Subject: [PATCH v2] t9832,t2200: avoid using pipes in git commands

From: "Amanda Shafack via GitGitGadget" <[email-masked]>
Date: Sun, 18 Oct 2020 14:42:04 +0000

From: Amanda Shafack <[email-masked]>

When a git command is upstream in a pipe, an unexpected failure of
the git command will go unnoticed.

Write out the output of the git command to a file, so as to actively
catch a failure of the git command.

Signed-off-by: Amanda Shafack <[email-masked]>
---
    [Outreachy][Patch v2] t9832,t2200: avoid using pipes in git related
    commands
    
    Changes since v1:
    
     * Merged patch [1/2] and [2/2] into a single patch
     * Revised commit message
    
    Signed-off-by: Amanda Shafack [email-masked]
    [[email-masked]]

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-885%2Flkmandy%2Favoid-pipes-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-885/lkmandy/avoid-pipes-v2
Pull-Request: https://github.com/git/git/pull/885

Range-diff vs v1:

 1:  fc2da014a6 ! 1:  bca393e69a t9832,t2200: avoid using pipes in git commands
     @@ Metadata
       ## Commit message ##
          t9832,t2200: avoid using pipes in git commands
      
     -    When the upstream of a pipe throws an error, the downstream still
     -    executes normally. This happens because the exit code of the upstream
     -    in a pipe is ignored. This behavior can make debugging very hard
     -    incase a test fails. Also, pipes are prone to deadlocks. If the
     -    upstream gets full, the commands downstream will never start.
     +    When a git command is upstream in a pipe, an unexpected failure of
     +    the git command will go unnoticed.
      
     -    Write out the output of the git command to a file, so as to test the
     -    exit codes of both commands.
     -
     -    Commit c6f44e1da5 (t9813: avoid using pipes, 2017-01-04) noticed that
     -    the exit code of upstream in the pipe is ignored, thus using pipes
     -    should be avoided.
     +    Write out the output of the git command to a file, so as to actively
     +    catch a failure of the git command.
      
          Signed-off-by: Amanda Shafack <[email-masked]>
      
     @@ t/t2200-add-update.sh: test_expect_success 'add -u resolves unmerged paths' '
       test_expect_success '"add -u non-existent" should fail' '
       	test_must_fail git add -u non-existent &&
      -	! (git ls-files | grep "non-existent")
     -+	! (
     -+		git ls-files >actual &&
     -+		grep "non-existent" actual
     -+	)
     ++	git ls-files >actual &&
     ++	! grep "non-existent" actual
       '
       
       test_done
 2:  0a1550cb22 < -:  ---------- t2200: modify code syntax

 t/t2200-add-update.sh | 3 ++-
 t/t9832-unshelve.sh   | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh
index f764b7e3f5..7cb7a70382 100755
--- a/t/t2200-add-update.sh
+++ b/t/t2200-add-update.sh
@@ -179,7 +179,8 @@ test_expect_success 'add -u resolves unmerged paths' '
 
 test_expect_success '"add -u non-existent" should fail' '
 	test_must_fail git add -u non-existent &&
-	! (git ls-files | grep "non-existent")
+	git ls-files >actual &&
+	! grep "non-existent" actual
 '
 
 test_done
diff --git a/t/t9832-unshelve.sh b/t/t9832-unshelve.sh
index 7194fb2855..6b3cb0414a 100755
--- a/t/t9832-unshelve.sh
+++ b/t/t9832-unshelve.sh
@@ -68,7 +68,8 @@ EOF
 		cd "$git" &&
 		change=$(last_shelved_change) &&
 		git p4 unshelve $change &&
-		git show refs/remotes/p4-unshelved/$change | grep -q "Further description" &&
+		git show refs/remotes/p4-unshelved/$change >actual &&
+		grep -q "Further description" actual &&
 		git cherry-pick refs/remotes/p4-unshelved/$change &&
 		test_path_is_file file2 &&
 		test_cmp file1 "$cli"/file1 &&

base-commit: a5fa49ff0a8f3252c6bff49f92b85e7683868f8a
-- 
gitgitgadget