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

From: Junio C Hamano <[email-masked]>
Date: Mon, 01 Mar 2021 13:43:27 -0800

Ævar Arnfjörð Bjarmason  <[email-masked]> writes:

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