Subject: Re: [PATCH 10/10] log tests: don't use "exit 1" outside a sub-shell

From: Junio C Hamano <[email-masked]>
Date: Wed, 20 Jul 2022 10:11:06 -0700

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

> Change an "exit 1" added in ac52d9410e5 (t4205: cover `git log
> --reflog -z` blindspot, 2019-11-19) to use "return 1" instead, which
> curiously was done in an adjacent test case added in the same commit.
>
> Using "exit 1" outside a sub-shell will cause the test framework
> itself to exit on failure, which isn't what we want to do here.
>
> This issue was spotted with the new
> "GIT_TEST_PASSING_SANITIZE_LEAK=check" mode, i.e. that "git show"
> command leaks memory, and we'd thus "exit 1". Another implementation
> of "GIT_TEST_PASSING_SANITIZE_LEAK=check" or "--invert-exit-code"
> might have intercepted the "exit 1", and thus hidden the underlying
> issue here, but we correctly distinguish the two.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <[email-masked]>
> ---
>  t/t4205-log-pretty-formats.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Let's not bury this, which does not have to depend on the rest of
the patches in this series, at the end.  Instead have this a
separate topic, on which this topic may depend on.

>
> diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
> index e448ef2928a..0404491d6ee 100755
> --- a/t/t4205-log-pretty-formats.sh
> +++ b/t/t4205-log-pretty-formats.sh
> @@ -156,7 +156,7 @@ test_expect_success 'NUL termination with --reflog --pretty=oneline' '
>  	for r in $revs
>  	do
>  		git show -s --pretty=oneline "$r" >raw &&
> -		cat raw | lf_to_nul || exit 1
> +		cat raw | lf_to_nul || return 1
>  	done >expect &&
>  	# the trailing NUL is already produced so we do not need to
>  	# output another one