Re: [PATCH v3 4/6] Refactor mechanics of testing in a sub test-lib

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 20, 2012, 21:13 UTC
Message-ID: <7vhaqs9z47.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1348080591-16430-1-git-send-email-git@adamspiers.org>

Adam Spiers <git@adamspiers.org> writes:

Show 44 quoted lines

> This will allow us to test the test framework more thoroughly
> without disrupting the top-level test metrics.
>
> Signed-off-by: Adam Spiers <git@adamspiers.org>
> ---
>  t/t0000-basic.sh | 67 ++++++++++++++++++++++++--------------------------------
>  1 file changed, 29 insertions(+), 38 deletions(-)
>
> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> index c6b42de..029e3bd 100755
> --- a/t/t0000-basic.sh
> +++ b/t/t0000-basic.sh
> @@ -55,39 +55,49 @@ test_expect_failure 'pretend we have a known breakage' '
>  	false
>  '
>  
> -test_expect_success 'pretend we have fixed a known breakage (run in sub test-lib)' "
> -	mkdir passing-todo &&
> -	(cd passing-todo &&
> -	cat >passing-todo.sh <<-EOF &&
> +run_sub_test_lib_test () {
> +	name="$1" descr="$2" # stdin is body of test code
> +	mkdir $name &&
> +	(cd $name &&
> +	cat >$name.sh <<-EOF &&
>  	#!$SHELL_PATH
>  
> -	test_description='A passing TODO test
> +	test_description='$descr (run in sub test-lib)
>  
>  	This is run in a sub test-lib so that we do not get incorrect
>  	passing metrics
>  	'
>  
>  	# Point to the t/test-lib.sh, which isn't in ../ as usual
> -	TEST_DIRECTORY=\"$TEST_DIRECTORY\"
> -	. \"\$TEST_DIRECTORY\"/test-lib.sh
> -
> -	test_expect_failure 'pretend we have fixed a known breakage' '
> -		:
> -	'
> +	TEST_DIRECTORY="$TEST_DIRECTORY"
> +	. "\$TEST_DIRECTORY"/test-lib.sh
> +	EOF

The quoting of $TEST_DIRECTORY in the assignment does not look correct (imagine a path with a double quote in it).

Removing the assignment and instead exporting TEST_DIRECTORY before calling name.sh may be a reasonable fix, than trying to quotemeta the value of $TEST_DIRECTORY here.

I'll re-queue this series in 'pu' with fixes and retitles; please eyeball them before submitting a reroll.

  b465316 tests: paint unexpectedly fixed known breakages in bold red
  7214717 tests: test the test framework more thoroughly
  03c772a [SQUASH] t/t0000-basic.sh: quoting of TEST_DIRECTORY is screwed up
  99fe0af tests: refactor mechanics of testing in a sub test-lib
  6af90bf tests: paint skipped tests in bold blue
  0b87581 tests: test number comes first in 'not ok $count - $message'
  1c55079 tests: paint known breakages in bold yellow

The third one from the tip looks like the following, to re-indent to make it readable and then minimally fix the quoting.

Thanks.

 t/t0000-basic.sh | 50 +++++++++++++++++++++++++++-----------------------
 1 file changed, 27 insertions(+), 23 deletions(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index ee78e68..c3345a9 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -56,33 +56,37 @@ test_expect_failure 'pretend we have a known breakage' '
 '
 
 run_sub_test_lib_test () {
-	name="$1" descr="$2" # stdin is body of test code
+	name="$1" descr="$2" # stdin is the body of the test code
 	mkdir $name &&
-	(cd $name &&
-	cat >$name.sh <<-EOF &&
-	#!$SHELL_PATH
-
-	test_description='$descr (run in sub test-lib)
-
-	This is run in a sub test-lib so that we do not get incorrect
-	passing metrics
-	'
-
-	# Point to the t/test-lib.sh, which isn't in ../ as usual
-	TEST_DIRECTORY="$TEST_DIRECTORY"
-	. "\$TEST_DIRECTORY"/test-lib.sh
-	EOF
-	cat >>$name.sh &&
-	chmod +x $name.sh &&
-	./$name.sh >out 2>err)
+	(
+		cd $name &&
+		cat >$name.sh <<-EOF &&
+		#!$SHELL_PATH
+
+		test_description='$descr (run in sub test-lib)
+
+		This is run in a sub test-lib so that we do not get incorrect
+		passing metrics
+		'
+
+		# Point to the t/test-lib.sh, which isn't in ../ as usual
+		. "\$TEST_DIRECTORY"/test-lib.sh
+		EOF
+		cat >>$name.sh &&
+		chmod +x $name.sh &&
+		export TEST_DIRECTORY &&
+		./$name.sh >out 2>err
+	)
 }
 
 check_sub_test_lib_test () {
-	name="$1" # stdin is expected output from the test
-	(cd $name &&
-	! test -s err &&
-	sed -e 's/^> //' -e 's/Z$//' >expect &&
-	test_cmp expect out)
+	name="$1" # stdin is the expected output from the test
+	(
+		cd $name &&
+		! test -s err &&
+		sed -e 's/^> //' -e 's/Z$//' >expect &&
+		test_cmp expect out
+	)
 }
 
 test_expect_success 'pretend we have fixed a known breakage' "

-- 
1.7.12.1.389.g3dff30b