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

[Jonathan Nieder <jrnieder@gmail.com>]

Ævar Arnfjörð Bjarmason wrote:

> 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. :)

	From: Johannes Sixt <j6t@kdbg.org>
	Subject: test-lib: Allow tests with multiple prerequisites

	Occasionally an especially taxing test comes around that
	makes multiple assumptions on the system running it.
	Currently the test suite works around that with such hacks as

	 if
		test_have_prereq SYMLINKS &&
		test_have_prereq POSIXPERM
	 then
		test_set_prereq SYMLINKPERM
	 fi
	 test_expect_success SYMLINKPERM 'funny symlink in work tree, un-unlink-able' '
		...
	 '

	or even worse, as in the test that example is based on, leaves
	out mention of the “easier” of the two assumptions.

	Maybe you would be happier to be able to write

	 test_expect_success SYMLINKS,POSIXPERM 'funny symlink' '
		...
	 '

	Based on a patch by Junio.

	[ab: added tests and documentation]

	Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

> +++ 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.

> +    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.

> +++ 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?

Except as noted above,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks.
---
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 2887677..cdb25af 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -58,35 +58,28 @@ test_expect_failure 'pretend we have fixed a known breakage' '
     :
 '
 test_set_prereq HAVEIT
+test_set_prereq ONE
+test_set_prereq TWO
 haveit=no
 test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '
     test_have_prereq HAVEIT &&
     haveit=yes
 '
+haveboth=no
+test_expect_success ONE,TWO 'test runs if prerequisites are satisfied' '
+    haveboth=yes
+'
 donthaveit=yes
 test_expect_success DONTHAVEIT 'unmet prerequisite causes test to be skipped' '
     donthaveit=no
 '
-if test $haveit$donthaveit != yesyes
-then
-	say "bug in test framework: prerequisite tags do not work reliably"
-	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 &&
-    haveit=yes
-'
-donthaveit=yes
-test_expect_success HAVEIT,DONTHAVEIT 'unmet prerequisites causes test to be skipped' '
-    donthaveit=no
+donthaveboth=yes
+test_expect_success THREE,ONE 'one unmet prerequisite is enough' '
+    donthaveboth=no
 '
-if test $haveit$donthaveit != yesyes
+if test $haveit$donthaveit$haveboth$donthaveboth != yesyesyesyes
 then
-	say "bug in test framework: multiple prerequisite tags do not work reliably"
+	say "bug in test framework: prerequisite tags do not work reliably"
 	exit 1
 fi
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 8701923..a3b0b22 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -338,7 +338,7 @@ test_have_prereq () {
 		*" $prerequisite "*)
 			: yes, have it ;;
 		*)
-			! : nope ;;
+			return 1
 		esac
 	done
 }
--