Re: [PATCH 1/3] t3404: avoid losing exit status with focus on `git show` and `git cat-file`
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Oct 14, 2024, 21:29 UTC
- Message-ID
- <Zw2NTPk+YwEqcydf@nand.local>
- In-Reply-To
- <bfff7937cd20737bb5a8791dc7492700b1d7881f.1728774574.git.gitgitgadget@gmail.com>
On Sat, Oct 12, 2024 at 11:09:32PM +0000, Usman Akinyemi via GitGitGadget wrote:
Show 26 quoted lines
> From: Usman Akinyemi <usmanakinyemi202@gmail.com> > > 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. This particular patch focuses on all `git show` and > some instances of `git cat-file`. > > Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com> > --- > t/t3404-rebase-interactive.sh | 71 +++++++++++++++++++++++------------ > 1 file changed, 48 insertions(+), 23 deletions(-) > > diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh > index f171af3061d..96a65783c47 100755 > --- a/t/t3404-rebase-interactive.sh > +++ b/t/t3404-rebase-interactive.sh > @@ -319,7 +319,8 @@ test_expect_success 'retain authorship' ' > GIT_AUTHOR_NAME="Twerp Snog" git commit -m "different author" && > git tag twerp && > git rebase -i --onto primary HEAD^ && > - git show HEAD | grep "^Author: Twerp Snog" > + git show HEAD >actual && > + grep "^Author: Twerp Snog" actual > '
Good.
Show 9 quoted lines
> @@ -397,7 +400,9 @@ test_expect_success 'multi-squash only fires up editor once' ' > git rebase -i $base > ) && > test $base = $(git rev-parse HEAD^) && > - test 1 = $(git show | grep ONCE | wc -l) > + git show >output && > + count=$(grep ONCE output | wc -l) && > + test 1 = $count > '
I think moving 'git show' out of the pipeline is a good step here, but I don't think we need to store $count in a separate variable. It would be fine to write:
git show >output &&
test 1 = $(grep ONCE output | wc -l)or even to replace the subshell with 'grep -c' instead of piping 'grep' to 'wc -l'.
Show 12 quoted lines
> test_expect_success 'multi-fixup does not fire up editor' '
> @@ -410,7 +415,9 @@ test_expect_success 'multi-fixup does not fire up editor' '
> git rebase -i $base
> ) &&
> test $base = $(git rev-parse HEAD^) &&
> - test 0 = $(git show | grep NEVER | wc -l) &&
> + git show >output &&
> + count=$(grep NEVER output | wc -l) &&
> + test 0 = $count &&
> git checkout @{-1} &&
> git branch -D multi-fixup
> 'Same notes from above here and the next two tests (elided from my response) below.
Show 10 quoted lines
> @@ -470,10 +481,10 @@ test_expect_success 'squash and fixup generate correct log messages' '
> ) &&
> git cat-file commit HEAD | sed -e 1,/^\$/d > actual-squash-fixup &&
> test_cmp expect-squash-fixup actual-squash-fixup &&
> - 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 &&Is there a more descriptive name for the output here than just 'actual'?
Thanks, Taylor