Subject: Re: [PATCH 2/2] test: cope better with use of return for errors

From: Johannes Sixt <[email-masked]>
Date: Tue, 09 Aug 2011 10:46:34 +0200

Am 8/8/2011 3:17, schrieb Jonathan Nieder:
> +test_eval_ () {
> +	# This is a separate function because some tests use
> +	# "return" to end a test_expect_success block early.
> +	eval >&3 2>&4 "$*"
> +}
> +
>  test_run_ () {
>  	test_cleanup=:
>  	expecting_failure=$2
> -	eval >&3 2>&4 "$1"
> +	test_eval_ "$1"
>  	eval_ret=$?
>  
>  	if test -z "$immediate" || test $eval_ret = 0 || test -n "$expecting_failure"
>  	then
> -		eval >&3 2>&4 "$test_cleanup"
> +		test_eval_ "$test_cleanup"
>  	fi
>  	if test "$verbose" = "t" && test -n "$HARNESS_ACTIVE"; then
>  		echo ""

This invalidates at least t3900.29, which accesses an unexpanded $3
from the test script. The patch below fixes this case.

I tried to detect other cases by poisoning test_run_ like this:

-       test_eval_ "$1"
+       test_eval_ :\; :\; :\; :\; :\; "$1"

in the hopes that ":;" is an error at the place that uses $1, $2, etc.
t3900.2[89] are the only tests that are uncovered in this way.

I noticed this because I have a patched test-lib.sh that calls
test_eval_ in a similarly modified manner.

--- >8 ---
From: Johannes Sixt <[email-masked]>
Subject: [PATCH] t3900: do not reference numbered arguments from the test
 script

The call to test_expect_success is nested inside a function, whose
arguments the test code wants to access. But it is not specified that any
unexpanded $1, $2, $3, etc in the test code will access the surrounding
function's arguments. Rather, they will access the arguments of the
function that happens to eval the test code.

Play safe by placing the argument in a named variable.

Signed-off-by: Johannes Sixt <[email-masked]>
---
 t/t3900-i18n-commit.sh |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh
index c06a5ee..3265fac 100755
--- a/t/t3900-i18n-commit.sh
+++ b/t/t3900-i18n-commit.sh
@@ -136,6 +136,7 @@ done
 test_commit_autosquash_flags () {
 	H=$1
 	flag=$2
+	mopt=$3
 	test_expect_success "commit --$flag with $H encoding" '
 		git config i18n.commitencoding $H &&
 		git checkout -b $H-$flag C0 &&
@@ -147,7 +148,7 @@ test_commit_autosquash_flags () {
 		git commit -a -m "intermediate commit" &&
 		test_tick &&
 		echo $H $flag >>F &&
-		git commit -a --$flag HEAD~1 $3 &&
+		git commit -a --$flag HEAD~1 $mopt &&
 		E=$(git cat-file commit '$H-$flag' |
 			sed -ne "s/^encoding //p") &&
 		test "z$E" = "z$H" &&
@@ -160,6 +161,6 @@ test_commit_autosquash_flags () {
 
 test_commit_autosquash_flags eucJP fixup
 
-test_commit_autosquash_flags ISO-2022-JP squash '-m "squash message"'
+test_commit_autosquash_flags ISO-2022-JP squash '-m squash_message'
 
 test_done
-- 
1.7.6.1618.gc932c