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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Dec 2, 2022, 01:02 UTC
Message-ID: <CAPig+cQK8BRU0u0g+jAkyJaqSamYBynJ6s00_6H2beb6gXaRDA@mail.gmail.com>
In-Reply-To: <patch-v2-1.8-7c9f8d2830f-20221202T000227Z-avarab@gmail.com>

On Thu, Dec 1, 2022 at 7:07 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 17 quoted lines

> 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.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> diff --git 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 &&

Using `return 1` here is "obviously correct" since this code is not inside a subshell. Furthermore, the exit code of the for-loop itself is not being lost down a pipe, so `return 1` is indeed an appropriate way to signal failure. Good.