{"thread":{"id":"62259","subject":"[PATCH 0/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","startedAt":"2024-10-06T05:33:37Z","lastAt":"2024-10-07T15:32:50Z","messageCount":63,"participants":["Usman Akinyemi via GitGitGadget","Eric Sunshine","Usman Akinyemi","shejialuo","Kristoffer Haugsbakk","Patrick Steinhardt","phillip.wood123@gmail.com","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"504149","messageId":"pull.1805.git.git.1728192814.gitgitgadget@gmail.com","threadId":"62259","inReplyTo":null,"subject":"[PATCH 0/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","fromName":"Usman Akinyemi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-06T05:33:32Z","receivedAt":"2024-10-06T05:33:37Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"Changes since v1:\n\n * Added \"tr -d '[:space:]'\" to handle whitespace on macOS\n\nSigned-off-by: Usman Akinyemi usmanakinyemi202@gmail.com\n\nUsman Akinyemi (2):\n  [Outreachy][Patch v1] t3404: avoid losing exit status to pipes\n  [Outreachy][Patch v2] t3404: avoid losing exit status to pipes\n\n t/t3404-rebase-interactive.sh | 71 +++++++++++++++++++++++------------\n 1 file changed, 48 insertions(+), 23 deletions(-)\n\n\nbase-commit: 90fe3800b92a49173530828c0a17951abd30f0e1\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1805%2FUnique-Usman%2Favoid_git_pipes-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1805/Unique-Usman/avoid_git_pipes-v1\nPull-Request: https://github.com/git/git/pull/1805\n-- \ngitgitgadget\n"},{"id":"504150","messageId":"5dd96eaf14cb6fbe130fb50ce2738646d7d3ac57.1728192814.git.gitgitgadget@gmail.com","threadId":"62259","inReplyTo":"pull.1805.git.git.1728192814.gitgitgadget@gmail.com","subject":"[PATCH 1/2] [Outreachy][Patch v1] t3404: avoid losing exit status to pipes","fromName":"Usman Akinyemi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-06T05:33:33Z","receivedAt":"2024-10-06T05:33:38Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"From: Usman Akinyemi <usmanakinyemi202@gmail.com>\n\nThe exit code of the preceding command in a pipe is disregarded. So\nif that preceding command is a Git command that fails, the test would\nnot fail. Instead, by saving the output of that Git command to a file,\nand removing the pipe, we make sure the test will fail if that Git\ncommand fails.\n\nSigned-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n---\n t/t3404-rebase-interactive.sh | 71 +++++++++++++++++++++++------------\n 1 file changed, 48 insertions(+), 23 deletions(-)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex f171af3061d..33ea1f05e2c 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -319,7 +319,8 @@ test_expect_success 'retain authorship' '\n \tGIT_AUTHOR_NAME=\"Twerp Snog\" git commit -m \"different author\" &&\n \tgit tag twerp &&\n \tgit rebase -i --onto primary HEAD^ &&\n-\tgit show HEAD | grep \"^Author: Twerp Snog\"\n+\tgit show HEAD >actual &&\n+\tgrep \"^Author: Twerp Snog\" actual\n '\n \n test_expect_success 'retain authorship w/ conflicts' '\n@@ -360,7 +361,8 @@ test_expect_success 'squash' '\n '\n \n test_expect_success 'retain authorship when squashing' '\n-\tgit show HEAD | grep \"^Author: Twerp Snog\"\n+\tgit show HEAD >actual &&\n+\tgrep \"^Author: Twerp Snog\" actual\n '\n \n test_expect_success '--continue tries to commit' '\n@@ -374,7 +376,8 @@ test_expect_success '--continue tries to commit' '\n \t\tFAKE_COMMIT_MESSAGE=\"chouette!\" git rebase --continue\n \t) &&\n \ttest_cmp_rev HEAD^ new-branch1 &&\n-\tgit show HEAD | grep chouette\n+\tgit show HEAD >actual &&\n+\tgrep chouette actual\n '\n \n test_expect_success 'verbose flag is heeded, even after --continue' '\n@@ -397,7 +400,9 @@ test_expect_success 'multi-squash only fires up editor once' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l)\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = \"$count\"\n '\n \n test_expect_success 'multi-fixup does not fire up editor' '\n@@ -410,7 +415,9 @@ test_expect_success 'multi-fixup does not fire up editor' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 0 = $(git show | grep NEVER | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep NEVER output | wc -l) &&\n+\ttest 0 = \"$count\" &&\n \tgit checkout @{-1} &&\n \tgit branch -D multi-fixup\n '\n@@ -428,7 +435,9 @@ test_expect_success 'commit message used after conflict' '\n \t\t\tgit rebase --continue\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = \"$count\" &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-fixup\n '\n@@ -446,7 +455,9 @@ test_expect_success 'commit message retained after conflict' '\n \t\t\tgit rebase --continue\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 2 = $(git show | grep TWICE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep TWICE output | wc -l) &&\n+\ttest 2 = \"$count\" &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-squash\n '\n@@ -470,10 +481,10 @@ test_expect_success 'squash and fixup generate correct log messages' '\n \t) &&\n \tgit cat-file commit HEAD | sed -e 1,/^\\$/d > actual-squash-fixup &&\n \ttest_cmp expect-squash-fixup actual-squash-fixup &&\n-\tgit cat-file commit HEAD@{2} |\n-\t\tgrep \"^# This is a combination of 3 commits\\.\"  &&\n-\tgit cat-file commit HEAD@{3} |\n-\t\tgrep \"^# This is a combination of 2 commits\\.\"  &&\n+\tgit cat-file commit HEAD@{2} >actual &&\n+\t\tgrep \"^# This is a combination of 3 commits\\.\" actual &&\n+\tgit cat-file commit HEAD@{3} >actual &&\n+\t\tgrep \"^# This is a combination of 2 commits\\.\" actual  &&\n \tgit checkout @{-1} &&\n \tgit branch -D squash-fixup\n '\n@@ -489,7 +500,9 @@ test_expect_success 'squash ignores comments' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = \"$count\" &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-comments\n '\n@@ -505,7 +518,9 @@ test_expect_success 'squash ignores blank lines' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = \"$count\" &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-blank-lines\n '\n@@ -572,7 +587,8 @@ test_expect_success '--continue tries to commit, even for \"edit\"' '\n \t\tFAKE_COMMIT_MESSAGE=\"chouette!\" git rebase --continue\n \t) &&\n \ttest edited = $(git show HEAD:file7) &&\n-\tgit show HEAD | grep chouette &&\n+\tgit show HEAD >actual &&\n+\tgrep chouette actual &&\n \ttest $parent = $(git rev-parse HEAD^)\n '\n \n@@ -757,19 +773,23 @@ test_expect_success 'reword' '\n \t\tset_fake_editor &&\n \t\tFAKE_LINES=\"1 2 3 reword 4\" FAKE_COMMIT_MESSAGE=\"E changed\" \\\n \t\t\tgit rebase -i A &&\n-\t\tgit show HEAD | grep \"E changed\" &&\n+\t\tgit show HEAD >actual &&\n+\t\tgrep \"E changed\" actual &&\n \t\ttest $(git rev-parse primary) != $(git rev-parse HEAD) &&\n \t\ttest_cmp_rev primary^ HEAD^ &&\n \t\tFAKE_LINES=\"1 2 reword 3 4\" FAKE_COMMIT_MESSAGE=\"D changed\" \\\n \t\t\tgit rebase -i A &&\n-\t\tgit show HEAD^ | grep \"D changed\" &&\n+\t\tgit show HEAD^ >actual &&\n+\t\tgrep \"D changed\" actual &&\n \t\tFAKE_LINES=\"reword 1 2 3 4\" FAKE_COMMIT_MESSAGE=\"B changed\" \\\n \t\t\tgit rebase -i A &&\n-\t\tgit show HEAD~3 | grep \"B changed\" &&\n+\t\tgit show HEAD~3 >actual &&\n+\t\tgrep \"B changed\" actual &&\n \t\tFAKE_LINES=\"1 r 2 pick 3 p 4\" FAKE_COMMIT_MESSAGE=\"C changed\" \\\n \t\t\tgit rebase -i A\n \t) &&\n-\tgit show HEAD~2 | grep \"C changed\"\n+\tgit show HEAD~2 >actual &&\n+\tgrep \"C changed\" actual\n '\n \n test_expect_success 'no uncommitted changes when rewording and the todo list is reloaded' '\n@@ -1003,8 +1023,10 @@ test_expect_success 'rebase -i --root retain root commit author and message' '\n \t\tset_fake_editor &&\n \t\tFAKE_LINES=\"2\" git rebase -i --root\n \t) &&\n-\tgit cat-file commit HEAD | grep -q \"^author Twerp Snog\" &&\n-\tgit cat-file commit HEAD | grep -q \"^different author$\"\n+\tgit cat-file commit HEAD >output &&\n+\tgrep -q \"^author Twerp Snog\" output &&\n+\tgit cat-file commit HEAD >actual &&\n+\tgrep -q \"^different author$\" actual\n '\n \n test_expect_success 'rebase -i --root temporary sentinel commit' '\n@@ -1013,7 +1035,8 @@ test_expect_success 'rebase -i --root temporary sentinel commit' '\n \t\tset_fake_editor &&\n \t\ttest_must_fail env FAKE_LINES=\"2\" git rebase -i --root\n \t) &&\n-\tgit cat-file commit HEAD | grep \"^tree $EMPTY_TREE\" &&\n+\tgit cat-file commit HEAD >actual &&\n+\tgrep \"^tree $EMPTY_TREE\" actual &&\n \tgit rebase --abort\n '\n \n@@ -1036,7 +1059,8 @@ test_expect_success 'rebase -i --root reword original root commit' '\n \t\tFAKE_LINES=\"reword 1 2\" FAKE_COMMIT_MESSAGE=\"A changed\" \\\n \t\t\tgit rebase -i --root\n \t) &&\n-\tgit show HEAD^ | grep \"A changed\" &&\n+\tgit show HEAD^ >actual &&\n+\tgrep \"A changed\" actual &&\n \ttest -z \"$(git show -s --format=%p HEAD^)\"\n '\n \n@@ -1048,7 +1072,8 @@ test_expect_success 'rebase -i --root reword new root commit' '\n \t\tFAKE_LINES=\"reword 3 1\" FAKE_COMMIT_MESSAGE=\"C changed\" \\\n \t\tgit rebase -i --root\n \t) &&\n-\tgit show HEAD^ | grep \"C changed\" &&\n+\tgit show HEAD^ >actual &&\n+\tgrep \"C changed\" actual &&\n \ttest -z \"$(git show -s --format=%p HEAD^)\"\n '\n \n-- \ngitgitgadget\n\n"},{"id":"504151","messageId":"4199434bd6ef2142192d1c720ccd877b1a80536b.1728192815.git.gitgitgadget@gmail.com","threadId":"62259","inReplyTo":"pull.1805.git.git.1728192814.gitgitgadget@gmail.com","subject":"[PATCH 2/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","fromName":"Usman Akinyemi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-06T05:33:34Z","receivedAt":"2024-10-06T05:33:39Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"From: Usman Akinyemi <usmanakinyemi202@gmail.com>\n\nChanges since v1:\n- Added \"tr -d '[:space:]'\" to handle whitespace on macOS\n\nSigned-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n---\n t/t3404-rebase-interactive.sh | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 33ea1f05e2c..3d8de69d6ee 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -401,7 +401,7 @@ test_expect_success 'multi-squash only fires up editor once' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep ONCE output | wc -l) &&\n+\tcount=$(grep ONCE output | wc -l | tr -d '[:space:]') &&\n \ttest 1 = \"$count\"\n '\n \n@@ -416,7 +416,7 @@ test_expect_success 'multi-fixup does not fire up editor' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep NEVER output | wc -l) &&\n+\tcount=$(grep NEVER output | wc -l | tr -d '[:space:]') &&\n \ttest 0 = \"$count\" &&\n \tgit checkout @{-1} &&\n \tgit branch -D multi-fixup\n@@ -436,7 +436,7 @@ test_expect_success 'commit message used after conflict' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep ONCE output | wc -l) &&\n+\tcount=$(grep ONCE output | wc -l | tr -d '[:space:]') &&\n \ttest 1 = \"$count\" &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-fixup\n@@ -456,7 +456,7 @@ test_expect_success 'commit message retained after conflict' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep TWICE output | wc -l) &&\n+\tcount=$(grep TWICE output | wc -l | tr -d '[:space:]') &&\n \ttest 2 = \"$count\" &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-squash\n@@ -501,7 +501,7 @@ test_expect_success 'squash ignores comments' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep ONCE output | wc -l) &&\n+\tcount=$(grep ONCE output | wc -l | tr -d '[:space:]') &&\n \ttest 1 = \"$count\" &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-comments\n@@ -519,7 +519,7 @@ test_expect_success 'squash ignores blank lines' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep ONCE output | wc -l) &&\n+\tcount=$(grep ONCE output | wc -l | tr -d '[:space:]') &&\n \ttest 1 = \"$count\" &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-blank-lines\n-- \ngitgitgadget\n"},{"id":"504153","messageId":"CAPig+cTb4mgpXnN79UrXvjvCnqGZhaR51oZX_Ds=HwdqQYFN9w@mail.gmail.com","threadId":"62259","inReplyTo":"4199434bd6ef2142192d1c720ccd877b1a80536b.1728192815.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-10-06T05:48:38Z","receivedAt":"2024-10-06T05:48:50Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Oct 6, 2024 at 1:33 AM Usman Akinyemi via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n> Changes since v1:\n> - Added \"tr -d '[:space:]'\" to handle whitespace on macOS\n>\n> Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> ---\n\nThanks for the submission. A few comments...\n\nThis second patch fixes problems with the first patch, but since this\nis an entirely new submission, you should instead \"squash\" these two\npatches together and then force-push them to the same branch that you\nused when submitting them via GitGitGadget, and re-submit them as a\nsingle patch. When you squash them, keep the commit message from the\nfirst patch.\n\nReviewers do appreciate that you explained what changed since the\nprevious version, but we'd like to see that information as commentary\nin the patch cover letter, not as the commit message of the patch\nitself. In GitGitGadget, the way you would do so is to write this as\nthe \"Description\" of the pull-request (possibly replacing or amending\nthe previous description).\n\nSome more observations below...\n\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> @@ -401,7 +401,7 @@ test_expect_success 'multi-squash only fires up editor once' '\n>         git show >output &&\n> -       count=$(grep ONCE output | wc -l) &&\n> +       count=$(grep ONCE output | wc -l | tr -d '[:space:]') &&\n>         test 1 = \"$count\"\n>  '\n\nThe reason this was failing for you was because you quoted $count. Had\nyou instead written:\n\n    test 1 = $count\n\nwhen it would have worked as expected. In other words, you don't need `tr`.\n\nThese days, instead of manually using `wc -l` and `test`, we would\ninstead write:\n\n    grep ONCE output >actual &&\n    test_line_count 1 actual\n\nHowever, that sort of change is independent of the purpose of this\npatch, so you probably should not make such a change in this patch. If\nyou're up to it, you could instead turn this into a two-patch series\nin which patch [1/2] fixes the \"Git upstream of a pipe\" problem, and\nthen patch [2/2] converts these cases to use test_line_count().\n"},{"id":"504155","messageId":"CAPig+cTO5_mDyEX=druLh6HkFazi=SBmDser8GdsVQP4iVaKug@mail.gmail.com","threadId":"62259","inReplyTo":"5dd96eaf14cb6fbe130fb50ce2738646d7d3ac57.1728192814.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] [Outreachy][Patch v1] t3404: avoid losing exit status to pipes","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-10-06T05:55:08Z","receivedAt":"2024-10-06T05:55:19Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Oct 6, 2024 at 1:33 AM Usman Akinyemi via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n> The exit code of the preceding command in a pipe is disregarded. So\n> if that preceding command is a Git command that fails, the test would\n> not fail. Instead, by saving the output of that Git command to a file,\n> and removing the pipe, we make sure the test will fail if that Git\n> command fails.\n\nOkay, makes sense.\n\nOne minor style comment below...\n\n> Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> ---\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> @@ -470,10 +481,10 @@ test_expect_success 'squash and fixup generate correct log messages' '\n> -       git cat-file commit HEAD@{2} |\n> -               grep \"^# This is a combination of 3 commits\\.\"  &&\n> -       git cat-file commit HEAD@{3} |\n> -               grep \"^# This is a combination of 2 commits\\.\"  &&\n> +       git cat-file commit HEAD@{2} >actual &&\n> +               grep \"^# This is a combination of 3 commits\\.\" actual &&\n> +       git cat-file commit HEAD@{3} >actual &&\n> +               grep \"^# This is a combination of 2 commits\\.\" actual  &&\n\nWe wouldn't normally indent the `grep` line like this. Now that you've\ndropped the patch, it would be best to lose the extra indentation, as\nwell:\n\n    git cat-file commit HEAD@{2} >actual &&\n    grep \"^# This is a combination of 3 commits\\.\" actual &&\n    ...\n"},{"id":"504165","messageId":"CAPSxiM_RKpp2_Y7HqhhFJqnavKpToViAjE3s6AiwCwUjGa8H4g@mail.gmail.com","threadId":"62259","inReplyTo":"CAPig+cTb4mgpXnN79UrXvjvCnqGZhaR51oZX_Ds=HwdqQYFN9w@mail.gmail.com","subject":"Re: [PATCH 2/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2024-10-06T06:30:47Z","receivedAt":"2024-10-06T06:30:59Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"On Sun, Oct 6, 2024 at 5:48 AM Eric Sunshine <sunshine@sunshineco.com> wrote:\n>\n> On Sun, Oct 6, 2024 at 1:33 AM Usman Akinyemi via GitGitGadget\n> <gitgitgadget@gmail.com> wrote:\n> > Changes since v1:\n> > - Added \"tr -d '[:space:]'\" to handle whitespace on macOS\n> >\n> > Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> > ---\n>\n> Thanks for the submission. A few comments...\n>\n> This second patch fixes problems with the first patch, but since this\n> is an entirely new submission, you should instead \"squash\" these two\n> patches together and then force-push them to the same branch that you\n> used when submitting them via GitGitGadget, and re-submit them as a\n> single patch. When you squash them, keep the commit message from the\n> first patch.\n>\n> Reviewers do appreciate that you explained what changed since the\n> previous version, but we'd like to see that information as commentary\n> in the patch cover letter, not as the commit message of the patch\n> itself. In GitGitGadget, the way you would do so is to write this as\n> the \"Description\" of the pull-request (possibly replacing or amending\n> the previous description).\n>\n> Some more observations below...\n>\n> > diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> > @@ -401,7 +401,7 @@ test_expect_success 'multi-squash only fires up editor once' '\n> >         git show >output &&\n> > -       count=$(grep ONCE output | wc -l) &&\n> > +       count=$(grep ONCE output | wc -l | tr -d '[:space:]') &&\n> >         test 1 = \"$count\"\n> >  '\n>\n> The reason this was failing for you was because you quoted $count. Had\n> you instead written:\n>\n>     test 1 = $count\n>\n> when it would have worked as expected. In other words, you don't need `tr`.\n>\n> These days, instead of manually using `wc -l` and `test`, we would\n> instead write:\n>\n>     grep ONCE output >actual &&\n>     test_line_count 1 actual\n>\n> However, that sort of change is independent of the purpose of this\n> patch, so you probably should not make such a change in this patch. If\n> you're up to it, you could instead turn this into a two-patch series\n> in which patch [1/2] fixes the \"Git upstream of a pipe\" problem, and\n> then patch [2/2] converts these cases to use test_line_count().\n\nHi  Eric Sunshine,\nthanks for the review. I really appreciate it. I have a couple of\ndoubts to clear.\n\nMy next patch should be the squash of my three patches which include\nmy first two patches and the new one on the same branch ?\nTwo patch series means two different commits on different patches ?\nBut, since they both depend on each other would not they lead to merge\nconflict ?\nAlso, to be clear, \"Description\" is the body of the commit message if\nI use the gitgitgadget while the \"commit message\" is the header ?\n\nThank you.\n"},{"id":"504166","messageId":"CAPSxiM9eJafyL4NiKHCjR83ZpL8FA_6sf=SYBjqEXP6tFrTzRw@mail.gmail.com","threadId":"62259","inReplyTo":"CAPig+cTO5_mDyEX=druLh6HkFazi=SBmDser8GdsVQP4iVaKug@mail.gmail.com","subject":"Re: [PATCH 1/2] [Outreachy][Patch v1] t3404: avoid losing exit status to pipes","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2024-10-06T06:31:12Z","receivedAt":"2024-10-06T06:31:23Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"On Sun, Oct 6, 2024 at 5:55 AM Eric Sunshine <sunshine@sunshineco.com> wrote:\n>\n> On Sun, Oct 6, 2024 at 1:33 AM Usman Akinyemi via GitGitGadget\n> <gitgitgadget@gmail.com> wrote:\n> > The exit code of the preceding command in a pipe is disregarded. So\n> > if that preceding command is a Git command that fails, the test would\n> > not fail. Instead, by saving the output of that Git command to a file,\n> > and removing the pipe, we make sure the test will fail if that Git\n> > command fails.\n>\n> Okay, makes sense.\n>\n> One minor style comment below...\n>\n> > Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> > ---\n> > diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> > @@ -470,10 +481,10 @@ test_expect_success 'squash and fixup generate correct log messages' '\n> > -       git cat-file commit HEAD@{2} |\n> > -               grep \"^# This is a combination of 3 commits\\.\"  &&\n> > -       git cat-file commit HEAD@{3} |\n> > -               grep \"^# This is a combination of 2 commits\\.\"  &&\n> > +       git cat-file commit HEAD@{2} >actual &&\n> > +               grep \"^# This is a combination of 3 commits\\.\" actual &&\n> > +       git cat-file commit HEAD@{3} >actual &&\n> > +               grep \"^# This is a combination of 2 commits\\.\" actual  &&\n>\n> We wouldn't normally indent the `grep` line like this. Now that you've\n> dropped the patch, it would be best to lose the extra indentation, as\n> well:\n>\n>     git cat-file commit HEAD@{2} >actual &&\n>     grep \"^# This is a combination of 3 commits\\.\" actual &&\n>     ...\n\nNoted and thanks.\n"},{"id":"504167","messageId":"CAPig+cTkj-wXqgLNM5hyKa9c3U3AT6L2oUpdbTg7nGUHpa=1Vg@mail.gmail.com","threadId":"62259","inReplyTo":"CAPSxiM_RKpp2_Y7HqhhFJqnavKpToViAjE3s6AiwCwUjGa8H4g@mail.gmail.com","subject":"Re: [PATCH 2/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-10-06T07:28:24Z","receivedAt":"2024-10-06T07:28:36Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Oct 6, 2024 at 2:31 AM Usman Akinyemi\n<usmanakinyemi202@gmail.com> wrote:\n> On Sun, Oct 6, 2024 at 5:48 AM Eric Sunshine <sunshine@sunshineco.com> wrote:\n> > This second patch fixes problems with the first patch, but since this\n> > is an entirely new submission, you should instead \"squash\" these two\n> > patches together and then force-push them to the same branch that you\n> > used when submitting them via GitGitGadget, and re-submit them as a\n> > single patch. When you squash them, keep the commit message from the\n> > first patch.\n> >\n> > Reviewers do appreciate that you explained what changed since the\n> > previous version, but we'd like to see that information as commentary\n> > in the patch cover letter, not as the commit message of the patch\n> > itself. In GitGitGadget, the way you would do so is to write this as\n> > the \"Description\" of the pull-request (possibly replacing or amending\n> > the previous description).\n> >\n> > These days, instead of manually using `wc -l` and `test`, we would\n> > instead write:\n> >\n> >     grep ONCE output >actual &&\n> >     test_line_count 1 actual\n> >\n> > However, that sort of change is independent of the purpose of this\n> > patch, so you probably should not make such a change in this patch. If\n> > you're up to it, you could instead turn this into a two-patch series\n> > in which patch [1/2] fixes the \"Git upstream of a pipe\" problem, and\n> > then patch [2/2] converts these cases to use test_line_count().\n>\n> thanks for the review. I really appreciate it. I have a couple of\n> doubts to clear.\n>\n> My next patch should be the squash of my three patches which include\n> my first two patches and the new one on the same branch ?\n\nI'm not sure which three patches you mean. Does the third patch, the\n\"new one on the same branch\", fix problems from the earlier two\npatches? Or does your third patch implement the suggestion regarding\ntest_line_count()?\n\n> Two patch series means two different commits on different patches ?\n> But, since they both depend on each other would not they lead to merge\n> conflict ?\n\nA patch series consists of one or more patches in sequence. Patches\nwithin a series don't conflict with one another; later patches build\nupon earlier patches. You create a series of commits on a single Git\nbranch. When you submit that branch as a pull-request via\nGitGitGadget, it turns the commits on that branch into a series of\npatch emails to the Git mailing list, one per commit.\n\nBefore submitting the patches via GitGitGadget, you polish them\nlocally to repair any problems before submitting them for review.\nRather than making subsequent commits fix problems with earlier\ncommits, you instead should fix the bad commits by using \"git rebase\n--interactive ...\" to either \"fixup\", \"squash\", or otherwise edit the\nbad commits. Once you are happy with the commits, submit them. This\nway, reviewers only see your polished result, not the series of steps\nyou made to arrive at the polished results.\n\nYou do the same thing after reviewers point out problems or ask for\nchanges. Edit and re-polish the existing commits to address reviewer\ncomments rather than merely making new commits on top of the existing\ncommits, and then resubmit once all the fixes have been applied and\npolished.\n\nWhen I suggested squashing your two original commits it was for the\nabove reason. In your original submission, patch [1/2] had some\nproblems which you fixed in patch [2/2], but reviewers don't need or\nwant to see that; they just want to see the polished end-result, which\nyou can obtain by squashing the two patches together. (However, in\nthis case, as I pointed out in my review, you don't even need to use\n`tr`; just use `test 1 = $count` instead of `test 1 = \"$count\"`.)\n\nIf you wanted to do the extra step of also updating the tests to use\ntest_line_count(), then that would be a separate patch, still on the\nsame branch, built on top of your \"fix Git upstream of pipe\" patch.\nThus, it would become a two-patch series: patch [1/2] fixing Git\nupstream of a pipe, and [2/2] employing test_line_count().\n\n> Also, to be clear, \"Description\" is the body of the commit message if\n> I use the gitgitgadget while the \"commit message\" is the header ?\n\nThe commit message is separate from the patch-series commentary. The\ncommit message of each patch explains what that patch changes or does.\n\nOnce you have polished your commit(s), force-push them to the\nGitGitGadget pull-request you already created. Then edit the very\nfirst (topmost) comment in the pull-request to explain what the patch\nseries is about and what you changed since the previous version. That\ncomment becomes the \"commentary\" portion of the patch series.\n"},{"id":"504169","messageId":"CAPSxiM99CoYzC04-OKRdmddHv2_=9GBgTCJeSEACZ=jQm5gLsA@mail.gmail.com","threadId":"62259","inReplyTo":"CAPig+cTkj-wXqgLNM5hyKa9c3U3AT6L2oUpdbTg7nGUHpa=1Vg@mail.gmail.com","subject":"Re: [PATCH 2/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2024-10-06T08:26:44Z","receivedAt":"2024-10-06T08:26:57Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"On Sun, Oct 6, 2024 at 7:28 AM Eric Sunshine <sunshine@sunshineco.com> wrote:\n>\n> On Sun, Oct 6, 2024 at 2:31 AM Usman Akinyemi\n> <usmanakinyemi202@gmail.com> wrote:\n> > On Sun, Oct 6, 2024 at 5:48 AM Eric Sunshine <sunshine@sunshineco.com> wrote:\n> > > This second patch fixes problems with the first patch, but since this\n> > > is an entirely new submission, you should instead \"squash\" these two\n> > > patches together and then force-push them to the same branch that you\n> > > used when submitting them via GitGitGadget, and re-submit them as a\n> > > single patch. When you squash them, keep the commit message from the\n> > > first patch.\n> > >\n> > > Reviewers do appreciate that you explained what changed since the\n> > > previous version, but we'd like to see that information as commentary\n> > > in the patch cover letter, not as the commit message of the patch\n> > > itself. In GitGitGadget, the way you would do so is to write this as\n> > > the \"Description\" of the pull-request (possibly replacing or amending\n> > > the previous description).\n> > >\n> > > These days, instead of manually using `wc -l` and `test`, we would\n> > > instead write:\n> > >\n> > >     grep ONCE output >actual &&\n> > >     test_line_count 1 actual\n> > >\n> > > However, that sort of change is independent of the purpose of this\n> > > patch, so you probably should not make such a change in this patch. If\n> > > you're up to it, you could instead turn this into a two-patch series\n> > > in which patch [1/2] fixes the \"Git upstream of a pipe\" problem, and\n> > > then patch [2/2] converts these cases to use test_line_count().\n> >\n> > thanks for the review. I really appreciate it. I have a couple of\n> > doubts to clear.\n> >\n> > My next patch should be the squash of my three patches which include\n> > my first two patches and the new one on the same branch ?\n>\n> I'm not sure which three patches you mean. Does the third patch, the\n> \"new one on the same branch\", fix problems from the earlier two\n> patches? Or does your third patch implement the suggestion regarding\n> test_line_count()?\n>\nThanks, I really appreciate your explanation. Thank you very much. By\nthe third patch, I meant the new one which I will be adding.\n\n> > Two patch series means two different commits on different patches ?\n> > But, since they both depend on each other would not they lead to merge\n> > conflict ?\n>\n> A patch series consists of one or more patches in sequence. Patches\n> within a series don't conflict with one another; later patches build\n> upon earlier patches. You create a series of commits on a single Git\n> branch. When you submit that branch as a pull-request via\n> GitGitGadget, it turns the commits on that branch into a series of\n> patch emails to the Git mailing list, one per commit.\n>\n> Before submitting the patches via GitGitGadget, you polish them\n> locally to repair any problems before submitting them for review.\n> Rather than making subsequent commits fix problems with earlier\n> commits, you instead should fix the bad commits by using \"git rebase\n> --interactive ...\" to either \"fixup\", \"squash\", or otherwise edit the\n> bad commits. Once you are happy with the commits, submit them. This\n> way, reviewers only see your polished result, not the series of steps\n> you made to arrive at the polished results.\n>\n> You do the same thing after reviewers point out problems or ask for\n> changes. Edit and re-polish the existing commits to address reviewer\n> comments rather than merely making new commits on top of the existing\n> commits, and then resubmit once all the fixes have been applied and\n> polished.\n>\n> When I suggested squashing your two original commits it was for the\n> above reason. In your original submission, patch [1/2] had some\n> problems which you fixed in patch [2/2], but reviewers don't need or\n> want to see that; they just want to see the polished end-result, which\n> you can obtain by squashing the two patches together. (However, in\n> this case, as I pointed out in my review, you don't even need to use\n> `tr`; just use `test 1 = $count` instead of `test 1 = \"$count\"`.)\n>\n> If you wanted to do the extra step of also updating the tests to use\n> test_line_count(), then that would be a separate patch, still on the\n> same branch, built on top of your \"fix Git upstream of pipe\" patch.\n> Thus, it would become a two-patch series: patch [1/2] fixing Git\n> upstream of a pipe, and [2/2] employing test_line_count().\n>\nThanks for this. I will submit the first patch, get feedback then\napproach the second one.\n> > Also, to be clear, \"Description\" is the body of the commit message if\n> > I use the gitgitgadget while the \"commit message\" is the header ?\n>\n> The commit message is separate from the patch-series commentary. The\n> commit message of each patch explains what that patch changes or does.\n>\n> Once you have polished your commit(s), force-push them to the\n> GitGitGadget pull-request you already created. Then edit the very\n> first (topmost) comment in the pull-request to explain what the patch\n> series is about and what you changed since the previous version. That\n> comment becomes the \"commentary\" portion of the patch series.\n"},{"id":"504170","messageId":"pull.1805.v2.git.git.1728203495287.gitgitgadget@gmail.com","threadId":"62259","inReplyTo":"pull.1805.git.git.1728192814.gitgitgadget@gmail.com","subject":"[PATCH v2] [Outreachy][Patch v1] t3404: avoid losing exit status to pipes","fromName":"Usman Akinyemi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-06T08:31:35Z","receivedAt":"2024-10-06T08:31:38Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"From: Usman Akinyemi <usmanakinyemi202@gmail.com>\n\nThe exit code of the preceding command in a pipe is disregarded. So\nif that preceding command is a Git command that fails, the test would\nnot fail. Instead, by saving the output of that Git command to a file,\nand removing the pipe, we make sure the test will fail if that Git\ncommand fails.\n\nSigned-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n---\n    [Outreachy][Patch v2] t3404: avoid losing exit status to pipes\n    \n    At the beginning of my task, I made the mistake of submitting two\n    patches for two separate commits instead of one. The first patch\n    addressed the issue of losing the Git exit status due to pipes.\n    \n    After submitting the first patch, I noticed that the output of wc -l was\n    failing due to trailing whitespace. I attempted to fix this by using tr\n    -d to remove the whitespace. However, instead of squashing the two\n    patches into one, I inadvertently created another commit.\n    \n    Eric Sunshine sunshine@sunshineco.com provided valuable feedback during\n    the review process. He explained the details of the patches to me and\n    pointed out that using tr -d was unnecessary to resolve the whitespace\n    issue.\n    \n    The root cause of the whitespace issue was quoting $count in the test\n    command, which led to the inclusion of whitespace in the comparison. By\n    removing the quotes around $count, the comparison works as expected\n    without the need for tr -d.\n    \n    Signed-off-by: Usman Akinyemi\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1805%2FUnique-Usman%2Favoid_git_pipes-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1805/Unique-Usman/avoid_git_pipes-v2\nPull-Request: https://github.com/git/git/pull/1805\n\nRange-diff vs v1:\n\n 1:  5dd96eaf14c ! 1:  be5a691e96f [Outreachy][Patch v1] t3404: avoid losing exit status to pipes\n     @@ t/t3404-rebase-interactive.sh: test_expect_success 'multi-squash only fires up e\n      -\ttest 1 = $(git show | grep ONCE | wc -l)\n      +\tgit show >output &&\n      +\tcount=$(grep ONCE output | wc -l) &&\n     -+\ttest 1 = \"$count\"\n     ++\ttest 1 = $count\n       '\n       \n       test_expect_success 'multi-fixup does not fire up editor' '\n     @@ t/t3404-rebase-interactive.sh: test_expect_success 'multi-fixup does not fire up\n      -\ttest 0 = $(git show | grep NEVER | wc -l) &&\n      +\tgit show >output &&\n      +\tcount=$(grep NEVER output | wc -l) &&\n     -+\ttest 0 = \"$count\" &&\n     ++\ttest 0 = $count &&\n       \tgit checkout @{-1} &&\n       \tgit branch -D multi-fixup\n       '\n     @@ t/t3404-rebase-interactive.sh: test_expect_success 'commit message used after co\n      -\ttest 1 = $(git show | grep ONCE | wc -l) &&\n      +\tgit show >output &&\n      +\tcount=$(grep ONCE output | wc -l) &&\n     -+\ttest 1 = \"$count\" &&\n     ++\ttest 1 = $count &&\n       \tgit checkout @{-1} &&\n       \tgit branch -D conflict-fixup\n       '\n     @@ t/t3404-rebase-interactive.sh: test_expect_success 'commit message retained afte\n      -\ttest 2 = $(git show | grep TWICE | wc -l) &&\n      +\tgit show >output &&\n      +\tcount=$(grep TWICE output | wc -l) &&\n     -+\ttest 2 = \"$count\" &&\n     ++\ttest 2 = $count &&\n       \tgit checkout @{-1} &&\n       \tgit branch -D conflict-squash\n       '\n     @@ t/t3404-rebase-interactive.sh: test_expect_success 'squash and fixup generate co\n      -\tgit cat-file commit HEAD@{3} |\n      -\t\tgrep \"^# This is a combination of 2 commits\\.\"  &&\n      +\tgit cat-file commit HEAD@{2} >actual &&\n     -+\t\tgrep \"^# This is a combination of 3 commits\\.\" actual &&\n     ++\tgrep \"^# This is a combination of 3 commits\\.\" actual &&\n      +\tgit cat-file commit HEAD@{3} >actual &&\n     -+\t\tgrep \"^# This is a combination of 2 commits\\.\" actual  &&\n     ++\tgrep \"^# This is a combination of 2 commits\\.\" actual  &&\n       \tgit checkout @{-1} &&\n       \tgit branch -D squash-fixup\n       '\n     @@ t/t3404-rebase-interactive.sh: test_expect_success 'squash ignores comments' '\n      -\ttest 1 = $(git show | grep ONCE | wc -l) &&\n      +\tgit show >output &&\n      +\tcount=$(grep ONCE output | wc -l) &&\n     -+\ttest 1 = \"$count\" &&\n     ++\ttest 1 = $count &&\n       \tgit checkout @{-1} &&\n       \tgit branch -D skip-comments\n       '\n     @@ t/t3404-rebase-interactive.sh: test_expect_success 'squash ignores blank lines'\n      -\ttest 1 = $(git show | grep ONCE | wc -l) &&\n      +\tgit show >output &&\n      +\tcount=$(grep ONCE output | wc -l) &&\n     -+\ttest 1 = \"$count\" &&\n     ++\ttest 1 = $count &&\n       \tgit checkout @{-1} &&\n       \tgit branch -D skip-blank-lines\n       '\n 2:  4199434bd6e < -:  ----------- [Outreachy][Patch v2] t3404: avoid losing exit status to pipes\n\n\n t/t3404-rebase-interactive.sh | 71 +++++++++++++++++++++++------------\n 1 file changed, 48 insertions(+), 23 deletions(-)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex f171af3061d..96a65783c47 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -319,7 +319,8 @@ test_expect_success 'retain authorship' '\n \tGIT_AUTHOR_NAME=\"Twerp Snog\" git commit -m \"different author\" &&\n \tgit tag twerp &&\n \tgit rebase -i --onto primary HEAD^ &&\n-\tgit show HEAD | grep \"^Author: Twerp Snog\"\n+\tgit show HEAD >actual &&\n+\tgrep \"^Author: Twerp Snog\" actual\n '\n \n test_expect_success 'retain authorship w/ conflicts' '\n@@ -360,7 +361,8 @@ test_expect_success 'squash' '\n '\n \n test_expect_success 'retain authorship when squashing' '\n-\tgit show HEAD | grep \"^Author: Twerp Snog\"\n+\tgit show HEAD >actual &&\n+\tgrep \"^Author: Twerp Snog\" actual\n '\n \n test_expect_success '--continue tries to commit' '\n@@ -374,7 +376,8 @@ test_expect_success '--continue tries to commit' '\n \t\tFAKE_COMMIT_MESSAGE=\"chouette!\" git rebase --continue\n \t) &&\n \ttest_cmp_rev HEAD^ new-branch1 &&\n-\tgit show HEAD | grep chouette\n+\tgit show HEAD >actual &&\n+\tgrep chouette actual\n '\n \n test_expect_success 'verbose flag is heeded, even after --continue' '\n@@ -397,7 +400,9 @@ test_expect_success 'multi-squash only fires up editor once' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l)\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = $count\n '\n \n test_expect_success 'multi-fixup does not fire up editor' '\n@@ -410,7 +415,9 @@ test_expect_success 'multi-fixup does not fire up editor' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 0 = $(git show | grep NEVER | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep NEVER output | wc -l) &&\n+\ttest 0 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D multi-fixup\n '\n@@ -428,7 +435,9 @@ test_expect_success 'commit message used after conflict' '\n \t\t\tgit rebase --continue\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-fixup\n '\n@@ -446,7 +455,9 @@ test_expect_success 'commit message retained after conflict' '\n \t\t\tgit rebase --continue\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 2 = $(git show | grep TWICE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep TWICE output | wc -l) &&\n+\ttest 2 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-squash\n '\n@@ -470,10 +481,10 @@ test_expect_success 'squash and fixup generate correct log messages' '\n \t) &&\n \tgit cat-file commit HEAD | sed -e 1,/^\\$/d > actual-squash-fixup &&\n \ttest_cmp expect-squash-fixup actual-squash-fixup &&\n-\tgit cat-file commit HEAD@{2} |\n-\t\tgrep \"^# This is a combination of 3 commits\\.\"  &&\n-\tgit cat-file commit HEAD@{3} |\n-\t\tgrep \"^# This is a combination of 2 commits\\.\"  &&\n+\tgit cat-file commit HEAD@{2} >actual &&\n+\tgrep \"^# This is a combination of 3 commits\\.\" actual &&\n+\tgit cat-file commit HEAD@{3} >actual &&\n+\tgrep \"^# This is a combination of 2 commits\\.\" actual  &&\n \tgit checkout @{-1} &&\n \tgit branch -D squash-fixup\n '\n@@ -489,7 +500,9 @@ test_expect_success 'squash ignores comments' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-comments\n '\n@@ -505,7 +518,9 @@ test_expect_success 'squash ignores blank lines' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-blank-lines\n '\n@@ -572,7 +587,8 @@ test_expect_success '--continue tries to commit, even for \"edit\"' '\n \t\tFAKE_COMMIT_MESSAGE=\"chouette!\" git rebase --continue\n \t) &&\n \ttest edited = $(git show HEAD:file7) &&\n-\tgit show HEAD | grep chouette &&\n+\tgit show HEAD >actual &&\n+\tgrep chouette actual &&\n \ttest $parent = $(git rev-parse HEAD^)\n '\n \n@@ -757,19 +773,23 @@ test_expect_success 'reword' '\n \t\tset_fake_editor &&\n \t\tFAKE_LINES=\"1 2 3 reword 4\" FAKE_COMMIT_MESSAGE=\"E changed\" \\\n \t\t\tgit rebase -i A &&\n-\t\tgit show HEAD | grep \"E changed\" &&\n+\t\tgit show HEAD >actual &&\n+\t\tgrep \"E changed\" actual &&\n \t\ttest $(git rev-parse primary) != $(git rev-parse HEAD) &&\n \t\ttest_cmp_rev primary^ HEAD^ &&\n \t\tFAKE_LINES=\"1 2 reword 3 4\" FAKE_COMMIT_MESSAGE=\"D changed\" \\\n \t\t\tgit rebase -i A &&\n-\t\tgit show HEAD^ | grep \"D changed\" &&\n+\t\tgit show HEAD^ >actual &&\n+\t\tgrep \"D changed\" actual &&\n \t\tFAKE_LINES=\"reword 1 2 3 4\" FAKE_COMMIT_MESSAGE=\"B changed\" \\\n \t\t\tgit rebase -i A &&\n-\t\tgit show HEAD~3 | grep \"B changed\" &&\n+\t\tgit show HEAD~3 >actual &&\n+\t\tgrep \"B changed\" actual &&\n \t\tFAKE_LINES=\"1 r 2 pick 3 p 4\" FAKE_COMMIT_MESSAGE=\"C changed\" \\\n \t\t\tgit rebase -i A\n \t) &&\n-\tgit show HEAD~2 | grep \"C changed\"\n+\tgit show HEAD~2 >actual &&\n+\tgrep \"C changed\" actual\n '\n \n test_expect_success 'no uncommitted changes when rewording and the todo list is reloaded' '\n@@ -1003,8 +1023,10 @@ test_expect_success 'rebase -i --root retain root commit author and message' '\n \t\tset_fake_editor &&\n \t\tFAKE_LINES=\"2\" git rebase -i --root\n \t) &&\n-\tgit cat-file commit HEAD | grep -q \"^author Twerp Snog\" &&\n-\tgit cat-file commit HEAD | grep -q \"^different author$\"\n+\tgit cat-file commit HEAD >output &&\n+\tgrep -q \"^author Twerp Snog\" output &&\n+\tgit cat-file commit HEAD >actual &&\n+\tgrep -q \"^different author$\" actual\n '\n \n test_expect_success 'rebase -i --root temporary sentinel commit' '\n@@ -1013,7 +1035,8 @@ test_expect_success 'rebase -i --root temporary sentinel commit' '\n \t\tset_fake_editor &&\n \t\ttest_must_fail env FAKE_LINES=\"2\" git rebase -i --root\n \t) &&\n-\tgit cat-file commit HEAD | grep \"^tree $EMPTY_TREE\" &&\n+\tgit cat-file commit HEAD >actual &&\n+\tgrep \"^tree $EMPTY_TREE\" actual &&\n \tgit rebase --abort\n '\n \n@@ -1036,7 +1059,8 @@ test_expect_success 'rebase -i --root reword original root commit' '\n \t\tFAKE_LINES=\"reword 1 2\" FAKE_COMMIT_MESSAGE=\"A changed\" \\\n \t\t\tgit rebase -i --root\n \t) &&\n-\tgit show HEAD^ | grep \"A changed\" &&\n+\tgit show HEAD^ >actual &&\n+\tgrep \"A changed\" actual &&\n \ttest -z \"$(git show -s --format=%p HEAD^)\"\n '\n \n@@ -1048,7 +1072,8 @@ test_expect_success 'rebase -i --root reword new root commit' '\n \t\tFAKE_LINES=\"reword 3 1\" FAKE_COMMIT_MESSAGE=\"C changed\" \\\n \t\tgit rebase -i --root\n \t) &&\n-\tgit show HEAD^ | grep \"C changed\" &&\n+\tgit show HEAD^ >actual &&\n+\tgrep \"C changed\" actual &&\n \ttest -z \"$(git show -s --format=%p HEAD^)\"\n '\n \n\nbase-commit: 90fe3800b92a49173530828c0a17951abd30f0e1\n-- \ngitgitgadget\n"},{"id":"504173","messageId":"CAPig+cRePbX6OyE6WhFp1GEZenZyC7HFeA3188F_nErt7Gin6A@mail.gmail.com","threadId":"62259","inReplyTo":"pull.1805.v2.git.git.1728203495287.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] [Outreachy][Patch v1] t3404: avoid losing exit status to pipes","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-10-06T09:19:13Z","receivedAt":"2024-10-06T09:19:25Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Oct 6, 2024 at 4:31 AM Usman Akinyemi via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n> The exit code of the preceding command in a pipe is disregarded. So\n> if that preceding command is a Git command that fails, the test would\n> not fail. Instead, by saving the output of that Git command to a file,\n> and removing the pipe, we make sure the test will fail if that Git\n> command fails.\n>\n> Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> ---\n>     After submitting the first patch, I noticed that the output of wc -l was\n>     failing due to trailing whitespace. I attempted to fix this by using tr\n>     -d to remove the whitespace. However, instead of squashing the two\n>     patches into one, I inadvertently created another commit.\n>\n>     Eric Sunshine sunshine@sunshineco.com provided valuable feedback during\n>     the review process. He explained the details of the patches to me and\n>     pointed out that using tr -d was unnecessary to resolve the whitespace\n>     issue.\n\nThanks. This version of the patch looks fine.\n\nI notice that there are still quite a few instances of `git` upstream\nof a pipe remaining in t3404 even after this patch. But that's okay;\nfixing all of them would have made the patch far longer and more\ntiresome to review, so it is not a problem that you selectively\nconverted only `git show` and `git cat-file` cases. It probably would\nhave been helpful to reviewers if the patch's commit message mentioned\nthat it only converts some of the instances, but it's not worth\nrerolling the patch just for that.\n\nSo, I think it makes sense to stop here and consider this microproject\nsuccessful (unless some other reviewer notices some problem or\nrequests some other change).\n"},{"id":"504174","messageId":"CAPSxiM-=wtsd_XH3FY+C+1V_MwNkiMtRDwOKQH4r+6Cfj5Vydw@mail.gmail.com","threadId":"62259","inReplyTo":"CAPig+cRePbX6OyE6WhFp1GEZenZyC7HFeA3188F_nErt7Gin6A@mail.gmail.com","subject":"Re: [PATCH v2] [Outreachy][Patch v1] t3404: avoid losing exit status to pipes","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2024-10-06T09:32:21Z","receivedAt":"2024-10-06T09:32:33Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"On Sun, Oct 6, 2024 at 9:19 AM Eric Sunshine <sunshine@sunshineco.com> wrote:\n>\n> On Sun, Oct 6, 2024 at 4:31 AM Usman Akinyemi via GitGitGadget\n> <gitgitgadget@gmail.com> wrote:\n> > The exit code of the preceding command in a pipe is disregarded. So\n> > if that preceding command is a Git command that fails, the test would\n> > not fail. Instead, by saving the output of that Git command to a file,\n> > and removing the pipe, we make sure the test will fail if that Git\n> > command fails.\n> >\n> > Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> > ---\n> >     After submitting the first patch, I noticed that the output of wc -l was\n> >     failing due to trailing whitespace. I attempted to fix this by using tr\n> >     -d to remove the whitespace. However, instead of squashing the two\n> >     patches into one, I inadvertently created another commit.\n> >\n> >     Eric Sunshine sunshine@sunshineco.com provided valuable feedback during\n> >     the review process. He explained the details of the patches to me and\n> >     pointed out that using tr -d was unnecessary to resolve the whitespace\n> >     issue.\n>\n> Thanks. This version of the patch looks fine.\n>\n> I notice that there are still quite a few instances of `git` upstream\n> of a pipe remaining in t3404 even after this patch. But that's okay;\n> fixing all of them would have made the patch far longer and more\n> tiresome to review, so it is not a problem that you selectively\n> converted only `git show` and `git cat-file` cases. It probably would\n> have been helpful to reviewers if the patch's commit message mentioned\n> that it only converts some of the instances, but it's not worth\n> rerolling the patch just for that.\n>\n> So, I think it makes sense to stop here and consider this microproject\n> successful (unless some other reviewer notices some problem or\n> requests some other change).\n\nThanks Eric Sunshine, I appreciate your time and review. I am more\neager to continue working on it after review from the others. And also\nwould like to work on the test_line_count also if permitted. Thank you\nvery much.\n"},{"id":"504177","messageId":"CAPig+cQfoMe2H=1QwnSmGNxcEG0abjfLQAC1W7tYaQiMjYvuOg@mail.gmail.com","threadId":"62259","inReplyTo":"CAPSxiM-=wtsd_XH3FY+C+1V_MwNkiMtRDwOKQH4r+6Cfj5Vydw@mail.gmail.com","subject":"Re: [PATCH v2] [Outreachy][Patch v1] t3404: avoid losing exit status to pipes","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-10-06T10:21:19Z","receivedAt":"2024-10-06T10:21:34Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Oct 6, 2024 at 5:32 AM Usman Akinyemi\n<usmanakinyemi202@gmail.com> wrote:\n> On Sun, Oct 6, 2024 at 9:19 AM Eric Sunshine <sunshine@sunshineco.com> wrote:\n> > So, I think it makes sense to stop here and consider this microproject\n> > successful (unless some other reviewer notices some problem or\n> > requests some other change).\n>\n> Thanks Eric Sunshine, I appreciate your time and review. I am more\n> eager to continue working on it after review from the others. And also\n> would like to work on the test_line_count also if permitted. Thank you\n> very much.\n\nYou don't have to ask permission; if you feel like converting the test\nto use test_line_count(), you are welcome to do so.\n"},{"id":"504180","messageId":"ZwJwsuf5ZOKiWbS1@ArchLinux","threadId":"62259","inReplyTo":"CAPig+cRePbX6OyE6WhFp1GEZenZyC7HFeA3188F_nErt7Gin6A@mail.gmail.com","subject":"Re: [PATCH v2] [Outreachy][Patch v1] t3404: avoid losing exit status to pipes","fromName":"shejialuo","fromEmail":"shejialuo@gmail.com","sentAt":"2024-10-06T11:12:50Z","receivedAt":"2024-10-06T11:12:44Z","isPatch":true,"sender":{"key":"shejialuo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56911263?v=4"},"body":"On Sun, Oct 06, 2024 at 05:19:13AM -0400, Eric Sunshine wrote:\n\n[snip]\n\n> It probably would have been helpful to reviewers if the patch's\n> commit message mentioned that it only converts some of the\n> instances, but it's not worth rerolling the patch just for that.\n\nExcept that, the commit title should not either include\n\"[Outreachy][Patch v1]\" here. From these two reasons, I think we should\nreroll the patch.\n\nThanks,\nJialuo\n"},{"id":"504182","messageId":"CAPig+cTVf=W5KXkE2iTxCSPmZjOSbmf90J_JugDuTH_xudcW2g@mail.gmail.com","threadId":"62259","inReplyTo":"ZwJwsuf5ZOKiWbS1@ArchLinux","subject":"Re: [PATCH v2] [Outreachy][Patch v1] t3404: avoid losing exit status to pipes","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-10-06T12:06:10Z","receivedAt":"2024-10-06T12:06:22Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Oct 6, 2024 at 7:12 AM shejialuo <shejialuo@gmail.com> wrote:\n> On Sun, Oct 06, 2024 at 05:19:13AM -0400, Eric Sunshine wrote:\n> > It probably would have been helpful to reviewers if the patch's\n> > commit message mentioned that it only converts some of the\n> > instances, but it's not worth rerolling the patch just for that.\n>\n> Except that, the commit title should not either include\n> \"[Outreachy][Patch v1]\" here. From these two reasons, I think we should\n> reroll the patch.\n\nYour observation about outdated/confusing \"[foo]\" annotations is\ncertainly something the submitter should take into consideration for\nfuture submissions, but does not seem worthy of a reroll, IMHO. First,\n`git am` will strip those off automatically, so they won't become part\nof the permanent project history anyhow when/if Junio picks up the\npatch. Second, asking for a reroll for something which does not impact\nthe correctness of either the patch or the commit message just makes\nbusy-work for the submitter and wastes reviewer time (which is a\nlimited resource on this project). Third, the point of a microproject\nis to expose the submitter to the workflow of the Git project and to\nthe review process, and for reviewers to see how the submitter\nresponds. That goal has already been achieved in this case, and\nrerolling for something so minor provides no additional benefit in\nthat regard.\n"},{"id":"504184","messageId":"CAPSxiM-bx7KdhP0OyajfiTczg-WqiJDPso1EAxLzntLqcuOUkA@mail.gmail.com","threadId":"62259","inReplyTo":"CAPig+cTVf=W5KXkE2iTxCSPmZjOSbmf90J_JugDuTH_xudcW2g@mail.gmail.com","subject":"Re: [PATCH v2] [Outreachy][Patch v1] t3404: avoid losing exit status to pipes","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2024-10-06T12:28:26Z","receivedAt":"2024-10-06T12:28:39Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"Hello,\n\nI really appreciate all the maintainers for their help and time.\n\nI am a bit confused now, I am already working on converting the\ntest_line_count right now. Can I update the patch or do something\nregarding the first patch ?\n\nThank you.\n\nOn Sun, Oct 6, 2024 at 12:06 PM Eric Sunshine <sunshine@sunshineco.com> wrote:\n>\n> On Sun, Oct 6, 2024 at 7:12 AM shejialuo <shejialuo@gmail.com> wrote:\n> > On Sun, Oct 06, 2024 at 05:19:13AM -0400, Eric Sunshine wrote:\n> > > It probably would have been helpful to reviewers if the patch's\n> > > commit message mentioned that it only converts some of the\n> > > instances, but it's not worth rerolling the patch just for that.\n> >\n> > Except that, the commit title should not either include\n> > \"[Outreachy][Patch v1]\" here. From these two reasons, I think we should\n> > reroll the patch.\n>\n> Your observation about outdated/confusing \"[foo]\" annotations is\n> certainly something the submitter should take into consideration for\n> future submissions, but does not seem worthy of a reroll, IMHO. First,\n> `git am` will strip those off automatically, so they won't become part\n> of the permanent project history anyhow when/if Junio picks up the\n> patch. Second, asking for a reroll for something which does not impact\n> the correctness of either the patch or the commit message just makes\n> busy-work for the submitter and wastes reviewer time (which is a\n> limited resource on this project). Third, the point of a microproject\n> is to expose the submitter to the workflow of the Git project and to\n> the review process, and for reviewers to see how the submitter\n> responds. That goal has already been achieved in this case, and\n> rerolling for something so minor provides no additional benefit in\n> that regard.\n"},{"id":"504185","messageId":"ZwKCxHzbWxNQ92KR@ArchLinux","threadId":"62259","inReplyTo":"CAPig+cTVf=W5KXkE2iTxCSPmZjOSbmf90J_JugDuTH_xudcW2g@mail.gmail.com","subject":"Re: [PATCH v2] [Outreachy][Patch v1] t3404: avoid losing exit status to pipes","fromName":"shejialuo","fromEmail":"shejialuo@gmail.com","sentAt":"2024-10-06T12:29:56Z","receivedAt":"2024-10-06T12:29:50Z","isPatch":true,"sender":{"key":"shejialuo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56911263?v=4"},"body":"On Sun, Oct 06, 2024 at 08:06:10AM -0400, Eric Sunshine wrote:\n\n> Your observation about outdated/confusing \"[foo]\" annotations is\n> certainly something the submitter should take into consideration for\n> future submissions, but does not seem worthy of a reroll, IMHO. First,\n> `git am` will strip those off automatically, so they won't become part\n> of the permanent project history anyhow when/if Junio picks up the\n> patch. Second, asking for a reroll for something which does not impact\n> the correctness of either the patch or the commit message just makes\n> busy-work for the submitter and wastes reviewer time (which is a\n> limited resource on this project). Third, the point of a microproject\n> is to expose the submitter to the workflow of the Git project and to\n> the review process, and for reviewers to see how the submitter\n> responds. That goal has already been achieved in this case, and\n> rerolling for something so minor provides no additional benefit in\n> that regard.\n\nThanks for your detailed explanation here. I don't know that \"git am\"\ncould strip those off automatically. I thought the maintainer would\ndelete \"[foo]\" manually. So, my main intention here is that I want the\nsubmitter to make it more perfect to reduce the overhead of the\nmaintainer and also pay attention to this for further submissions.\n\nAnd from my perspective, the reroll would not bring much overhead for\nthe submitter, so I expressed my words in the previous email. I know you\nconcerned that my words would frustrate the Usman. And I wanna say this\nis not my intention here. I think Usamn has already done a great job for\nthis microproject to understand the workflow of the Git project. So,\nactually we are on the same boat here.\n\nLet me withdraw my previous words (\"We should reroll the patch\"). This\npatch is good and don't need a reroll.\n\nThanks,\nJialuo\n"},{"id":"504187","messageId":"CAPSxiM85QuiJK+0HYDPT-4-hoBpeZ42uzG+ZoMdPEZYC1C_yZw@mail.gmail.com","threadId":"62259","inReplyTo":"ZwKCxHzbWxNQ92KR@ArchLinux","subject":"Re: [PATCH v2] [Outreachy][Patch v1] t3404: avoid losing exit status to pipes","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2024-10-06T12:46:13Z","receivedAt":"2024-10-06T12:46:26Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"Thanks Shejialuo and Eric, I really appreciate both of your time and\nhelp. I am not frustrated at all and I see any changes as a perfect\nopportunity to learn something new. I have also learned from my\nmistake and your guidance and would keep it in mind for future\nsubmissions.\n\nIn that case, I will send the second patch.\nThank you.\n\nOn Sun, Oct 6, 2024 at 12:29 PM shejialuo <shejialuo@gmail.com> wrote:\n>\n> On Sun, Oct 06, 2024 at 08:06:10AM -0400, Eric Sunshine wrote:\n>\n> > Your observation about outdated/confusing \"[foo]\" annotations is\n> > certainly something the submitter should take into consideration for\n> > future submissions, but does not seem worthy of a reroll, IMHO. First,\n> > `git am` will strip those off automatically, so they won't become part\n> > of the permanent project history anyhow when/if Junio picks up the\n> > patch. Second, asking for a reroll for something which does not impact\n> > the correctness of either the patch or the commit message just makes\n> > busy-work for the submitter and wastes reviewer time (which is a\n> > limited resource on this project). Third, the point of a microproject\n> > is to expose the submitter to the workflow of the Git project and to\n> > the review process, and for reviewers to see how the submitter\n> > responds. That goal has already been achieved in this case, and\n> > rerolling for something so minor provides no additional benefit in\n> > that regard.\n>\n> Thanks for your detailed explanation here. I don't know that \"git am\"\n> could strip those off automatically. I thought the maintainer would\n> delete \"[foo]\" manually. So, my main intention here is that I want the\n> submitter to make it more perfect to reduce the overhead of the\n> maintainer and also pay attention to this for further submissions.\n>\n> And from my perspective, the reroll would not bring much overhead for\n> the submitter, so I expressed my words in the previous email. I know you\n> concerned that my words would frustrate the Usman. And I wanna say this\n> is not my intention here. I think Usamn has already done a great job for\n> this microproject to understand the workflow of the Git project. So,\n> actually we are on the same boat here.\n>\n> Let me withdraw my previous words (\"We should reroll the patch\"). This\n> patch is good and don't need a reroll.\n>\n> Thanks,\n> Jialuo\n"},{"id":"504188","messageId":"ZwKKhzbeyQ5e_H9Q@ArchLinux","threadId":"62259","inReplyTo":"CAPSxiM-bx7KdhP0OyajfiTczg-WqiJDPso1EAxLzntLqcuOUkA@mail.gmail.com","subject":"Re: [PATCH v2] [Outreachy][Patch v1] t3404: avoid losing exit status to pipes","fromName":"shejialuo","fromEmail":"shejialuo@gmail.com","sentAt":"2024-10-06T13:03:03Z","receivedAt":"2024-10-06T13:02:57Z","isPatch":true,"sender":{"key":"shejialuo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56911263?v=4"},"body":"On Sun, Oct 06, 2024 at 12:28:26PM +0000, Usman Akinyemi wrote:\n> I am a bit confused now, I am already working on converting the\n> test_line_count right now. Can I update the patch or do something\n> regarding the first patch ?\n\nHi Usman:\n\nI have just scanned the review from Eric.\n\n> These days, instead of manually using `wc -l` and `test`, we would\n> instead write:\n\n>    grep ONCE output >actual &&\n>    test_line_count 1 actual\n\n> However, that sort of change is independent of the purpose of this\n> patch, so you probably should not make such a change in this patch. If\n> you're up to it, you could instead turn this into a two-patch series\n> in which patch [1/2] fixes the \"Git upstream of a pipe\" problem, and\n> then patch [2/2] converts these cases to use test_line_count().\n\nIf you decide to do this. As Eric has commented in [1], you should add a\nnew commit (a new patch) followed by current patch to convert to the\n`test_line_count`. Then you should re-send the series to the mailing\nlist. And thus you could enhance the commit message of the first patch.\nIf you do not decide to do this (the current patch is enough for the\nmicroproject), you don't need to reroll for the minor things.\n\nSo, you should never update the current patch for converting the test\nusing `test_line_count`. Instead, create a new commit to do this. and\nBTW you could change the commit message of the first patch if you want.\n\nThanks,\nJialuo\n\n[1] https://lore.kernel.org/git/CAPig+cTb4mgpXnN79UrXvjvCnqGZhaR51oZX_Ds=HwdqQYFN9w@mail.gmail.com/\n"},{"id":"504189","messageId":"CAPSxiM_+LCH04ZbTZm8oZWDc=Mz2JNJ_Uw7u+gO4e_ie4yPw5g@mail.gmail.com","threadId":"62259","inReplyTo":"ZwKKhzbeyQ5e_H9Q@ArchLinux","subject":"Re: [PATCH v2] [Outreachy][Patch v1] t3404: avoid losing exit status to pipes","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2024-10-06T13:27:00Z","receivedAt":"2024-10-06T13:27:12Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"Thanks very much Jialuo,\n\nMy understanding of this, kindly tell me if I am wrong.\n\n1. Make a new commit for test_line_count on the same branch right ?\n2. I should remove the \"Outreachy][Patch v1]\" in the first patch to\nenhance the commit message right?\n\nThank you.\n\nOn Sun, Oct 6, 2024 at 1:02 PM shejialuo <shejialuo@gmail.com> wrote:\n>\n> On Sun, Oct 06, 2024 at 12:28:26PM +0000, Usman Akinyemi wrote:\n> > I am a bit confused now, I am already working on converting the\n> > test_line_count right now. Can I update the patch or do something\n> > regarding the first patch ?\n>\n> Hi Usman:\n>\n> I have just scanned the review from Eric.\n>\n> > These days, instead of manually using `wc -l` and `test`, we would\n> > instead write:\n>\n> >    grep ONCE output >actual &&\n> >    test_line_count 1 actual\n>\n> > However, that sort of change is independent of the purpose of this\n> > patch, so you probably should not make such a change in this patch. If\n> > you're up to it, you could instead turn this into a two-patch series\n> > in which patch [1/2] fixes the \"Git upstream of a pipe\" problem, and\n> > then patch [2/2] converts these cases to use test_line_count().\n>\n> If you decide to do this. As Eric has commented in [1], you should add a\n> new commit (a new patch) followed by current patch to convert to the\n> `test_line_count`. Then you should re-send the series to the mailing\n> list. And thus you could enhance the commit message of the first patch.\n> If you do not decide to do this (the current patch is enough for the\n> microproject), you don't need to reroll for the minor things.\n>\n> So, you should never update the current patch for converting the test\n> using `test_line_count`. Instead, create a new commit to do this. and\n> BTW you could change the commit message of the first patch if you want.\n>\n> Thanks,\n> Jialuo\n>\n> [1] https://lore.kernel.org/git/CAPig+cTb4mgpXnN79UrXvjvCnqGZhaR51oZX_Ds=HwdqQYFN9w@mail.gmail.com/\n"},{"id":"504190","messageId":"ZwKSR48mlSME9J0w@ArchLinux","threadId":"62259","inReplyTo":"CAPSxiM_+LCH04ZbTZm8oZWDc=Mz2JNJ_Uw7u+gO4e_ie4yPw5g@mail.gmail.com","subject":"Re: [PATCH v2] [Outreachy][Patch v1] t3404: avoid losing exit status to pipes","fromName":"shejialuo","fromEmail":"shejialuo@gmail.com","sentAt":"2024-10-06T13:36:07Z","receivedAt":"2024-10-06T13:36:02Z","isPatch":true,"sender":{"key":"shejialuo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56911263?v=4"},"body":"On Sun, Oct 06, 2024 at 01:27:00PM +0000, Usman Akinyemi wrote:\n> Thanks very much Jialuo,\n> \n> My understanding of this, kindly tell me if I am wrong.\n> \n> 1. Make a new commit for test_line_count on the same branch right ?\n> 2. I should remove the \"Outreachy][Patch v1]\" in the first patch to\n> enhance the commit message right?\n> \n\nFor 1, it is correct. For 2, if you decide to change the commit message\nof the first patch, you should also follow the advice from Eric in [1].\n\n> It probably would have been helpful to reviewers if the patch's\n> commit message mentioned that it only converts some of the\n> instances, but it's not worth rerolling the patch just for that.\n\n[1] https://lore.kernel.org/git/CAPig+cRePbX6OyE6WhFp1GEZenZyC7HFeA3188F_nErt7Gin6A@mail.gmail.com/\n"},{"id":"504194","messageId":"pull.1805.v3.git.git.1728230769.gitgitgadget@gmail.com","threadId":"62259","inReplyTo":"pull.1805.v2.git.git.1728203495287.gitgitgadget@gmail.com","subject":"[PATCH v3 0/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","fromName":"Usman Akinyemi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-06T16:06:07Z","receivedAt":"2024-10-06T16:06:12Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"At the beginning of my task, I made the mistake of submitting two patches\nfor two separate commits instead of one. The first patch addressed the issue\nof losing the Git exit status due to pipes.\n\nAfter submitting the first patch, I noticed that the output of wc -l was\nfailing due to trailing whitespace. I attempted to fix this by using tr -d\nto remove the whitespace. However, instead of squashing the two patches into\none, I inadvertently created another commit.\n\nEric Sunshine sunshine@sunshineco.com provided valuable feedback during the\nreview process. He explained the details of the patches to me and pointed\nout that using tr -d was unnecessary to resolve the whitespace issue.\n\nThe root cause of the whitespace issue was quoting $count in the test\ncommand, which led to the inclusion of whitespace in the comparison. By\nremoving the quotes around $count, the comparison works as expected without\nthe need for tr -d.\n\nSigned-off-by: Usman Akinyemi\n\nUsman Akinyemi (2):\n  t3404: avoid losing exit status with focus on `git show` and `git\n    cat-files`\n  [Outreachy][Patch v1] t3404: employing test_line_count() to replace\n    test\n\n t/t3404-rebase-interactive.sh | 74 +++++++++++++++++++++++------------\n 1 file changed, 50 insertions(+), 24 deletions(-)\n\n\nbase-commit: 90fe3800b92a49173530828c0a17951abd30f0e1\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1805%2FUnique-Usman%2Favoid_git_pipes-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1805/Unique-Usman/avoid_git_pipes-v3\nPull-Request: https://github.com/git/git/pull/1805\n\nRange-diff vs v2:\n\n 1:  be5a691e96f ! 1:  c9a0cca179b [Outreachy][Patch v1] t3404: avoid losing exit status to pipes\n     @@ Metadata\n      Author: Usman Akinyemi <usmanakinyemi202@gmail.com>\n      \n       ## Commit message ##\n     -    [Outreachy][Patch v1] t3404: avoid losing exit status to pipes\n     +    t3404: avoid losing exit status with focus on `git show` and `git cat-files`\n      \n          The exit code of the preceding command in a pipe is disregarded. So\n          if that preceding command is a Git command that fails, the test would\n          not fail. Instead, by saving the output of that Git command to a file,\n          and removing the pipe, we make sure the test will fail if that Git\n     -    command fails.\n     +    command fails. This particular patch focuses on some of the instances\n     +    which include `git show` and `git cat-files`.\n      \n          Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n      \n -:  ----------- > 2:  37b1411ee2c [Outreachy][Patch v1] t3404: employing test_line_count() to replace test\n\n-- \ngitgitgadget\n"},{"id":"504195","messageId":"c9a0cca179bfad129017c7f1085ff87201afb417.1728230769.git.gitgitgadget@gmail.com","threadId":"62259","inReplyTo":"pull.1805.v3.git.git.1728230769.gitgitgadget@gmail.com","subject":"[PATCH v3 1/2] t3404: avoid losing exit status with focus on `git show` and `git cat-files`","fromName":"Usman Akinyemi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-06T16:06:08Z","receivedAt":"2024-10-06T16:06:13Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"From: Usman Akinyemi <usmanakinyemi202@gmail.com>\n\nThe exit code of the preceding command in a pipe is disregarded. So\nif that preceding command is a Git command that fails, the test would\nnot fail. Instead, by saving the output of that Git command to a file,\nand removing the pipe, we make sure the test will fail if that Git\ncommand fails. This particular patch focuses on some of the instances\nwhich include `git show` and `git cat-files`.\n\nSigned-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n---\n t/t3404-rebase-interactive.sh | 71 +++++++++++++++++++++++------------\n 1 file changed, 48 insertions(+), 23 deletions(-)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex f171af3061d..96a65783c47 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -319,7 +319,8 @@ test_expect_success 'retain authorship' '\n \tGIT_AUTHOR_NAME=\"Twerp Snog\" git commit -m \"different author\" &&\n \tgit tag twerp &&\n \tgit rebase -i --onto primary HEAD^ &&\n-\tgit show HEAD | grep \"^Author: Twerp Snog\"\n+\tgit show HEAD >actual &&\n+\tgrep \"^Author: Twerp Snog\" actual\n '\n \n test_expect_success 'retain authorship w/ conflicts' '\n@@ -360,7 +361,8 @@ test_expect_success 'squash' '\n '\n \n test_expect_success 'retain authorship when squashing' '\n-\tgit show HEAD | grep \"^Author: Twerp Snog\"\n+\tgit show HEAD >actual &&\n+\tgrep \"^Author: Twerp Snog\" actual\n '\n \n test_expect_success '--continue tries to commit' '\n@@ -374,7 +376,8 @@ test_expect_success '--continue tries to commit' '\n \t\tFAKE_COMMIT_MESSAGE=\"chouette!\" git rebase --continue\n \t) &&\n \ttest_cmp_rev HEAD^ new-branch1 &&\n-\tgit show HEAD | grep chouette\n+\tgit show HEAD >actual &&\n+\tgrep chouette actual\n '\n \n test_expect_success 'verbose flag is heeded, even after --continue' '\n@@ -397,7 +400,9 @@ test_expect_success 'multi-squash only fires up editor once' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l)\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = $count\n '\n \n test_expect_success 'multi-fixup does not fire up editor' '\n@@ -410,7 +415,9 @@ test_expect_success 'multi-fixup does not fire up editor' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 0 = $(git show | grep NEVER | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep NEVER output | wc -l) &&\n+\ttest 0 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D multi-fixup\n '\n@@ -428,7 +435,9 @@ test_expect_success 'commit message used after conflict' '\n \t\t\tgit rebase --continue\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-fixup\n '\n@@ -446,7 +455,9 @@ test_expect_success 'commit message retained after conflict' '\n \t\t\tgit rebase --continue\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 2 = $(git show | grep TWICE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep TWICE output | wc -l) &&\n+\ttest 2 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-squash\n '\n@@ -470,10 +481,10 @@ test_expect_success 'squash and fixup generate correct log messages' '\n \t) &&\n \tgit cat-file commit HEAD | sed -e 1,/^\\$/d > actual-squash-fixup &&\n \ttest_cmp expect-squash-fixup actual-squash-fixup &&\n-\tgit cat-file commit HEAD@{2} |\n-\t\tgrep \"^# This is a combination of 3 commits\\.\"  &&\n-\tgit cat-file commit HEAD@{3} |\n-\t\tgrep \"^# This is a combination of 2 commits\\.\"  &&\n+\tgit cat-file commit HEAD@{2} >actual &&\n+\tgrep \"^# This is a combination of 3 commits\\.\" actual &&\n+\tgit cat-file commit HEAD@{3} >actual &&\n+\tgrep \"^# This is a combination of 2 commits\\.\" actual  &&\n \tgit checkout @{-1} &&\n \tgit branch -D squash-fixup\n '\n@@ -489,7 +500,9 @@ test_expect_success 'squash ignores comments' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-comments\n '\n@@ -505,7 +518,9 @@ test_expect_success 'squash ignores blank lines' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-blank-lines\n '\n@@ -572,7 +587,8 @@ test_expect_success '--continue tries to commit, even for \"edit\"' '\n \t\tFAKE_COMMIT_MESSAGE=\"chouette!\" git rebase --continue\n \t) &&\n \ttest edited = $(git show HEAD:file7) &&\n-\tgit show HEAD | grep chouette &&\n+\tgit show HEAD >actual &&\n+\tgrep chouette actual &&\n \ttest $parent = $(git rev-parse HEAD^)\n '\n \n@@ -757,19 +773,23 @@ test_expect_success 'reword' '\n \t\tset_fake_editor &&\n \t\tFAKE_LINES=\"1 2 3 reword 4\" FAKE_COMMIT_MESSAGE=\"E changed\" \\\n \t\t\tgit rebase -i A &&\n-\t\tgit show HEAD | grep \"E changed\" &&\n+\t\tgit show HEAD >actual &&\n+\t\tgrep \"E changed\" actual &&\n \t\ttest $(git rev-parse primary) != $(git rev-parse HEAD) &&\n \t\ttest_cmp_rev primary^ HEAD^ &&\n \t\tFAKE_LINES=\"1 2 reword 3 4\" FAKE_COMMIT_MESSAGE=\"D changed\" \\\n \t\t\tgit rebase -i A &&\n-\t\tgit show HEAD^ | grep \"D changed\" &&\n+\t\tgit show HEAD^ >actual &&\n+\t\tgrep \"D changed\" actual &&\n \t\tFAKE_LINES=\"reword 1 2 3 4\" FAKE_COMMIT_MESSAGE=\"B changed\" \\\n \t\t\tgit rebase -i A &&\n-\t\tgit show HEAD~3 | grep \"B changed\" &&\n+\t\tgit show HEAD~3 >actual &&\n+\t\tgrep \"B changed\" actual &&\n \t\tFAKE_LINES=\"1 r 2 pick 3 p 4\" FAKE_COMMIT_MESSAGE=\"C changed\" \\\n \t\t\tgit rebase -i A\n \t) &&\n-\tgit show HEAD~2 | grep \"C changed\"\n+\tgit show HEAD~2 >actual &&\n+\tgrep \"C changed\" actual\n '\n \n test_expect_success 'no uncommitted changes when rewording and the todo list is reloaded' '\n@@ -1003,8 +1023,10 @@ test_expect_success 'rebase -i --root retain root commit author and message' '\n \t\tset_fake_editor &&\n \t\tFAKE_LINES=\"2\" git rebase -i --root\n \t) &&\n-\tgit cat-file commit HEAD | grep -q \"^author Twerp Snog\" &&\n-\tgit cat-file commit HEAD | grep -q \"^different author$\"\n+\tgit cat-file commit HEAD >output &&\n+\tgrep -q \"^author Twerp Snog\" output &&\n+\tgit cat-file commit HEAD >actual &&\n+\tgrep -q \"^different author$\" actual\n '\n \n test_expect_success 'rebase -i --root temporary sentinel commit' '\n@@ -1013,7 +1035,8 @@ test_expect_success 'rebase -i --root temporary sentinel commit' '\n \t\tset_fake_editor &&\n \t\ttest_must_fail env FAKE_LINES=\"2\" git rebase -i --root\n \t) &&\n-\tgit cat-file commit HEAD | grep \"^tree $EMPTY_TREE\" &&\n+\tgit cat-file commit HEAD >actual &&\n+\tgrep \"^tree $EMPTY_TREE\" actual &&\n \tgit rebase --abort\n '\n \n@@ -1036,7 +1059,8 @@ test_expect_success 'rebase -i --root reword original root commit' '\n \t\tFAKE_LINES=\"reword 1 2\" FAKE_COMMIT_MESSAGE=\"A changed\" \\\n \t\t\tgit rebase -i --root\n \t) &&\n-\tgit show HEAD^ | grep \"A changed\" &&\n+\tgit show HEAD^ >actual &&\n+\tgrep \"A changed\" actual &&\n \ttest -z \"$(git show -s --format=%p HEAD^)\"\n '\n \n@@ -1048,7 +1072,8 @@ test_expect_success 'rebase -i --root reword new root commit' '\n \t\tFAKE_LINES=\"reword 3 1\" FAKE_COMMIT_MESSAGE=\"C changed\" \\\n \t\tgit rebase -i --root\n \t) &&\n-\tgit show HEAD^ | grep \"C changed\" &&\n+\tgit show HEAD^ >actual &&\n+\tgrep \"C changed\" actual &&\n \ttest -z \"$(git show -s --format=%p HEAD^)\"\n '\n \n-- \ngitgitgadget\n\n"},{"id":"504196","messageId":"37b1411ee2c420f1a8578d27a2f7d54ccd3f329f.1728230769.git.gitgitgadget@gmail.com","threadId":"62259","inReplyTo":"pull.1805.v3.git.git.1728230769.gitgitgadget@gmail.com","subject":"[PATCH v3 2/2] [Outreachy][Patch v1] t3404: employing test_line_count() to replace test","fromName":"Usman Akinyemi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-06T16:06:09Z","receivedAt":"2024-10-06T16:06:13Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"From: Usman Akinyemi <usmanakinyemi202@gmail.com>\n\nRefactor t3404 to replace instances of `test` with `test_line_count()`\nfor checking line counts. This improves readability and aligns with Git's\ncurrent test practices.\n\nSigned-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n---\n t/t3404-rebase-interactive.sh | 23 ++++++++++++-----------\n 1 file changed, 12 insertions(+), 11 deletions(-)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 96a65783c47..19390eaf331 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -281,7 +281,8 @@ test_expect_success 'stop on conflicting pick' '\n \ttest_cmp expect2 file1 &&\n \ttest \"$(git diff --name-status |\n \t\tsed -n -e \"/^U/s/^U[^a-z]*//p\")\" = file1 &&\n-\ttest 4 = $(grep -v \"^#\" < .git/rebase-merge/done | wc -l) &&\n+\tgrep -v \"^#\" < .git/rebase-merge/done > actual &&\n+\ttest_line_count = 4 actual &&\n \ttest 0 = $(grep -c \"^[^#]\" < .git/rebase-merge/git-rebase-todo)\n '\n \n@@ -401,8 +402,8 @@ test_expect_success 'multi-squash only fires up editor once' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep ONCE output | wc -l) &&\n-\ttest 1 = $count\n+\tgrep ONCE output >actual &&\n+\ttest_line_count = 1 actual\n '\n \n test_expect_success 'multi-fixup does not fire up editor' '\n@@ -436,8 +437,8 @@ test_expect_success 'commit message used after conflict' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep ONCE output | wc -l) &&\n-\ttest 1 = $count &&\n+\tgrep ONCE output >actual &&\n+\ttest_line_count = 1 actual &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-fixup\n '\n@@ -456,8 +457,8 @@ test_expect_success 'commit message retained after conflict' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep TWICE output | wc -l) &&\n-\ttest 2 = $count &&\n+\tgrep TWICE output >actual &&\n+\ttest_line_count = 2 actual &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-squash\n '\n@@ -501,8 +502,8 @@ test_expect_success 'squash ignores comments' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep ONCE output | wc -l) &&\n-\ttest 1 = $count &&\n+\tgrep ONCE output >actual &&\n+\ttest_line_count = 1 actual &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-comments\n '\n@@ -519,8 +520,8 @@ test_expect_success 'squash ignores blank lines' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep ONCE output | wc -l) &&\n-\ttest 1 = $count &&\n+\tgrep ONCE output >actual &&\n+\ttest_line_count = 1 actual &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-blank-lines\n '\n-- \ngitgitgadget\n"},{"id":"504197","messageId":"CAPSxiM9eExGxy5biEr_23tCKXaUQg=tsD57VR7e2SVdvVJR+Kg@mail.gmail.com","threadId":"62259","inReplyTo":"pull.1805.v3.git.git.1728230769.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 0/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2024-10-06T16:18:34Z","receivedAt":"2024-10-06T16:18:46Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"Hello,\n\nKindly, help take a look if this is okay now.\n\nAlso, I wanted to change this also to use test_line_count,\n test 0 = $(grep -c \"^[^#]\" < .git/rebase-merge/git-rebase-todo)\n\n But, I tried a different approach and the test kept failing.\n\nSimilar as\n\ngit show >output &&\ncount=$(grep NEVER output | wc -l) &&\ntest 0 = $count &&\n\nThank you.\n\nOn Sun, Oct 6, 2024 at 4:06 PM Usman Akinyemi via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n>\n> At the beginning of my task, I made the mistake of submitting two patches\n> for two separate commits instead of one. The first patch addressed the issue\n> of losing the Git exit status due to pipes.\n>\n> After submitting the first patch, I noticed that the output of wc -l was\n> failing due to trailing whitespace. I attempted to fix this by using tr -d\n> to remove the whitespace. However, instead of squashing the two patches into\n> one, I inadvertently created another commit.\n>\n> Eric Sunshine sunshine@sunshineco.com provided valuable feedback during the\n> review process. He explained the details of the patches to me and pointed\n> out that using tr -d was unnecessary to resolve the whitespace issue.\n>\n> The root cause of the whitespace issue was quoting $count in the test\n> command, which led to the inclusion of whitespace in the comparison. By\n> removing the quotes around $count, the comparison works as expected without\n> the need for tr -d.\n>\n> Signed-off-by: Usman Akinyemi\n>\n> Usman Akinyemi (2):\n>   t3404: avoid losing exit status with focus on `git show` and `git\n>     cat-files`\n>   [Outreachy][Patch v1] t3404: employing test_line_count() to replace\n>     test\n>\n>  t/t3404-rebase-interactive.sh | 74 +++++++++++++++++++++++------------\n>  1 file changed, 50 insertions(+), 24 deletions(-)\n>\n>\n> base-commit: 90fe3800b92a49173530828c0a17951abd30f0e1\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1805%2FUnique-Usman%2Favoid_git_pipes-v3\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1805/Unique-Usman/avoid_git_pipes-v3\n> Pull-Request: https://github.com/git/git/pull/1805\n>\n> Range-diff vs v2:\n>\n>  1:  be5a691e96f ! 1:  c9a0cca179b [Outreachy][Patch v1] t3404: avoid losing exit status to pipes\n>      @@ Metadata\n>       Author: Usman Akinyemi <usmanakinyemi202@gmail.com>\n>\n>        ## Commit message ##\n>      -    [Outreachy][Patch v1] t3404: avoid losing exit status to pipes\n>      +    t3404: avoid losing exit status with focus on `git show` and `git cat-files`\n>\n>           The exit code of the preceding command in a pipe is disregarded. So\n>           if that preceding command is a Git command that fails, the test would\n>           not fail. Instead, by saving the output of that Git command to a file,\n>           and removing the pipe, we make sure the test will fail if that Git\n>      -    command fails.\n>      +    command fails. This particular patch focuses on some of the instances\n>      +    which include `git show` and `git cat-files`.\n>\n>           Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n>\n>  -:  ----------- > 2:  37b1411ee2c [Outreachy][Patch v1] t3404: employing test_line_count() to replace test\n>\n> --\n> gitgitgadget\n"},{"id":"504198","messageId":"92496582-8414-4d5b-8883-8b0a0b3c0988@app.fastmail.com","threadId":"62259","inReplyTo":"pull.1805.v3.git.git.1728230769.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 0/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2024-10-06T16:21:14Z","receivedAt":"2024-10-06T16:21:36Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Sun, Oct 6, 2024, at 18:06, Usman Akinyemi via GitGitGadget wrote:\n>\n> The root cause of the whitespace issue was quoting $count in the test\n> command, which led to the inclusion of whitespace in the comparison. By\n> removing the quotes around $count, the comparison works as expected without\n> the need for tr -d.\n>\n> Signed-off-by: Usman Akinyemi\n>\n> Usman Akinyemi (2):\n>   t3404: avoid losing exit status with focus on `git show` and `git\n>     cat-files`\n>   [Outreachy][Patch v1] t3404: employing test_line_count() to replace\n>     test\n\nYou don’t have to sign off on your cover letter.  Just the patches. :)\n\nOf course do what you prefer.\n"},{"id":"504199","messageId":"CAPSxiM-jKgsuqkF_aHDJxiTz0FWwuT==FYxJPKUP0O_wwDahEg@mail.gmail.com","threadId":"62259","inReplyTo":"92496582-8414-4d5b-8883-8b0a0b3c0988@app.fastmail.com","subject":"Re: [PATCH v3 0/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2024-10-06T16:26:03Z","receivedAt":"2024-10-06T16:26:16Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"Thanks for this. I will definitely take note of this next time. Thank you.\n\nOn Sun, Oct 6, 2024 at 4:21 PM Kristoffer Haugsbakk\n<kristofferhaugsbakk@fastmail.com> wrote:\n>\n> On Sun, Oct 6, 2024, at 18:06, Usman Akinyemi via GitGitGadget wrote:\n> >\n> > The root cause of the whitespace issue was quoting $count in the test\n> > command, which led to the inclusion of whitespace in the comparison. By\n> > removing the quotes around $count, the comparison works as expected without\n> > the need for tr -d.\n> >\n> > Signed-off-by: Usman Akinyemi\n> >\n> > Usman Akinyemi (2):\n> >   t3404: avoid losing exit status with focus on `git show` and `git\n> >     cat-files`\n> >   [Outreachy][Patch v1] t3404: employing test_line_count() to replace\n> >     test\n>\n> You don’t have to sign off on your cover letter.  Just the patches. :)\n>\n> Of course do what you prefer.\n"},{"id":"504200","messageId":"5a0f09e0-d181-44e8-ab04-5b544bddd21a@app.fastmail.com","threadId":"62259","inReplyTo":"37b1411ee2c420f1a8578d27a2f7d54ccd3f329f.1728230769.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 2/2] [Outreachy][Patch v1] t3404: employing test_line_count() to replace test","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2024-10-06T16:31:13Z","receivedAt":"2024-10-06T16:31:35Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Sun, Oct 6, 2024, at 18:06, Usman Akinyemi via GitGitGadget wrote:\n> From: Usman Akinyemi <usmanakinyemi202@gmail.com>\n>\n> Refactor t3404 to replace instances of `test` with `test_line_count()`\n> for checking line counts. This improves readability and aligns with Git's\n> current test practices.\n>\n> Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n\nNice commit message.\n\n-- \nKristoffer Haugsbakk\n\n"},{"id":"504237","messageId":"CAPig+cQHqXrhBQbTx0msqzhNpeK1Uzv2XKZ2SmTmgaFiCcktZQ@mail.gmail.com","threadId":"62259","inReplyTo":"ZwKCxHzbWxNQ92KR@ArchLinux","subject":"Re: [PATCH v2] [Outreachy][Patch v1] t3404: avoid losing exit status to pipes","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-10-07T04:19:23Z","receivedAt":"2024-10-07T04:19:35Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Oct 6, 2024 at 8:29 AM shejialuo <shejialuo@gmail.com> wrote:\n> On Sun, Oct 06, 2024 at 08:06:10AM -0400, Eric Sunshine wrote:\n> > Your observation about outdated/confusing \"[foo]\" annotations is\n> > certainly something the submitter should take into consideration for\n> > future submissions, but does not seem worthy of a reroll, IMHO. First,\n> > `git am` will strip those off automatically, so they won't become part\n> > of the permanent project history anyhow when/if Junio picks up the\n> > patch. Second, asking for a reroll for something which does not impact\n> > the correctness of either the patch or the commit message just makes\n> > busy-work for the submitter and wastes reviewer time (which is a\n> > limited resource on this project). Third, the point of a microproject\n> > is to expose the submitter to the workflow of the Git project and to\n> > the review process, and for reviewers to see how the submitter\n> > responds. That goal has already been achieved in this case, and\n> > rerolling for something so minor provides no additional benefit in\n> > that regard.\n>\n> Thanks for your detailed explanation here. I don't know that \"git am\"\n> could strip those off automatically. I thought the maintainer would\n> delete \"[foo]\" manually. So, my main intention here is that I want the\n> submitter to make it more perfect to reduce the overhead of the\n> maintainer and also pay attention to this for further submissions.\n\nOkay, that makes sense. Fortunately, the behavior of git-am means that\nwe don't have to worry about that particular issue.\n\n> And from my perspective, the reroll would not bring much overhead for\n> the submitter, so I expressed my words in the previous email. I know you\n> concerned that my words would frustrate the Usman.\n\nIt's true that I try to be careful to avoid asking submitters to do\nunnecessary work, but my bigger concern is that there are many patches\nbeing submitted but very few people reviewing them, so it is a good\nidea to avoid piling more work on reviewers if possible.\n\nBy the way, I appreciate that you are helping to review patches on\nthis list; not just this series, but also larger and more complicated\nseries such as the one for making git-worktree employ relative paths.\n\n> And I wanna say this\n> is not my intention here. I think Usamn has already done a great job for\n> this microproject to understand the workflow of the Git project. So,\n> actually we are on the same boat here.\n\nAgreed.\n\n> Let me withdraw my previous words (\"We should reroll the patch\"). This\n> patch is good and don't need a reroll.\n"},{"id":"504238","messageId":"CAPig+cQ1xC9ky8+NZO0ywmYAa6Vxe2ZUbs_Ae4OTTcWM8tHY_Q@mail.gmail.com","threadId":"62259","inReplyTo":"CAPSxiM9eExGxy5biEr_23tCKXaUQg=tsD57VR7e2SVdvVJR+Kg@mail.gmail.com","subject":"Re: [PATCH v3 0/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-10-07T04:24:23Z","receivedAt":"2024-10-07T04:24:35Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Oct 6, 2024 at 12:18 PM Usman Akinyemi\n<usmanakinyemi202@gmail.com> wrote:\n> Kindly, help take a look if this is okay now.\n>\n> Also, I wanted to change this also to use test_line_count,\n>  test 0 = $(grep -c \"^[^#]\" < .git/rebase-merge/git-rebase-todo)\n>\n>  But, I tried a different approach and the test kept failing.\n>\n> Similar as\n>\n> git show >output &&\n> count=$(grep NEVER output | wc -l) &&\n> test 0 = $count &&\n\nWhat is the actual error you encountered?\n\nBy the way, we have a handy function, test_must_be_empty(), which can\nbe used if you expect the output to not contain anything. As an\nexample:\n\n    git show >output &&\n    grep NEVER output >actual &&\n    test_must_be_empty actual\n"},{"id":"504243","messageId":"CAPig+cR7w-ZgkN8qDe+_zy_WJi6puoLVk9Wk+9TRuzW_-dAA=A@mail.gmail.com","threadId":"62259","inReplyTo":"37b1411ee2c420f1a8578d27a2f7d54ccd3f329f.1728230769.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 2/2] [Outreachy][Patch v1] t3404: employing test_line_count() to replace test","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-10-07T04:38:10Z","receivedAt":"2024-10-07T04:38:22Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Oct 6, 2024 at 12:06 PM Usman Akinyemi via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n> Refactor t3404 to replace instances of `test` with `test_line_count()`\n> for checking line counts. This improves readability and aligns with Git's\n> current test practices.\n>\n> Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> ---\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> @@ -281,7 +281,8 @@ test_expect_success 'stop on conflicting pick' '\n> -       test 4 = $(grep -v \"^#\" < .git/rebase-merge/done | wc -l) &&\n> +       grep -v \"^#\" < .git/rebase-merge/done > actual &&\n> +       test_line_count = 4 actual &&\n\nThanks. Both patches in this series look fine.\n\nLet's consider this a successfully-completed microproject.\n"},{"id":"504249","messageId":"ZwN3sz_gPGqKAWqO@pks.im","threadId":"62259","inReplyTo":"CAPSxiM-jKgsuqkF_aHDJxiTz0FWwuT==FYxJPKUP0O_wwDahEg@mail.gmail.com","subject":"Re: [PATCH v3 0/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-07T05:55:08Z","receivedAt":"2024-10-07T05:55:14Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sun, Oct 06, 2024 at 04:26:03PM +0000, Usman Akinyemi wrote:\n> Thanks for this. I will definitely take note of this next time. Thank you.\n\nPlease note that we tend to not top-post on this mailing list. Instead,\nyour reply should be either inline in the quoted parts in case you want\nto reply to individual parts of it, or below the quoted part as others\nare doing :)\n\nPatrick\n\n> On Sun, Oct 6, 2024 at 4:21 PM Kristoffer Haugsbakk\n> <kristofferhaugsbakk@fastmail.com> wrote:\n> >\n> > On Sun, Oct 6, 2024, at 18:06, Usman Akinyemi via GitGitGadget wrote:\n> > >\n> > > The root cause of the whitespace issue was quoting $count in the test\n> > > command, which led to the inclusion of whitespace in the comparison. By\n> > > removing the quotes around $count, the comparison works as expected without\n> > > the need for tr -d.\n> > >\n> > > Signed-off-by: Usman Akinyemi\n> > >\n> > > Usman Akinyemi (2):\n> > >   t3404: avoid losing exit status with focus on `git show` and `git\n> > >     cat-files`\n> > >   [Outreachy][Patch v1] t3404: employing test_line_count() to replace\n> > >     test\n> >\n> > You don’t have to sign off on your cover letter.  Just the patches. :)\n> >\n> > Of course do what you prefer.\n"},{"id":"504251","messageId":"ZwN6BNPHiXyKGd1t@pks.im","threadId":"62259","inReplyTo":"c9a0cca179bfad129017c7f1085ff87201afb417.1728230769.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 1/2] t3404: avoid losing exit status with focus on `git show` and `git cat-files`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-07T06:04:59Z","receivedAt":"2024-10-07T06:05:05Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sun, Oct 06, 2024 at 04:06:08PM +0000, Usman Akinyemi via GitGitGadget wrote:\n> From: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> \n> The exit code of the preceding command in a pipe is disregarded. So\n> if that preceding command is a Git command that fails, the test would\n> not fail. Instead, by saving the output of that Git command to a file,\n> and removing the pipe, we make sure the test will fail if that Git\n> command fails. This particular patch focuses on some of the instances\n> which include `git show` and `git cat-files`.\n\nWell-described.\n\n> Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> ---\n>  t/t3404-rebase-interactive.sh | 71 +++++++++++++++++++++++------------\n>  1 file changed, 48 insertions(+), 23 deletions(-)\n> \n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index f171af3061d..96a65783c47 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -319,7 +319,8 @@ test_expect_success 'retain authorship' '\n>  \tGIT_AUTHOR_NAME=\"Twerp Snog\" git commit -m \"different author\" &&\n>  \tgit tag twerp &&\n>  \tgit rebase -i --onto primary HEAD^ &&\n> -\tgit show HEAD | grep \"^Author: Twerp Snog\"\n> +\tgit show HEAD >actual &&\n> +\tgrep \"^Author: Twerp Snog\" actual\n>  '\n\nOne thing to note is that it would be preferable to use `test_grep`\ninstead of `grep` here. `test_grep` brings in some additional benefits\nover plain `grep` like better diagnosis of issues, and it would also\nprint the file if things didn't match. That makes it way easier e.g. in\nour CI to see what the actual output was.\n\nThere is no need to reroll this patch series just because of that\nthough, as your changes are a strict improvement by themselves already.\nBut if you do end up rerolling it would be nice to incorporate this.\n\n> @@ -397,7 +400,9 @@ test_expect_success 'multi-squash only fires up editor once' '\n>  \t\t\tgit rebase -i $base\n>  \t) &&\n>  \ttest $base = $(git rev-parse HEAD^) &&\n> -\ttest 1 = $(git show | grep ONCE | wc -l)\n> +\tgit show >output &&\n> +\tcount=$(grep ONCE output | wc -l) &&\n> +\ttest 1 = $count\n>  '\n>  \n>  test_expect_success 'multi-fixup does not fire up editor' '\n\nI was wondering whether the following might be nicer:\n\n    git show >output &&\n    grep ONCE output >output.filtered &&\n    test_line_count = 1 output.filtered\n\nBut after seeing this I don't strongly lean into one or another\ndirection. So please feel free to use either your current or my proposed\nstyle, I'm fine with either.\n\n> @@ -470,10 +481,10 @@ test_expect_success 'squash and fixup generate correct log messages' '\n>  \t) &&\n>  \tgit cat-file commit HEAD | sed -e 1,/^\\$/d > actual-squash-fixup &&\n>  \ttest_cmp expect-squash-fixup actual-squash-fixup &&\n> -\tgit cat-file commit HEAD@{2} |\n> -\t\tgrep \"^# This is a combination of 3 commits\\.\"  &&\n> -\tgit cat-file commit HEAD@{3} |\n> -\t\tgrep \"^# This is a combination of 2 commits\\.\"  &&\n> +\tgit cat-file commit HEAD@{2} >actual &&\n> +\tgrep \"^# This is a combination of 3 commits\\.\" actual &&\n> +\tgit cat-file commit HEAD@{3} >actual &&\n> +\tgrep \"^# This is a combination of 2 commits\\.\" actual  &&\n>  \tgit checkout @{-1} &&\n>  \tgit branch -D squash-fixup\n>  '\n\nNice cleanups while at it.\n\nPatrick\n"},{"id":"504252","messageId":"ZwN6D_-Ak1FoE0ix@pks.im","threadId":"62259","inReplyTo":"37b1411ee2c420f1a8578d27a2f7d54ccd3f329f.1728230769.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 2/2] [Outreachy][Patch v1] t3404: employing test_line_count() to replace test","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-07T06:05:03Z","receivedAt":"2024-10-07T06:05:08Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sun, Oct 06, 2024 at 04:06:09PM +0000, Usman Akinyemi via GitGitGadget wrote:\n> @@ -401,8 +402,8 @@ test_expect_success 'multi-squash only fires up editor once' '\n>  \t) &&\n>  \ttest $base = $(git rev-parse HEAD^) &&\n>  \tgit show >output &&\n> -\tcount=$(grep ONCE output | wc -l) &&\n> -\ttest 1 = $count\n> +\tgrep ONCE output >actual &&\n> +\ttest_line_count = 1 actual\n>  '\n>  \n>  test_expect_success 'multi-fixup does not fire up editor' '\n\nOh, you already do the change I proposed on the first commit. It's a bit\nfunny that we first change things one way and then touch it up again in\nanother commit as it leaves the reviewer wondering for a bit.\n\nBut I guess that's okay, especially for a microproject. So overall I\ndon't see a strong reason to reroll this series, thanks!\n\nPatrick\n"},{"id":"504261","messageId":"CAPSxiM9Z3q5Oz0PkadC72J0cU2F9D8Rh5eAtNz7aZoUZxp6PUA@mail.gmail.com","threadId":"62259","inReplyTo":"CAPig+cQ1xC9ky8+NZO0ywmYAa6Vxe2ZUbs_Ae4OTTcWM8tHY_Q@mail.gmail.com","subject":"Re: [PATCH v3 0/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2024-10-07T07:25:44Z","receivedAt":"2024-10-07T07:25:56Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"On Mon, Oct 7, 2024 at 4:24 AM Eric Sunshine <sunshine@sunshineco.com> wrote:\n>\n> On Sun, Oct 6, 2024 at 12:18 PM Usman Akinyemi\n> <usmanakinyemi202@gmail.com> wrote:\n> > Kindly, help take a look if this is okay now.\n> >\n> > Also, I wanted to change this also to use test_line_count,\n> >  test 0 = $(grep -c \"^[^#]\" < .git/rebase-merge/git-rebase-todo)\n> >\n> >  But, I tried a different approach and the test kept failing.\n> >\n> > Similar as\n> >\n> > git show >output &&\n> > count=$(grep NEVER output | wc -l) &&\n> > test 0 = $count &&\n>\n> What is the actual error you encountered?\n>\n> By the way, we have a handy function, test_must_be_empty(), which can\n> be used if you expect the output to not contain anything. As an\n> example:\n>\n>     git show >output &&\n>     grep NEVER output >actual &&\n>     test_must_be_empty actual\n\nThanks for your review, I really appreciate it. I tried this approach,\nbut I was getting this particular error for the testing.\n\nnot ok 32 - multi-fixup does not fire up editor\n#\n# git checkout -b multi-fixup E &&\n# base=$(git rev-parse HEAD~4) &&\n# (\n# set_fake_editor &&\n# FAKE_COMMIT_AMEND=\"NEVER\" \\\n# FAKE_LINES=\"1 fixup 2 fixup 3 fixup 4\" \\\n# git rebase -i $base\n# ) &&\n# test $base = $(git rev-parse HEAD^) &&\n# git show >output &&\n# grep NEVER output >actual &&\n# test_must_be_empty actual &&\n# git checkout @{-1} &&\n# git branch -D multi-fixup\n#\nBelow is the particular test case\n\ntest_expect_success 'multi-fixup does not fire up editor' '\ngit checkout -b multi-fixup E &&\nbase=$(git rev-parse HEAD~4) &&\n(\nset_fake_editor &&\nFAKE_COMMIT_AMEND=\"NEVER\" \\\nFAKE_LINES=\"1 fixup 2 fixup 3 fixup 4\" \\\ngit rebase -i $base\n) &&\ntest $base = $(git rev-parse HEAD^) &&\ngit show >output &&\ngrep NEVER output >actual &&\ntest_must_be_empty actual &&\ngit checkout @{-1} &&\ngit branch -D multi-fixup\n'\n"},{"id":"504262","messageId":"CAPSxiM-Lm-1AvpFuEOtYuA2rDEJ0PY9YTV9SwahW0vBdRdjeGA@mail.gmail.com","threadId":"62259","inReplyTo":"ZwN6D_-Ak1FoE0ix@pks.im","subject":"Re: [PATCH v3 2/2] [Outreachy][Patch v1] t3404: employing test_line_count() to replace test","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2024-10-07T07:32:47Z","receivedAt":"2024-10-07T07:32:58Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"On Mon, Oct 7, 2024 at 6:05 AM Patrick Steinhardt <ps@pks.im> wrote:\n>\n> On Sun, Oct 06, 2024 at 04:06:09PM +0000, Usman Akinyemi via GitGitGadget wrote:\n> > @@ -401,8 +402,8 @@ test_expect_success 'multi-squash only fires up editor once' '\n> >       ) &&\n> >       test $base = $(git rev-parse HEAD^) &&\n> >       git show >output &&\n> > -     count=$(grep ONCE output | wc -l) &&\n> > -     test 1 = $count\n> > +     grep ONCE output >actual &&\n> > +     test_line_count = 1 actual\n> >  '\n> >\n> >  test_expect_success 'multi-fixup does not fire up editor' '\n>\n> Oh, you already do the change I proposed on the first commit. It's a bit\n> funny that we first change things one way and then touch it up again in\n> another commit as it leaves the reviewer wondering for a bit.\n>\n> But I guess that's okay, especially for a microproject. So overall I\n> don't see a strong reason to reroll this series, thanks!\n>\n> Patrick\n\nThank you very much for the review. I really appreciate it. I had to\nmake this separate commit as recommended in the resources you provided\nand reviews from other reviewers.\n"},{"id":"504264","messageId":"ZwOW96K1_12Kzefo@pks.im","threadId":"62259","inReplyTo":"CAPSxiM9Z3q5Oz0PkadC72J0cU2F9D8Rh5eAtNz7aZoUZxp6PUA@mail.gmail.com","subject":"Re: [PATCH v3 0/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-07T08:08:29Z","receivedAt":"2024-10-07T08:08:34Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Oct 07, 2024 at 07:25:44AM +0000, Usman Akinyemi wrote:\n> On Mon, Oct 7, 2024 at 4:24 AM Eric Sunshine <sunshine@sunshineco.com> wrote:\n> >\n> > On Sun, Oct 6, 2024 at 12:18 PM Usman Akinyemi\n> > <usmanakinyemi202@gmail.com> wrote:\n> > > Kindly, help take a look if this is okay now.\n> > >\n> > > Also, I wanted to change this also to use test_line_count,\n> > >  test 0 = $(grep -c \"^[^#]\" < .git/rebase-merge/git-rebase-todo)\n> > >\n> > >  But, I tried a different approach and the test kept failing.\n> > >\n> > > Similar as\n> > >\n> > > git show >output &&\n> > > count=$(grep NEVER output | wc -l) &&\n> > > test 0 = $count &&\n> >\n> > What is the actual error you encountered?\n> >\n> > By the way, we have a handy function, test_must_be_empty(), which can\n> > be used if you expect the output to not contain anything. As an\n> > example:\n> >\n> >     git show >output &&\n> >     grep NEVER output >actual &&\n> >     test_must_be_empty actual\n> \n> Thanks for your review, I really appreciate it. I tried this approach,\n> but I was getting this particular error for the testing.\n> \n> not ok 32 - multi-fixup does not fire up editor\n> #\n> # git checkout -b multi-fixup E &&\n> # base=$(git rev-parse HEAD~4) &&\n> # (\n> # set_fake_editor &&\n> # FAKE_COMMIT_AMEND=\"NEVER\" \\\n> # FAKE_LINES=\"1 fixup 2 fixup 3 fixup 4\" \\\n> # git rebase -i $base\n> # ) &&\n> # test $base = $(git rev-parse HEAD^) &&\n> # git show >output &&\n> # grep NEVER output >actual &&\n> # test_must_be_empty actual &&\n> # git checkout @{-1} &&\n> # git branch -D multi-fixup\n> #\n> Below is the particular test case\n> \n> test_expect_success 'multi-fixup does not fire up editor' '\n> git checkout -b multi-fixup E &&\n> base=$(git rev-parse HEAD~4) &&\n> (\n> set_fake_editor &&\n> FAKE_COMMIT_AMEND=\"NEVER\" \\\n> FAKE_LINES=\"1 fixup 2 fixup 3 fixup 4\" \\\n> git rebase -i $base\n> ) &&\n> test $base = $(git rev-parse HEAD^) &&\n> git show >output &&\n> grep NEVER output >actual &&\n> test_must_be_empty actual &&\n> git checkout @{-1} &&\n> git branch -D multi-fixup\n> '\n\nThat makes sense. The expectation here is that `output` shouldn't\ncontain the string \"NEVER\" at all. And as grep(1) would fail when it\ndoesn't find a match the whole test would fail like this. So the below\nwould likely be the best solution.\n\nPatrick\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex f171af3061..978fdfc2f1 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -410,7 +410,8 @@ test_expect_success 'multi-fixup does not fire up editor' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 0 = $(git show | grep NEVER | wc -l) &&\n+\tgit show >output &&\n+\t! grep NEVER output &&\n \tgit checkout @{-1} &&\n \tgit branch -D multi-fixup\n '\n"},{"id":"504265","messageId":"CAPig+cTGESaCMfCzP7Zt3Y6JmG_uroCn1cd6_12W1QKLU7hk8A@mail.gmail.com","threadId":"62259","inReplyTo":"ZwOW96K1_12Kzefo@pks.im","subject":"Re: [PATCH v3 0/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-10-07T08:11:29Z","receivedAt":"2024-10-07T08:11:42Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Oct 7, 2024 at 4:08 AM Patrick Steinhardt <ps@pks.im> wrote:\n> On Mon, Oct 07, 2024 at 07:25:44AM +0000, Usman Akinyemi wrote:\n> > test $base = $(git rev-parse HEAD^) &&\n> > git show >output &&\n> > grep NEVER output >actual &&\n> > test_must_be_empty actual &&\n>\n> That makes sense. The expectation here is that `output` shouldn't\n> contain the string \"NEVER\" at all. And as grep(1) would fail when it\n> doesn't find a match the whole test would fail like this. So the below\n> would likely be the best solution.\n\nThanks. I was just about to respond with the same answer. As a bit of\nextra explanation, the &&-chaining means that every command in the\nchain must return \"success\" (status 0), but the return code of `grep`\ndepends upon whether or not it matched any lines. In this case, it\nreturned non-zero which caused the test to fail.\n"},{"id":"504277","messageId":"b4a725aa-eb90-4640-8e45-b24b9f89eab6@gmail.com","threadId":"62259","inReplyTo":"c9a0cca179bfad129017c7f1085ff87201afb417.1728230769.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 1/2] t3404: avoid losing exit status with focus on `git show` and `git cat-files`","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-10-07T08:52:38Z","receivedAt":"2024-10-07T08:52:42Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Usman\n\nOn 06/10/2024 17:06, Usman Akinyemi via GitGitGadget wrote:\n> From: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> \n> The exit code of the preceding command in a pipe is disregarded. So\n> if that preceding command is a Git command that fails, the test would\n> not fail. Instead, by saving the output of that Git command to a file,\n> and removing the pipe, we make sure the test will fail if that Git\n> command fails.\n\nThis is a good description of the reason for making this change.\n\n> This particular patch focuses on some of the instances\n> which include `git show` and `git cat-files`.\n\nThis patch seems to fix all of the instances of \"git show\" rather than \nsome. It also fixes a few instances of \"git cat-file\" (note there is no \ntrailing \"s\" in the command name). It is not immediately clear to me why \nthose instances of \"git cat-file\" were selected for conversion but not \nothers. However they are a worthwhile improvement and converting them \nall in this patch would make it bit too big to comfortably review so I'm \nhappy with the changes as they are.\n\nThe patch itself looks good.\n\nThanks\n\nPhillip\n\n> Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> ---\n>   t/t3404-rebase-interactive.sh | 71 +++++++++++++++++++++++------------\n>   1 file changed, 48 insertions(+), 23 deletions(-)\n> \n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index f171af3061d..96a65783c47 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -319,7 +319,8 @@ test_expect_success 'retain authorship' '\n>   \tGIT_AUTHOR_NAME=\"Twerp Snog\" git commit -m \"different author\" &&\n>   \tgit tag twerp &&\n>   \tgit rebase -i --onto primary HEAD^ &&\n> -\tgit show HEAD | grep \"^Author: Twerp Snog\"\n> +\tgit show HEAD >actual &&\n> +\tgrep \"^Author: Twerp Snog\" actual\n>   '\n>   \n>   test_expect_success 'retain authorship w/ conflicts' '\n> @@ -360,7 +361,8 @@ test_expect_success 'squash' '\n>   '\n>   \n>   test_expect_success 'retain authorship when squashing' '\n> -\tgit show HEAD | grep \"^Author: Twerp Snog\"\n> +\tgit show HEAD >actual &&\n> +\tgrep \"^Author: Twerp Snog\" actual\n>   '\n>   \n>   test_expect_success '--continue tries to commit' '\n> @@ -374,7 +376,8 @@ test_expect_success '--continue tries to commit' '\n>   \t\tFAKE_COMMIT_MESSAGE=\"chouette!\" git rebase --continue\n>   \t) &&\n>   \ttest_cmp_rev HEAD^ new-branch1 &&\n> -\tgit show HEAD | grep chouette\n> +\tgit show HEAD >actual &&\n> +\tgrep chouette actual\n>   '\n>   \n>   test_expect_success 'verbose flag is heeded, even after --continue' '\n> @@ -397,7 +400,9 @@ test_expect_success 'multi-squash only fires up editor once' '\n>   \t\t\tgit rebase -i $base\n>   \t) &&\n>   \ttest $base = $(git rev-parse HEAD^) &&\n> -\ttest 1 = $(git show | grep ONCE | wc -l)\n> +\tgit show >output &&\n> +\tcount=$(grep ONCE output | wc -l) &&\n> +\ttest 1 = $count\n>   '\n>   \n>   test_expect_success 'multi-fixup does not fire up editor' '\n> @@ -410,7 +415,9 @@ test_expect_success 'multi-fixup does not fire up editor' '\n>   \t\t\tgit rebase -i $base\n>   \t) &&\n>   \ttest $base = $(git rev-parse HEAD^) &&\n> -\ttest 0 = $(git show | grep NEVER | wc -l) &&\n> +\tgit show >output &&\n> +\tcount=$(grep NEVER output | wc -l) &&\n> +\ttest 0 = $count &&\n>   \tgit checkout @{-1} &&\n>   \tgit branch -D multi-fixup\n>   '\n> @@ -428,7 +435,9 @@ test_expect_success 'commit message used after conflict' '\n>   \t\t\tgit rebase --continue\n>   \t) &&\n>   \ttest $base = $(git rev-parse HEAD^) &&\n> -\ttest 1 = $(git show | grep ONCE | wc -l) &&\n> +\tgit show >output &&\n> +\tcount=$(grep ONCE output | wc -l) &&\n> +\ttest 1 = $count &&\n>   \tgit checkout @{-1} &&\n>   \tgit branch -D conflict-fixup\n>   '\n> @@ -446,7 +455,9 @@ test_expect_success 'commit message retained after conflict' '\n>   \t\t\tgit rebase --continue\n>   \t) &&\n>   \ttest $base = $(git rev-parse HEAD^) &&\n> -\ttest 2 = $(git show | grep TWICE | wc -l) &&\n> +\tgit show >output &&\n> +\tcount=$(grep TWICE output | wc -l) &&\n> +\ttest 2 = $count &&\n>   \tgit checkout @{-1} &&\n>   \tgit branch -D conflict-squash\n>   '\n> @@ -470,10 +481,10 @@ test_expect_success 'squash and fixup generate correct log messages' '\n>   \t) &&\n>   \tgit cat-file commit HEAD | sed -e 1,/^\\$/d > actual-squash-fixup &&\n>   \ttest_cmp expect-squash-fixup actual-squash-fixup &&\n> -\tgit cat-file commit HEAD@{2} |\n> -\t\tgrep \"^# This is a combination of 3 commits\\.\"  &&\n> -\tgit cat-file commit HEAD@{3} |\n> -\t\tgrep \"^# This is a combination of 2 commits\\.\"  &&\n> +\tgit cat-file commit HEAD@{2} >actual &&\n> +\tgrep \"^# This is a combination of 3 commits\\.\" actual &&\n> +\tgit cat-file commit HEAD@{3} >actual &&\n> +\tgrep \"^# This is a combination of 2 commits\\.\" actual  &&\n>   \tgit checkout @{-1} &&\n>   \tgit branch -D squash-fixup\n>   '\n> @@ -489,7 +500,9 @@ test_expect_success 'squash ignores comments' '\n>   \t\t\tgit rebase -i $base\n>   \t) &&\n>   \ttest $base = $(git rev-parse HEAD^) &&\n> -\ttest 1 = $(git show | grep ONCE | wc -l) &&\n> +\tgit show >output &&\n> +\tcount=$(grep ONCE output | wc -l) &&\n> +\ttest 1 = $count &&\n>   \tgit checkout @{-1} &&\n>   \tgit branch -D skip-comments\n>   '\n> @@ -505,7 +518,9 @@ test_expect_success 'squash ignores blank lines' '\n>   \t\t\tgit rebase -i $base\n>   \t) &&\n>   \ttest $base = $(git rev-parse HEAD^) &&\n> -\ttest 1 = $(git show | grep ONCE | wc -l) &&\n> +\tgit show >output &&\n> +\tcount=$(grep ONCE output | wc -l) &&\n> +\ttest 1 = $count &&\n>   \tgit checkout @{-1} &&\n>   \tgit branch -D skip-blank-lines\n>   '\n> @@ -572,7 +587,8 @@ test_expect_success '--continue tries to commit, even for \"edit\"' '\n>   \t\tFAKE_COMMIT_MESSAGE=\"chouette!\" git rebase --continue\n>   \t) &&\n>   \ttest edited = $(git show HEAD:file7) &&\n> -\tgit show HEAD | grep chouette &&\n> +\tgit show HEAD >actual &&\n> +\tgrep chouette actual &&\n>   \ttest $parent = $(git rev-parse HEAD^)\n>   '\n>   \n> @@ -757,19 +773,23 @@ test_expect_success 'reword' '\n>   \t\tset_fake_editor &&\n>   \t\tFAKE_LINES=\"1 2 3 reword 4\" FAKE_COMMIT_MESSAGE=\"E changed\" \\\n>   \t\t\tgit rebase -i A &&\n> -\t\tgit show HEAD | grep \"E changed\" &&\n> +\t\tgit show HEAD >actual &&\n> +\t\tgrep \"E changed\" actual &&\n>   \t\ttest $(git rev-parse primary) != $(git rev-parse HEAD) &&\n>   \t\ttest_cmp_rev primary^ HEAD^ &&\n>   \t\tFAKE_LINES=\"1 2 reword 3 4\" FAKE_COMMIT_MESSAGE=\"D changed\" \\\n>   \t\t\tgit rebase -i A &&\n> -\t\tgit show HEAD^ | grep \"D changed\" &&\n> +\t\tgit show HEAD^ >actual &&\n> +\t\tgrep \"D changed\" actual &&\n>   \t\tFAKE_LINES=\"reword 1 2 3 4\" FAKE_COMMIT_MESSAGE=\"B changed\" \\\n>   \t\t\tgit rebase -i A &&\n> -\t\tgit show HEAD~3 | grep \"B changed\" &&\n> +\t\tgit show HEAD~3 >actual &&\n> +\t\tgrep \"B changed\" actual &&\n>   \t\tFAKE_LINES=\"1 r 2 pick 3 p 4\" FAKE_COMMIT_MESSAGE=\"C changed\" \\\n>   \t\t\tgit rebase -i A\n>   \t) &&\n> -\tgit show HEAD~2 | grep \"C changed\"\n> +\tgit show HEAD~2 >actual &&\n> +\tgrep \"C changed\" actual\n>   '\n>   \n>   test_expect_success 'no uncommitted changes when rewording and the todo list is reloaded' '\n> @@ -1003,8 +1023,10 @@ test_expect_success 'rebase -i --root retain root commit author and message' '\n>   \t\tset_fake_editor &&\n>   \t\tFAKE_LINES=\"2\" git rebase -i --root\n>   \t) &&\n> -\tgit cat-file commit HEAD | grep -q \"^author Twerp Snog\" &&\n> -\tgit cat-file commit HEAD | grep -q \"^different author$\"\n> +\tgit cat-file commit HEAD >output &&\n> +\tgrep -q \"^author Twerp Snog\" output &&\n> +\tgit cat-file commit HEAD >actual &&\n> +\tgrep -q \"^different author$\" actual\n>   '\n>   \n>   test_expect_success 'rebase -i --root temporary sentinel commit' '\n> @@ -1013,7 +1035,8 @@ test_expect_success 'rebase -i --root temporary sentinel commit' '\n>   \t\tset_fake_editor &&\n>   \t\ttest_must_fail env FAKE_LINES=\"2\" git rebase -i --root\n>   \t) &&\n> -\tgit cat-file commit HEAD | grep \"^tree $EMPTY_TREE\" &&\n> +\tgit cat-file commit HEAD >actual &&\n> +\tgrep \"^tree $EMPTY_TREE\" actual &&\n>   \tgit rebase --abort\n>   '\n>   \n> @@ -1036,7 +1059,8 @@ test_expect_success 'rebase -i --root reword original root commit' '\n>   \t\tFAKE_LINES=\"reword 1 2\" FAKE_COMMIT_MESSAGE=\"A changed\" \\\n>   \t\t\tgit rebase -i --root\n>   \t) &&\n> -\tgit show HEAD^ | grep \"A changed\" &&\n> +\tgit show HEAD^ >actual &&\n> +\tgrep \"A changed\" actual &&\n>   \ttest -z \"$(git show -s --format=%p HEAD^)\"\n>   '\n>   \n> @@ -1048,7 +1072,8 @@ test_expect_success 'rebase -i --root reword new root commit' '\n>   \t\tFAKE_LINES=\"reword 3 1\" FAKE_COMMIT_MESSAGE=\"C changed\" \\\n>   \t\tgit rebase -i --root\n>   \t) &&\n> -\tgit show HEAD^ | grep \"C changed\" &&\n> +\tgit show HEAD^ >actual &&\n> +\tgrep \"C changed\" actual &&\n>   \ttest -z \"$(git show -s --format=%p HEAD^)\"\n>   '\n>   \n\n"},{"id":"504278","messageId":"75485b2f-4ce4-40f4-bed2-343b3adc830e@gmail.com","threadId":"62259","inReplyTo":"ZwN6D_-Ak1FoE0ix@pks.im","subject":"Re: [PATCH v3 2/2] [Outreachy][Patch v1] t3404: employing test_line_count() to replace test","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-10-07T09:00:15Z","receivedAt":"2024-10-07T09:00:18Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Patrick\n\nOn 07/10/2024 07:05, Patrick Steinhardt wrote:\n> \n> Oh, you already do the change I proposed on the first commit. It's a bit\n> funny that we first change things one way and then touch it up again in\n> another commit as it leaves the reviewer wondering for a bit.\n> \n> But I guess that's okay, especially for a microproject. So overall I\n> don't see a strong reason to reroll this series, thanks!\n\nIt looks like Eric suggested making this change in a separate patch. \nWhile they could perhaps be made in the same patch fixing the pipelines \nand using test_line_count are essentially independent changes so I think \nsplitting them into two patches is good practice and makes sense from \npedagogical point of view.\n\nBest Wishes\n\nPhillip\n\n"},{"id":"504279","messageId":"CAPSxiM9=CzKzjjQCmqeyt5mNf4jFMTwNzN7pzPhAgbLsM8Um+g@mail.gmail.com","threadId":"62259","inReplyTo":"CAPig+cTGESaCMfCzP7Zt3Y6JmG_uroCn1cd6_12W1QKLU7hk8A@mail.gmail.com","subject":"Re: [PATCH v3 0/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2024-10-07T09:01:49Z","receivedAt":"2024-10-07T09:02:02Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"On Mon, Oct 7, 2024 at 8:11 AM Eric Sunshine <sunshine@sunshineco.com> wrote:\n>\n> On Mon, Oct 7, 2024 at 4:08 AM Patrick Steinhardt <ps@pks.im> wrote:\n> > On Mon, Oct 07, 2024 at 07:25:44AM +0000, Usman Akinyemi wrote:\n> > > test $base = $(git rev-parse HEAD^) &&\n> > > git show >output &&\n> > > grep NEVER output >actual &&\n> > > test_must_be_empty actual &&\n> >\n> > That makes sense. The expectation here is that `output` shouldn't\n> > contain the string \"NEVER\" at all. And as grep(1) would fail when it\n> > doesn't find a match the whole test would fail like this. So the below\n> > would likely be the best solution.\n>\n> Thanks. I was just about to respond with the same answer. As a bit of\n> extra explanation, the &&-chaining means that every command in the\n> chain must return \"success\" (status 0), but the return code of `grep`\n> depends upon whether or not it matched any lines. In this case, it\n> returned non-zero which caused the test to fail.\nThanks Eric and Partrick. Thanks for the explanation. I will update\nthe patch then.\n"},{"id":"504280","messageId":"CAPSxiM-b3aNRWM7+aYX83NUnYdVvJ3O1Hm30KhkXCMccmTF0Xw@mail.gmail.com","threadId":"62259","inReplyTo":"b4a725aa-eb90-4640-8e45-b24b9f89eab6@gmail.com","subject":"Re: [PATCH v3 1/2] t3404: avoid losing exit status with focus on `git show` and `git cat-files`","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2024-10-07T09:05:50Z","receivedAt":"2024-10-07T09:06:03Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"On Mon, Oct 7, 2024 at 8:52 AM <phillip.wood123@gmail.com> wrote:\n>\n> Hi Usman\n>\n> On 06/10/2024 17:06, Usman Akinyemi via GitGitGadget wrote:\n> > From: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> >\n> > The exit code of the preceding command in a pipe is disregarded. So\n> > if that preceding command is a Git command that fails, the test would\n> > not fail. Instead, by saving the output of that Git command to a file,\n> > and removing the pipe, we make sure the test will fail if that Git\n> > command fails.\n>\n> This is a good description of the reason for making this change.\n>\n> > This particular patch focuses on some of the instances\n> > which include `git show` and `git cat-files`.\n>\n> This patch seems to fix all of the instances of \"git show\" rather than\n> some. It also fixes a few instances of \"git cat-file\" (note there is no\n> trailing \"s\" in the command name). It is not immediately clear to me why\n> those instances of \"git cat-file\" were selected for conversion but not\n> others. However they are a worthwhile improvement and converting them\n> all in this patch would make it bit too big to comfortably review so I'm\n> happy with the changes as they are.\n>\n> The patch itself looks good.\n>\n> Thanks\n>\n> Phillip\nSince, I am updating a patch. I will update this commit message also.\nThank you for the review.\n>\n> > Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> > ---\n> >   t/t3404-rebase-interactive.sh | 71 +++++++++++++++++++++++------------\n> >   1 file changed, 48 insertions(+), 23 deletions(-)\n> >\n> > diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> > index f171af3061d..96a65783c47 100755\n> > --- a/t/t3404-rebase-interactive.sh\n> > +++ b/t/t3404-rebase-interactive.sh\n> > @@ -319,7 +319,8 @@ test_expect_success 'retain authorship' '\n> >       GIT_AUTHOR_NAME=\"Twerp Snog\" git commit -m \"different author\" &&\n> >       git tag twerp &&\n> >       git rebase -i --onto primary HEAD^ &&\n> > -     git show HEAD | grep \"^Author: Twerp Snog\"\n> > +     git show HEAD >actual &&\n> > +     grep \"^Author: Twerp Snog\" actual\n> >   '\n> >\n> >   test_expect_success 'retain authorship w/ conflicts' '\n> > @@ -360,7 +361,8 @@ test_expect_success 'squash' '\n> >   '\n> >\n> >   test_expect_success 'retain authorship when squashing' '\n> > -     git show HEAD | grep \"^Author: Twerp Snog\"\n> > +     git show HEAD >actual &&\n> > +     grep \"^Author: Twerp Snog\" actual\n> >   '\n> >\n> >   test_expect_success '--continue tries to commit' '\n> > @@ -374,7 +376,8 @@ test_expect_success '--continue tries to commit' '\n> >               FAKE_COMMIT_MESSAGE=\"chouette!\" git rebase --continue\n> >       ) &&\n> >       test_cmp_rev HEAD^ new-branch1 &&\n> > -     git show HEAD | grep chouette\n> > +     git show HEAD >actual &&\n> > +     grep chouette actual\n> >   '\n> >\n> >   test_expect_success 'verbose flag is heeded, even after --continue' '\n> > @@ -397,7 +400,9 @@ test_expect_success 'multi-squash only fires up editor once' '\n> >                       git rebase -i $base\n> >       ) &&\n> >       test $base = $(git rev-parse HEAD^) &&\n> > -     test 1 = $(git show | grep ONCE | wc -l)\n> > +     git show >output &&\n> > +     count=$(grep ONCE output | wc -l) &&\n> > +     test 1 = $count\n> >   '\n> >\n> >   test_expect_success 'multi-fixup does not fire up editor' '\n> > @@ -410,7 +415,9 @@ test_expect_success 'multi-fixup does not fire up editor' '\n> >                       git rebase -i $base\n> >       ) &&\n> >       test $base = $(git rev-parse HEAD^) &&\n> > -     test 0 = $(git show | grep NEVER | wc -l) &&\n> > +     git show >output &&\n> > +     count=$(grep NEVER output | wc -l) &&\n> > +     test 0 = $count &&\n> >       git checkout @{-1} &&\n> >       git branch -D multi-fixup\n> >   '\n> > @@ -428,7 +435,9 @@ test_expect_success 'commit message used after conflict' '\n> >                       git rebase --continue\n> >       ) &&\n> >       test $base = $(git rev-parse HEAD^) &&\n> > -     test 1 = $(git show | grep ONCE | wc -l) &&\n> > +     git show >output &&\n> > +     count=$(grep ONCE output | wc -l) &&\n> > +     test 1 = $count &&\n> >       git checkout @{-1} &&\n> >       git branch -D conflict-fixup\n> >   '\n> > @@ -446,7 +455,9 @@ test_expect_success 'commit message retained after conflict' '\n> >                       git rebase --continue\n> >       ) &&\n> >       test $base = $(git rev-parse HEAD^) &&\n> > -     test 2 = $(git show | grep TWICE | wc -l) &&\n> > +     git show >output &&\n> > +     count=$(grep TWICE output | wc -l) &&\n> > +     test 2 = $count &&\n> >       git checkout @{-1} &&\n> >       git branch -D conflict-squash\n> >   '\n> > @@ -470,10 +481,10 @@ test_expect_success 'squash and fixup generate correct log messages' '\n> >       ) &&\n> >       git cat-file commit HEAD | sed -e 1,/^\\$/d > actual-squash-fixup &&\n> >       test_cmp expect-squash-fixup actual-squash-fixup &&\n> > -     git cat-file commit HEAD@{2} |\n> > -             grep \"^# This is a combination of 3 commits\\.\"  &&\n> > -     git cat-file commit HEAD@{3} |\n> > -             grep \"^# This is a combination of 2 commits\\.\"  &&\n> > +     git cat-file commit HEAD@{2} >actual &&\n> > +     grep \"^# This is a combination of 3 commits\\.\" actual &&\n> > +     git cat-file commit HEAD@{3} >actual &&\n> > +     grep \"^# This is a combination of 2 commits\\.\" actual  &&\n> >       git checkout @{-1} &&\n> >       git branch -D squash-fixup\n> >   '\n> > @@ -489,7 +500,9 @@ test_expect_success 'squash ignores comments' '\n> >                       git rebase -i $base\n> >       ) &&\n> >       test $base = $(git rev-parse HEAD^) &&\n> > -     test 1 = $(git show | grep ONCE | wc -l) &&\n> > +     git show >output &&\n> > +     count=$(grep ONCE output | wc -l) &&\n> > +     test 1 = $count &&\n> >       git checkout @{-1} &&\n> >       git branch -D skip-comments\n> >   '\n> > @@ -505,7 +518,9 @@ test_expect_success 'squash ignores blank lines' '\n> >                       git rebase -i $base\n> >       ) &&\n> >       test $base = $(git rev-parse HEAD^) &&\n> > -     test 1 = $(git show | grep ONCE | wc -l) &&\n> > +     git show >output &&\n> > +     count=$(grep ONCE output | wc -l) &&\n> > +     test 1 = $count &&\n> >       git checkout @{-1} &&\n> >       git branch -D skip-blank-lines\n> >   '\n> > @@ -572,7 +587,8 @@ test_expect_success '--continue tries to commit, even for \"edit\"' '\n> >               FAKE_COMMIT_MESSAGE=\"chouette!\" git rebase --continue\n> >       ) &&\n> >       test edited = $(git show HEAD:file7) &&\n> > -     git show HEAD | grep chouette &&\n> > +     git show HEAD >actual &&\n> > +     grep chouette actual &&\n> >       test $parent = $(git rev-parse HEAD^)\n> >   '\n> >\n> > @@ -757,19 +773,23 @@ test_expect_success 'reword' '\n> >               set_fake_editor &&\n> >               FAKE_LINES=\"1 2 3 reword 4\" FAKE_COMMIT_MESSAGE=\"E changed\" \\\n> >                       git rebase -i A &&\n> > -             git show HEAD | grep \"E changed\" &&\n> > +             git show HEAD >actual &&\n> > +             grep \"E changed\" actual &&\n> >               test $(git rev-parse primary) != $(git rev-parse HEAD) &&\n> >               test_cmp_rev primary^ HEAD^ &&\n> >               FAKE_LINES=\"1 2 reword 3 4\" FAKE_COMMIT_MESSAGE=\"D changed\" \\\n> >                       git rebase -i A &&\n> > -             git show HEAD^ | grep \"D changed\" &&\n> > +             git show HEAD^ >actual &&\n> > +             grep \"D changed\" actual &&\n> >               FAKE_LINES=\"reword 1 2 3 4\" FAKE_COMMIT_MESSAGE=\"B changed\" \\\n> >                       git rebase -i A &&\n> > -             git show HEAD~3 | grep \"B changed\" &&\n> > +             git show HEAD~3 >actual &&\n> > +             grep \"B changed\" actual &&\n> >               FAKE_LINES=\"1 r 2 pick 3 p 4\" FAKE_COMMIT_MESSAGE=\"C changed\" \\\n> >                       git rebase -i A\n> >       ) &&\n> > -     git show HEAD~2 | grep \"C changed\"\n> > +     git show HEAD~2 >actual &&\n> > +     grep \"C changed\" actual\n> >   '\n> >\n> >   test_expect_success 'no uncommitted changes when rewording and the todo list is reloaded' '\n> > @@ -1003,8 +1023,10 @@ test_expect_success 'rebase -i --root retain root commit author and message' '\n> >               set_fake_editor &&\n> >               FAKE_LINES=\"2\" git rebase -i --root\n> >       ) &&\n> > -     git cat-file commit HEAD | grep -q \"^author Twerp Snog\" &&\n> > -     git cat-file commit HEAD | grep -q \"^different author$\"\n> > +     git cat-file commit HEAD >output &&\n> > +     grep -q \"^author Twerp Snog\" output &&\n> > +     git cat-file commit HEAD >actual &&\n> > +     grep -q \"^different author$\" actual\n> >   '\n> >\n> >   test_expect_success 'rebase -i --root temporary sentinel commit' '\n> > @@ -1013,7 +1035,8 @@ test_expect_success 'rebase -i --root temporary sentinel commit' '\n> >               set_fake_editor &&\n> >               test_must_fail env FAKE_LINES=\"2\" git rebase -i --root\n> >       ) &&\n> > -     git cat-file commit HEAD | grep \"^tree $EMPTY_TREE\" &&\n> > +     git cat-file commit HEAD >actual &&\n> > +     grep \"^tree $EMPTY_TREE\" actual &&\n> >       git rebase --abort\n> >   '\n> >\n> > @@ -1036,7 +1059,8 @@ test_expect_success 'rebase -i --root reword original root commit' '\n> >               FAKE_LINES=\"reword 1 2\" FAKE_COMMIT_MESSAGE=\"A changed\" \\\n> >                       git rebase -i --root\n> >       ) &&\n> > -     git show HEAD^ | grep \"A changed\" &&\n> > +     git show HEAD^ >actual &&\n> > +     grep \"A changed\" actual &&\n> >       test -z \"$(git show -s --format=%p HEAD^)\"\n> >   '\n> >\n> > @@ -1048,7 +1072,8 @@ test_expect_success 'rebase -i --root reword new root commit' '\n> >               FAKE_LINES=\"reword 3 1\" FAKE_COMMIT_MESSAGE=\"C changed\" \\\n> >               git rebase -i --root\n> >       ) &&\n> > -     git show HEAD^ | grep \"C changed\" &&\n> > +     git show HEAD^ >actual &&\n> > +     grep \"C changed\" actual &&\n> >       test -z \"$(git show -s --format=%p HEAD^)\"\n> >   '\n> >\n>\n"},{"id":"504295","messageId":"bfff7937cd20737bb5a8791dc7492700b1d7881f.1728296531.git.gitgitgadget@gmail.com","threadId":"62259","inReplyTo":"pull.1805.v4.git.git.1728296531.gitgitgadget@gmail.com","subject":"[PATCH v4 1/2] t3404: avoid losing exit status with focus on `git show` and `git cat-file`","fromName":"Usman Akinyemi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-07T10:22:10Z","receivedAt":"2024-10-07T10:22:15Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"From: Usman Akinyemi <usmanakinyemi202@gmail.com>\n\nThe exit code of the preceding command in a pipe is disregarded. So\nif that preceding command is a Git command that fails, the test would\nnot fail. Instead, by saving the output of that Git command to a file,\nand removing the pipe, we make sure the test will fail if that Git\ncommand fails. This particular patch focuses on all `git show` and\nsome instances of `git cat-file`.\n\nSigned-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n---\n t/t3404-rebase-interactive.sh | 71 +++++++++++++++++++++++------------\n 1 file changed, 48 insertions(+), 23 deletions(-)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex f171af3061d..96a65783c47 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -319,7 +319,8 @@ test_expect_success 'retain authorship' '\n \tGIT_AUTHOR_NAME=\"Twerp Snog\" git commit -m \"different author\" &&\n \tgit tag twerp &&\n \tgit rebase -i --onto primary HEAD^ &&\n-\tgit show HEAD | grep \"^Author: Twerp Snog\"\n+\tgit show HEAD >actual &&\n+\tgrep \"^Author: Twerp Snog\" actual\n '\n \n test_expect_success 'retain authorship w/ conflicts' '\n@@ -360,7 +361,8 @@ test_expect_success 'squash' '\n '\n \n test_expect_success 'retain authorship when squashing' '\n-\tgit show HEAD | grep \"^Author: Twerp Snog\"\n+\tgit show HEAD >actual &&\n+\tgrep \"^Author: Twerp Snog\" actual\n '\n \n test_expect_success '--continue tries to commit' '\n@@ -374,7 +376,8 @@ test_expect_success '--continue tries to commit' '\n \t\tFAKE_COMMIT_MESSAGE=\"chouette!\" git rebase --continue\n \t) &&\n \ttest_cmp_rev HEAD^ new-branch1 &&\n-\tgit show HEAD | grep chouette\n+\tgit show HEAD >actual &&\n+\tgrep chouette actual\n '\n \n test_expect_success 'verbose flag is heeded, even after --continue' '\n@@ -397,7 +400,9 @@ test_expect_success 'multi-squash only fires up editor once' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l)\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = $count\n '\n \n test_expect_success 'multi-fixup does not fire up editor' '\n@@ -410,7 +415,9 @@ test_expect_success 'multi-fixup does not fire up editor' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 0 = $(git show | grep NEVER | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep NEVER output | wc -l) &&\n+\ttest 0 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D multi-fixup\n '\n@@ -428,7 +435,9 @@ test_expect_success 'commit message used after conflict' '\n \t\t\tgit rebase --continue\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-fixup\n '\n@@ -446,7 +455,9 @@ test_expect_success 'commit message retained after conflict' '\n \t\t\tgit rebase --continue\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 2 = $(git show | grep TWICE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep TWICE output | wc -l) &&\n+\ttest 2 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-squash\n '\n@@ -470,10 +481,10 @@ test_expect_success 'squash and fixup generate correct log messages' '\n \t) &&\n \tgit cat-file commit HEAD | sed -e 1,/^\\$/d > actual-squash-fixup &&\n \ttest_cmp expect-squash-fixup actual-squash-fixup &&\n-\tgit cat-file commit HEAD@{2} |\n-\t\tgrep \"^# This is a combination of 3 commits\\.\"  &&\n-\tgit cat-file commit HEAD@{3} |\n-\t\tgrep \"^# This is a combination of 2 commits\\.\"  &&\n+\tgit cat-file commit HEAD@{2} >actual &&\n+\tgrep \"^# This is a combination of 3 commits\\.\" actual &&\n+\tgit cat-file commit HEAD@{3} >actual &&\n+\tgrep \"^# This is a combination of 2 commits\\.\" actual  &&\n \tgit checkout @{-1} &&\n \tgit branch -D squash-fixup\n '\n@@ -489,7 +500,9 @@ test_expect_success 'squash ignores comments' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-comments\n '\n@@ -505,7 +518,9 @@ test_expect_success 'squash ignores blank lines' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-blank-lines\n '\n@@ -572,7 +587,8 @@ test_expect_success '--continue tries to commit, even for \"edit\"' '\n \t\tFAKE_COMMIT_MESSAGE=\"chouette!\" git rebase --continue\n \t) &&\n \ttest edited = $(git show HEAD:file7) &&\n-\tgit show HEAD | grep chouette &&\n+\tgit show HEAD >actual &&\n+\tgrep chouette actual &&\n \ttest $parent = $(git rev-parse HEAD^)\n '\n \n@@ -757,19 +773,23 @@ test_expect_success 'reword' '\n \t\tset_fake_editor &&\n \t\tFAKE_LINES=\"1 2 3 reword 4\" FAKE_COMMIT_MESSAGE=\"E changed\" \\\n \t\t\tgit rebase -i A &&\n-\t\tgit show HEAD | grep \"E changed\" &&\n+\t\tgit show HEAD >actual &&\n+\t\tgrep \"E changed\" actual &&\n \t\ttest $(git rev-parse primary) != $(git rev-parse HEAD) &&\n \t\ttest_cmp_rev primary^ HEAD^ &&\n \t\tFAKE_LINES=\"1 2 reword 3 4\" FAKE_COMMIT_MESSAGE=\"D changed\" \\\n \t\t\tgit rebase -i A &&\n-\t\tgit show HEAD^ | grep \"D changed\" &&\n+\t\tgit show HEAD^ >actual &&\n+\t\tgrep \"D changed\" actual &&\n \t\tFAKE_LINES=\"reword 1 2 3 4\" FAKE_COMMIT_MESSAGE=\"B changed\" \\\n \t\t\tgit rebase -i A &&\n-\t\tgit show HEAD~3 | grep \"B changed\" &&\n+\t\tgit show HEAD~3 >actual &&\n+\t\tgrep \"B changed\" actual &&\n \t\tFAKE_LINES=\"1 r 2 pick 3 p 4\" FAKE_COMMIT_MESSAGE=\"C changed\" \\\n \t\t\tgit rebase -i A\n \t) &&\n-\tgit show HEAD~2 | grep \"C changed\"\n+\tgit show HEAD~2 >actual &&\n+\tgrep \"C changed\" actual\n '\n \n test_expect_success 'no uncommitted changes when rewording and the todo list is reloaded' '\n@@ -1003,8 +1023,10 @@ test_expect_success 'rebase -i --root retain root commit author and message' '\n \t\tset_fake_editor &&\n \t\tFAKE_LINES=\"2\" git rebase -i --root\n \t) &&\n-\tgit cat-file commit HEAD | grep -q \"^author Twerp Snog\" &&\n-\tgit cat-file commit HEAD | grep -q \"^different author$\"\n+\tgit cat-file commit HEAD >output &&\n+\tgrep -q \"^author Twerp Snog\" output &&\n+\tgit cat-file commit HEAD >actual &&\n+\tgrep -q \"^different author$\" actual\n '\n \n test_expect_success 'rebase -i --root temporary sentinel commit' '\n@@ -1013,7 +1035,8 @@ test_expect_success 'rebase -i --root temporary sentinel commit' '\n \t\tset_fake_editor &&\n \t\ttest_must_fail env FAKE_LINES=\"2\" git rebase -i --root\n \t) &&\n-\tgit cat-file commit HEAD | grep \"^tree $EMPTY_TREE\" &&\n+\tgit cat-file commit HEAD >actual &&\n+\tgrep \"^tree $EMPTY_TREE\" actual &&\n \tgit rebase --abort\n '\n \n@@ -1036,7 +1059,8 @@ test_expect_success 'rebase -i --root reword original root commit' '\n \t\tFAKE_LINES=\"reword 1 2\" FAKE_COMMIT_MESSAGE=\"A changed\" \\\n \t\t\tgit rebase -i --root\n \t) &&\n-\tgit show HEAD^ | grep \"A changed\" &&\n+\tgit show HEAD^ >actual &&\n+\tgrep \"A changed\" actual &&\n \ttest -z \"$(git show -s --format=%p HEAD^)\"\n '\n \n@@ -1048,7 +1072,8 @@ test_expect_success 'rebase -i --root reword new root commit' '\n \t\tFAKE_LINES=\"reword 3 1\" FAKE_COMMIT_MESSAGE=\"C changed\" \\\n \t\tgit rebase -i --root\n \t) &&\n-\tgit show HEAD^ | grep \"C changed\" &&\n+\tgit show HEAD^ >actual &&\n+\tgrep \"C changed\" actual &&\n \ttest -z \"$(git show -s --format=%p HEAD^)\"\n '\n \n-- \ngitgitgadget\n\n"},{"id":"504296","messageId":"864b00997b725dbd7f3bea06391570df7d428f3b.1728296531.git.gitgitgadget@gmail.com","threadId":"62259","inReplyTo":"pull.1805.v4.git.git.1728296531.gitgitgadget@gmail.com","subject":"[PATCH v4 2/2] [Outreachy][Patch v1] t3404: employing test_line_count() to replace test","fromName":"Usman Akinyemi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-07T10:22:11Z","receivedAt":"2024-10-07T10:22:16Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"From: Usman Akinyemi <usmanakinyemi202@gmail.com>\n\nRefactor t3404 to replace instances of `test` with `test_line_count()`\nfor checking line counts. This improves readability and aligns with Git's\ncurrent test practices.\n\nSigned-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n\nremoved test\n---\n t/t3404-rebase-interactive.sh | 26 +++++++++++++-------------\n 1 file changed, 13 insertions(+), 13 deletions(-)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 96a65783c47..81f984f4cf5 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -281,7 +281,8 @@ test_expect_success 'stop on conflicting pick' '\n \ttest_cmp expect2 file1 &&\n \ttest \"$(git diff --name-status |\n \t\tsed -n -e \"/^U/s/^U[^a-z]*//p\")\" = file1 &&\n-\ttest 4 = $(grep -v \"^#\" < .git/rebase-merge/done | wc -l) &&\n+\tgrep -v \"^#\" < .git/rebase-merge/done > actual &&\n+\ttest_line_count = 4 actual &&\n \ttest 0 = $(grep -c \"^[^#]\" < .git/rebase-merge/git-rebase-todo)\n '\n \n@@ -401,8 +402,8 @@ test_expect_success 'multi-squash only fires up editor once' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep ONCE output | wc -l) &&\n-\ttest 1 = $count\n+\tgrep ONCE output >actual &&\n+\ttest_line_count = 1 actual\n '\n \n test_expect_success 'multi-fixup does not fire up editor' '\n@@ -416,8 +417,7 @@ test_expect_success 'multi-fixup does not fire up editor' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep NEVER output | wc -l) &&\n-\ttest 0 = $count &&\n+\t! grep NEVER output &&\n \tgit checkout @{-1} &&\n \tgit branch -D multi-fixup\n '\n@@ -436,8 +436,8 @@ test_expect_success 'commit message used after conflict' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep ONCE output | wc -l) &&\n-\ttest 1 = $count &&\n+\tgrep ONCE output >actual &&\n+\ttest_line_count = 1 actual &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-fixup\n '\n@@ -456,8 +456,8 @@ test_expect_success 'commit message retained after conflict' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep TWICE output | wc -l) &&\n-\ttest 2 = $count &&\n+\tgrep TWICE output >actual &&\n+\ttest_line_count = 2 actual &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-squash\n '\n@@ -501,8 +501,8 @@ test_expect_success 'squash ignores comments' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep ONCE output | wc -l) &&\n-\ttest 1 = $count &&\n+\tgrep ONCE output >actual &&\n+\ttest_line_count = 1 actual &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-comments\n '\n@@ -519,8 +519,8 @@ test_expect_success 'squash ignores blank lines' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep ONCE output | wc -l) &&\n-\ttest 1 = $count &&\n+\tgrep ONCE output >actual &&\n+\ttest_line_count = 1 actual &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-blank-lines\n '\n-- \ngitgitgadget\n"},{"id":"504297","messageId":"pull.1805.v4.git.git.1728296531.gitgitgadget@gmail.com","threadId":"62259","inReplyTo":"pull.1805.v3.git.git.1728230769.gitgitgadget@gmail.com","subject":"[PATCH v4 0/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","fromName":"Usman Akinyemi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-07T10:22:09Z","receivedAt":"2024-10-07T10:22:17Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"Changes since v3:\n\n * Replaced the use of grep | wc -l with a simplified ! grep command to\n   directly check for the absence of \"NEVER\" in the output. I was not able\n   to add this in the second patch ( employing test_line_count() to replace\n   test). I was able to come to this solution with the help of Eric and\n   Patriack.\n\nUsman Akinyemi (2):\n  t3404: avoid losing exit status with focus on `git show` and `git\n    cat-file`\n  [Outreachy][Patch v1] t3404: employing test_line_count() to replace\n    test\n\n t/t3404-rebase-interactive.sh | 73 +++++++++++++++++++++++------------\n 1 file changed, 49 insertions(+), 24 deletions(-)\n\n\nbase-commit: 90fe3800b92a49173530828c0a17951abd30f0e1\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1805%2FUnique-Usman%2Favoid_git_pipes-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1805/Unique-Usman/avoid_git_pipes-v4\nPull-Request: https://github.com/git/git/pull/1805\n\nRange-diff vs v3:\n\n 1:  c9a0cca179b ! 1:  bfff7937cd2 t3404: avoid losing exit status with focus on `git show` and `git cat-files`\n     @@ Metadata\n      Author: Usman Akinyemi <usmanakinyemi202@gmail.com>\n      \n       ## Commit message ##\n     -    t3404: avoid losing exit status with focus on `git show` and `git cat-files`\n     +    t3404: avoid losing exit status with focus on `git show` and `git cat-file`\n      \n          The exit code of the preceding command in a pipe is disregarded. So\n          if that preceding command is a Git command that fails, the test would\n          not fail. Instead, by saving the output of that Git command to a file,\n          and removing the pipe, we make sure the test will fail if that Git\n     -    command fails. This particular patch focuses on some of the instances\n     -    which include `git show` and `git cat-files`.\n     +    command fails. This particular patch focuses on all `git show` and\n     +    some instances of `git cat-file`.\n      \n          Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n      \n 2:  37b1411ee2c ! 2:  864b00997b7 [Outreachy][Patch v1] t3404: employing test_line_count() to replace test\n     @@ Commit message\n      \n          Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n      \n     +    removed test\n     +\n       ## t/t3404-rebase-interactive.sh ##\n      @@ t/t3404-rebase-interactive.sh: test_expect_success 'stop on conflicting pick' '\n       \ttest_cmp expect2 file1 &&\n     @@ t/t3404-rebase-interactive.sh: test_expect_success 'multi-squash only fires up e\n       '\n       \n       test_expect_success 'multi-fixup does not fire up editor' '\n     +@@ t/t3404-rebase-interactive.sh: test_expect_success 'multi-fixup does not fire up editor' '\n     + \t) &&\n     + \ttest $base = $(git rev-parse HEAD^) &&\n     + \tgit show >output &&\n     +-\tcount=$(grep NEVER output | wc -l) &&\n     +-\ttest 0 = $count &&\n     ++\t! grep NEVER output &&\n     + \tgit checkout @{-1} &&\n     + \tgit branch -D multi-fixup\n     + '\n      @@ t/t3404-rebase-interactive.sh: test_expect_success 'commit message used after conflict' '\n       \t) &&\n       \ttest $base = $(git rev-parse HEAD^) &&\n\n-- \ngitgitgadget\n"},{"id":"504299","messageId":"ZwO30-cFO9w3frca@pks.im","threadId":"62259","inReplyTo":"864b00997b725dbd7f3bea06391570df7d428f3b.1728296531.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v4 2/2] [Outreachy][Patch v1] t3404: employing test_line_count() to replace test","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-07T10:28:35Z","receivedAt":"2024-10-07T10:28:42Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Oct 07, 2024 at 10:22:11AM +0000, Usman Akinyemi via GitGitGadget wrote:\n\nThe title of this commit still has the \"[Outreachy][Patch v1]\" prefix\nthat you likely want to remove.\n\n> From: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> \n> Refactor t3404 to replace instances of `test` with `test_line_count()`\n> for checking line counts. This improves readability and aligns with Git's\n> current test practices.\n> \n> Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> \n> removed test\n\nI think this line got here by accident :)\n\n> ---\n>  t/t3404-rebase-interactive.sh | 26 +++++++++++++-------------\n>  1 file changed, 13 insertions(+), 13 deletions(-)\n> \n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 96a65783c47..81f984f4cf5 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -281,7 +281,8 @@ test_expect_success 'stop on conflicting pick' '\n>  \ttest_cmp expect2 file1 &&\n>  \ttest \"$(git diff --name-status |\n>  \t\tsed -n -e \"/^U/s/^U[^a-z]*//p\")\" = file1 &&\n> -\ttest 4 = $(grep -v \"^#\" < .git/rebase-merge/done | wc -l) &&\n> +\tgrep -v \"^#\" < .git/rebase-merge/done > actual &&\n\nWe do not use a space after redirects, so this should be\n\"<.git/rebase-merge/done\" and \">actual\".\n\nOther than that this patch looks good to me, thanks!\n\nPatrick\n"},{"id":"504300","messageId":"pull.1805.v5.git.git.1728298308.gitgitgadget@gmail.com","threadId":"62259","inReplyTo":"pull.1805.v4.git.git.1728296531.gitgitgadget@gmail.com","subject":"[PATCH v5 0/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","fromName":"Usman Akinyemi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-07T10:51:46Z","receivedAt":"2024-10-07T10:51:52Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"Changes since v3:\n\n * Replaced the use of grep | wc -l with a simplified ! grep command to\n   directly check for the absence of \"NEVER\" in the output. I was not able\n   to add this in the second patch ( employing test_line_count() to replace\n   test). I was able to come to this solution with the help of Eric and\n   Patriack.\n\nUsman Akinyemi (2):\n  t3404: avoid losing exit status with focus on `git show` and `git\n    cat-file`\n  t3404: employing test_line_count() to replace test\n\n t/t3404-rebase-interactive.sh | 73 +++++++++++++++++++++++------------\n 1 file changed, 49 insertions(+), 24 deletions(-)\n\n\nbase-commit: 90fe3800b92a49173530828c0a17951abd30f0e1\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1805%2FUnique-Usman%2Favoid_git_pipes-v5\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1805/Unique-Usman/avoid_git_pipes-v5\nPull-Request: https://github.com/git/git/pull/1805\n\nRange-diff vs v4:\n\n 1:  bfff7937cd2 = 1:  bfff7937cd2 t3404: avoid losing exit status with focus on `git show` and `git cat-file`\n 2:  864b00997b7 ! 2:  0ce40300fa3 [Outreachy][Patch v1] t3404: employing test_line_count() to replace test\n     @@ Metadata\n      Author: Usman Akinyemi <usmanakinyemi202@gmail.com>\n      \n       ## Commit message ##\n     -    [Outreachy][Patch v1] t3404: employing test_line_count() to replace test\n     +    t3404: employing test_line_count() to replace test\n      \n          Refactor t3404 to replace instances of `test` with `test_line_count()`\n          for checking line counts. This improves readability and aligns with Git's\n     @@ Commit message\n      \n          Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n      \n     -    removed test\n     -\n       ## t/t3404-rebase-interactive.sh ##\n      @@ t/t3404-rebase-interactive.sh: test_expect_success 'stop on conflicting pick' '\n       \ttest_cmp expect2 file1 &&\n       \ttest \"$(git diff --name-status |\n       \t\tsed -n -e \"/^U/s/^U[^a-z]*//p\")\" = file1 &&\n      -\ttest 4 = $(grep -v \"^#\" < .git/rebase-merge/done | wc -l) &&\n     -+\tgrep -v \"^#\" < .git/rebase-merge/done > actual &&\n     ++\tgrep -v \"^#\" < .git/rebase-merge/done >actual &&\n      +\ttest_line_count = 4 actual &&\n       \ttest 0 = $(grep -c \"^[^#]\" < .git/rebase-merge/git-rebase-todo)\n       '\n\n-- \ngitgitgadget\n"},{"id":"504301","messageId":"bfff7937cd20737bb5a8791dc7492700b1d7881f.1728298308.git.gitgitgadget@gmail.com","threadId":"62259","inReplyTo":"pull.1805.v5.git.git.1728298308.gitgitgadget@gmail.com","subject":"[PATCH v5 1/2] t3404: avoid losing exit status with focus on `git show` and `git cat-file`","fromName":"Usman Akinyemi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-07T10:51:47Z","receivedAt":"2024-10-07T10:51:52Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"From: Usman Akinyemi <usmanakinyemi202@gmail.com>\n\nThe exit code of the preceding command in a pipe is disregarded. So\nif that preceding command is a Git command that fails, the test would\nnot fail. Instead, by saving the output of that Git command to a file,\nand removing the pipe, we make sure the test will fail if that Git\ncommand fails. This particular patch focuses on all `git show` and\nsome instances of `git cat-file`.\n\nSigned-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n---\n t/t3404-rebase-interactive.sh | 71 +++++++++++++++++++++++------------\n 1 file changed, 48 insertions(+), 23 deletions(-)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex f171af3061d..96a65783c47 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -319,7 +319,8 @@ test_expect_success 'retain authorship' '\n \tGIT_AUTHOR_NAME=\"Twerp Snog\" git commit -m \"different author\" &&\n \tgit tag twerp &&\n \tgit rebase -i --onto primary HEAD^ &&\n-\tgit show HEAD | grep \"^Author: Twerp Snog\"\n+\tgit show HEAD >actual &&\n+\tgrep \"^Author: Twerp Snog\" actual\n '\n \n test_expect_success 'retain authorship w/ conflicts' '\n@@ -360,7 +361,8 @@ test_expect_success 'squash' '\n '\n \n test_expect_success 'retain authorship when squashing' '\n-\tgit show HEAD | grep \"^Author: Twerp Snog\"\n+\tgit show HEAD >actual &&\n+\tgrep \"^Author: Twerp Snog\" actual\n '\n \n test_expect_success '--continue tries to commit' '\n@@ -374,7 +376,8 @@ test_expect_success '--continue tries to commit' '\n \t\tFAKE_COMMIT_MESSAGE=\"chouette!\" git rebase --continue\n \t) &&\n \ttest_cmp_rev HEAD^ new-branch1 &&\n-\tgit show HEAD | grep chouette\n+\tgit show HEAD >actual &&\n+\tgrep chouette actual\n '\n \n test_expect_success 'verbose flag is heeded, even after --continue' '\n@@ -397,7 +400,9 @@ test_expect_success 'multi-squash only fires up editor once' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l)\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = $count\n '\n \n test_expect_success 'multi-fixup does not fire up editor' '\n@@ -410,7 +415,9 @@ test_expect_success 'multi-fixup does not fire up editor' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 0 = $(git show | grep NEVER | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep NEVER output | wc -l) &&\n+\ttest 0 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D multi-fixup\n '\n@@ -428,7 +435,9 @@ test_expect_success 'commit message used after conflict' '\n \t\t\tgit rebase --continue\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-fixup\n '\n@@ -446,7 +455,9 @@ test_expect_success 'commit message retained after conflict' '\n \t\t\tgit rebase --continue\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 2 = $(git show | grep TWICE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep TWICE output | wc -l) &&\n+\ttest 2 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-squash\n '\n@@ -470,10 +481,10 @@ test_expect_success 'squash and fixup generate correct log messages' '\n \t) &&\n \tgit cat-file commit HEAD | sed -e 1,/^\\$/d > actual-squash-fixup &&\n \ttest_cmp expect-squash-fixup actual-squash-fixup &&\n-\tgit cat-file commit HEAD@{2} |\n-\t\tgrep \"^# This is a combination of 3 commits\\.\"  &&\n-\tgit cat-file commit HEAD@{3} |\n-\t\tgrep \"^# This is a combination of 2 commits\\.\"  &&\n+\tgit cat-file commit HEAD@{2} >actual &&\n+\tgrep \"^# This is a combination of 3 commits\\.\" actual &&\n+\tgit cat-file commit HEAD@{3} >actual &&\n+\tgrep \"^# This is a combination of 2 commits\\.\" actual  &&\n \tgit checkout @{-1} &&\n \tgit branch -D squash-fixup\n '\n@@ -489,7 +500,9 @@ test_expect_success 'squash ignores comments' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-comments\n '\n@@ -505,7 +518,9 @@ test_expect_success 'squash ignores blank lines' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-blank-lines\n '\n@@ -572,7 +587,8 @@ test_expect_success '--continue tries to commit, even for \"edit\"' '\n \t\tFAKE_COMMIT_MESSAGE=\"chouette!\" git rebase --continue\n \t) &&\n \ttest edited = $(git show HEAD:file7) &&\n-\tgit show HEAD | grep chouette &&\n+\tgit show HEAD >actual &&\n+\tgrep chouette actual &&\n \ttest $parent = $(git rev-parse HEAD^)\n '\n \n@@ -757,19 +773,23 @@ test_expect_success 'reword' '\n \t\tset_fake_editor &&\n \t\tFAKE_LINES=\"1 2 3 reword 4\" FAKE_COMMIT_MESSAGE=\"E changed\" \\\n \t\t\tgit rebase -i A &&\n-\t\tgit show HEAD | grep \"E changed\" &&\n+\t\tgit show HEAD >actual &&\n+\t\tgrep \"E changed\" actual &&\n \t\ttest $(git rev-parse primary) != $(git rev-parse HEAD) &&\n \t\ttest_cmp_rev primary^ HEAD^ &&\n \t\tFAKE_LINES=\"1 2 reword 3 4\" FAKE_COMMIT_MESSAGE=\"D changed\" \\\n \t\t\tgit rebase -i A &&\n-\t\tgit show HEAD^ | grep \"D changed\" &&\n+\t\tgit show HEAD^ >actual &&\n+\t\tgrep \"D changed\" actual &&\n \t\tFAKE_LINES=\"reword 1 2 3 4\" FAKE_COMMIT_MESSAGE=\"B changed\" \\\n \t\t\tgit rebase -i A &&\n-\t\tgit show HEAD~3 | grep \"B changed\" &&\n+\t\tgit show HEAD~3 >actual &&\n+\t\tgrep \"B changed\" actual &&\n \t\tFAKE_LINES=\"1 r 2 pick 3 p 4\" FAKE_COMMIT_MESSAGE=\"C changed\" \\\n \t\t\tgit rebase -i A\n \t) &&\n-\tgit show HEAD~2 | grep \"C changed\"\n+\tgit show HEAD~2 >actual &&\n+\tgrep \"C changed\" actual\n '\n \n test_expect_success 'no uncommitted changes when rewording and the todo list is reloaded' '\n@@ -1003,8 +1023,10 @@ test_expect_success 'rebase -i --root retain root commit author and message' '\n \t\tset_fake_editor &&\n \t\tFAKE_LINES=\"2\" git rebase -i --root\n \t) &&\n-\tgit cat-file commit HEAD | grep -q \"^author Twerp Snog\" &&\n-\tgit cat-file commit HEAD | grep -q \"^different author$\"\n+\tgit cat-file commit HEAD >output &&\n+\tgrep -q \"^author Twerp Snog\" output &&\n+\tgit cat-file commit HEAD >actual &&\n+\tgrep -q \"^different author$\" actual\n '\n \n test_expect_success 'rebase -i --root temporary sentinel commit' '\n@@ -1013,7 +1035,8 @@ test_expect_success 'rebase -i --root temporary sentinel commit' '\n \t\tset_fake_editor &&\n \t\ttest_must_fail env FAKE_LINES=\"2\" git rebase -i --root\n \t) &&\n-\tgit cat-file commit HEAD | grep \"^tree $EMPTY_TREE\" &&\n+\tgit cat-file commit HEAD >actual &&\n+\tgrep \"^tree $EMPTY_TREE\" actual &&\n \tgit rebase --abort\n '\n \n@@ -1036,7 +1059,8 @@ test_expect_success 'rebase -i --root reword original root commit' '\n \t\tFAKE_LINES=\"reword 1 2\" FAKE_COMMIT_MESSAGE=\"A changed\" \\\n \t\t\tgit rebase -i --root\n \t) &&\n-\tgit show HEAD^ | grep \"A changed\" &&\n+\tgit show HEAD^ >actual &&\n+\tgrep \"A changed\" actual &&\n \ttest -z \"$(git show -s --format=%p HEAD^)\"\n '\n \n@@ -1048,7 +1072,8 @@ test_expect_success 'rebase -i --root reword new root commit' '\n \t\tFAKE_LINES=\"reword 3 1\" FAKE_COMMIT_MESSAGE=\"C changed\" \\\n \t\tgit rebase -i --root\n \t) &&\n-\tgit show HEAD^ | grep \"C changed\" &&\n+\tgit show HEAD^ >actual &&\n+\tgrep \"C changed\" actual &&\n \ttest -z \"$(git show -s --format=%p HEAD^)\"\n '\n \n-- \ngitgitgadget\n\n"},{"id":"504302","messageId":"0ce40300fa32bde27c789fae9e0c642498710a25.1728298308.git.gitgitgadget@gmail.com","threadId":"62259","inReplyTo":"pull.1805.v5.git.git.1728298308.gitgitgadget@gmail.com","subject":"[PATCH v5 2/2] t3404: employing test_line_count() to replace test","fromName":"Usman Akinyemi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-07T10:51:48Z","receivedAt":"2024-10-07T10:51:54Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"From: Usman Akinyemi <usmanakinyemi202@gmail.com>\n\nRefactor t3404 to replace instances of `test` with `test_line_count()`\nfor checking line counts. This improves readability and aligns with Git's\ncurrent test practices.\n\nSigned-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n---\n t/t3404-rebase-interactive.sh | 26 +++++++++++++-------------\n 1 file changed, 13 insertions(+), 13 deletions(-)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 96a65783c47..1073eb88fa2 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -281,7 +281,8 @@ test_expect_success 'stop on conflicting pick' '\n \ttest_cmp expect2 file1 &&\n \ttest \"$(git diff --name-status |\n \t\tsed -n -e \"/^U/s/^U[^a-z]*//p\")\" = file1 &&\n-\ttest 4 = $(grep -v \"^#\" < .git/rebase-merge/done | wc -l) &&\n+\tgrep -v \"^#\" < .git/rebase-merge/done >actual &&\n+\ttest_line_count = 4 actual &&\n \ttest 0 = $(grep -c \"^[^#]\" < .git/rebase-merge/git-rebase-todo)\n '\n \n@@ -401,8 +402,8 @@ test_expect_success 'multi-squash only fires up editor once' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep ONCE output | wc -l) &&\n-\ttest 1 = $count\n+\tgrep ONCE output >actual &&\n+\ttest_line_count = 1 actual\n '\n \n test_expect_success 'multi-fixup does not fire up editor' '\n@@ -416,8 +417,7 @@ test_expect_success 'multi-fixup does not fire up editor' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep NEVER output | wc -l) &&\n-\ttest 0 = $count &&\n+\t! grep NEVER output &&\n \tgit checkout @{-1} &&\n \tgit branch -D multi-fixup\n '\n@@ -436,8 +436,8 @@ test_expect_success 'commit message used after conflict' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep ONCE output | wc -l) &&\n-\ttest 1 = $count &&\n+\tgrep ONCE output >actual &&\n+\ttest_line_count = 1 actual &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-fixup\n '\n@@ -456,8 +456,8 @@ test_expect_success 'commit message retained after conflict' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep TWICE output | wc -l) &&\n-\ttest 2 = $count &&\n+\tgrep TWICE output >actual &&\n+\ttest_line_count = 2 actual &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-squash\n '\n@@ -501,8 +501,8 @@ test_expect_success 'squash ignores comments' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep ONCE output | wc -l) &&\n-\ttest 1 = $count &&\n+\tgrep ONCE output >actual &&\n+\ttest_line_count = 1 actual &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-comments\n '\n@@ -519,8 +519,8 @@ test_expect_success 'squash ignores blank lines' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep ONCE output | wc -l) &&\n-\ttest 1 = $count &&\n+\tgrep ONCE output >actual &&\n+\ttest_line_count = 1 actual &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-blank-lines\n '\n-- \ngitgitgadget\n"},{"id":"504303","messageId":"ZwO96ACW05R_9DPb@pks.im","threadId":"62259","inReplyTo":"0ce40300fa32bde27c789fae9e0c642498710a25.1728298308.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v5 2/2] t3404: employing test_line_count() to replace test","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-07T10:54:32Z","receivedAt":"2024-10-07T10:54:39Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Oct 07, 2024 at 10:51:48AM +0000, Usman Akinyemi via GitGitGadget wrote:\n> From: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 96a65783c47..1073eb88fa2 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -281,7 +281,8 @@ test_expect_success 'stop on conflicting pick' '\n>  \ttest_cmp expect2 file1 &&\n>  \ttest \"$(git diff --name-status |\n>  \t\tsed -n -e \"/^U/s/^U[^a-z]*//p\")\" = file1 &&\n> -\ttest 4 = $(grep -v \"^#\" < .git/rebase-merge/done | wc -l) &&\n> +\tgrep -v \"^#\" < .git/rebase-merge/done >actual &&\n> +\ttest_line_count = 4 actual &&\n>  \ttest 0 = $(grep -c \"^[^#]\" < .git/rebase-merge/git-rebase-todo)\n>  '\n\nThere's still the space between `<` and `.git/rebase-merge/git-rebase-todo`.\nBut overall this version looks good enough to me, so I don't think this\nrequires a reroll. Thanks!\n\nPatrick\n"},{"id":"504307","messageId":"CAPSxiM8UFdS_T5CHSOUPt6ax=i1KuNf7-9XWnNeQQCtOiR6KCg@mail.gmail.com","threadId":"62259","inReplyTo":"ZwO96ACW05R_9DPb@pks.im","subject":"Re: [PATCH v5 2/2] t3404: employing test_line_count() to replace test","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2024-10-07T11:10:56Z","receivedAt":"2024-10-07T11:11:09Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"On Mon, Oct 7, 2024 at 10:54 AM Patrick Steinhardt <ps@pks.im> wrote:\n>\n> On Mon, Oct 07, 2024 at 10:51:48AM +0000, Usman Akinyemi via GitGitGadget wrote:\n> > From: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> > diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> > index 96a65783c47..1073eb88fa2 100755\n> > --- a/t/t3404-rebase-interactive.sh\n> > +++ b/t/t3404-rebase-interactive.sh\n> > @@ -281,7 +281,8 @@ test_expect_success 'stop on conflicting pick' '\n> >       test_cmp expect2 file1 &&\n> >       test \"$(git diff --name-status |\n> >               sed -n -e \"/^U/s/^U[^a-z]*//p\")\" = file1 &&\n> > -     test 4 = $(grep -v \"^#\" < .git/rebase-merge/done | wc -l) &&\n> > +     grep -v \"^#\" < .git/rebase-merge/done >actual &&\n> > +     test_line_count = 4 actual &&\n> >       test 0 = $(grep -c \"^[^#]\" < .git/rebase-merge/git-rebase-todo)\n> >  '\n>\n> There's still the space between `<` and `.git/rebase-merge/git-rebase-todo`.\n> But overall this version looks good enough to me, so I don't think this\n> requires a reroll. Thanks!\n>\n> Patrick\nThanks for the review. I think it is better to fix it now. I already\ndid. Thank you.\n"},{"id":"504308","messageId":"pull.1805.v6.git.git.1728299466.gitgitgadget@gmail.com","threadId":"62259","inReplyTo":"pull.1805.v5.git.git.1728298308.gitgitgadget@gmail.com","subject":"[PATCH v6 0/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","fromName":"Usman Akinyemi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-07T11:11:04Z","receivedAt":"2024-10-07T11:11:09Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"Changes since v5:\n\n * Remove spaces between redirect to conform with git style.\n\nUsman Akinyemi (2):\n  t3404: avoid losing exit status with focus on `git show` and `git\n    cat-file`\n  t3404: employing test_line_count() to replace test\n\n t/t3404-rebase-interactive.sh | 75 +++++++++++++++++++++++------------\n 1 file changed, 50 insertions(+), 25 deletions(-)\n\n\nbase-commit: 90fe3800b92a49173530828c0a17951abd30f0e1\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1805%2FUnique-Usman%2Favoid_git_pipes-v6\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1805/Unique-Usman/avoid_git_pipes-v6\nPull-Request: https://github.com/git/git/pull/1805\n\nRange-diff vs v5:\n\n 1:  bfff7937cd2 = 1:  bfff7937cd2 t3404: avoid losing exit status with focus on `git show` and `git cat-file`\n 2:  0ce40300fa3 ! 2:  b3d3deced25 t3404: employing test_line_count() to replace test\n     @@ t/t3404-rebase-interactive.sh: test_expect_success 'stop on conflicting pick' '\n       \ttest \"$(git diff --name-status |\n       \t\tsed -n -e \"/^U/s/^U[^a-z]*//p\")\" = file1 &&\n      -\ttest 4 = $(grep -v \"^#\" < .git/rebase-merge/done | wc -l) &&\n     -+\tgrep -v \"^#\" < .git/rebase-merge/done >actual &&\n     +-\ttest 0 = $(grep -c \"^[^#]\" < .git/rebase-merge/git-rebase-todo)\n     ++\tgrep -v \"^#\" <.git/rebase-merge/done >actual &&\n      +\ttest_line_count = 4 actual &&\n     - \ttest 0 = $(grep -c \"^[^#]\" < .git/rebase-merge/git-rebase-todo)\n     ++\ttest 0 = $(grep -c \"^[^#]\" <.git/rebase-merge/git-rebase-todo)\n       '\n       \n     + test_expect_success 'show conflicted patch' '\n      @@ t/t3404-rebase-interactive.sh: test_expect_success 'multi-squash only fires up editor once' '\n       \t) &&\n       \ttest $base = $(git rev-parse HEAD^) &&\n\n-- \ngitgitgadget\n"},{"id":"504309","messageId":"bfff7937cd20737bb5a8791dc7492700b1d7881f.1728299466.git.gitgitgadget@gmail.com","threadId":"62259","inReplyTo":"pull.1805.v6.git.git.1728299466.gitgitgadget@gmail.com","subject":"[PATCH v6 1/2] t3404: avoid losing exit status with focus on `git show` and `git cat-file`","fromName":"Usman Akinyemi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-07T11:11:05Z","receivedAt":"2024-10-07T11:11:10Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"From: Usman Akinyemi <usmanakinyemi202@gmail.com>\n\nThe exit code of the preceding command in a pipe is disregarded. So\nif that preceding command is a Git command that fails, the test would\nnot fail. Instead, by saving the output of that Git command to a file,\nand removing the pipe, we make sure the test will fail if that Git\ncommand fails. This particular patch focuses on all `git show` and\nsome instances of `git cat-file`.\n\nSigned-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n---\n t/t3404-rebase-interactive.sh | 71 +++++++++++++++++++++++------------\n 1 file changed, 48 insertions(+), 23 deletions(-)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex f171af3061d..96a65783c47 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -319,7 +319,8 @@ test_expect_success 'retain authorship' '\n \tGIT_AUTHOR_NAME=\"Twerp Snog\" git commit -m \"different author\" &&\n \tgit tag twerp &&\n \tgit rebase -i --onto primary HEAD^ &&\n-\tgit show HEAD | grep \"^Author: Twerp Snog\"\n+\tgit show HEAD >actual &&\n+\tgrep \"^Author: Twerp Snog\" actual\n '\n \n test_expect_success 'retain authorship w/ conflicts' '\n@@ -360,7 +361,8 @@ test_expect_success 'squash' '\n '\n \n test_expect_success 'retain authorship when squashing' '\n-\tgit show HEAD | grep \"^Author: Twerp Snog\"\n+\tgit show HEAD >actual &&\n+\tgrep \"^Author: Twerp Snog\" actual\n '\n \n test_expect_success '--continue tries to commit' '\n@@ -374,7 +376,8 @@ test_expect_success '--continue tries to commit' '\n \t\tFAKE_COMMIT_MESSAGE=\"chouette!\" git rebase --continue\n \t) &&\n \ttest_cmp_rev HEAD^ new-branch1 &&\n-\tgit show HEAD | grep chouette\n+\tgit show HEAD >actual &&\n+\tgrep chouette actual\n '\n \n test_expect_success 'verbose flag is heeded, even after --continue' '\n@@ -397,7 +400,9 @@ test_expect_success 'multi-squash only fires up editor once' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l)\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = $count\n '\n \n test_expect_success 'multi-fixup does not fire up editor' '\n@@ -410,7 +415,9 @@ test_expect_success 'multi-fixup does not fire up editor' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 0 = $(git show | grep NEVER | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep NEVER output | wc -l) &&\n+\ttest 0 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D multi-fixup\n '\n@@ -428,7 +435,9 @@ test_expect_success 'commit message used after conflict' '\n \t\t\tgit rebase --continue\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-fixup\n '\n@@ -446,7 +455,9 @@ test_expect_success 'commit message retained after conflict' '\n \t\t\tgit rebase --continue\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 2 = $(git show | grep TWICE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep TWICE output | wc -l) &&\n+\ttest 2 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-squash\n '\n@@ -470,10 +481,10 @@ test_expect_success 'squash and fixup generate correct log messages' '\n \t) &&\n \tgit cat-file commit HEAD | sed -e 1,/^\\$/d > actual-squash-fixup &&\n \ttest_cmp expect-squash-fixup actual-squash-fixup &&\n-\tgit cat-file commit HEAD@{2} |\n-\t\tgrep \"^# This is a combination of 3 commits\\.\"  &&\n-\tgit cat-file commit HEAD@{3} |\n-\t\tgrep \"^# This is a combination of 2 commits\\.\"  &&\n+\tgit cat-file commit HEAD@{2} >actual &&\n+\tgrep \"^# This is a combination of 3 commits\\.\" actual &&\n+\tgit cat-file commit HEAD@{3} >actual &&\n+\tgrep \"^# This is a combination of 2 commits\\.\" actual  &&\n \tgit checkout @{-1} &&\n \tgit branch -D squash-fixup\n '\n@@ -489,7 +500,9 @@ test_expect_success 'squash ignores comments' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-comments\n '\n@@ -505,7 +518,9 @@ test_expect_success 'squash ignores blank lines' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-blank-lines\n '\n@@ -572,7 +587,8 @@ test_expect_success '--continue tries to commit, even for \"edit\"' '\n \t\tFAKE_COMMIT_MESSAGE=\"chouette!\" git rebase --continue\n \t) &&\n \ttest edited = $(git show HEAD:file7) &&\n-\tgit show HEAD | grep chouette &&\n+\tgit show HEAD >actual &&\n+\tgrep chouette actual &&\n \ttest $parent = $(git rev-parse HEAD^)\n '\n \n@@ -757,19 +773,23 @@ test_expect_success 'reword' '\n \t\tset_fake_editor &&\n \t\tFAKE_LINES=\"1 2 3 reword 4\" FAKE_COMMIT_MESSAGE=\"E changed\" \\\n \t\t\tgit rebase -i A &&\n-\t\tgit show HEAD | grep \"E changed\" &&\n+\t\tgit show HEAD >actual &&\n+\t\tgrep \"E changed\" actual &&\n \t\ttest $(git rev-parse primary) != $(git rev-parse HEAD) &&\n \t\ttest_cmp_rev primary^ HEAD^ &&\n \t\tFAKE_LINES=\"1 2 reword 3 4\" FAKE_COMMIT_MESSAGE=\"D changed\" \\\n \t\t\tgit rebase -i A &&\n-\t\tgit show HEAD^ | grep \"D changed\" &&\n+\t\tgit show HEAD^ >actual &&\n+\t\tgrep \"D changed\" actual &&\n \t\tFAKE_LINES=\"reword 1 2 3 4\" FAKE_COMMIT_MESSAGE=\"B changed\" \\\n \t\t\tgit rebase -i A &&\n-\t\tgit show HEAD~3 | grep \"B changed\" &&\n+\t\tgit show HEAD~3 >actual &&\n+\t\tgrep \"B changed\" actual &&\n \t\tFAKE_LINES=\"1 r 2 pick 3 p 4\" FAKE_COMMIT_MESSAGE=\"C changed\" \\\n \t\t\tgit rebase -i A\n \t) &&\n-\tgit show HEAD~2 | grep \"C changed\"\n+\tgit show HEAD~2 >actual &&\n+\tgrep \"C changed\" actual\n '\n \n test_expect_success 'no uncommitted changes when rewording and the todo list is reloaded' '\n@@ -1003,8 +1023,10 @@ test_expect_success 'rebase -i --root retain root commit author and message' '\n \t\tset_fake_editor &&\n \t\tFAKE_LINES=\"2\" git rebase -i --root\n \t) &&\n-\tgit cat-file commit HEAD | grep -q \"^author Twerp Snog\" &&\n-\tgit cat-file commit HEAD | grep -q \"^different author$\"\n+\tgit cat-file commit HEAD >output &&\n+\tgrep -q \"^author Twerp Snog\" output &&\n+\tgit cat-file commit HEAD >actual &&\n+\tgrep -q \"^different author$\" actual\n '\n \n test_expect_success 'rebase -i --root temporary sentinel commit' '\n@@ -1013,7 +1035,8 @@ test_expect_success 'rebase -i --root temporary sentinel commit' '\n \t\tset_fake_editor &&\n \t\ttest_must_fail env FAKE_LINES=\"2\" git rebase -i --root\n \t) &&\n-\tgit cat-file commit HEAD | grep \"^tree $EMPTY_TREE\" &&\n+\tgit cat-file commit HEAD >actual &&\n+\tgrep \"^tree $EMPTY_TREE\" actual &&\n \tgit rebase --abort\n '\n \n@@ -1036,7 +1059,8 @@ test_expect_success 'rebase -i --root reword original root commit' '\n \t\tFAKE_LINES=\"reword 1 2\" FAKE_COMMIT_MESSAGE=\"A changed\" \\\n \t\t\tgit rebase -i --root\n \t) &&\n-\tgit show HEAD^ | grep \"A changed\" &&\n+\tgit show HEAD^ >actual &&\n+\tgrep \"A changed\" actual &&\n \ttest -z \"$(git show -s --format=%p HEAD^)\"\n '\n \n@@ -1048,7 +1072,8 @@ test_expect_success 'rebase -i --root reword new root commit' '\n \t\tFAKE_LINES=\"reword 3 1\" FAKE_COMMIT_MESSAGE=\"C changed\" \\\n \t\tgit rebase -i --root\n \t) &&\n-\tgit show HEAD^ | grep \"C changed\" &&\n+\tgit show HEAD^ >actual &&\n+\tgrep \"C changed\" actual &&\n \ttest -z \"$(git show -s --format=%p HEAD^)\"\n '\n \n-- \ngitgitgadget\n\n"},{"id":"504310","messageId":"b3d3deced25f038defe6afe7d474350464328299.1728299466.git.gitgitgadget@gmail.com","threadId":"62259","inReplyTo":"pull.1805.v6.git.git.1728299466.gitgitgadget@gmail.com","subject":"[PATCH v6 2/2] t3404: employing test_line_count() to replace test","fromName":"Usman Akinyemi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-07T11:11:06Z","receivedAt":"2024-10-07T11:11:10Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"From: Usman Akinyemi <usmanakinyemi202@gmail.com>\n\nRefactor t3404 to replace instances of `test` with `test_line_count()`\nfor checking line counts. This improves readability and aligns with Git's\ncurrent test practices.\n\nSigned-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n---\n t/t3404-rebase-interactive.sh | 28 ++++++++++++++--------------\n 1 file changed, 14 insertions(+), 14 deletions(-)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 96a65783c47..2ab660ef30f 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -281,8 +281,9 @@ test_expect_success 'stop on conflicting pick' '\n \ttest_cmp expect2 file1 &&\n \ttest \"$(git diff --name-status |\n \t\tsed -n -e \"/^U/s/^U[^a-z]*//p\")\" = file1 &&\n-\ttest 4 = $(grep -v \"^#\" < .git/rebase-merge/done | wc -l) &&\n-\ttest 0 = $(grep -c \"^[^#]\" < .git/rebase-merge/git-rebase-todo)\n+\tgrep -v \"^#\" <.git/rebase-merge/done >actual &&\n+\ttest_line_count = 4 actual &&\n+\ttest 0 = $(grep -c \"^[^#]\" <.git/rebase-merge/git-rebase-todo)\n '\n \n test_expect_success 'show conflicted patch' '\n@@ -401,8 +402,8 @@ test_expect_success 'multi-squash only fires up editor once' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep ONCE output | wc -l) &&\n-\ttest 1 = $count\n+\tgrep ONCE output >actual &&\n+\ttest_line_count = 1 actual\n '\n \n test_expect_success 'multi-fixup does not fire up editor' '\n@@ -416,8 +417,7 @@ test_expect_success 'multi-fixup does not fire up editor' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep NEVER output | wc -l) &&\n-\ttest 0 = $count &&\n+\t! grep NEVER output &&\n \tgit checkout @{-1} &&\n \tgit branch -D multi-fixup\n '\n@@ -436,8 +436,8 @@ test_expect_success 'commit message used after conflict' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep ONCE output | wc -l) &&\n-\ttest 1 = $count &&\n+\tgrep ONCE output >actual &&\n+\ttest_line_count = 1 actual &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-fixup\n '\n@@ -456,8 +456,8 @@ test_expect_success 'commit message retained after conflict' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep TWICE output | wc -l) &&\n-\ttest 2 = $count &&\n+\tgrep TWICE output >actual &&\n+\ttest_line_count = 2 actual &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-squash\n '\n@@ -501,8 +501,8 @@ test_expect_success 'squash ignores comments' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep ONCE output | wc -l) &&\n-\ttest 1 = $count &&\n+\tgrep ONCE output >actual &&\n+\ttest_line_count = 1 actual &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-comments\n '\n@@ -519,8 +519,8 @@ test_expect_success 'squash ignores blank lines' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep ONCE output | wc -l) &&\n-\ttest 1 = $count &&\n+\tgrep ONCE output >actual &&\n+\ttest_line_count = 1 actual &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-blank-lines\n '\n-- \ngitgitgadget\n"},{"id":"504315","messageId":"ZwPCDAjeQVf4VFc_@pks.im","threadId":"62259","inReplyTo":"pull.1805.v6.git.git.1728299466.gitgitgadget@gmail.com","subject":"Re: [PATCH v6 0/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-07T11:12:12Z","receivedAt":"2024-10-07T11:12:18Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Oct 07, 2024 at 11:11:04AM +0000, Usman Akinyemi via GitGitGadget wrote:\n> Changes since v5:\n> \n>  * Remove spaces between redirect to conform with git style.\n\nThanks, this version looks good to me now.\n\nPatrick\n"},{"id":"504316","messageId":"CAPSxiM8FpPyCgvRqrH1gEhZEskGRAeBO_2+_1FamTHaezr9+Cw@mail.gmail.com","threadId":"62259","inReplyTo":"ZwPCDAjeQVf4VFc_@pks.im","subject":"Re: [PATCH v6 0/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2024-10-07T11:16:49Z","receivedAt":"2024-10-07T11:17:02Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"On Mon, Oct 7, 2024 at 11:12 AM Patrick Steinhardt <ps@pks.im> wrote:\n>\n> On Mon, Oct 07, 2024 at 11:11:04AM +0000, Usman Akinyemi via GitGitGadget wrote:\n> > Changes since v5:\n> >\n> >  * Remove spaces between redirect to conform with git style.\n>\n> Thanks, this version looks good to me now.\n>\n> Patrick\n\nThank you very much and I appreciate all the guidance from all\nreviewers. I Learned a great lot of new things.\nGoing forward, what should be my next step? Can I work on other tasks\nnow or wait for this to be merged? Are there any other things also\nexpected from my side as an outreachy applicant ? Thank you  very\nmuch.\n"},{"id":"504319","messageId":"ZwPG3QHarKerpN3K@pks.im","threadId":"62259","inReplyTo":"CAPSxiM8FpPyCgvRqrH1gEhZEskGRAeBO_2+_1FamTHaezr9+Cw@mail.gmail.com","subject":"Re: [PATCH v6 0/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-10-07T11:32:56Z","receivedAt":"2024-10-07T11:33:01Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Oct 07, 2024 at 11:16:49AM +0000, Usman Akinyemi wrote:\n> On Mon, Oct 7, 2024 at 11:12 AM Patrick Steinhardt <ps@pks.im> wrote:\n> >\n> > On Mon, Oct 07, 2024 at 11:11:04AM +0000, Usman Akinyemi via GitGitGadget wrote:\n> > > Changes since v5:\n> > >\n> > >  * Remove spaces between redirect to conform with git style.\n> >\n> > Thanks, this version looks good to me now.\n> >\n> > Patrick\n> \n> Thank you very much and I appreciate all the guidance from all\n> reviewers. I Learned a great lot of new things.\n> Going forward, what should be my next step? Can I work on other tasks\n> now or wait for this to be merged? Are there any other things also\n> expected from my side as an outreachy applicant ? Thank you  very\n> much.\n\nFor now I'd recommend to wait a couple of days until the patch series\nyou have sent gets picked up by Junio and merged to `next`. You should\nwatch out for the \"What's cooking?\" reports that he sends out every\ncouple days and observe how your topic progresses in it. Note that\nthings may go a bit slower right now due to the pending Git v2.47\nrelease, so it may take a while before he picks your topic.\n\nWhen things go smoothly: congrats, you have checked the first box and\nhave successfully completed your microproject :) I'd recommend to keep\non reading the mailing list to get familiar with how things work over\nhere and get some familiarity with the code in question.\n\nYou are of course free to send additional patches, but this is not a\nrequirement. If you still want to do so I'd recommend to not pick up a\nmicroproject, but try to find a work item on your own, e.g. by searching\nfor \"#leftoverbits\" in our mailing list. The reason for why I don't\nrecommend to pick up another microproject is so that other applicants\nhave items to work on, and it decreases the likelihood that your changes\ncollide with the work of another intern.\n\nPatrick\n"},{"id":"504320","messageId":"CAPSxiM8cpX9NYB02fAPA7WbLsLrvmFfsE2=VN=Ncguif7mQN9Q@mail.gmail.com","threadId":"62259","inReplyTo":"ZwPG3QHarKerpN3K@pks.im","subject":"Re: [PATCH v6 0/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2024-10-07T11:44:31Z","receivedAt":"2024-10-07T11:44:43Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"On Mon, Oct 7, 2024 at 11:33 AM Patrick Steinhardt <ps@pks.im> wrote:\n>\n> On Mon, Oct 07, 2024 at 11:16:49AM +0000, Usman Akinyemi wrote:\n> > On Mon, Oct 7, 2024 at 11:12 AM Patrick Steinhardt <ps@pks.im> wrote:\n> > >\n> > > On Mon, Oct 07, 2024 at 11:11:04AM +0000, Usman Akinyemi via GitGitGadget wrote:\n> > > > Changes since v5:\n> > > >\n> > > >  * Remove spaces between redirect to conform with git style.\n> > >\n> > > Thanks, this version looks good to me now.\n> > >\n> > > Patrick\n> >\n> > Thank you very much and I appreciate all the guidance from all\n> > reviewers. I Learned a great lot of new things.\n> > Going forward, what should be my next step? Can I work on other tasks\n> > now or wait for this to be merged? Are there any other things also\n> > expected from my side as an outreachy applicant ? Thank you  very\n> > much.\n>\n> For now I'd recommend to wait a couple of days until the patch series\n> you have sent gets picked up by Junio and merged to `next`. You should\n> watch out for the \"What's cooking?\" reports that he sends out every\n> couple days and observe how your topic progresses in it. Note that\n> things may go a bit slower right now due to the pending Git v2.47\n> release, so it may take a while before he picks your topic.\n>\n> When things go smoothly: congrats, you have checked the first box and\n> have successfully completed your microproject :) I'd recommend to keep\n> on reading the mailing list to get familiar with how things work over\n> here and get some familiarity with the code in question.\n>\n> You are of course free to send additional patches, but this is not a\n> requirement. If you still want to do so I'd recommend to not pick up a\n> microproject, but try to find a work item on your own, e.g. by searching\n> for \"#leftoverbits\" in our mailing list. The reason for why I don't\n> recommend to pick up another microproject is so that other applicants\n> have items to work on, and it decreases the likelihood that your changes\n> collide with the work of another intern.\n>\n> Patrick\n\nThanks a lot, I really appreciate it.\n"},{"id":"504332","messageId":"4f79f144-9f48-4fbe-b646-2d146e57606a@gmail.com","threadId":"62259","inReplyTo":"b3d3deced25f038defe6afe7d474350464328299.1728299466.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v6 2/2] t3404: employing test_line_count() to replace test","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-10-07T15:07:28Z","receivedAt":"2024-10-07T15:07:31Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Usman\n\nI missed this when I read the earlier version of this series but our \ncommit subjects use the imperative so we would say\n\n     t3403: employ test_line_count() to replace test\n\nor\n\n     t3404: replace test with test_line_count()\n\nI agree with Patrick that we should let this series settle for a couple \nof days to allow others to comment before sending any new version.\n\nThis series is looking good, congratulations\n\nPhillip\n\nOn 07/10/2024 12:11, Usman Akinyemi via GitGitGadget wrote:\n> From: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> \n> Refactor t3404 to replace instances of `test` with `test_line_count()`\n> for checking line counts. This improves readability and aligns with Git's\n> current test practices.\n> \n> Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> ---\n>   t/t3404-rebase-interactive.sh | 28 ++++++++++++++--------------\n>   1 file changed, 14 insertions(+), 14 deletions(-)\n> \n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 96a65783c47..2ab660ef30f 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -281,8 +281,9 @@ test_expect_success 'stop on conflicting pick' '\n>   \ttest_cmp expect2 file1 &&\n>   \ttest \"$(git diff --name-status |\n>   \t\tsed -n -e \"/^U/s/^U[^a-z]*//p\")\" = file1 &&\n> -\ttest 4 = $(grep -v \"^#\" < .git/rebase-merge/done | wc -l) &&\n> -\ttest 0 = $(grep -c \"^[^#]\" < .git/rebase-merge/git-rebase-todo)\n> +\tgrep -v \"^#\" <.git/rebase-merge/done >actual &&\n> +\ttest_line_count = 4 actual &&\n> +\ttest 0 = $(grep -c \"^[^#]\" <.git/rebase-merge/git-rebase-todo)\n>   '\n>   \n>   test_expect_success 'show conflicted patch' '\n> @@ -401,8 +402,8 @@ test_expect_success 'multi-squash only fires up editor once' '\n>   \t) &&\n>   \ttest $base = $(git rev-parse HEAD^) &&\n>   \tgit show >output &&\n> -\tcount=$(grep ONCE output | wc -l) &&\n> -\ttest 1 = $count\n> +\tgrep ONCE output >actual &&\n> +\ttest_line_count = 1 actual\n>   '\n>   \n>   test_expect_success 'multi-fixup does not fire up editor' '\n> @@ -416,8 +417,7 @@ test_expect_success 'multi-fixup does not fire up editor' '\n>   \t) &&\n>   \ttest $base = $(git rev-parse HEAD^) &&\n>   \tgit show >output &&\n> -\tcount=$(grep NEVER output | wc -l) &&\n> -\ttest 0 = $count &&\n> +\t! grep NEVER output &&\n>   \tgit checkout @{-1} &&\n>   \tgit branch -D multi-fixup\n>   '\n> @@ -436,8 +436,8 @@ test_expect_success 'commit message used after conflict' '\n>   \t) &&\n>   \ttest $base = $(git rev-parse HEAD^) &&\n>   \tgit show >output &&\n> -\tcount=$(grep ONCE output | wc -l) &&\n> -\ttest 1 = $count &&\n> +\tgrep ONCE output >actual &&\n> +\ttest_line_count = 1 actual &&\n>   \tgit checkout @{-1} &&\n>   \tgit branch -D conflict-fixup\n>   '\n> @@ -456,8 +456,8 @@ test_expect_success 'commit message retained after conflict' '\n>   \t) &&\n>   \ttest $base = $(git rev-parse HEAD^) &&\n>   \tgit show >output &&\n> -\tcount=$(grep TWICE output | wc -l) &&\n> -\ttest 2 = $count &&\n> +\tgrep TWICE output >actual &&\n> +\ttest_line_count = 2 actual &&\n>   \tgit checkout @{-1} &&\n>   \tgit branch -D conflict-squash\n>   '\n> @@ -501,8 +501,8 @@ test_expect_success 'squash ignores comments' '\n>   \t) &&\n>   \ttest $base = $(git rev-parse HEAD^) &&\n>   \tgit show >output &&\n> -\tcount=$(grep ONCE output | wc -l) &&\n> -\ttest 1 = $count &&\n> +\tgrep ONCE output >actual &&\n> +\ttest_line_count = 1 actual &&\n>   \tgit checkout @{-1} &&\n>   \tgit branch -D skip-comments\n>   '\n> @@ -519,8 +519,8 @@ test_expect_success 'squash ignores blank lines' '\n>   \t) &&\n>   \ttest $base = $(git rev-parse HEAD^) &&\n>   \tgit show >output &&\n> -\tcount=$(grep ONCE output | wc -l) &&\n> -\ttest 1 = $count &&\n> +\tgrep ONCE output >actual &&\n> +\ttest_line_count = 1 actual &&\n>   \tgit checkout @{-1} &&\n>   \tgit branch -D skip-blank-lines\n>   '\n\n"},{"id":"504339","messageId":"pull.1805.v7.git.git.1728315124.gitgitgadget@gmail.com","threadId":"62259","inReplyTo":"pull.1805.v6.git.git.1728299466.gitgitgadget@gmail.com","subject":"[PATCH v7 0/2] [Outreachy][Patch v2] t3404: avoid losing exit status to pipes","fromName":"Usman Akinyemi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-07T15:32:01Z","receivedAt":"2024-10-07T15:32:08Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"Changes since v6:\n\n * Updated commit subjects to use imperative for proper Git commit subject.\n\nUsman Akinyemi (2):\n  t3404: avoid losing exit status with focus on `git show` and `git\n    cat-file`\n  t3404: replace test with test_line_count()\n\n t/t3404-rebase-interactive.sh | 75 +++++++++++++++++++++++------------\n 1 file changed, 50 insertions(+), 25 deletions(-)\n\n\nbase-commit: 90fe3800b92a49173530828c0a17951abd30f0e1\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1805%2FUnique-Usman%2Favoid_git_pipes-v7\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1805/Unique-Usman/avoid_git_pipes-v7\nPull-Request: https://github.com/git/git/pull/1805\n\nRange-diff vs v6:\n\n 1:  bfff7937cd2 = 1:  bfff7937cd2 t3404: avoid losing exit status with focus on `git show` and `git cat-file`\n 2:  b3d3deced25 ! 2:  e2cae7f3a51 t3404: employing test_line_count() to replace test\n     @@ Metadata\n      Author: Usman Akinyemi <usmanakinyemi202@gmail.com>\n      \n       ## Commit message ##\n     -    t3404: employing test_line_count() to replace test\n     +    t3404: replace test with test_line_count()\n      \n          Refactor t3404 to replace instances of `test` with `test_line_count()`\n          for checking line counts. This improves readability and aligns with Git's\n\n-- \ngitgitgadget\n"},{"id":"504340","messageId":"bfff7937cd20737bb5a8791dc7492700b1d7881f.1728315124.git.gitgitgadget@gmail.com","threadId":"62259","inReplyTo":"pull.1805.v7.git.git.1728315124.gitgitgadget@gmail.com","subject":"[PATCH v7 1/2] t3404: avoid losing exit status with focus on `git show` and `git cat-file`","fromName":"Usman Akinyemi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-07T15:32:02Z","receivedAt":"2024-10-07T15:32:09Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"From: Usman Akinyemi <usmanakinyemi202@gmail.com>\n\nThe exit code of the preceding command in a pipe is disregarded. So\nif that preceding command is a Git command that fails, the test would\nnot fail. Instead, by saving the output of that Git command to a file,\nand removing the pipe, we make sure the test will fail if that Git\ncommand fails. This particular patch focuses on all `git show` and\nsome instances of `git cat-file`.\n\nSigned-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n---\n t/t3404-rebase-interactive.sh | 71 +++++++++++++++++++++++------------\n 1 file changed, 48 insertions(+), 23 deletions(-)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex f171af3061d..96a65783c47 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -319,7 +319,8 @@ test_expect_success 'retain authorship' '\n \tGIT_AUTHOR_NAME=\"Twerp Snog\" git commit -m \"different author\" &&\n \tgit tag twerp &&\n \tgit rebase -i --onto primary HEAD^ &&\n-\tgit show HEAD | grep \"^Author: Twerp Snog\"\n+\tgit show HEAD >actual &&\n+\tgrep \"^Author: Twerp Snog\" actual\n '\n \n test_expect_success 'retain authorship w/ conflicts' '\n@@ -360,7 +361,8 @@ test_expect_success 'squash' '\n '\n \n test_expect_success 'retain authorship when squashing' '\n-\tgit show HEAD | grep \"^Author: Twerp Snog\"\n+\tgit show HEAD >actual &&\n+\tgrep \"^Author: Twerp Snog\" actual\n '\n \n test_expect_success '--continue tries to commit' '\n@@ -374,7 +376,8 @@ test_expect_success '--continue tries to commit' '\n \t\tFAKE_COMMIT_MESSAGE=\"chouette!\" git rebase --continue\n \t) &&\n \ttest_cmp_rev HEAD^ new-branch1 &&\n-\tgit show HEAD | grep chouette\n+\tgit show HEAD >actual &&\n+\tgrep chouette actual\n '\n \n test_expect_success 'verbose flag is heeded, even after --continue' '\n@@ -397,7 +400,9 @@ test_expect_success 'multi-squash only fires up editor once' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l)\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = $count\n '\n \n test_expect_success 'multi-fixup does not fire up editor' '\n@@ -410,7 +415,9 @@ test_expect_success 'multi-fixup does not fire up editor' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 0 = $(git show | grep NEVER | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep NEVER output | wc -l) &&\n+\ttest 0 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D multi-fixup\n '\n@@ -428,7 +435,9 @@ test_expect_success 'commit message used after conflict' '\n \t\t\tgit rebase --continue\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-fixup\n '\n@@ -446,7 +455,9 @@ test_expect_success 'commit message retained after conflict' '\n \t\t\tgit rebase --continue\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 2 = $(git show | grep TWICE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep TWICE output | wc -l) &&\n+\ttest 2 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-squash\n '\n@@ -470,10 +481,10 @@ test_expect_success 'squash and fixup generate correct log messages' '\n \t) &&\n \tgit cat-file commit HEAD | sed -e 1,/^\\$/d > actual-squash-fixup &&\n \ttest_cmp expect-squash-fixup actual-squash-fixup &&\n-\tgit cat-file commit HEAD@{2} |\n-\t\tgrep \"^# This is a combination of 3 commits\\.\"  &&\n-\tgit cat-file commit HEAD@{3} |\n-\t\tgrep \"^# This is a combination of 2 commits\\.\"  &&\n+\tgit cat-file commit HEAD@{2} >actual &&\n+\tgrep \"^# This is a combination of 3 commits\\.\" actual &&\n+\tgit cat-file commit HEAD@{3} >actual &&\n+\tgrep \"^# This is a combination of 2 commits\\.\" actual  &&\n \tgit checkout @{-1} &&\n \tgit branch -D squash-fixup\n '\n@@ -489,7 +500,9 @@ test_expect_success 'squash ignores comments' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-comments\n '\n@@ -505,7 +518,9 @@ test_expect_success 'squash ignores blank lines' '\n \t\t\tgit rebase -i $base\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n-\ttest 1 = $(git show | grep ONCE | wc -l) &&\n+\tgit show >output &&\n+\tcount=$(grep ONCE output | wc -l) &&\n+\ttest 1 = $count &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-blank-lines\n '\n@@ -572,7 +587,8 @@ test_expect_success '--continue tries to commit, even for \"edit\"' '\n \t\tFAKE_COMMIT_MESSAGE=\"chouette!\" git rebase --continue\n \t) &&\n \ttest edited = $(git show HEAD:file7) &&\n-\tgit show HEAD | grep chouette &&\n+\tgit show HEAD >actual &&\n+\tgrep chouette actual &&\n \ttest $parent = $(git rev-parse HEAD^)\n '\n \n@@ -757,19 +773,23 @@ test_expect_success 'reword' '\n \t\tset_fake_editor &&\n \t\tFAKE_LINES=\"1 2 3 reword 4\" FAKE_COMMIT_MESSAGE=\"E changed\" \\\n \t\t\tgit rebase -i A &&\n-\t\tgit show HEAD | grep \"E changed\" &&\n+\t\tgit show HEAD >actual &&\n+\t\tgrep \"E changed\" actual &&\n \t\ttest $(git rev-parse primary) != $(git rev-parse HEAD) &&\n \t\ttest_cmp_rev primary^ HEAD^ &&\n \t\tFAKE_LINES=\"1 2 reword 3 4\" FAKE_COMMIT_MESSAGE=\"D changed\" \\\n \t\t\tgit rebase -i A &&\n-\t\tgit show HEAD^ | grep \"D changed\" &&\n+\t\tgit show HEAD^ >actual &&\n+\t\tgrep \"D changed\" actual &&\n \t\tFAKE_LINES=\"reword 1 2 3 4\" FAKE_COMMIT_MESSAGE=\"B changed\" \\\n \t\t\tgit rebase -i A &&\n-\t\tgit show HEAD~3 | grep \"B changed\" &&\n+\t\tgit show HEAD~3 >actual &&\n+\t\tgrep \"B changed\" actual &&\n \t\tFAKE_LINES=\"1 r 2 pick 3 p 4\" FAKE_COMMIT_MESSAGE=\"C changed\" \\\n \t\t\tgit rebase -i A\n \t) &&\n-\tgit show HEAD~2 | grep \"C changed\"\n+\tgit show HEAD~2 >actual &&\n+\tgrep \"C changed\" actual\n '\n \n test_expect_success 'no uncommitted changes when rewording and the todo list is reloaded' '\n@@ -1003,8 +1023,10 @@ test_expect_success 'rebase -i --root retain root commit author and message' '\n \t\tset_fake_editor &&\n \t\tFAKE_LINES=\"2\" git rebase -i --root\n \t) &&\n-\tgit cat-file commit HEAD | grep -q \"^author Twerp Snog\" &&\n-\tgit cat-file commit HEAD | grep -q \"^different author$\"\n+\tgit cat-file commit HEAD >output &&\n+\tgrep -q \"^author Twerp Snog\" output &&\n+\tgit cat-file commit HEAD >actual &&\n+\tgrep -q \"^different author$\" actual\n '\n \n test_expect_success 'rebase -i --root temporary sentinel commit' '\n@@ -1013,7 +1035,8 @@ test_expect_success 'rebase -i --root temporary sentinel commit' '\n \t\tset_fake_editor &&\n \t\ttest_must_fail env FAKE_LINES=\"2\" git rebase -i --root\n \t) &&\n-\tgit cat-file commit HEAD | grep \"^tree $EMPTY_TREE\" &&\n+\tgit cat-file commit HEAD >actual &&\n+\tgrep \"^tree $EMPTY_TREE\" actual &&\n \tgit rebase --abort\n '\n \n@@ -1036,7 +1059,8 @@ test_expect_success 'rebase -i --root reword original root commit' '\n \t\tFAKE_LINES=\"reword 1 2\" FAKE_COMMIT_MESSAGE=\"A changed\" \\\n \t\t\tgit rebase -i --root\n \t) &&\n-\tgit show HEAD^ | grep \"A changed\" &&\n+\tgit show HEAD^ >actual &&\n+\tgrep \"A changed\" actual &&\n \ttest -z \"$(git show -s --format=%p HEAD^)\"\n '\n \n@@ -1048,7 +1072,8 @@ test_expect_success 'rebase -i --root reword new root commit' '\n \t\tFAKE_LINES=\"reword 3 1\" FAKE_COMMIT_MESSAGE=\"C changed\" \\\n \t\tgit rebase -i --root\n \t) &&\n-\tgit show HEAD^ | grep \"C changed\" &&\n+\tgit show HEAD^ >actual &&\n+\tgrep \"C changed\" actual &&\n \ttest -z \"$(git show -s --format=%p HEAD^)\"\n '\n \n-- \ngitgitgadget\n\n"},{"id":"504341","messageId":"e2cae7f3a510027864303fe91dcf447f63eb0873.1728315124.git.gitgitgadget@gmail.com","threadId":"62259","inReplyTo":"pull.1805.v7.git.git.1728315124.gitgitgadget@gmail.com","subject":"[PATCH v7 2/2] t3404: replace test with test_line_count()","fromName":"Usman Akinyemi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-10-07T15:32:03Z","receivedAt":"2024-10-07T15:32:09Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"From: Usman Akinyemi <usmanakinyemi202@gmail.com>\n\nRefactor t3404 to replace instances of `test` with `test_line_count()`\nfor checking line counts. This improves readability and aligns with Git's\ncurrent test practices.\n\nSigned-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n---\n t/t3404-rebase-interactive.sh | 28 ++++++++++++++--------------\n 1 file changed, 14 insertions(+), 14 deletions(-)\n\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 96a65783c47..2ab660ef30f 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -281,8 +281,9 @@ test_expect_success 'stop on conflicting pick' '\n \ttest_cmp expect2 file1 &&\n \ttest \"$(git diff --name-status |\n \t\tsed -n -e \"/^U/s/^U[^a-z]*//p\")\" = file1 &&\n-\ttest 4 = $(grep -v \"^#\" < .git/rebase-merge/done | wc -l) &&\n-\ttest 0 = $(grep -c \"^[^#]\" < .git/rebase-merge/git-rebase-todo)\n+\tgrep -v \"^#\" <.git/rebase-merge/done >actual &&\n+\ttest_line_count = 4 actual &&\n+\ttest 0 = $(grep -c \"^[^#]\" <.git/rebase-merge/git-rebase-todo)\n '\n \n test_expect_success 'show conflicted patch' '\n@@ -401,8 +402,8 @@ test_expect_success 'multi-squash only fires up editor once' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep ONCE output | wc -l) &&\n-\ttest 1 = $count\n+\tgrep ONCE output >actual &&\n+\ttest_line_count = 1 actual\n '\n \n test_expect_success 'multi-fixup does not fire up editor' '\n@@ -416,8 +417,7 @@ test_expect_success 'multi-fixup does not fire up editor' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep NEVER output | wc -l) &&\n-\ttest 0 = $count &&\n+\t! grep NEVER output &&\n \tgit checkout @{-1} &&\n \tgit branch -D multi-fixup\n '\n@@ -436,8 +436,8 @@ test_expect_success 'commit message used after conflict' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep ONCE output | wc -l) &&\n-\ttest 1 = $count &&\n+\tgrep ONCE output >actual &&\n+\ttest_line_count = 1 actual &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-fixup\n '\n@@ -456,8 +456,8 @@ test_expect_success 'commit message retained after conflict' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep TWICE output | wc -l) &&\n-\ttest 2 = $count &&\n+\tgrep TWICE output >actual &&\n+\ttest_line_count = 2 actual &&\n \tgit checkout @{-1} &&\n \tgit branch -D conflict-squash\n '\n@@ -501,8 +501,8 @@ test_expect_success 'squash ignores comments' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep ONCE output | wc -l) &&\n-\ttest 1 = $count &&\n+\tgrep ONCE output >actual &&\n+\ttest_line_count = 1 actual &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-comments\n '\n@@ -519,8 +519,8 @@ test_expect_success 'squash ignores blank lines' '\n \t) &&\n \ttest $base = $(git rev-parse HEAD^) &&\n \tgit show >output &&\n-\tcount=$(grep ONCE output | wc -l) &&\n-\ttest 1 = $count &&\n+\tgrep ONCE output >actual &&\n+\ttest_line_count = 1 actual &&\n \tgit checkout @{-1} &&\n \tgit branch -D skip-blank-lines\n '\n-- \ngitgitgadget\n"},{"id":"504342","messageId":"CAPSxiM_eyWJAfFV7jZzerBZaG1RY3h78rd+hVcRBUjdgGtCkrg@mail.gmail.com","threadId":"62259","inReplyTo":"4f79f144-9f48-4fbe-b646-2d146e57606a@gmail.com","subject":"Re: [PATCH v6 2/2] t3404: employing test_line_count() to replace test","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2024-10-07T15:32:38Z","receivedAt":"2024-10-07T15:32:50Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"On Mon, Oct 7, 2024 at 3:07 PM Phillip Wood <phillip.wood123@gmail.com> wrote:\n>\n> Hi Usman\n>\n> I missed this when I read the earlier version of this series but our\n> commit subjects use the imperative so we would say\n>\n>      t3403: employ test_line_count() to replace test\n>\n> or\n>\n>      t3404: replace test with test_line_count()\n>\nThanks for the review. I just improved the commit subject now.\n> I agree with Patrick that we should let this series settle for a couple\n> of days to allow others to comment before sending any new version.\n>\n> This series is looking good, congratulations\nThanks Philip.\n>\n> Phillip\n>\n> On 07/10/2024 12:11, Usman Akinyemi via GitGitGadget wrote:\n> > From: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> >\n> > Refactor t3404 to replace instances of `test` with `test_line_count()`\n> > for checking line counts. This improves readability and aligns with Git's\n> > current test practices.\n> >\n> > Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n> > ---\n> >   t/t3404-rebase-interactive.sh | 28 ++++++++++++++--------------\n> >   1 file changed, 14 insertions(+), 14 deletions(-)\n> >\n> > diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> > index 96a65783c47..2ab660ef30f 100755\n> > --- a/t/t3404-rebase-interactive.sh\n> > +++ b/t/t3404-rebase-interactive.sh\n> > @@ -281,8 +281,9 @@ test_expect_success 'stop on conflicting pick' '\n> >       test_cmp expect2 file1 &&\n> >       test \"$(git diff --name-status |\n> >               sed -n -e \"/^U/s/^U[^a-z]*//p\")\" = file1 &&\n> > -     test 4 = $(grep -v \"^#\" < .git/rebase-merge/done | wc -l) &&\n> > -     test 0 = $(grep -c \"^[^#]\" < .git/rebase-merge/git-rebase-todo)\n> > +     grep -v \"^#\" <.git/rebase-merge/done >actual &&\n> > +     test_line_count = 4 actual &&\n> > +     test 0 = $(grep -c \"^[^#]\" <.git/rebase-merge/git-rebase-todo)\n> >   '\n> >\n> >   test_expect_success 'show conflicted patch' '\n> > @@ -401,8 +402,8 @@ test_expect_success 'multi-squash only fires up editor once' '\n> >       ) &&\n> >       test $base = $(git rev-parse HEAD^) &&\n> >       git show >output &&\n> > -     count=$(grep ONCE output | wc -l) &&\n> > -     test 1 = $count\n> > +     grep ONCE output >actual &&\n> > +     test_line_count = 1 actual\n> >   '\n> >\n> >   test_expect_success 'multi-fixup does not fire up editor' '\n> > @@ -416,8 +417,7 @@ test_expect_success 'multi-fixup does not fire up editor' '\n> >       ) &&\n> >       test $base = $(git rev-parse HEAD^) &&\n> >       git show >output &&\n> > -     count=$(grep NEVER output | wc -l) &&\n> > -     test 0 = $count &&\n> > +     ! grep NEVER output &&\n> >       git checkout @{-1} &&\n> >       git branch -D multi-fixup\n> >   '\n> > @@ -436,8 +436,8 @@ test_expect_success 'commit message used after conflict' '\n> >       ) &&\n> >       test $base = $(git rev-parse HEAD^) &&\n> >       git show >output &&\n> > -     count=$(grep ONCE output | wc -l) &&\n> > -     test 1 = $count &&\n> > +     grep ONCE output >actual &&\n> > +     test_line_count = 1 actual &&\n> >       git checkout @{-1} &&\n> >       git branch -D conflict-fixup\n> >   '\n> > @@ -456,8 +456,8 @@ test_expect_success 'commit message retained after conflict' '\n> >       ) &&\n> >       test $base = $(git rev-parse HEAD^) &&\n> >       git show >output &&\n> > -     count=$(grep TWICE output | wc -l) &&\n> > -     test 2 = $count &&\n> > +     grep TWICE output >actual &&\n> > +     test_line_count = 2 actual &&\n> >       git checkout @{-1} &&\n> >       git branch -D conflict-squash\n> >   '\n> > @@ -501,8 +501,8 @@ test_expect_success 'squash ignores comments' '\n> >       ) &&\n> >       test $base = $(git rev-parse HEAD^) &&\n> >       git show >output &&\n> > -     count=$(grep ONCE output | wc -l) &&\n> > -     test 1 = $count &&\n> > +     grep ONCE output >actual &&\n> > +     test_line_count = 1 actual &&\n> >       git checkout @{-1} &&\n> >       git branch -D skip-comments\n> >   '\n> > @@ -519,8 +519,8 @@ test_expect_success 'squash ignores blank lines' '\n> >       ) &&\n> >       test $base = $(git rev-parse HEAD^) &&\n> >       git show >output &&\n> > -     count=$(grep ONCE output | wc -l) &&\n> > -     test 1 = $count &&\n> > +     grep ONCE output >actual &&\n> > +     test_line_count = 1 actual &&\n> >       git checkout @{-1} &&\n> >       git branch -D skip-blank-lines\n> >   '\n>\n"}]}