Subject: [PATCH/RFC] t0000-*.sh: Fix the GIT_SKIP_TESTS sub-tests

From: Ramsay Jones <[email-masked]>
Date: Tue, 20 May 2014 22:26:58 +0100

Signed-off-by: Ramsay Jones <[email-masked]>
---

The test suite has been failing for me on the pu branch for
a while now. I finally found a few minutes to take a look.

This failure is specific to the dash shell (/bin/sh) on my
system (ie it may well affect other shells, but I haven't
tested them all ... :). This does not affect bash (or bash
as /bin/sh aka bash --posix).

The GIT_SKIP_TESTS, 13-15, all fail with verbose output
similar to:

  $ ./t0000-basic.sh -i -v
  ...
  --- expect	2014-05-20 20:55:54.138342361 +0000
  +++ out	2014-05-20 20:55:54.134342341 +0000
  @@ -1,5 +1,5 @@
   ok 1 - passing test #1
  -ok 2 # skip passing test #2 (GIT_SKIP_TESTS)
  +ok 2 - passing test #2
   ok 3 - passing test #3
   # passed all 3 test(s)
   1..3
  not ok 13 - GIT_SKIP_TESTS
  #	
  #		GIT_SKIP_TESTS='git.2' 		run_sub_test_lib_test git-skip-tests-basic 		'GIT_SKIP_TESTS' <<-\EOF &&
  #		for i in 1 2 3
  #		do
  #			test_expect_success "passing test #$i" 'true'
  #		done
  #		test_done
  #		EOF
  #		check_sub_test_lib_test git-skip-tests-basic <<-\EOF
  #		> ok 1 - passing test #1
  #		> ok 2 # skip passing test #2 (GIT_SKIP_TESTS)
  #		> ok 3 - passing test #3
  #		> # passed all 3 test(s)
  #		> 1..3
  #		EOF
  #	
  $ 

... which looks like the sub-test does not see the GIT_SKIP_TESTS
variable at all. Indeed, if I put the entire test in a sub-shell
and replace the use of that variable on the run_sub_test_lib_test
invocation with a separate explicit assignment and export, then
the tests start working. (ie if I do the opposite of some other
recent commits!)

This patch is an RFC, because I take a different approach to the
above solution, only because the diff is much smaller and easier
to read! Is it a better solution?

ATB,
Ramsay Jones

 t/t0000-basic.sh | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 8345c8a..373ad8f 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -296,8 +296,9 @@ test_expect_success 'test --verbose-only' '
 '
 
 test_expect_success 'GIT_SKIP_TESTS' "
-	GIT_SKIP_TESTS='git.2' \
-		run_sub_test_lib_test git-skip-tests-basic \
+	GIT_SKIP_TESTS='git.2' && export GIT_SKIP_TESTS &&
+	test_when_finished sane_unset GIT_SKIP_TESTS &&
+	run_sub_test_lib_test git-skip-tests-basic \
 		'GIT_SKIP_TESTS' <<-\\EOF &&
 	for i in 1 2 3
 	do
@@ -315,8 +316,9 @@ test_expect_success 'GIT_SKIP_TESTS' "
 "
 
 test_expect_success 'GIT_SKIP_TESTS several tests' "
-	GIT_SKIP_TESTS='git.2 git.5' \
-		run_sub_test_lib_test git-skip-tests-several \
+	GIT_SKIP_TESTS='git.2 git.5' && export GIT_SKIP_TESTS &&
+	test_when_finished sane_unset GIT_SKIP_TESTS &&
+	run_sub_test_lib_test git-skip-tests-several \
 		'GIT_SKIP_TESTS several tests' <<-\\EOF &&
 	for i in 1 2 3 4 5 6
 	do
@@ -337,8 +339,9 @@ test_expect_success 'GIT_SKIP_TESTS several tests' "
 "
 
 test_expect_success 'GIT_SKIP_TESTS sh pattern' "
-	GIT_SKIP_TESTS='git.[2-5]' \
-		run_sub_test_lib_test git-skip-tests-sh-pattern \
+	GIT_SKIP_TESTS='git.[2-5]' && export GIT_SKIP_TESTS &&
+	test_when_finished sane_unset GIT_SKIP_TESTS &&
+	run_sub_test_lib_test git-skip-tests-sh-pattern \
 		'GIT_SKIP_TESTS sh pattern' <<-\\EOF &&
 	for i in 1 2 3 4 5 6
 	do
-- 
1.9.0