[PATCH v2] [Outreachy][Patch v1] t3404: avoid losing exit status to pipes

["Usman Akinyemi via GitGitGadget" <gitgitgadget@gmail.com>]

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.

Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
---
    [Outreachy][Patch v2] t3404: avoid losing exit status to pipes
    
    At the beginning of my task, I made the mistake of submitting two
    patches for two separate commits instead of one. The first patch
    addressed the issue of losing the Git exit status due to pipes.
    
    After submitting the first patch, I noticed that the output of wc -l was
    failing due to trailing whitespace. I attempted to fix this by using tr
    -d to remove the whitespace. However, instead of squashing the two
    patches into one, I inadvertently created another commit.
    
    Eric Sunshine sunshine@sunshineco.com provided valuable feedback during
    the review process. He explained the details of the patches to me and
    pointed out that using tr -d was unnecessary to resolve the whitespace
    issue.
    
    The root cause of the whitespace issue was quoting $count in the test
    command, which led to the inclusion of whitespace in the comparison. By
    removing the quotes around $count, the comparison works as expected
    without the need for tr -d.
    
    Signed-off-by: Usman Akinyemi

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1805%2FUnique-Usman%2Favoid_git_pipes-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1805/Unique-Usman/avoid_git_pipes-v2
Pull-Request: https://github.com/git/git/pull/1805

Range-diff vs v1:

 1:  5dd96eaf14c ! 1:  be5a691e96f [Outreachy][Patch v1] t3404: avoid losing exit status to pipes
     @@ t/t3404-rebase-interactive.sh: test_expect_success 'multi-squash only fires up e
      -	test 1 = $(git show | grep ONCE | wc -l)
      +	git show >output &&
      +	count=$(grep ONCE output | wc -l) &&
     -+	test 1 = "$count"
     ++	test 1 = $count
       '
       
       test_expect_success 'multi-fixup does not fire up editor' '
     @@ t/t3404-rebase-interactive.sh: test_expect_success 'multi-fixup does not fire up
      -	test 0 = $(git show | grep NEVER | wc -l) &&
      +	git show >output &&
      +	count=$(grep NEVER output | wc -l) &&
     -+	test 0 = "$count" &&
     ++	test 0 = $count &&
       	git checkout @{-1} &&
       	git branch -D multi-fixup
       '
     @@ t/t3404-rebase-interactive.sh: test_expect_success 'commit message used after co
      -	test 1 = $(git show | grep ONCE | wc -l) &&
      +	git show >output &&
      +	count=$(grep ONCE output | wc -l) &&
     -+	test 1 = "$count" &&
     ++	test 1 = $count &&
       	git checkout @{-1} &&
       	git branch -D conflict-fixup
       '
     @@ t/t3404-rebase-interactive.sh: test_expect_success 'commit message retained afte
      -	test 2 = $(git show | grep TWICE | wc -l) &&
      +	git show >output &&
      +	count=$(grep TWICE output | wc -l) &&
     -+	test 2 = "$count" &&
     ++	test 2 = $count &&
       	git checkout @{-1} &&
       	git branch -D conflict-squash
       '
     @@ t/t3404-rebase-interactive.sh: test_expect_success 'squash and fixup generate co
      -	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 &&
     ++	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  &&
     ++	grep "^# This is a combination of 2 commits\." actual  &&
       	git checkout @{-1} &&
       	git branch -D squash-fixup
       '
     @@ t/t3404-rebase-interactive.sh: test_expect_success 'squash ignores comments' '
      -	test 1 = $(git show | grep ONCE | wc -l) &&
      +	git show >output &&
      +	count=$(grep ONCE output | wc -l) &&
     -+	test 1 = "$count" &&
     ++	test 1 = $count &&
       	git checkout @{-1} &&
       	git branch -D skip-comments
       '
     @@ t/t3404-rebase-interactive.sh: test_expect_success 'squash ignores blank lines'
      -	test 1 = $(git show | grep ONCE | wc -l) &&
      +	git show >output &&
      +	count=$(grep ONCE output | wc -l) &&
     -+	test 1 = "$count" &&
     ++	test 1 = $count &&
       	git checkout @{-1} &&
       	git branch -D skip-blank-lines
       '
 2:  4199434bd6e < -:  ----------- [Outreachy][Patch v2] t3404: avoid losing exit status to pipes


 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
 '
 
 test_expect_success 'retain authorship w/ conflicts' '
@@ -360,7 +361,8 @@ test_expect_success 'squash' '
 '
 
 test_expect_success 'retain authorship when squashing' '
-	git show HEAD | grep "^Author: Twerp Snog"
+	git show HEAD >actual &&
+	grep "^Author: Twerp Snog" actual
 '
 
 test_expect_success '--continue tries to commit' '
@@ -374,7 +376,8 @@ test_expect_success '--continue tries to commit' '
 		FAKE_COMMIT_MESSAGE="chouette!" git rebase --continue
 	) &&
 	test_cmp_rev HEAD^ new-branch1 &&
-	git show HEAD | grep chouette
+	git show HEAD >actual &&
+	grep chouette actual
 '
 
 test_expect_success 'verbose flag is heeded, even after --continue' '
@@ -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
 '
 
 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
 '
@@ -428,7 +435,9 @@ test_expect_success 'commit message used after conflict' '
 			git rebase --continue
 	) &&
 	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 &&
 	git checkout @{-1} &&
 	git branch -D conflict-fixup
 '
@@ -446,7 +455,9 @@ test_expect_success 'commit message retained after conflict' '
 			git rebase --continue
 	) &&
 	test $base = $(git rev-parse HEAD^) &&
-	test 2 = $(git show | grep TWICE | wc -l) &&
+	git show >output &&
+	count=$(grep TWICE output | wc -l) &&
+	test 2 = $count &&
 	git checkout @{-1} &&
 	git branch -D conflict-squash
 '
@@ -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 &&
+	git cat-file commit HEAD@{3} >actual &&
+	grep "^# This is a combination of 2 commits\." actual  &&
 	git checkout @{-1} &&
 	git branch -D squash-fixup
 '
@@ -489,7 +500,9 @@ test_expect_success 'squash ignores comments' '
 			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 &&
 	git checkout @{-1} &&
 	git branch -D skip-comments
 '
@@ -505,7 +518,9 @@ test_expect_success 'squash ignores blank lines' '
 			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 &&
 	git checkout @{-1} &&
 	git branch -D skip-blank-lines
 '
@@ -572,7 +587,8 @@ test_expect_success '--continue tries to commit, even for "edit"' '
 		FAKE_COMMIT_MESSAGE="chouette!" git rebase --continue
 	) &&
 	test edited = $(git show HEAD:file7) &&
-	git show HEAD | grep chouette &&
+	git show HEAD >actual &&
+	grep chouette actual &&
 	test $parent = $(git rev-parse HEAD^)
 '
 
@@ -757,19 +773,23 @@ test_expect_success 'reword' '
 		set_fake_editor &&
 		FAKE_LINES="1 2 3 reword 4" FAKE_COMMIT_MESSAGE="E changed" \
 			git rebase -i A &&
-		git show HEAD | grep "E changed" &&
+		git show HEAD >actual &&
+		grep "E changed" actual &&
 		test $(git rev-parse primary) != $(git rev-parse HEAD) &&
 		test_cmp_rev primary^ HEAD^ &&
 		FAKE_LINES="1 2 reword 3 4" FAKE_COMMIT_MESSAGE="D changed" \
 			git rebase -i A &&
-		git show HEAD^ | grep "D changed" &&
+		git show HEAD^ >actual &&
+		grep "D changed" actual &&
 		FAKE_LINES="reword 1 2 3 4" FAKE_COMMIT_MESSAGE="B changed" \
 			git rebase -i A &&
-		git show HEAD~3 | grep "B changed" &&
+		git show HEAD~3 >actual &&
+		grep "B changed" actual &&
 		FAKE_LINES="1 r 2 pick 3 p 4" FAKE_COMMIT_MESSAGE="C changed" \
 			git rebase -i A
 	) &&
-	git show HEAD~2 | grep "C changed"
+	git show HEAD~2 >actual &&
+	grep "C changed" actual
 '
 
 test_expect_success 'no uncommitted changes when rewording and the todo list is reloaded' '
@@ -1003,8 +1023,10 @@ test_expect_success 'rebase -i --root retain root commit author and message' '
 		set_fake_editor &&
 		FAKE_LINES="2" git rebase -i --root
 	) &&
-	git cat-file commit HEAD | grep -q "^author Twerp Snog" &&
-	git cat-file commit HEAD | grep -q "^different author$"
+	git cat-file commit HEAD >output &&
+	grep -q "^author Twerp Snog" output &&
+	git cat-file commit HEAD >actual &&
+	grep -q "^different author$" actual
 '
 
 test_expect_success 'rebase -i --root temporary sentinel commit' '
@@ -1013,7 +1035,8 @@ test_expect_success 'rebase -i --root temporary sentinel commit' '
 		set_fake_editor &&
 		test_must_fail env FAKE_LINES="2" git rebase -i --root
 	) &&
-	git cat-file commit HEAD | grep "^tree $EMPTY_TREE" &&
+	git cat-file commit HEAD >actual &&
+	grep "^tree $EMPTY_TREE" actual &&
 	git rebase --abort
 '
 
@@ -1036,7 +1059,8 @@ test_expect_success 'rebase -i --root reword original root commit' '
 		FAKE_LINES="reword 1 2" FAKE_COMMIT_MESSAGE="A changed" \
 			git rebase -i --root
 	) &&
-	git show HEAD^ | grep "A changed" &&
+	git show HEAD^ >actual &&
+	grep "A changed" actual &&
 	test -z "$(git show -s --format=%p HEAD^)"
 '
 
@@ -1048,7 +1072,8 @@ test_expect_success 'rebase -i --root reword new root commit' '
 		FAKE_LINES="reword 3 1" FAKE_COMMIT_MESSAGE="C changed" \
 		git rebase -i --root
 	) &&
-	git show HEAD^ | grep "C changed" &&
+	git show HEAD^ >actual &&
+	grep "C changed" actual &&
 	test -z "$(git show -s --format=%p HEAD^)"
 '
 

base-commit: 90fe3800b92a49173530828c0a17951abd30f0e1
-- 
gitgitgadget