Re: [PATCH v3 0/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes

[Patrick Steinhardt <ps@pks.im>]

On Mon, Oct 07, 2024 at 07:25:44AM +0000, Usman Akinyemi wrote:
> On Mon, Oct 7, 2024 at 4:24 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> >
> > On Sun, Oct 6, 2024 at 12:18 PM Usman Akinyemi
> > <usmanakinyemi202@gmail.com> wrote:
> > > Kindly, help take a look if this is okay now.
> > >
> > > Also, I wanted to change this also to use test_line_count,
> > >  test 0 = $(grep -c "^[^#]" < .git/rebase-merge/git-rebase-todo)
> > >
> > >  But, I tried a different approach and the test kept failing.
> > >
> > > Similar as
> > >
> > > git show >output &&
> > > count=$(grep NEVER output | wc -l) &&
> > > test 0 = $count &&
> >
> > What is the actual error you encountered?
> >
> > By the way, we have a handy function, test_must_be_empty(), which can
> > be used if you expect the output to not contain anything. As an
> > example:
> >
> >     git show >output &&
> >     grep NEVER output >actual &&
> >     test_must_be_empty actual
> 
> Thanks for your review, I really appreciate it. I tried this approach,
> but I was getting this particular error for the testing.
> 
> not ok 32 - multi-fixup does not fire up editor
> #
> # git checkout -b multi-fixup E &&
> # base=$(git rev-parse HEAD~4) &&
> # (
> # set_fake_editor &&
> # FAKE_COMMIT_AMEND="NEVER" \
> # FAKE_LINES="1 fixup 2 fixup 3 fixup 4" \
> # git rebase -i $base
> # ) &&
> # test $base = $(git rev-parse HEAD^) &&
> # git show >output &&
> # grep NEVER output >actual &&
> # test_must_be_empty actual &&
> # git checkout @{-1} &&
> # git branch -D multi-fixup
> #
> Below is the particular test case
> 
> test_expect_success 'multi-fixup does not fire up editor' '
> git checkout -b multi-fixup E &&
> base=$(git rev-parse HEAD~4) &&
> (
> set_fake_editor &&
> FAKE_COMMIT_AMEND="NEVER" \
> FAKE_LINES="1 fixup 2 fixup 3 fixup 4" \
> git rebase -i $base
> ) &&
> test $base = $(git rev-parse HEAD^) &&
> git show >output &&
> grep NEVER output >actual &&
> test_must_be_empty actual &&
> git checkout @{-1} &&
> git branch -D multi-fixup
> '

That makes sense. The expectation here is that `output` shouldn't
contain the string "NEVER" at all. And as grep(1) would fail when it
doesn't find a match the whole test would fail like this. So the below
would likely be the best solution.

Patrick

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index f171af3061..978fdfc2f1 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -410,7 +410,8 @@ 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 &&
+	! grep NEVER output &&
 	git checkout @{-1} &&
 	git branch -D multi-fixup
 '