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

From: Junio C Hamano <gitster@pobox.com>
Date: May 20, 2014, 21:33 UTC
Message-ID: <xmqqppj8rv82.fsf@gitster.dls.corp.google.com>
In-Reply-To: <537BC8A2.9060009@ramsay1.demon.co.uk>

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

Show 24 quoted lines

> 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 &&

The original is clearly wrong if run_sub_test_lib_test is a shell function. I thought we hunted those down and killed them already, but apparently we didn't.

I think exporting the variable and then clearing it in test-when-finished is fine, and doing the export and run in a subshell so that you do not have to clear is also fine.

Show 26 quoted lines

>  	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