Subject: Re: [PATCH 2/4] t0024, t5000: use test_lazy_prereq for UNZIP

From: Matt Kraai <[email-masked]>
Date: Sun, 6 Jan 2013 10:06:21 -0800

On Sun, Jan 06, 2013 at 06:49:00PM +0100, René Scharfe wrote:
> This change makes the code smaller and we can put it at the top of
> the script, its rightful place as setup code.

Would it be better to add the setting of GIT_UNZIP and
test_lazy_prereq to test-lib.sh so they aren't duplicated in both
t0024-crlf-archive.sh and t5000-tar-tree.sh, something like the
following (modulo UNZIP/GIT_UNZIP)?

-- 
Matt Kraai
https://ftbfs.org/kraai

diff --git a/t/t0024-crlf-archive.sh b/t/t0024-crlf-archive.sh
index ec6c1b3..084f33c 100755
--- a/t/t0024-crlf-archive.sh
+++ b/t/t0024-crlf-archive.sh
@@ -3,7 +3,6 @@
 test_description='respect crlf in git archive'
 
 . ./test-lib.sh
-UNZIP=${UNZIP:-unzip}
 
 test_expect_success setup '
 
@@ -26,13 +25,6 @@ test_expect_success 'tar archive' '
 
 '
 
-"$UNZIP" -v >/dev/null 2>&1
-if [ $? -eq 127 ]; then
-	say "Skipping ZIP test, because unzip was not found"
-else
-	test_set_prereq UNZIP
-fi
-
 test_expect_success UNZIP 'zip archive' '
 
 	git archive --format=zip HEAD >test.zip &&
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index ecf00ed..85b64ae 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -25,7 +25,6 @@ commit id embedding:
 '
 
 . ./test-lib.sh
-UNZIP=${UNZIP:-unzip}
 GZIP=${GZIP:-gzip}
 GUNZIP=${GUNZIP:-gzip -d}
 
@@ -201,13 +200,6 @@ test_expect_success \
       test_cmp a/substfile2 g/prefix/a/substfile2
 '
 
-$UNZIP -v >/dev/null 2>&1
-if [ $? -eq 127 ]; then
-	say "Skipping ZIP tests, because unzip was not found"
-else
-	test_set_prereq UNZIP
-fi
-
 test_expect_success \
     'git archive --format=zip' \
     'git archive --format=zip HEAD >d.zip'
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 8a12cbb..4ceabad 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -752,6 +752,13 @@ test_lazy_prereq AUTOIDENT '
 	git var GIT_AUTHOR_IDENT
 '
 
+UNZIP=${UNZIP:-unzip}
+
+test_lazy_prereq UNZIP '
+	"$UNZIP" -v >/dev/null 2>&1
+	test $? -ne 127
+'
+
 # When the tests are run as root, permission tests will report that
 # things are writable when they shouldn't be.
 test -w / || test_set_prereq SANITY