Re: [PATCH 1/3] test-lib: Add support for multiple test prerequisites

On Sun, Aug 8, 2010 at 01:57, Jonathan Nieder [off-list ref] wrote:

Ævar Arnfjörð Bjarmason wrote:

quoted

Change the test_have_prereq function in test-lib.sh to support a
comma-separated list of prerequisites. This is useful for tests that
need e.g. both POSIXPERM and SANITY.

The implementation was stolen from Junio C Hamano and Johannes Sixt,
the tests and documentation were not.

I think you can sell it better. :)

As Johannes points out the patch is different enough that I changed
the authorship. Actually I wouldn't modify any patch under someone
else's name without getting them to sign off on it.

quoted

+++ b/t/t0000-basic.sh

@@ -73,6 +73,23 @@ then

      exit 1
 fi

+test_set_prereq HAVETHIS
+haveit=no
+test_expect_success HAVETHIS,HAVEIT 'test runs if prerequisites are satisfied' '
+    test_have_prereq HAVEIT &&
+    test_have_prereq HAVETHIS &&

I think the similar code above was just a way to sneak in a sanity
check for test_have_prereq().  I’d leave it out.

It's a sanity test. If that wasn't there the test might succeed
e.g. if the implementation was broken and only did a OR on the comma
delimited list, not a AND.

quoted

+    haveit=yes
+'
+donthaveit=yes
+test_expect_success HAVEIT,DONTHAVEIT 'unmet prerequisites causes test to be skipped' '
+    donthaveit=no
+'
+if test $haveit$donthaveit != yesyes
+then
+     say "bug in test framework: multiple prerequisite tags do not work reliably"
+     exit 1
+fi

Maybe it would be simpler to squash this in with the other similar checks.

I think that's too much complexity for one test, especially in the
sanity file. I didn't squash it with the existing "yesyes" test
because we're testing basic functionality first (one prereq) then the
fancy stuff (multiple).

quoted

+++ b/t/test-lib.sh

@@ -327,12 +327,20 @@ test_set_prereq () {

 satisfied=" "

 test_have_prereq () {
-     case $satisfied in
-     *" $1 "*)
-             : yes, have it ;;
-     *)
-             ! : nope ;;
-     esac
+     # prerequisites can be concatenated with ','
+     save_IFS=$IFS
+     IFS=,
+     set -- $*
+     IFS=$save_IFS
+     for prerequisite
+     do
+             case $satisfied in
+             *" $prerequisite "*)
+                     : yes, have it ;;
+             *)
+                     ! : nope ;;
+             esac
+     done

Does that work?

It passes all the tests :)

Except as noted above,
Reviewed-by: Jonathan Nieder <redacted>

Thanks.

Likewise.