Re: [PATCH 10/10] test-lib: return 1 from test_expect_{success,failure}

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 1, 2021, 21:43 UTC
Message-ID: <xmqqft1eo8hs.fsf@gitster.c.googlers.com>
In-Reply-To: <20210228195414.21372-11-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

Show 20 quoted lines

> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index c6cdabf53e..3dd68091bb 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -636,6 +636,7 @@ test_expect_failure () {
>  		fi
>  	fi
>  	test_finish_
> +	return 1
>  }
>  
>  test_expect_success () {
> @@ -656,6 +657,7 @@ test_expect_success () {
>  		fi
>  	fi
>  	test_finish_
> +	return 1
>  }
>  
>  # test_external runs external test scripts that provide continuous

Hmph.

This does not catch if the outer expect_success does not catch a failure in the inner expect_success and signal a failure.

When I asked if this kind of breakage is an easy mistake to catch by the test lint, I had something along this in mind:

	test_expect_success () {
		if test -n "$GIT_IN_TEST_EXPECT"
		then
			BUG caling "$GIT_IN_TEST_EXPECT" inside test_expect_success
		fi
		GIT_IN_TEST_EXPECT=test_expect_success

		... do the 'eval the given test body' thing ..

		GIT_IN_TEST_EXPECT=
	}

After all, the error is in the outer expect_success in that it called another one, so it feels more natural that the called inner expect_success to notice the situation and barf.

Thanks.