Re: [PATCH v3 3/8] test-lib: rearrange start/end of test_expect_* and test_skip

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 18, 2013, 18:21 UTC
Message-ID: <7v4ncv5lff.fsf@alter.siamese.dyndns.org>
In-Reply-To: <0c11f94a00644b0fe8d2fcc3620ece530868f5af.1371557431.git.trast@inf.ethz.ch>

Thomas Rast <trast@inf.ethz.ch> writes:

Show 21 quoted lines

> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 4fa141a..e99b0ea 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -369,8 +369,15 @@ test_run_ () {
>  	return "$eval_ret"
>  }
>  
> -test_skip () {
> +test_start_ () {
>  	test_count=$(($test_count+1))
> +}
> +
> +test_finish_ () {
> +	echo >&3 ""
> +}
> +
> +test_skip () {
>  	to_skip=
>  	if match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS
>  	then

This puzzled me for a few minutes, until I realized that the code before this patch was using the call to test_skip, whose primary purpose is to answer "do we want to run this test, or do we want to skip it?", as a way to increment the test_count variable. Arguably each test would call test_skip once, so it may not be too bad, but it does look like it is depending on a subtle side-effect.

That increment does logically belong to "now we are starting a new test" much better. This change makes perfect sense.

Thanks.