Re: [PATCH 1/2] [Outreachy][Patch v1] t3404: avoid losing exit status to pipes

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Oct 6, 2024, 05:55 UTC
Message-ID: <CAPig+cTO5_mDyEX=druLh6HkFazi=SBmDser8GdsVQP4iVaKug@mail.gmail.com>
In-Reply-To: <5dd96eaf14cb6fbe130fb50ce2738646d7d3ac57.1728192814.git.gitgitgadget@gmail.com>

On Sun, Oct 6, 2024 at 1:33 AM Usman Akinyemi via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 5 quoted lines

> The exit code of the preceding command in a pipe is disregarded. So
> if that preceding command is a Git command that fails, the test would
> not fail. Instead, by saving the output of that Git command to a file,
> and removing the pipe, we make sure the test will fail if that Git
> command fails.

Okay, makes sense.

One minor style comment below...

Show 12 quoted lines

> Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
> ---
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> @@ -470,10 +481,10 @@ test_expect_success 'squash and fixup generate correct log messages' '
> -       git cat-file commit HEAD@{2} |
> -               grep "^# This is a combination of 3 commits\."  &&
> -       git cat-file commit HEAD@{3} |
> -               grep "^# This is a combination of 2 commits\."  &&
> +       git cat-file commit HEAD@{2} >actual &&
> +               grep "^# This is a combination of 3 commits\." actual &&
> +       git cat-file commit HEAD@{3} >actual &&
> +               grep "^# This is a combination of 2 commits\." actual  &&

We wouldn't normally indent the `grep` line like this. Now that you've dropped the patch, it would be best to lose the extra indentation, as well:

    git cat-file commit HEAD@{2} >actual &&
    grep "^# This is a combination of 3 commits\." actual &&
    ...