threads / patch / 62285

patcht6050: avoid pipes in git related commands

Subject: [Outreachy][PATCH] t6050: avoid pipes in git related commands

## tl;dr

25 messages between Oct 8, 2024 and Oct 22, 2024. Diffs are folded; open one to read it.

replies: 24people: 6as markdown or json

chizobajames21@gmail.com· Oct 8, 2024, 16:21 UTC · lore
From: Chizoba ODINAKA <chizobajames21@gmail.com>

In pipes, the exit code of a chain of commands is determined by the downstream command. For more accurate info on exit code tests, write output of upstreams into a file.

Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com>
---
 t/t6050-replace.sh | 86 +++++++++++++++++++++++-----------------------
 1 file changed, 43 insertions(+), 43 deletions(-)
Show changes to t/t6050-replace.sh +43 −43
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index d7702fc756..6b9811ed67 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -98,30 +98,30 @@ test_expect_success 'set up buggy branch' '
 '
 
 test_expect_success 'replace the author' '
-	git cat-file commit $HASH2 | grep "author A U Thor" &&
-	R=$(git cat-file commit $HASH2 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
-	git cat-file commit $R | grep "author O Thor" &&
+	git cat-file commit $HASH2 >actual && grep "author A U Thor" actual &&
+	R=$(git cat-file commit $HASH2 >actual && sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
+	git cat-file commit $R >actual && grep "author O Thor" actual &&
 	git update-ref refs/replace/$HASH2 $R &&
-	git show HEAD~5 | grep "O Thor" &&
-	git show $HASH2 | grep "O Thor"
+	git show HEAD~5 >actual && grep "O Thor" actual &&
+	git show $HASH2 >actual && grep "O Thor" actual
 '
 
 test_expect_success 'test --no-replace-objects option' '
-	git cat-file commit $HASH2 | grep "author O Thor" &&
-	git --no-replace-objects cat-file commit $HASH2 | grep "author A U Thor" &&
-	git show $HASH2 | grep "O Thor" &&
-	git --no-replace-objects show $HASH2 | grep "A U Thor"
+	git cat-file commit $HASH2 >actual && grep "author O Thor" actual &&
+	git --no-replace-objects cat-file commit $HASH2 >actual && grep "author A U Thor" actual &&
+	git show $HASH2 >actual && grep "O Thor" actual &&
+	git --no-replace-objects show $HASH2 >actual && grep "A U Thor" actual
 '
 
 test_expect_success 'test GIT_NO_REPLACE_OBJECTS env variable' '
-	GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 | grep "author A U Thor" &&
-	GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 | grep "A U Thor"
+	GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 >actual && grep "author A U Thor"  actual &&
+	GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 >actual && grep "A U Thor" actual
 '
 
 test_expect_success 'test core.usereplacerefs config option' '
 	test_config core.usereplacerefs false &&
-	git cat-file commit $HASH2 | grep "author A U Thor" &&
-	git show $HASH2 | grep "A U Thor"
+	git cat-file commit $HASH2 >actual && grep "author A U Thor" actual &&
+	git show $HASH2 >actual && grep "A U Thor" actual
 '
 
 cat >tag.sig <<EOF
@@ -148,14 +148,14 @@ test_expect_success 'repack, clone and fetch work' '
 	git clone --no-hardlinks . clone_dir &&
 	(
 		cd clone_dir &&
-		git show HEAD~5 | grep "A U Thor" &&
-		git show $HASH2 | grep "A U Thor" &&
+		git show HEAD~5 >actual && grep "A U Thor" actual &&
+		git show $HASH2 >actual && grep "A U Thor" actual &&
 		git cat-file commit $R &&
 		git repack -a -d &&
 		test_must_fail git cat-file commit $R &&
 		git fetch ../ "refs/replace/*:refs/replace/*" &&
-		git show HEAD~5 | grep "O Thor" &&
-		git show $HASH2 | grep "O Thor" &&
+		git show HEAD~5 >actual && grep "O Thor" actual &&
+		git show $HASH2 >actual && grep "O Thor" actual &&
 		git cat-file commit $R
 	)
 '
@@ -169,13 +169,13 @@ test_expect_success '"git replace" listing and deleting' '
 	test_must_fail git replace --delete &&
 	test_must_fail git replace -l -d $HASH2 &&
 	git replace -d $HASH2 &&
-	git show $HASH2 | grep "A U Thor" &&
+	git show $HASH2 >actual && grep "A U Thor" actual &&
 	test -z "$(git replace -l)"
 '
 
 test_expect_success '"git replace" replacing' '
 	git replace $HASH2 $R &&
-	git show $HASH2 | grep "O Thor" &&
+	git show $HASH2 >actual && grep "O Thor" actual &&
 	test_must_fail git replace $HASH2 $R &&
 	git replace -f $HASH2 $R &&
 	test_must_fail git replace -f &&
@@ -186,7 +186,7 @@ test_expect_success '"git replace" resolves sha1' '
 	SHORTHASH2=$(git rev-parse --short=8 $HASH2) &&
 	git replace -d $SHORTHASH2 &&
 	git replace $SHORTHASH2 $R &&
-	git show $HASH2 | grep "O Thor" &&
+	git show $HASH2 >actual && grep "O Thor" actual &&
 	test_must_fail git replace $HASH2 $R &&
 	git replace -f $HASH2 $R &&
 	test_must_fail git replace --force &&
@@ -209,10 +209,10 @@ test_expect_success '"git replace" resolves sha1' '
 #
 test_expect_success 'create parallel branch without the bug' '
 	git replace -d $HASH2 &&
-	git show $HASH2 | grep "A U Thor" &&
+	git show $HASH2 >actual && grep "A U Thor" actual &&
 	git checkout $HASH1 &&
 	git cherry-pick $HASH2 &&
-	git show $HASH5 | git apply &&
+	git show $HASH5 >actual && git apply actual &&
 	git commit --amend -m "hello: 4 more lines WITHOUT the bug" hello &&
 	PARA2=$(git rev-parse --verify HEAD) &&
 	git cherry-pick $HASH3 &&
@@ -225,7 +225,7 @@ test_expect_success 'create parallel branch without the bug' '
 	git checkout main &&
 	cur=$(git rev-parse --verify HEAD) &&
 	test "$cur" = "$HASH7" &&
-	git log --pretty=oneline | grep $PARA2 &&
+	git log --pretty=oneline >actual && grep $PARA2 actual &&
 	git remote add cloned ./clone_dir
 '
 
@@ -234,23 +234,23 @@ test_expect_success 'push to cloned repo' '
 	(
 		cd clone_dir &&
 		git checkout parallel &&
-		git log --pretty=oneline | grep $PARA2
+		git log --pretty=oneline >actual && grep $PARA2 actual
 	)
 '
 
 test_expect_success 'push branch with replacement' '
-	git cat-file commit $PARA3 | grep "author A U Thor" &&
-	S=$(git cat-file commit $PARA3 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
-	git cat-file commit $S | grep "author O Thor" &&
+	git cat-file commit $PARA3 >actual && grep "author A U Thor" actual &&
+	S=$(git cat-file commit $PARA3 >actual && sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
+	git cat-file commit $S >actual && grep "author O Thor" actual &&
 	git replace $PARA3 $S &&
-	git show $HASH6~2 | grep "O Thor" &&
-	git show $PARA3 | grep "O Thor" &&
+	git show $HASH6~2 >actual && grep "O Thor" actual &&
+	git show $PARA3 >actual && grep "O Thor" actual &&
 	git push cloned $HASH6^:refs/heads/parallel2 &&
 	(
 		cd clone_dir &&
 		git checkout parallel2 &&
-		git log --pretty=oneline | grep $PARA3 &&
-		git show $PARA3 | grep "A U Thor"
+		git log --pretty=oneline >actual && grep $PARA3 actual &&
+		git show $PARA3 >actual && grep "A U Thor" actual
 	)
 '
 
@@ -284,8 +284,8 @@ test_expect_success 'bisect and replacements' '
 '
 
 test_expect_success 'index-pack and replacements' '
-	git --no-replace-objects rev-list --objects HEAD |
-	git --no-replace-objects pack-objects test- &&
+	git --no-replace-objects rev-list --objects HEAD >actual &&
+	git --no-replace-objects pack-objects test- <actual &&
 	git index-pack test-*.pack
 '
 
@@ -319,7 +319,7 @@ test_expect_success '-f option bypasses the type check' '
 '
 
 test_expect_success 'git cat-file --batch works on replace objects' '
-	git replace | grep $PARA3 &&
+	git replace >actual && grep $PARA3 actual &&
 	echo $PARA3 | git cat-file --batch
 '
 
@@ -344,7 +344,7 @@ test_expect_success 'test --format medium' '
 		echo "$PARA3 -> $S" &&
 		echo "$MYTAG -> $HASH1"
 	} | sort >expected &&
-	git replace -l --format medium | sort >actual &&
+	git replace -l --format medium >actual && sort actual &&
 	test_cmp expected actual
 '
 
@@ -356,7 +356,7 @@ test_expect_success 'test --format long' '
 		echo "$PARA3 (commit) -> $S (commit)" &&
 		echo "$MYTAG (tag) -> $HASH1 (commit)"
 	} | sort >expected &&
-	git replace --format=long | sort >actual &&
+	git replace --format=long >actual && sort actual &&
 	test_cmp expected actual
 '
 
@@ -374,12 +374,12 @@ test_expect_success 'setup fake editors' '
 test_expect_success '--edit with and without already replaced object' '
 	test_must_fail env GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --force --edit "$PARA3" &&
-	git replace -l | grep "$PARA3" &&
-	git cat-file commit "$PARA3" | grep "A fake Thor" &&
+	git replace -l >actual && grep "$PARA3" actual &&
+	git cat-file commit "$PARA3" >actual && grep "A fake Thor" actual &&
 	git replace -d "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
-	git replace -l | grep "$PARA3" &&
-	git cat-file commit "$PARA3" | grep "A fake Thor"
+	git replace -l >actual && grep "$PARA3" actual &&
+	git cat-file commit "$PARA3" >actual && grep "A fake Thor" actual
 '
 
 test_expect_success '--edit and change nothing or command failed' '
@@ -387,8 +387,8 @@ test_expect_success '--edit and change nothing or command failed' '
 	test_must_fail env GIT_EDITOR=true git replace --edit "$PARA3" &&
 	test_must_fail env GIT_EDITOR="./failingfakeeditor" git replace --edit "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
-	git replace -l | grep "$PARA3" &&
-	git cat-file commit "$PARA3" | grep "A fake Thor"
+	git replace -l >actual && grep "$PARA3" actual &&
+	git cat-file commit "$PARA3" >actual && grep "A fake Thor" actual
 '
 
 test_expect_success 'replace ref cleanup' '
@@ -468,7 +468,7 @@ test_expect_success GPG 'set up a merge commit with a mergetag' '
 	git checkout main &&
 	git merge -s ours test_tag &&
 	HASH10=$(git rev-parse --verify HEAD) &&
-	git cat-file commit $HASH10 | grep "^mergetag object"
+	git cat-file commit $HASH10 >actual && grep "^mergetag object" actual
 '
 
 test_expect_success GPG '--graft on a commit with a mergetag' '

-- 
2.34.1
Patrick Steinhardt· Oct 9, 2024, 07:28 UTC · re: chizobajames21@gmail.com · lore

Re: [Outreachy][PATCH] t6050: avoid pipes in git related commands

On Tue, Oct 08, 2024 at 05:21:17PM +0100, chizobajames21@gmail.com wrote:
Show 5 quoted lines
> From: Chizoba ODINAKA <chizobajames21@gmail.com>
> 
> In pipes, the exit code of a chain of commands is determined by
> the downstream command. For more accurate info on exit code tests,
> write output of upstreams into a file.
Nit: it isn't really about accuracy, but rather about losing the return
code entirely. I'd also mention as part of your observation that t6050
still contains this pattern, which isn't currently obvious from just
reading the commit message standalone.

I'd also propose the following subject: "t6050: avoid pipes with downstream Git commands", which reflects the fact that Git commands can be at the end of the pipe without much of an issue.

Show 26 quoted lines
> Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com>
> ---
>  t/t6050-replace.sh | 86 +++++++++++++++++++++++-----------------------
>  1 file changed, 43 insertions(+), 43 deletions(-)
> 
> diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
> index d7702fc756..6b9811ed67 100755
> --- a/t/t6050-replace.sh
> +++ b/t/t6050-replace.sh
> @@ -98,30 +98,30 @@ test_expect_success 'set up buggy branch' '
>  '
>  
>  test_expect_success 'replace the author' '
> -	git cat-file commit $HASH2 | grep "author A U Thor" &&
> -	R=$(git cat-file commit $HASH2 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
> -	git cat-file commit $R | grep "author O Thor" &&
> +	git cat-file commit $HASH2 >actual && grep "author A U Thor" actual &&
> +	R=$(git cat-file commit $HASH2 >actual && sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
> +	git cat-file commit $R >actual && grep "author O Thor" actual &&
> 
>  	git update-ref refs/replace/$HASH2 $R &&
> -	git show HEAD~5 | grep "O Thor" &&
> -	git show $HASH2 | grep "O Thor"
> +	git show HEAD~5 >actual && grep "O Thor" actual &&
> +	git show $HASH2 >actual && grep "O Thor" actual
>  '

We don't typically chain multiple commands on the same line, as it becomes hard to read very fast. So these should all be split across multiple lines. The same is true for other tests you have converted.

Furthermore, I'd recommend to replace "grep" with "test_grep", which is a convenience wrapper that provides more context in case the grep might have failed. It would for example output the contents of "actual", which helps quite a lot in the context of failing CI jobs.

Thanks!
Patrick
Chizoba ODINAKA· Oct 10, 2024, 07:26 UTC · re: Patrick Steinhardt · lore

Re: [Outreachy][PATCH] t6050: avoid pipes in git related commands

On Wed, 9 Oct 2024 at 08:28, Patrick Steinhardt <ps@pks.im> wrote:
Show 13 quoted lines
>
> On Tue, Oct 08, 2024 at 05:21:17PM +0100, chizobajames21@gmail.com wrote:
> > From: Chizoba ODINAKA <chizobajames21@gmail.com>
> >
> > In pipes, the exit code of a chain of commands is determined by
> > the downstream command. For more accurate info on exit code tests,
> > write output of upstreams into a file.
>
> Nit: it isn't really about accuracy, but rather about losing the return
> code entirely. I'd also mention as part of your observation that t6050
> still contains this pattern, which isn't currently obvious from just
> reading the commit message standalone.
>

Thanks Patrick for the review, and for pointing this out, I totally agree with you.

> I'd also propose the following subject: "t6050: avoid pipes with
> downstream Git commands", which reflects the fact that Git commands can
> be at the end of the pipe without much of an issue.
>
And I will effect this proposal the next change.
Show 36 quoted lines
> > Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com>
> > ---
> >  t/t6050-replace.sh | 86 +++++++++++++++++++++++-----------------------
> >  1 file changed, 43 insertions(+), 43 deletions(-)
> >
> > diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
> > index d7702fc756..6b9811ed67 100755
> > --- a/t/t6050-replace.sh
> > +++ b/t/t6050-replace.sh
> > @@ -98,30 +98,30 @@ test_expect_success 'set up buggy branch' '
> >  '
> >
> >  test_expect_success 'replace the author' '
> > -     git cat-file commit $HASH2 | grep "author A U Thor" &&
> > -     R=$(git cat-file commit $HASH2 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
> > -     git cat-file commit $R | grep "author O Thor" &&
> > +     git cat-file commit $HASH2 >actual && grep "author A U Thor" actual &&
> > +     R=$(git cat-file commit $HASH2 >actual && sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
> > +     git cat-file commit $R >actual && grep "author O Thor" actual &&
> >
> >       git update-ref refs/replace/$HASH2 $R &&
> > -     git show HEAD~5 | grep "O Thor" &&
> > -     git show $HASH2 | grep "O Thor"
> > +     git show HEAD~5 >actual && grep "O Thor" actual &&
> > +     git show $HASH2 >actual && grep "O Thor" actual
> >  '
>
> We don't typically chain multiple commands on the same line, as it
> becomes hard to read very fast. So these should all be split across
> multiple lines. The same is true for other tests you have converted.
>
> Furthermore, I'd recommend to replace "grep" with "test_grep", which is
> a convenience wrapper that provides more context in case the grep might
> have failed. It would for example output the contents of "actual", which
> helps quite a lot in the context of failing CI jobs.
>
I made these recommended changes.
> Thanks!
>
> Patrick

Thank you. Chizoba

chizobajames21@gmail.com· Oct 10, 2024, 06:39 UTC · re: chizobajames21@gmail.com · lore

[Outreachy][PATCH v2] t6050: avoid pipes with downstream Git commands

From: Chizoba ODINAKA <chizobajames21@gmail.com>

In pipes, the exit code of a chain of commands is determined by the downstream command. In order not to loss the entire result code of tests, write output of upstreams into a file.

Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com>
---
Changes in v2:
- split multiple commands chain on the same line across multiple line,
  for easier readability
- replace "grep" with "test_grep", for more context in case of a "grep"
  failure
 t/t6050-replace.sh | 133 +++++++++++++++++++++++++++++----------------
 1 file changed, 87 insertions(+), 46 deletions(-)
Show changes to t/t6050-replace.sh +87 −46
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index d7702fc756..599b6bc66c 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -98,30 +98,43 @@ test_expect_success 'set up buggy branch' '
 '
 
 test_expect_success 'replace the author' '
-	git cat-file commit $HASH2 | grep "author A U Thor" &&
-	R=$(git cat-file commit $HASH2 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
-	git cat-file commit $R | grep "author O Thor" &&
+	git cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor" actual &&
+	git cat-file commit $HASH2 >actual &&
+	R=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
+	git cat-file commit $R >actual &&
+	test_grep "author O Thor" actual &&
 	git update-ref refs/replace/$HASH2 $R &&
-	git show HEAD~5 | grep "O Thor" &&
-	git show $HASH2 | grep "O Thor"
+	git show HEAD~5 >actual &&
+	test_grep "O Thor" actual &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual
 '
 
 test_expect_success 'test --no-replace-objects option' '
-	git cat-file commit $HASH2 | grep "author O Thor" &&
-	git --no-replace-objects cat-file commit $HASH2 | grep "author A U Thor" &&
-	git show $HASH2 | grep "O Thor" &&
-	git --no-replace-objects show $HASH2 | grep "A U Thor"
+	git cat-file commit $HASH2 >actual &&
+	test_grep "author O Thor" actual &&
+	git --no-replace-objects cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor" actual &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual &&
+	git --no-replace-objects show $HASH2 >actual &&
+	test_grep "A U Thor" actual
 '
 
 test_expect_success 'test GIT_NO_REPLACE_OBJECTS env variable' '
-	GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 | grep "author A U Thor" &&
-	GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 | grep "A U Thor"
+	GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor"  actual &&
+	GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 >actual &&
+	test_grep "A U Thor" actual
 '
 
 test_expect_success 'test core.usereplacerefs config option' '
 	test_config core.usereplacerefs false &&
-	git cat-file commit $HASH2 | grep "author A U Thor" &&
-	git show $HASH2 | grep "A U Thor"
+	git cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor" actual &&
+	git show $HASH2 >actual &&
+	test_grep "A U Thor" actual
 '
 
 cat >tag.sig <<EOF
@@ -148,14 +161,18 @@ test_expect_success 'repack, clone and fetch work' '
 	git clone --no-hardlinks . clone_dir &&
 	(
 		cd clone_dir &&
-		git show HEAD~5 | grep "A U Thor" &&
-		git show $HASH2 | grep "A U Thor" &&
+		git show HEAD~5 >actual &&
+		test_grep "A U Thor" actual &&
+		git show $HASH2 >actual &&
+		test_grep "A U Thor" actual &&
 		git cat-file commit $R &&
 		git repack -a -d &&
 		test_must_fail git cat-file commit $R &&
 		git fetch ../ "refs/replace/*:refs/replace/*" &&
-		git show HEAD~5 | grep "O Thor" &&
-		git show $HASH2 | grep "O Thor" &&
+		git show HEAD~5 >actual &&
+		test_grep "O Thor" actual &&
+		git show $HASH2 >actual &&
+		test_grep "O Thor" actual &&
 		git cat-file commit $R
 	)
 '
@@ -169,13 +186,15 @@ test_expect_success '"git replace" listing and deleting' '
 	test_must_fail git replace --delete &&
 	test_must_fail git replace -l -d $HASH2 &&
 	git replace -d $HASH2 &&
-	git show $HASH2 | grep "A U Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "A U Thor" actual &&
 	test -z "$(git replace -l)"
 '
 
 test_expect_success '"git replace" replacing' '
 	git replace $HASH2 $R &&
-	git show $HASH2 | grep "O Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual &&
 	test_must_fail git replace $HASH2 $R &&
 	git replace -f $HASH2 $R &&
 	test_must_fail git replace -f &&
@@ -186,7 +205,8 @@ test_expect_success '"git replace" resolves sha1' '
 	SHORTHASH2=$(git rev-parse --short=8 $HASH2) &&
 	git replace -d $SHORTHASH2 &&
 	git replace $SHORTHASH2 $R &&
-	git show $HASH2 | grep "O Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual &&
 	test_must_fail git replace $HASH2 $R &&
 	git replace -f $HASH2 $R &&
 	test_must_fail git replace --force &&
@@ -209,10 +229,12 @@ test_expect_success '"git replace" resolves sha1' '
 #
 test_expect_success 'create parallel branch without the bug' '
 	git replace -d $HASH2 &&
-	git show $HASH2 | grep "A U Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "A U Thor" actual &&
 	git checkout $HASH1 &&
 	git cherry-pick $HASH2 &&
-	git show $HASH5 | git apply &&
+	git show $HASH5 >actual &&
+	git apply actual &&
 	git commit --amend -m "hello: 4 more lines WITHOUT the bug" hello &&
 	PARA2=$(git rev-parse --verify HEAD) &&
 	git cherry-pick $HASH3 &&
@@ -225,7 +247,8 @@ test_expect_success 'create parallel branch without the bug' '
 	git checkout main &&
 	cur=$(git rev-parse --verify HEAD) &&
 	test "$cur" = "$HASH7" &&
-	git log --pretty=oneline | grep $PARA2 &&
+	git log --pretty=oneline >actual &&
+	test_grep $PARA2 actual &&
 	git remote add cloned ./clone_dir
 '
 
@@ -234,23 +257,31 @@ test_expect_success 'push to cloned repo' '
 	(
 		cd clone_dir &&
 		git checkout parallel &&
-		git log --pretty=oneline | grep $PARA2
+		git log --pretty=oneline >actual &&
+		test_grep $PARA2 actual
 	)
 '
 
 test_expect_success 'push branch with replacement' '
-	git cat-file commit $PARA3 | grep "author A U Thor" &&
-	S=$(git cat-file commit $PARA3 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
-	git cat-file commit $S | grep "author O Thor" &&
+	git cat-file commit $PARA3 >actual &&
+	test_grep "author A U Thor" actual &&
+	git cat-file commit $PARA3 >actual &&
+	S=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
+	git cat-file commit $S >actual &&
+	test_grep "author O Thor" actual &&
 	git replace $PARA3 $S &&
-	git show $HASH6~2 | grep "O Thor" &&
-	git show $PARA3 | grep "O Thor" &&
+	git show $HASH6~2 >actual &&
+	test_grep "O Thor" actual &&
+	git show $PARA3 >actual &&
+	test_grep "O Thor" actual &&
 	git push cloned $HASH6^:refs/heads/parallel2 &&
 	(
 		cd clone_dir &&
 		git checkout parallel2 &&
-		git log --pretty=oneline | grep $PARA3 &&
-		git show $PARA3 | grep "A U Thor"
+		git log --pretty=oneline >actual &&
+		test_grep $PARA3 actual &&
+		git show $PARA3 >actual &&
+		test_grep "A U Thor" actual
 	)
 '
 
@@ -260,14 +291,14 @@ test_expect_success 'fetch branch with replacement' '
 		cd clone_dir &&
 		git fetch origin refs/heads/tofetch:refs/heads/parallel3 &&
 		git log --pretty=oneline parallel3 >output.txt &&
-		! grep $PARA3 output.txt &&
+		! test_grep $PARA3 output.txt &&
 		git show $PARA3 >para3.txt &&
-		grep "A U Thor" para3.txt &&
+		test_grep "A U Thor" para3.txt &&
 		git fetch origin "refs/replace/*:refs/replace/*" &&
 		git log --pretty=oneline parallel3 >output.txt &&
-		grep $PARA3 output.txt &&
+		test_grep $PARA3 output.txt &&
 		git show $PARA3 >para3.txt &&
-		grep "O Thor" para3.txt
+		test_grep "O Thor" para3.txt
 	)
 '
 
@@ -284,7 +315,7 @@ test_expect_success 'bisect and replacements' '
 '
 
 test_expect_success 'index-pack and replacements' '
-	git --no-replace-objects rev-list --objects HEAD |
+	git --no-replace-objects rev-list --objects HEAD >actual &&
 	git --no-replace-objects pack-objects test- &&
 	git index-pack test-*.pack
 '
@@ -319,7 +350,8 @@ test_expect_success '-f option bypasses the type check' '
 '
 
 test_expect_success 'git cat-file --batch works on replace objects' '
-	git replace | grep $PARA3 &&
+	git replace >actual &&
+	test_grep $PARA3 actual &&
 	echo $PARA3 | git cat-file --batch
 '
 
@@ -344,7 +376,8 @@ test_expect_success 'test --format medium' '
 		echo "$PARA3 -> $S" &&
 		echo "$MYTAG -> $HASH1"
 	} | sort >expected &&
-	git replace -l --format medium | sort >actual &&
+	git replace -l --format medium >actual &&
+	sort actual &&
 	test_cmp expected actual
 '
 
@@ -356,7 +389,8 @@ test_expect_success 'test --format long' '
 		echo "$PARA3 (commit) -> $S (commit)" &&
 		echo "$MYTAG (tag) -> $HASH1 (commit)"
 	} | sort >expected &&
-	git replace --format=long | sort >actual &&
+	git replace --format=long >actual &&
+	sort actual &&
 	test_cmp expected actual
 '
 
@@ -374,12 +408,16 @@ test_expect_success 'setup fake editors' '
 test_expect_success '--edit with and without already replaced object' '
 	test_must_fail env GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --force --edit "$PARA3" &&
-	git replace -l | grep "$PARA3" &&
-	git cat-file commit "$PARA3" | grep "A fake Thor" &&
+	git replace -l >actual &&
+	test_grep "$PARA3" actual &&
+	git cat-file commit "$PARA3" >actual &&
+	test_grep "A fake Thor" actual &&
 	git replace -d "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
-	git replace -l | grep "$PARA3" &&
-	git cat-file commit "$PARA3" | grep "A fake Thor"
+	git replace -l >actual &&
+	test_grep "$PARA3" actual &&
+	git cat-file commit "$PARA3" >actual &&
+	test_grep "A fake Thor" actual
 '
 
 test_expect_success '--edit and change nothing or command failed' '
@@ -387,8 +425,10 @@ test_expect_success '--edit and change nothing or command failed' '
 	test_must_fail env GIT_EDITOR=true git replace --edit "$PARA3" &&
 	test_must_fail env GIT_EDITOR="./failingfakeeditor" git replace --edit "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
-	git replace -l | grep "$PARA3" &&
-	git cat-file commit "$PARA3" | grep "A fake Thor"
+	git replace -l >actual &&
+	test_grep "$PARA3" actual &&
+	git cat-file commit "$PARA3" >actual &&
+	test_grep "A fake Thor" actual
 '
 
 test_expect_success 'replace ref cleanup' '
@@ -468,7 +508,8 @@ test_expect_success GPG 'set up a merge commit with a mergetag' '
 	git checkout main &&
 	git merge -s ours test_tag &&
 	HASH10=$(git rev-parse --verify HEAD) &&
-	git cat-file commit $HASH10 | grep "^mergetag object"
+	git cat-file commit $HASH10 >actual &&
+	test_grep "^mergetag object" actual
 '
 
 test_expect_success GPG '--graft on a commit with a mergetag' '

-- 
2.34.1
Phillip Wood· Oct 10, 2024, 14:08 UTC · re: chizobajames21@gmail.com · lore

Re: [Outreachy][PATCH v2] t6050: avoid pipes with downstream Git commands

Hi Chizoba
On 10/10/2024 07:39, chizobajames21@gmail.com wrote:
> From: Chizoba ODINAKA <chizobajames21@gmail.com>
> 
> In pipes, the exit code of a chain of commands is determined by
> the downstream command.

I would perhaps say "final command" rather than "downstream command" as in a pipeline "cmd1 | cmd2 | cmd3" cmd2 and cmd3 are downstream of cmd1 but it is the exit code of cmd3 that will be used

> In order not to loss the entire result code of tests,
> write output of upstreams into a file.

We're interested in checking the exit code of git, but not other commands so it would be helpful to make that clear. Usman's patch [1] has a good explanation of this.

This patch also changes instances of "grep" to "test_grep" so the commit message needs to explain the reason for that change which is that it gives a better debugging experience if the test fails.

The patch is looking pretty good, most of the conversions look correct. I've left a few comments below

[1] https://lore.kernel.org/git/bfff7937cd20737bb5a8791dc7492700b1d7881f.1728315124.git.gitgitgadget@gmail.com

Show 7 quoted lines
>   test_expect_success 'replace the author' '
> -	git cat-file commit $HASH2 | grep "author A U Thor" &&
> -	R=$(git cat-file commit $HASH2 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
> -	git cat-file commit $R | grep "author O Thor" &&
> +	git cat-file commit $HASH2 >actual &&
> +	test_grep "author A U Thor" actual &&
> +	git cat-file commit $HASH2 >actual &&

You don't need to repeat this command now that we are saving the output of "git cat-file commit $HASH2"

> +	R=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
> +	git cat-file commit $R >actual &&
> +	test_grep "author O Thor" actual &&
Show 7 quoted lines
>   test_expect_success 'push branch with replacement' '
> -	git cat-file commit $PARA3 | grep "author A U Thor" &&
> -	S=$(git cat-file commit $PARA3 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
> -	git cat-file commit $S | grep "author O Thor" &&
> +	git cat-file commit $PARA3 >actual &&
> +	test_grep "author A U Thor" actual &&
> +	git cat-file commit $PARA3 >actual &&
We can drop this line for the same reason as above
> +	S=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
> +	git cat-file commit $S >actual &&
> +	test_grep "author O Thor" actual &&
Show 6 quoted lines
> @@ -260,14 +291,14 @@ test_expect_success 'fetch branch with replacement' '
>   		cd clone_dir &&
>   		git fetch origin refs/heads/tofetch:refs/heads/parallel3 &&
>   		git log --pretty=oneline parallel3 >output.txt &&
> -		! grep $PARA3 output.txt &&
> +		! test_grep $PARA3 output.txt &&

test_grep will print an error message the pattern does not match. In this case we expect it not to match so we want to print an error if it does match. We can do that with

	test_grep ! $PARA3 output.txt &&
>   test_expect_success 'index-pack and replacements' '
> -	git --no-replace-objects rev-list --objects HEAD |
> +	git --no-replace-objects rev-list --objects HEAD >actual &&
>   	git --no-replace-objects pack-objects test- &&

This command has lost its input - you need to use '<actual' to get it to read output from "git rev-list". This test itself could probably do a better job of checking that index-pack does what we expect but that is outside the scope of this patch.

>   	git index-pack test-*.pack
>   '
Everything that I've not commented on looks correct to me.
Best Wishes
Phillip
Chizoba ODINAKA· Oct 10, 2024, 18:51 UTC · re: Phillip Wood · lore

Re: [Outreachy][PATCH v2] t6050: avoid pipes with downstream Git commands

Hi Philip, On Thu, 10 Oct 2024 at 15:08, Phillip Wood <phillip.wood123@gmail.com> wrote:

Show 13 quoted lines
>
> Hi Chizoba
>
> On 10/10/2024 07:39, chizobajames21@gmail.com wrote:
> > From: Chizoba ODINAKA <chizobajames21@gmail.com>
> >
> > In pipes, the exit code of a chain of commands is determined by
> > the downstream command.
>
> I would perhaps say "final command" rather than "downstream command" as
> in a pipeline "cmd1 | cmd2 | cmd3" cmd2 and cmd3 are downstream of cmd1
> but it is the exit code of cmd3 that will be used
>
Yes, final command is a more appropriate explain.
Show 7 quoted lines
> > In order not to loss the entire result code of tests,
> > write output of upstreams into a file.
>
> We're interested in checking the exit code of git, but not other
> commands so it would be helpful to make that clear. Usman's patch [1]
> has a good explanation of this.
>

I just read that sentence again, it obviously needs some clarity. "In order not to miss the exit code of any Git command, avoid using pipe and write output into a file" has more clarity. I will look up on Usman's patch [1], before my next changes.

> This patch also changes instances of "grep" to "test_grep" so the commit
> message needs to explain the reason for that change which is that it
> gives a better debugging experience if the test fails.
>
I had included that in my "Changes in v2", appended beneath my "Sign-off-by".
"Changes in v2:
- split multiple commands chain on the same line across multiple line,
  for easier readability
- replace "grep" with "test_grep", for more context in case of a "grep"
  failure"
Maybe it was not so obvious that you didn't notice, or it is not the
traditional way of including it.
Show 64 quoted lines
> The patch is looking pretty good, most of the conversions look correct.
> I've left a few comments below
>
>
> [1]
> https://lore.kernel.org/git/bfff7937cd20737bb5a8791dc7492700b1d7881f.1728315124.git.gitgitgadget@gmail.com
>
> >   test_expect_success 'replace the author' '
> > -     git cat-file commit $HASH2 | grep "author A U Thor" &&
> > -     R=$(git cat-file commit $HASH2 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
> > -     git cat-file commit $R | grep "author O Thor" &&
> > +     git cat-file commit $HASH2 >actual &&
> > +     test_grep "author A U Thor" actual &&
> > +     git cat-file commit $HASH2 >actual &&
>
> You don't need to repeat this command now that we are saving the output
> of "git cat-file commit $HASH2"
>
> > +     R=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
> > +     git cat-file commit $R >actual &&
> > +     test_grep "author O Thor" actual &&
>
> >   test_expect_success 'push branch with replacement' '
> > -     git cat-file commit $PARA3 | grep "author A U Thor" &&
> > -     S=$(git cat-file commit $PARA3 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
> > -     git cat-file commit $S | grep "author O Thor" &&
> > +     git cat-file commit $PARA3 >actual &&
> > +     test_grep "author A U Thor" actual &&
> > +     git cat-file commit $PARA3 >actual &&
>
> We can drop this line for the same reason as above
>
> > +     S=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
> > +     git cat-file commit $S >actual &&
> > +     test_grep "author O Thor" actual &&
>
> > @@ -260,14 +291,14 @@ test_expect_success 'fetch branch with replacement' '
> >               cd clone_dir &&
> >               git fetch origin refs/heads/tofetch:refs/heads/parallel3 &&
> >               git log --pretty=oneline parallel3 >output.txt &&
> > -             ! grep $PARA3 output.txt &&
> > +             ! test_grep $PARA3 output.txt &&
>
> test_grep will print an error message the pattern does not match. In
> this case we expect it not to match so we want to print an error if it
> does match. We can do that with
>
>         test_grep ! $PARA3 output.txt &&
>
> >   test_expect_success 'index-pack and replacements' '
> > -     git --no-replace-objects rev-list --objects HEAD |
> > +     git --no-replace-objects rev-list --objects HEAD >actual &&
> >       git --no-replace-objects pack-objects test- &&
>
> This command has lost its input - you need to use '<actual' to get it to
> read output from "git rev-list". This test itself could probably do a
> better job of checking that index-pack does what we expect but that is
> outside the scope of this patch.
>
> >       git index-pack test-*.pack
> >   '
>
> Everything that I've not commented on looks correct to me.
>

Thanks Philip for the review, I will make the needed changes in my next patch. And look into the index-pack proposal in a new patch, since it is outside this scope.

> Best Wishes
>
> Phillip

Thanks Chizoba

phillip.wood123@gmail.com· Oct 11, 2024, 09:17 UTC · re: Chizoba ODINAKA · lore

Re: [Outreachy][PATCH v2] t6050: avoid pipes with downstream Git commands

Hi Chizoba
On 10/10/2024 19:51, Chizoba ODINAKA wrote:
Show 28 quoted lines
> On Thu, 10 Oct 2024 at 15:08, Phillip Wood <phillip.wood123@gmail.com> wrote:
>> On 10/10/2024 07:39, chizobajames21@gmail.com wrote:
>>> From: Chizoba ODINAKA <chizobajames21@gmail.com>
>>>
>>> In order not to loss the entire result code of tests,
>>> write output of upstreams into a file.
>>
>> We're interested in checking the exit code of git, but not other
>> commands so it would be helpful to make that clear. Usman's patch [1]
>> has a good explanation of this.
>>
> I just read that sentence again, it obviously needs some clarity. "In order not
> to miss the exit code of any Git command, avoid using pipe and write
> output into a file"
> has more clarity. I will look up on Usman's patch [1], before my next changes.
> 
>> This patch also changes instances of "grep" to "test_grep" so the commit
>> message needs to explain the reason for that change which is that it
>> gives a better debugging experience if the test fails.
>>
> I had included that in my "Changes in v2", appended beneath my "Sign-off-by".
> "Changes in v2:
> - split multiple commands chain on the same line across multiple line,
>    for easier readability
> - replace "grep" with "test_grep", for more context in case of a "grep"
>    failure"
> Maybe it was not so obvious that you didn't notice, or it is not the
> traditional way of including it.

It's great that you listed the changes between versions below the "---" line - that is really helpful for reviewers. However those comments are not part of the commit message when the patch is applied. It is important for the commit message to explain the reasons for all the changes in a patch so that someone reading it later can understand why the change was made. Therefore the grep->test_grep change should be explained in the commit message. In general one should avoid making unrelated changes in the same commit. In this case I think one can argue that the changes are small enough that combining them is fine.

> Thanks Philip for the review, I will make the needed changes in my
> next patch.
That's great, I look forward to reviewing it
> And look into
> the index-pack proposal in a new patch, since it is outside this scope.

Let's finish this patch first. I'm not sure what the best way to improve that test is at the moment.

Best Wishes
Phillip
chizobajames21@gmail.com· Oct 11, 2024, 15:45 UTC · re: chizobajames21@gmail.com · lore

[Outreachy][PATCH v3] t6050: avoid pipes with downstream Git commands

From: Chizoba ODINAKA <chizobajames21@gmail.com>

In pipes, the exit code of a chain of commands is determined by the final command. In order not to miss the exit code of a failed Git command, avoid pipes instead write output of Git commands into a file. For better debugging experience, instances of "grep" were changed to "test_grep". "test_grep" provides more context in case of a failed "grep".

Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com>
---
Changes in v3:
- remove some duplicated lines of code
- fixed some typo, an ommitted input redirection sign, <,
  and another omitted logic negation operator, !
Range-diff against v2:
1:  a084472fa6 ! 1:  3ef388e9aa t6050: avoid pipes with downstream Git commands
    @@ Commit message
         t6050: avoid pipes with downstream Git commands
     
         In pipes, the exit code of a chain of commands is determined by
    -    the downstream command. In order not to loss the entire result code of tests,
    -    write output of upstreams into a file.
    +    the final command. In order not to miss the exit code of a failed
    +    Git command, avoid pipes instead write output of Git commands
    +    into a file.
    +    For better debugging experience, instances of "grep" were changed
    +    to "test_grep". "test_grep" provides more context in case of a
    +    failed "grep".
     
         Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com>
     
    @@ t/t6050-replace.sh: test_expect_success 'set up buggy branch' '
     -	git cat-file commit $R | grep "author O Thor" &&
     +	git cat-file commit $HASH2 >actual &&
     +	test_grep "author A U Thor" actual &&
    -+	git cat-file commit $HASH2 >actual &&
     +	R=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
     +	git cat-file commit $R >actual &&
     +	test_grep "author O Thor" actual &&
    @@ t/t6050-replace.sh: test_expect_success 'push to cloned repo' '
     -	git cat-file commit $S | grep "author O Thor" &&
     +	git cat-file commit $PARA3 >actual &&
     +	test_grep "author A U Thor" actual &&
    -+	git cat-file commit $PARA3 >actual &&
     +	S=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
     +	git cat-file commit $S >actual &&
     +	test_grep "author O Thor" actual &&
    @@ t/t6050-replace.sh: test_expect_success 'fetch branch with replacement' '
      		git fetch origin refs/heads/tofetch:refs/heads/parallel3 &&
      		git log --pretty=oneline parallel3 >output.txt &&
     -		! grep $PARA3 output.txt &&
    -+		! test_grep $PARA3 output.txt &&
    ++		test_grep ! $PARA3 output.txt &&
      		git show $PARA3 >para3.txt &&
     -		grep "A U Thor" para3.txt &&
     +		test_grep "A U Thor" para3.txt &&
    @@ t/t6050-replace.sh: test_expect_success 'bisect and replacements' '
      
      test_expect_success 'index-pack and replacements' '
     -	git --no-replace-objects rev-list --objects HEAD |
    +-	git --no-replace-objects pack-objects test- &&
     +	git --no-replace-objects rev-list --objects HEAD >actual &&
    - 	git --no-replace-objects pack-objects test- &&
    ++	git --no-replace-objects pack-objects test- <actual &&
      	git index-pack test-*.pack
      '
    + 
     @@ t/t6050-replace.sh: test_expect_success '-f option bypasses the type check' '
      '
      
 t/t6050-replace.sh | 133 +++++++++++++++++++++++++++++----------------
 1 file changed, 86 insertions(+), 47 deletions(-)
Show changes to t/t6050-replace.sh +86 −47
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index d7702fc756..f09055beac 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -98,30 +98,42 @@ test_expect_success 'set up buggy branch' '
 '
 
 test_expect_success 'replace the author' '
-	git cat-file commit $HASH2 | grep "author A U Thor" &&
-	R=$(git cat-file commit $HASH2 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
-	git cat-file commit $R | grep "author O Thor" &&
+	git cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor" actual &&
+	R=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
+	git cat-file commit $R >actual &&
+	test_grep "author O Thor" actual &&
 	git update-ref refs/replace/$HASH2 $R &&
-	git show HEAD~5 | grep "O Thor" &&
-	git show $HASH2 | grep "O Thor"
+	git show HEAD~5 >actual &&
+	test_grep "O Thor" actual &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual
 '
 
 test_expect_success 'test --no-replace-objects option' '
-	git cat-file commit $HASH2 | grep "author O Thor" &&
-	git --no-replace-objects cat-file commit $HASH2 | grep "author A U Thor" &&
-	git show $HASH2 | grep "O Thor" &&
-	git --no-replace-objects show $HASH2 | grep "A U Thor"
+	git cat-file commit $HASH2 >actual &&
+	test_grep "author O Thor" actual &&
+	git --no-replace-objects cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor" actual &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual &&
+	git --no-replace-objects show $HASH2 >actual &&
+	test_grep "A U Thor" actual
 '
 
 test_expect_success 'test GIT_NO_REPLACE_OBJECTS env variable' '
-	GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 | grep "author A U Thor" &&
-	GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 | grep "A U Thor"
+	GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor"  actual &&
+	GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 >actual &&
+	test_grep "A U Thor" actual
 '
 
 test_expect_success 'test core.usereplacerefs config option' '
 	test_config core.usereplacerefs false &&
-	git cat-file commit $HASH2 | grep "author A U Thor" &&
-	git show $HASH2 | grep "A U Thor"
+	git cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor" actual &&
+	git show $HASH2 >actual &&
+	test_grep "A U Thor" actual
 '
 
 cat >tag.sig <<EOF
@@ -148,14 +160,18 @@ test_expect_success 'repack, clone and fetch work' '
 	git clone --no-hardlinks . clone_dir &&
 	(
 		cd clone_dir &&
-		git show HEAD~5 | grep "A U Thor" &&
-		git show $HASH2 | grep "A U Thor" &&
+		git show HEAD~5 >actual &&
+		test_grep "A U Thor" actual &&
+		git show $HASH2 >actual &&
+		test_grep "A U Thor" actual &&
 		git cat-file commit $R &&
 		git repack -a -d &&
 		test_must_fail git cat-file commit $R &&
 		git fetch ../ "refs/replace/*:refs/replace/*" &&
-		git show HEAD~5 | grep "O Thor" &&
-		git show $HASH2 | grep "O Thor" &&
+		git show HEAD~5 >actual &&
+		test_grep "O Thor" actual &&
+		git show $HASH2 >actual &&
+		test_grep "O Thor" actual &&
 		git cat-file commit $R
 	)
 '
@@ -169,13 +185,15 @@ test_expect_success '"git replace" listing and deleting' '
 	test_must_fail git replace --delete &&
 	test_must_fail git replace -l -d $HASH2 &&
 	git replace -d $HASH2 &&
-	git show $HASH2 | grep "A U Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "A U Thor" actual &&
 	test -z "$(git replace -l)"
 '
 
 test_expect_success '"git replace" replacing' '
 	git replace $HASH2 $R &&
-	git show $HASH2 | grep "O Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual &&
 	test_must_fail git replace $HASH2 $R &&
 	git replace -f $HASH2 $R &&
 	test_must_fail git replace -f &&
@@ -186,7 +204,8 @@ test_expect_success '"git replace" resolves sha1' '
 	SHORTHASH2=$(git rev-parse --short=8 $HASH2) &&
 	git replace -d $SHORTHASH2 &&
 	git replace $SHORTHASH2 $R &&
-	git show $HASH2 | grep "O Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual &&
 	test_must_fail git replace $HASH2 $R &&
 	git replace -f $HASH2 $R &&
 	test_must_fail git replace --force &&
@@ -209,10 +228,12 @@ test_expect_success '"git replace" resolves sha1' '
 #
 test_expect_success 'create parallel branch without the bug' '
 	git replace -d $HASH2 &&
-	git show $HASH2 | grep "A U Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "A U Thor" actual &&
 	git checkout $HASH1 &&
 	git cherry-pick $HASH2 &&
-	git show $HASH5 | git apply &&
+	git show $HASH5 >actual &&
+	git apply actual &&
 	git commit --amend -m "hello: 4 more lines WITHOUT the bug" hello &&
 	PARA2=$(git rev-parse --verify HEAD) &&
 	git cherry-pick $HASH3 &&
@@ -225,7 +246,8 @@ test_expect_success 'create parallel branch without the bug' '
 	git checkout main &&
 	cur=$(git rev-parse --verify HEAD) &&
 	test "$cur" = "$HASH7" &&
-	git log --pretty=oneline | grep $PARA2 &&
+	git log --pretty=oneline >actual &&
+	test_grep $PARA2 actual &&
 	git remote add cloned ./clone_dir
 '
 
@@ -234,23 +256,30 @@ test_expect_success 'push to cloned repo' '
 	(
 		cd clone_dir &&
 		git checkout parallel &&
-		git log --pretty=oneline | grep $PARA2
+		git log --pretty=oneline >actual &&
+		test_grep $PARA2 actual
 	)
 '
 
 test_expect_success 'push branch with replacement' '
-	git cat-file commit $PARA3 | grep "author A U Thor" &&
-	S=$(git cat-file commit $PARA3 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
-	git cat-file commit $S | grep "author O Thor" &&
+	git cat-file commit $PARA3 >actual &&
+	test_grep "author A U Thor" actual &&
+	S=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
+	git cat-file commit $S >actual &&
+	test_grep "author O Thor" actual &&
 	git replace $PARA3 $S &&
-	git show $HASH6~2 | grep "O Thor" &&
-	git show $PARA3 | grep "O Thor" &&
+	git show $HASH6~2 >actual &&
+	test_grep "O Thor" actual &&
+	git show $PARA3 >actual &&
+	test_grep "O Thor" actual &&
 	git push cloned $HASH6^:refs/heads/parallel2 &&
 	(
 		cd clone_dir &&
 		git checkout parallel2 &&
-		git log --pretty=oneline | grep $PARA3 &&
-		git show $PARA3 | grep "A U Thor"
+		git log --pretty=oneline >actual &&
+		test_grep $PARA3 actual &&
+		git show $PARA3 >actual &&
+		test_grep "A U Thor" actual
 	)
 '
 
@@ -260,14 +289,14 @@ test_expect_success 'fetch branch with replacement' '
 		cd clone_dir &&
 		git fetch origin refs/heads/tofetch:refs/heads/parallel3 &&
 		git log --pretty=oneline parallel3 >output.txt &&
-		! grep $PARA3 output.txt &&
+		test_grep ! $PARA3 output.txt &&
 		git show $PARA3 >para3.txt &&
-		grep "A U Thor" para3.txt &&
+		test_grep "A U Thor" para3.txt &&
 		git fetch origin "refs/replace/*:refs/replace/*" &&
 		git log --pretty=oneline parallel3 >output.txt &&
-		grep $PARA3 output.txt &&
+		test_grep $PARA3 output.txt &&
 		git show $PARA3 >para3.txt &&
-		grep "O Thor" para3.txt
+		test_grep "O Thor" para3.txt
 	)
 '
 
@@ -284,8 +313,8 @@ test_expect_success 'bisect and replacements' '
 '
 
 test_expect_success 'index-pack and replacements' '
-	git --no-replace-objects rev-list --objects HEAD |
-	git --no-replace-objects pack-objects test- &&
+	git --no-replace-objects rev-list --objects HEAD >actual &&
+	git --no-replace-objects pack-objects test- <actual &&
 	git index-pack test-*.pack
 '
 
@@ -319,7 +348,8 @@ test_expect_success '-f option bypasses the type check' '
 '
 
 test_expect_success 'git cat-file --batch works on replace objects' '
-	git replace | grep $PARA3 &&
+	git replace >actual &&
+	test_grep $PARA3 actual &&
 	echo $PARA3 | git cat-file --batch
 '
 
@@ -344,7 +374,8 @@ test_expect_success 'test --format medium' '
 		echo "$PARA3 -> $S" &&
 		echo "$MYTAG -> $HASH1"
 	} | sort >expected &&
-	git replace -l --format medium | sort >actual &&
+	git replace -l --format medium >actual &&
+	sort actual &&
 	test_cmp expected actual
 '
 
@@ -356,7 +387,8 @@ test_expect_success 'test --format long' '
 		echo "$PARA3 (commit) -> $S (commit)" &&
 		echo "$MYTAG (tag) -> $HASH1 (commit)"
 	} | sort >expected &&
-	git replace --format=long | sort >actual &&
+	git replace --format=long >actual &&
+	sort actual &&
 	test_cmp expected actual
 '
 
@@ -374,12 +406,16 @@ test_expect_success 'setup fake editors' '
 test_expect_success '--edit with and without already replaced object' '
 	test_must_fail env GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --force --edit "$PARA3" &&
-	git replace -l | grep "$PARA3" &&
-	git cat-file commit "$PARA3" | grep "A fake Thor" &&
+	git replace -l >actual &&
+	test_grep "$PARA3" actual &&
+	git cat-file commit "$PARA3" >actual &&
+	test_grep "A fake Thor" actual &&
 	git replace -d "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
-	git replace -l | grep "$PARA3" &&
-	git cat-file commit "$PARA3" | grep "A fake Thor"
+	git replace -l >actual &&
+	test_grep "$PARA3" actual &&
+	git cat-file commit "$PARA3" >actual &&
+	test_grep "A fake Thor" actual
 '
 
 test_expect_success '--edit and change nothing or command failed' '
@@ -387,8 +423,10 @@ test_expect_success '--edit and change nothing or command failed' '
 	test_must_fail env GIT_EDITOR=true git replace --edit "$PARA3" &&
 	test_must_fail env GIT_EDITOR="./failingfakeeditor" git replace --edit "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
-	git replace -l | grep "$PARA3" &&
-	git cat-file commit "$PARA3" | grep "A fake Thor"
+	git replace -l >actual &&
+	test_grep "$PARA3" actual &&
+	git cat-file commit "$PARA3" >actual &&
+	test_grep "A fake Thor" actual
 '
 
 test_expect_success 'replace ref cleanup' '
@@ -468,7 +506,8 @@ test_expect_success GPG 'set up a merge commit with a mergetag' '
 	git checkout main &&
 	git merge -s ours test_tag &&
 	HASH10=$(git rev-parse --verify HEAD) &&
-	git cat-file commit $HASH10 | grep "^mergetag object"
+	git cat-file commit $HASH10 >actual &&
+	test_grep "^mergetag object" actual
 '
 
 test_expect_success GPG '--graft on a commit with a mergetag' '

-- 
2.34.1
Junio C Hamano· Oct 11, 2024, 16:02 UTC · re: chizobajames21@gmail.com · lore

Re: [Outreachy][PATCH v3] t6050: avoid pipes with downstream Git commands

chizobajames21@gmail.com writes:
Show 16 quoted lines
> From: Chizoba ODINAKA <chizobajames21@gmail.com>
>
> In pipes, the exit code of a chain of commands is determined by
> the final command. In order not to miss the exit code of a failed
> Git command, avoid pipes instead write output of Git commands
> into a file.
> For better debugging experience, instances of "grep" were changed
> to "test_grep". "test_grep" provides more context in case of a
> failed "grep".
>
> Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com>
> ---
> Changes in v3:
> - remove some duplicated lines of code
> - fixed some typo, an ommitted input redirection sign, <,
>   and another omitted logic negation operator, !

I'd suggest looking at your commit title again. "with downstream Git commands" sounds like the git being tested is at the tail, not in front, of a pipeline, which would be perfectly OK arrangement to see what its exit status is.

The updated patch does indeed use such a construct, e.g.
> +	git cat-file commit $HASH2 >actual &&
> +	test_grep "author A U Thor" actual &&
> +	R=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
here.
    Subject: [Outreachy][PATCH v3] t6050: don't lose exit status of Git to pipes
or something, perhaps.
Other than that, looking very good.  Big thanks to all of you.
Eric Sunshine· Oct 11, 2024, 16:03 UTC · re: chizobajames21@gmail.com · lore

Re: [Outreachy][PATCH v3] t6050: avoid pipes with downstream Git commands

On Fri, Oct 11, 2024 at 11:46 AM <chizobajames21@gmail.com> wrote:
Show 16 quoted lines
> In pipes, the exit code of a chain of commands is determined by
> the final command. In order not to miss the exit code of a failed
> Git command, avoid pipes instead write output of Git commands
> into a file.
> For better debugging experience, instances of "grep" were changed
> to "test_grep". "test_grep" provides more context in case of a
> failed "grep".
>
> Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com>
> ---
> diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
> @@ -344,7 +374,8 @@ test_expect_success 'test --format medium' '
> -       git replace -l --format medium | sort >actual &&
> +       git replace -l --format medium >actual &&
> +       sort actual &&
>         test_cmp expected actual

Isn't this conversion broken? The `sort` command emits the sorted content to its standard output stream; it does not sort the file in place. So this is doing the sort but then simply throwing the output away, not capturing it to a file for comparison via `test_cmp`.

Junio C Hamano· Oct 11, 2024, 17:49 UTC · re: Eric Sunshine · lore

Re: [Outreachy][PATCH v3] t6050: avoid pipes with downstream Git commands

Eric Sunshine <sunshine@sunshineco.com> writes:
Show 22 quoted lines
> On Fri, Oct 11, 2024 at 11:46 AM <chizobajames21@gmail.com> wrote:
>> In pipes, the exit code of a chain of commands is determined by
>> the final command. In order not to miss the exit code of a failed
>> Git command, avoid pipes instead write output of Git commands
>> into a file.
>> For better debugging experience, instances of "grep" were changed
>> to "test_grep". "test_grep" provides more context in case of a
>> failed "grep".
>>
>> Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com>
>> ---
>> diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
>> @@ -344,7 +374,8 @@ test_expect_success 'test --format medium' '
>> -       git replace -l --format medium | sort >actual &&
>> +       git replace -l --format medium >actual &&
>> +       sort actual &&
>>         test_cmp expected actual
>
> Isn't this conversion broken? The `sort` command emits the sorted
> content to its standard output stream; it does not sort the file in
> place. So this is doing the sort but then simply throwing the output
> away, not capturing it to a file for comparison via `test_cmp`.
Good eyes.
chizobajames21@gmail.com· Oct 11, 2024, 23:59 UTC · re: Junio C Hamano · lore

[Outreachy][PATCH v4] t6050: avoid pipes with upstream Git commands

From: Chizoba ODINAKA <chizobajames21@gmail.com>

In pipes, the exit code of a chain of commands is determined by the final command. In order not to miss the exit code of a failed Git command, avoid pipes instead write output of Git commands into a file. For better debugging experience, instances of "grep" were changed to "test_grep". "test_grep" provides more context in case of a failed "grep".

Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com>
---
Changes in v4:
- Correct the subject
- fixed some ommitted output option in sorts that where writing to
  stdout
Range-diff against v3:
1:  3ef388e9aa ! 1:  28cab4c752 t6050: avoid pipes with downstream Git commands
    @@ Metadata
     Author: Chizoba ODINAKA <chizobajames21@gmail.com>
     
      ## Commit message ##
    -    t6050: avoid pipes with downstream Git commands
    +    t6050: avoid pipes with upstream Git commands
     
         In pipes, the exit code of a chain of commands is determined by
         the final command. In order not to miss the exit code of a failed
    @@ Commit message
         failed "grep".
     
         Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com>
    +    Changes in v4:
    +    - Correct the subject
    +    - fixed some ommitted output option in sorts that where writing to
    +      stdout
     
      ## t/t6050-replace.sh ##
     @@ t/t6050-replace.sh: test_expect_success 'set up buggy branch' '
    @@ t/t6050-replace.sh: test_expect_success 'test --format medium' '
      	} | sort >expected &&
     -	git replace -l --format medium | sort >actual &&
     +	git replace -l --format medium >actual &&
    -+	sort actual &&
    ++	sort actual -o actual &&
      	test_cmp expected actual
      '
      
    @@ t/t6050-replace.sh: test_expect_success 'test --format long' '
      	} | sort >expected &&
     -	git replace --format=long | sort >actual &&
     +	git replace --format=long >actual &&
    -+	sort actual &&
    ++	sort actual -o actual &&
      	test_cmp expected actual
      '
      
 t/t6050-replace.sh | 133 +++++++++++++++++++++++++++++----------------
 1 file changed, 86 insertions(+), 47 deletions(-)
Show changes to t/t6050-replace.sh +86 −47
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index d7702fc756..018d672c1b 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -98,30 +98,42 @@ test_expect_success 'set up buggy branch' '
 '
 
 test_expect_success 'replace the author' '
-	git cat-file commit $HASH2 | grep "author A U Thor" &&
-	R=$(git cat-file commit $HASH2 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
-	git cat-file commit $R | grep "author O Thor" &&
+	git cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor" actual &&
+	R=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
+	git cat-file commit $R >actual &&
+	test_grep "author O Thor" actual &&
 	git update-ref refs/replace/$HASH2 $R &&
-	git show HEAD~5 | grep "O Thor" &&
-	git show $HASH2 | grep "O Thor"
+	git show HEAD~5 >actual &&
+	test_grep "O Thor" actual &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual
 '
 
 test_expect_success 'test --no-replace-objects option' '
-	git cat-file commit $HASH2 | grep "author O Thor" &&
-	git --no-replace-objects cat-file commit $HASH2 | grep "author A U Thor" &&
-	git show $HASH2 | grep "O Thor" &&
-	git --no-replace-objects show $HASH2 | grep "A U Thor"
+	git cat-file commit $HASH2 >actual &&
+	test_grep "author O Thor" actual &&
+	git --no-replace-objects cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor" actual &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual &&
+	git --no-replace-objects show $HASH2 >actual &&
+	test_grep "A U Thor" actual
 '
 
 test_expect_success 'test GIT_NO_REPLACE_OBJECTS env variable' '
-	GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 | grep "author A U Thor" &&
-	GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 | grep "A U Thor"
+	GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor"  actual &&
+	GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 >actual &&
+	test_grep "A U Thor" actual
 '
 
 test_expect_success 'test core.usereplacerefs config option' '
 	test_config core.usereplacerefs false &&
-	git cat-file commit $HASH2 | grep "author A U Thor" &&
-	git show $HASH2 | grep "A U Thor"
+	git cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor" actual &&
+	git show $HASH2 >actual &&
+	test_grep "A U Thor" actual
 '
 
 cat >tag.sig <<EOF
@@ -148,14 +160,18 @@ test_expect_success 'repack, clone and fetch work' '
 	git clone --no-hardlinks . clone_dir &&
 	(
 		cd clone_dir &&
-		git show HEAD~5 | grep "A U Thor" &&
-		git show $HASH2 | grep "A U Thor" &&
+		git show HEAD~5 >actual &&
+		test_grep "A U Thor" actual &&
+		git show $HASH2 >actual &&
+		test_grep "A U Thor" actual &&
 		git cat-file commit $R &&
 		git repack -a -d &&
 		test_must_fail git cat-file commit $R &&
 		git fetch ../ "refs/replace/*:refs/replace/*" &&
-		git show HEAD~5 | grep "O Thor" &&
-		git show $HASH2 | grep "O Thor" &&
+		git show HEAD~5 >actual &&
+		test_grep "O Thor" actual &&
+		git show $HASH2 >actual &&
+		test_grep "O Thor" actual &&
 		git cat-file commit $R
 	)
 '
@@ -169,13 +185,15 @@ test_expect_success '"git replace" listing and deleting' '
 	test_must_fail git replace --delete &&
 	test_must_fail git replace -l -d $HASH2 &&
 	git replace -d $HASH2 &&
-	git show $HASH2 | grep "A U Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "A U Thor" actual &&
 	test -z "$(git replace -l)"
 '
 
 test_expect_success '"git replace" replacing' '
 	git replace $HASH2 $R &&
-	git show $HASH2 | grep "O Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual &&
 	test_must_fail git replace $HASH2 $R &&
 	git replace -f $HASH2 $R &&
 	test_must_fail git replace -f &&
@@ -186,7 +204,8 @@ test_expect_success '"git replace" resolves sha1' '
 	SHORTHASH2=$(git rev-parse --short=8 $HASH2) &&
 	git replace -d $SHORTHASH2 &&
 	git replace $SHORTHASH2 $R &&
-	git show $HASH2 | grep "O Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual &&
 	test_must_fail git replace $HASH2 $R &&
 	git replace -f $HASH2 $R &&
 	test_must_fail git replace --force &&
@@ -209,10 +228,12 @@ test_expect_success '"git replace" resolves sha1' '
 #
 test_expect_success 'create parallel branch without the bug' '
 	git replace -d $HASH2 &&
-	git show $HASH2 | grep "A U Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "A U Thor" actual &&
 	git checkout $HASH1 &&
 	git cherry-pick $HASH2 &&
-	git show $HASH5 | git apply &&
+	git show $HASH5 >actual &&
+	git apply actual &&
 	git commit --amend -m "hello: 4 more lines WITHOUT the bug" hello &&
 	PARA2=$(git rev-parse --verify HEAD) &&
 	git cherry-pick $HASH3 &&
@@ -225,7 +246,8 @@ test_expect_success 'create parallel branch without the bug' '
 	git checkout main &&
 	cur=$(git rev-parse --verify HEAD) &&
 	test "$cur" = "$HASH7" &&
-	git log --pretty=oneline | grep $PARA2 &&
+	git log --pretty=oneline >actual &&
+	test_grep $PARA2 actual &&
 	git remote add cloned ./clone_dir
 '
 
@@ -234,23 +256,30 @@ test_expect_success 'push to cloned repo' '
 	(
 		cd clone_dir &&
 		git checkout parallel &&
-		git log --pretty=oneline | grep $PARA2
+		git log --pretty=oneline >actual &&
+		test_grep $PARA2 actual
 	)
 '
 
 test_expect_success 'push branch with replacement' '
-	git cat-file commit $PARA3 | grep "author A U Thor" &&
-	S=$(git cat-file commit $PARA3 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
-	git cat-file commit $S | grep "author O Thor" &&
+	git cat-file commit $PARA3 >actual &&
+	test_grep "author A U Thor" actual &&
+	S=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
+	git cat-file commit $S >actual &&
+	test_grep "author O Thor" actual &&
 	git replace $PARA3 $S &&
-	git show $HASH6~2 | grep "O Thor" &&
-	git show $PARA3 | grep "O Thor" &&
+	git show $HASH6~2 >actual &&
+	test_grep "O Thor" actual &&
+	git show $PARA3 >actual &&
+	test_grep "O Thor" actual &&
 	git push cloned $HASH6^:refs/heads/parallel2 &&
 	(
 		cd clone_dir &&
 		git checkout parallel2 &&
-		git log --pretty=oneline | grep $PARA3 &&
-		git show $PARA3 | grep "A U Thor"
+		git log --pretty=oneline >actual &&
+		test_grep $PARA3 actual &&
+		git show $PARA3 >actual &&
+		test_grep "A U Thor" actual
 	)
 '
 
@@ -260,14 +289,14 @@ test_expect_success 'fetch branch with replacement' '
 		cd clone_dir &&
 		git fetch origin refs/heads/tofetch:refs/heads/parallel3 &&
 		git log --pretty=oneline parallel3 >output.txt &&
-		! grep $PARA3 output.txt &&
+		test_grep ! $PARA3 output.txt &&
 		git show $PARA3 >para3.txt &&
-		grep "A U Thor" para3.txt &&
+		test_grep "A U Thor" para3.txt &&
 		git fetch origin "refs/replace/*:refs/replace/*" &&
 		git log --pretty=oneline parallel3 >output.txt &&
-		grep $PARA3 output.txt &&
+		test_grep $PARA3 output.txt &&
 		git show $PARA3 >para3.txt &&
-		grep "O Thor" para3.txt
+		test_grep "O Thor" para3.txt
 	)
 '
 
@@ -284,8 +313,8 @@ test_expect_success 'bisect and replacements' '
 '
 
 test_expect_success 'index-pack and replacements' '
-	git --no-replace-objects rev-list --objects HEAD |
-	git --no-replace-objects pack-objects test- &&
+	git --no-replace-objects rev-list --objects HEAD >actual &&
+	git --no-replace-objects pack-objects test- <actual &&
 	git index-pack test-*.pack
 '
 
@@ -319,7 +348,8 @@ test_expect_success '-f option bypasses the type check' '
 '
 
 test_expect_success 'git cat-file --batch works on replace objects' '
-	git replace | grep $PARA3 &&
+	git replace >actual &&
+	test_grep $PARA3 actual &&
 	echo $PARA3 | git cat-file --batch
 '
 
@@ -344,7 +374,8 @@ test_expect_success 'test --format medium' '
 		echo "$PARA3 -> $S" &&
 		echo "$MYTAG -> $HASH1"
 	} | sort >expected &&
-	git replace -l --format medium | sort >actual &&
+	git replace -l --format medium >actual &&
+	sort actual -o actual &&
 	test_cmp expected actual
 '
 
@@ -356,7 +387,8 @@ test_expect_success 'test --format long' '
 		echo "$PARA3 (commit) -> $S (commit)" &&
 		echo "$MYTAG (tag) -> $HASH1 (commit)"
 	} | sort >expected &&
-	git replace --format=long | sort >actual &&
+	git replace --format=long >actual &&
+	sort actual -o actual &&
 	test_cmp expected actual
 '
 
@@ -374,12 +406,16 @@ test_expect_success 'setup fake editors' '
 test_expect_success '--edit with and without already replaced object' '
 	test_must_fail env GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --force --edit "$PARA3" &&
-	git replace -l | grep "$PARA3" &&
-	git cat-file commit "$PARA3" | grep "A fake Thor" &&
+	git replace -l >actual &&
+	test_grep "$PARA3" actual &&
+	git cat-file commit "$PARA3" >actual &&
+	test_grep "A fake Thor" actual &&
 	git replace -d "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
-	git replace -l | grep "$PARA3" &&
-	git cat-file commit "$PARA3" | grep "A fake Thor"
+	git replace -l >actual &&
+	test_grep "$PARA3" actual &&
+	git cat-file commit "$PARA3" >actual &&
+	test_grep "A fake Thor" actual
 '
 
 test_expect_success '--edit and change nothing or command failed' '
@@ -387,8 +423,10 @@ test_expect_success '--edit and change nothing or command failed' '
 	test_must_fail env GIT_EDITOR=true git replace --edit "$PARA3" &&
 	test_must_fail env GIT_EDITOR="./failingfakeeditor" git replace --edit "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
-	git replace -l | grep "$PARA3" &&
-	git cat-file commit "$PARA3" | grep "A fake Thor"
+	git replace -l >actual &&
+	test_grep "$PARA3" actual &&
+	git cat-file commit "$PARA3" >actual &&
+	test_grep "A fake Thor" actual
 '
 
 test_expect_success 'replace ref cleanup' '
@@ -468,7 +506,8 @@ test_expect_success GPG 'set up a merge commit with a mergetag' '
 	git checkout main &&
 	git merge -s ours test_tag &&
 	HASH10=$(git rev-parse --verify HEAD) &&
-	git cat-file commit $HASH10 | grep "^mergetag object"
+	git cat-file commit $HASH10 >actual &&
+	test_grep "^mergetag object" actual
 '
 
 test_expect_success GPG '--graft on a commit with a mergetag' '

-- 
2.34.1
Eric Sunshine· Oct 12, 2024, 05:35 UTC · re: chizobajames21@gmail.com · lore

Re: [Outreachy][PATCH v4] t6050: avoid pipes with upstream Git commands

On Fri, Oct 11, 2024 at 8:00 PM <chizobajames21@gmail.com> wrote:
Show 16 quoted lines
> In pipes, the exit code of a chain of commands is determined by
> the final command. In order not to miss the exit code of a failed
> Git command, avoid pipes instead write output of Git commands
> into a file.
> For better debugging experience, instances of "grep" were changed
> to "test_grep". "test_grep" provides more context in case of a
> failed "grep".
>
> Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com>
> ---
> diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
> @@ -344,7 +374,8 @@ test_expect_success 'test --format medium' '
> -       git replace -l --format medium | sort >actual &&
> +       git replace -l --format medium >actual &&
> +       sort actual -o actual &&
>         test_cmp expected actual

Although `sort -o` is POSIX, it may not be supported by all platforms on which Git is built. Moreover, as far as I can tell, `sort -o` is not used at all in the Git test suite, so we should be wary of using it here. Therefore, rather than introducing a novel use of this command, which might possibly break on some platform, instead stick with what is known to work without problem:

    git replace -l --format medium >output &&
    sort output >actual &&
Chizoba ODINAKA· Oct 12, 2024, 06:28 UTC · re: Eric Sunshine · lore

Re: [Outreachy][PATCH v4] t6050: avoid pipes with upstream Git commands

On Sat, 12 Oct 2024 at 06:35, Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 28 quoted lines
>
> On Fri, Oct 11, 2024 at 8:00 PM <chizobajames21@gmail.com> wrote:
> > In pipes, the exit code of a chain of commands is determined by
> > the final command. In order not to miss the exit code of a failed
> > Git command, avoid pipes instead write output of Git commands
> > into a file.
> > For better debugging experience, instances of "grep" were changed
> > to "test_grep". "test_grep" provides more context in case of a
> > failed "grep".
> >
> > Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com>
> > ---
> > diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
> > @@ -344,7 +374,8 @@ test_expect_success 'test --format medium' '
> > -       git replace -l --format medium | sort >actual &&
> > +       git replace -l --format medium >actual &&
> > +       sort actual -o actual &&
> >         test_cmp expected actual
>
> Although `sort -o` is POSIX, it may not be supported by all platforms
> on which Git is built. Moreover, as far as I can tell, `sort -o` is
> not used at all in the Git test suite, so we should be wary of using
> it here. Therefore, rather than introducing a novel use of this
> command, which might possibly break on some platform, instead stick
> with what is known to work without problem:
>
>     git replace -l --format medium >output &&
>     sort output >actual &&
Fixed. Thanks Eric.
chizobajames21@gmail.com· Oct 12, 2024, 06:21 UTC · re: chizobajames21@gmail.com · lore

[Outreachy][PATCH v5] t6050: avoid pipes with upstream Git commands

From: Chizoba ODINAKA <chizobajames21@gmail.com>

In pipes, the exit code of a chain of commands is determined by the final command. In order not to miss the exit code of a failed Git command, avoid pipes instead write output of Git commands into a file. For better debugging experience, instances of "grep" were changed to "test_grep". "test_grep" provides more context in case of a failed "grep".

Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com>
Changes in v5:
- replace 'sort -o' which might not be supported by some platforms
  to the more conventional output redirection, > .
---
Range-diff against v4:
1:  28cab4c752 ! 1:  b339b1f550 t6050: avoid pipes with upstream Git commands
    @@ Commit message
         failed "grep".
     
         Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com>
    -    Changes in v4:
    -    - Correct the subject
    -    - fixed some ommitted output option in sorts that where writing to
    -      stdout
    +    Changes in v5:
    +    - replace 'sort -o' which might not be supported by some platforms
    +      to the more conventional output redirection, > .
     
      ## t/t6050-replace.sh ##
     @@ t/t6050-replace.sh: test_expect_success 'set up buggy branch' '
    @@ t/t6050-replace.sh: test_expect_success 'test --format medium' '
      		echo "$MYTAG -> $HASH1"
      	} | sort >expected &&
     -	git replace -l --format medium | sort >actual &&
    -+	git replace -l --format medium >actual &&
    -+	sort actual -o actual &&
    ++	git replace -l --format medium >output &&
    ++	sort output >actual &&
      	test_cmp expected actual
      '
      
    @@ t/t6050-replace.sh: test_expect_success 'test --format long' '
      		echo "$MYTAG (tag) -> $HASH1 (commit)"
      	} | sort >expected &&
     -	git replace --format=long | sort >actual &&
    -+	git replace --format=long >actual &&
    -+	sort actual -o actual &&
    ++	git replace --format=long >output &&
    ++	sort output >actual &&
      	test_cmp expected actual
      '
      
 t/t6050-replace.sh | 133 +++++++++++++++++++++++++++++----------------
 1 file changed, 86 insertions(+), 47 deletions(-)
Show changes to t/t6050-replace.sh +86 −47
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index d7702fc756..37cb6547b6 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -98,30 +98,42 @@ test_expect_success 'set up buggy branch' '
 '
 
 test_expect_success 'replace the author' '
-	git cat-file commit $HASH2 | grep "author A U Thor" &&
-	R=$(git cat-file commit $HASH2 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
-	git cat-file commit $R | grep "author O Thor" &&
+	git cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor" actual &&
+	R=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
+	git cat-file commit $R >actual &&
+	test_grep "author O Thor" actual &&
 	git update-ref refs/replace/$HASH2 $R &&
-	git show HEAD~5 | grep "O Thor" &&
-	git show $HASH2 | grep "O Thor"
+	git show HEAD~5 >actual &&
+	test_grep "O Thor" actual &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual
 '
 
 test_expect_success 'test --no-replace-objects option' '
-	git cat-file commit $HASH2 | grep "author O Thor" &&
-	git --no-replace-objects cat-file commit $HASH2 | grep "author A U Thor" &&
-	git show $HASH2 | grep "O Thor" &&
-	git --no-replace-objects show $HASH2 | grep "A U Thor"
+	git cat-file commit $HASH2 >actual &&
+	test_grep "author O Thor" actual &&
+	git --no-replace-objects cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor" actual &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual &&
+	git --no-replace-objects show $HASH2 >actual &&
+	test_grep "A U Thor" actual
 '
 
 test_expect_success 'test GIT_NO_REPLACE_OBJECTS env variable' '
-	GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 | grep "author A U Thor" &&
-	GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 | grep "A U Thor"
+	GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor"  actual &&
+	GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 >actual &&
+	test_grep "A U Thor" actual
 '
 
 test_expect_success 'test core.usereplacerefs config option' '
 	test_config core.usereplacerefs false &&
-	git cat-file commit $HASH2 | grep "author A U Thor" &&
-	git show $HASH2 | grep "A U Thor"
+	git cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor" actual &&
+	git show $HASH2 >actual &&
+	test_grep "A U Thor" actual
 '
 
 cat >tag.sig <<EOF
@@ -148,14 +160,18 @@ test_expect_success 'repack, clone and fetch work' '
 	git clone --no-hardlinks . clone_dir &&
 	(
 		cd clone_dir &&
-		git show HEAD~5 | grep "A U Thor" &&
-		git show $HASH2 | grep "A U Thor" &&
+		git show HEAD~5 >actual &&
+		test_grep "A U Thor" actual &&
+		git show $HASH2 >actual &&
+		test_grep "A U Thor" actual &&
 		git cat-file commit $R &&
 		git repack -a -d &&
 		test_must_fail git cat-file commit $R &&
 		git fetch ../ "refs/replace/*:refs/replace/*" &&
-		git show HEAD~5 | grep "O Thor" &&
-		git show $HASH2 | grep "O Thor" &&
+		git show HEAD~5 >actual &&
+		test_grep "O Thor" actual &&
+		git show $HASH2 >actual &&
+		test_grep "O Thor" actual &&
 		git cat-file commit $R
 	)
 '
@@ -169,13 +185,15 @@ test_expect_success '"git replace" listing and deleting' '
 	test_must_fail git replace --delete &&
 	test_must_fail git replace -l -d $HASH2 &&
 	git replace -d $HASH2 &&
-	git show $HASH2 | grep "A U Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "A U Thor" actual &&
 	test -z "$(git replace -l)"
 '
 
 test_expect_success '"git replace" replacing' '
 	git replace $HASH2 $R &&
-	git show $HASH2 | grep "O Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual &&
 	test_must_fail git replace $HASH2 $R &&
 	git replace -f $HASH2 $R &&
 	test_must_fail git replace -f &&
@@ -186,7 +204,8 @@ test_expect_success '"git replace" resolves sha1' '
 	SHORTHASH2=$(git rev-parse --short=8 $HASH2) &&
 	git replace -d $SHORTHASH2 &&
 	git replace $SHORTHASH2 $R &&
-	git show $HASH2 | grep "O Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual &&
 	test_must_fail git replace $HASH2 $R &&
 	git replace -f $HASH2 $R &&
 	test_must_fail git replace --force &&
@@ -209,10 +228,12 @@ test_expect_success '"git replace" resolves sha1' '
 #
 test_expect_success 'create parallel branch without the bug' '
 	git replace -d $HASH2 &&
-	git show $HASH2 | grep "A U Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "A U Thor" actual &&
 	git checkout $HASH1 &&
 	git cherry-pick $HASH2 &&
-	git show $HASH5 | git apply &&
+	git show $HASH5 >actual &&
+	git apply actual &&
 	git commit --amend -m "hello: 4 more lines WITHOUT the bug" hello &&
 	PARA2=$(git rev-parse --verify HEAD) &&
 	git cherry-pick $HASH3 &&
@@ -225,7 +246,8 @@ test_expect_success 'create parallel branch without the bug' '
 	git checkout main &&
 	cur=$(git rev-parse --verify HEAD) &&
 	test "$cur" = "$HASH7" &&
-	git log --pretty=oneline | grep $PARA2 &&
+	git log --pretty=oneline >actual &&
+	test_grep $PARA2 actual &&
 	git remote add cloned ./clone_dir
 '
 
@@ -234,23 +256,30 @@ test_expect_success 'push to cloned repo' '
 	(
 		cd clone_dir &&
 		git checkout parallel &&
-		git log --pretty=oneline | grep $PARA2
+		git log --pretty=oneline >actual &&
+		test_grep $PARA2 actual
 	)
 '
 
 test_expect_success 'push branch with replacement' '
-	git cat-file commit $PARA3 | grep "author A U Thor" &&
-	S=$(git cat-file commit $PARA3 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
-	git cat-file commit $S | grep "author O Thor" &&
+	git cat-file commit $PARA3 >actual &&
+	test_grep "author A U Thor" actual &&
+	S=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
+	git cat-file commit $S >actual &&
+	test_grep "author O Thor" actual &&
 	git replace $PARA3 $S &&
-	git show $HASH6~2 | grep "O Thor" &&
-	git show $PARA3 | grep "O Thor" &&
+	git show $HASH6~2 >actual &&
+	test_grep "O Thor" actual &&
+	git show $PARA3 >actual &&
+	test_grep "O Thor" actual &&
 	git push cloned $HASH6^:refs/heads/parallel2 &&
 	(
 		cd clone_dir &&
 		git checkout parallel2 &&
-		git log --pretty=oneline | grep $PARA3 &&
-		git show $PARA3 | grep "A U Thor"
+		git log --pretty=oneline >actual &&
+		test_grep $PARA3 actual &&
+		git show $PARA3 >actual &&
+		test_grep "A U Thor" actual
 	)
 '
 
@@ -260,14 +289,14 @@ test_expect_success 'fetch branch with replacement' '
 		cd clone_dir &&
 		git fetch origin refs/heads/tofetch:refs/heads/parallel3 &&
 		git log --pretty=oneline parallel3 >output.txt &&
-		! grep $PARA3 output.txt &&
+		test_grep ! $PARA3 output.txt &&
 		git show $PARA3 >para3.txt &&
-		grep "A U Thor" para3.txt &&
+		test_grep "A U Thor" para3.txt &&
 		git fetch origin "refs/replace/*:refs/replace/*" &&
 		git log --pretty=oneline parallel3 >output.txt &&
-		grep $PARA3 output.txt &&
+		test_grep $PARA3 output.txt &&
 		git show $PARA3 >para3.txt &&
-		grep "O Thor" para3.txt
+		test_grep "O Thor" para3.txt
 	)
 '
 
@@ -284,8 +313,8 @@ test_expect_success 'bisect and replacements' '
 '
 
 test_expect_success 'index-pack and replacements' '
-	git --no-replace-objects rev-list --objects HEAD |
-	git --no-replace-objects pack-objects test- &&
+	git --no-replace-objects rev-list --objects HEAD >actual &&
+	git --no-replace-objects pack-objects test- <actual &&
 	git index-pack test-*.pack
 '
 
@@ -319,7 +348,8 @@ test_expect_success '-f option bypasses the type check' '
 '
 
 test_expect_success 'git cat-file --batch works on replace objects' '
-	git replace | grep $PARA3 &&
+	git replace >actual &&
+	test_grep $PARA3 actual &&
 	echo $PARA3 | git cat-file --batch
 '
 
@@ -344,7 +374,8 @@ test_expect_success 'test --format medium' '
 		echo "$PARA3 -> $S" &&
 		echo "$MYTAG -> $HASH1"
 	} | sort >expected &&
-	git replace -l --format medium | sort >actual &&
+	git replace -l --format medium >output &&
+	sort output >actual &&
 	test_cmp expected actual
 '
 
@@ -356,7 +387,8 @@ test_expect_success 'test --format long' '
 		echo "$PARA3 (commit) -> $S (commit)" &&
 		echo "$MYTAG (tag) -> $HASH1 (commit)"
 	} | sort >expected &&
-	git replace --format=long | sort >actual &&
+	git replace --format=long >output &&
+	sort output >actual &&
 	test_cmp expected actual
 '
 
@@ -374,12 +406,16 @@ test_expect_success 'setup fake editors' '
 test_expect_success '--edit with and without already replaced object' '
 	test_must_fail env GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --force --edit "$PARA3" &&
-	git replace -l | grep "$PARA3" &&
-	git cat-file commit "$PARA3" | grep "A fake Thor" &&
+	git replace -l >actual &&
+	test_grep "$PARA3" actual &&
+	git cat-file commit "$PARA3" >actual &&
+	test_grep "A fake Thor" actual &&
 	git replace -d "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
-	git replace -l | grep "$PARA3" &&
-	git cat-file commit "$PARA3" | grep "A fake Thor"
+	git replace -l >actual &&
+	test_grep "$PARA3" actual &&
+	git cat-file commit "$PARA3" >actual &&
+	test_grep "A fake Thor" actual
 '
 
 test_expect_success '--edit and change nothing or command failed' '
@@ -387,8 +423,10 @@ test_expect_success '--edit and change nothing or command failed' '
 	test_must_fail env GIT_EDITOR=true git replace --edit "$PARA3" &&
 	test_must_fail env GIT_EDITOR="./failingfakeeditor" git replace --edit "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
-	git replace -l | grep "$PARA3" &&
-	git cat-file commit "$PARA3" | grep "A fake Thor"
+	git replace -l >actual &&
+	test_grep "$PARA3" actual &&
+	git cat-file commit "$PARA3" >actual &&
+	test_grep "A fake Thor" actual
 '
 
 test_expect_success 'replace ref cleanup' '
@@ -468,7 +506,8 @@ test_expect_success GPG 'set up a merge commit with a mergetag' '
 	git checkout main &&
 	git merge -s ours test_tag &&
 	HASH10=$(git rev-parse --verify HEAD) &&
-	git cat-file commit $HASH10 | grep "^mergetag object"
+	git cat-file commit $HASH10 >actual &&
+	test_grep "^mergetag object" actual
 '
 
 test_expect_success GPG '--graft on a commit with a mergetag' '

-- 
2.34.1
Phillip Wood· Oct 14, 2024, 14:00 UTC · re: chizobajames21@gmail.com · lore

Re: [Outreachy][PATCH v5] t6050: avoid pipes with upstream Git commands

Hi Chizoba
On 12/10/2024 07:21, chizobajames21@gmail.com wrote:
Show 14 quoted lines
> From: Chizoba ODINAKA <chizobajames21@gmail.com>
> 
> In pipes, the exit code of a chain of commands is determined by
> the final command. In order not to miss the exit code of a failed
> Git command, avoid pipes instead write output of Git commands
> into a file.
> For better debugging experience, instances of "grep" were changed
> to "test_grep". "test_grep" provides more context in case of a
> failed "grep".
> 
> Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com>
> Changes in v5:
> - replace 'sort -o' which might not be supported by some platforms
>    to the more conventional output redirection, > .

It's really helpful to list the changes like this, but please do so below the "---" line so it does not become part of the commit message when the patch is applied. The range-diff looks good, with a fixed up commit message I think we can declare victory for this microproject.

Best Wishes
Phillip
Show 322 quoted lines
> ---
> Range-diff against v4:
> 1:  28cab4c752 ! 1:  b339b1f550 t6050: avoid pipes with upstream Git commands
>      @@ Commit message
>           failed "grep".
>       
>           Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com>
>      -    Changes in v4:
>      -    - Correct the subject
>      -    - fixed some ommitted output option in sorts that where writing to
>      -      stdout
>      +    Changes in v5:
>      +    - replace 'sort -o' which might not be supported by some platforms
>      +      to the more conventional output redirection, > .
>       
>        ## t/t6050-replace.sh ##
>       @@ t/t6050-replace.sh: test_expect_success 'set up buggy branch' '
>      @@ t/t6050-replace.sh: test_expect_success 'test --format medium' '
>        		echo "$MYTAG -> $HASH1"
>        	} | sort >expected &&
>       -	git replace -l --format medium | sort >actual &&
>      -+	git replace -l --format medium >actual &&
>      -+	sort actual -o actual &&
>      ++	git replace -l --format medium >output &&
>      ++	sort output >actual &&
>        	test_cmp expected actual
>        '
>        
>      @@ t/t6050-replace.sh: test_expect_success 'test --format long' '
>        		echo "$MYTAG (tag) -> $HASH1 (commit)"
>        	} | sort >expected &&
>       -	git replace --format=long | sort >actual &&
>      -+	git replace --format=long >actual &&
>      -+	sort actual -o actual &&
>      ++	git replace --format=long >output &&
>      ++	sort output >actual &&
>        	test_cmp expected actual
>        '
>        
> 
>   t/t6050-replace.sh | 133 +++++++++++++++++++++++++++++----------------
>   1 file changed, 86 insertions(+), 47 deletions(-)
> 
> diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
> index d7702fc756..37cb6547b6 100755
> --- a/t/t6050-replace.sh
> +++ b/t/t6050-replace.sh
> @@ -98,30 +98,42 @@ test_expect_success 'set up buggy branch' '
>   '
>   
>   test_expect_success 'replace the author' '
> -	git cat-file commit $HASH2 | grep "author A U Thor" &&
> -	R=$(git cat-file commit $HASH2 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
> -	git cat-file commit $R | grep "author O Thor" &&
> +	git cat-file commit $HASH2 >actual &&
> +	test_grep "author A U Thor" actual &&
> +	R=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
> +	git cat-file commit $R >actual &&
> +	test_grep "author O Thor" actual &&
>   	git update-ref refs/replace/$HASH2 $R &&
> -	git show HEAD~5 | grep "O Thor" &&
> -	git show $HASH2 | grep "O Thor"
> +	git show HEAD~5 >actual &&
> +	test_grep "O Thor" actual &&
> +	git show $HASH2 >actual &&
> +	test_grep "O Thor" actual
>   '
>   
>   test_expect_success 'test --no-replace-objects option' '
> -	git cat-file commit $HASH2 | grep "author O Thor" &&
> -	git --no-replace-objects cat-file commit $HASH2 | grep "author A U Thor" &&
> -	git show $HASH2 | grep "O Thor" &&
> -	git --no-replace-objects show $HASH2 | grep "A U Thor"
> +	git cat-file commit $HASH2 >actual &&
> +	test_grep "author O Thor" actual &&
> +	git --no-replace-objects cat-file commit $HASH2 >actual &&
> +	test_grep "author A U Thor" actual &&
> +	git show $HASH2 >actual &&
> +	test_grep "O Thor" actual &&
> +	git --no-replace-objects show $HASH2 >actual &&
> +	test_grep "A U Thor" actual
>   '
>   
>   test_expect_success 'test GIT_NO_REPLACE_OBJECTS env variable' '
> -	GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 | grep "author A U Thor" &&
> -	GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 | grep "A U Thor"
> +	GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 >actual &&
> +	test_grep "author A U Thor"  actual &&
> +	GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 >actual &&
> +	test_grep "A U Thor" actual
>   '
>   
>   test_expect_success 'test core.usereplacerefs config option' '
>   	test_config core.usereplacerefs false &&
> -	git cat-file commit $HASH2 | grep "author A U Thor" &&
> -	git show $HASH2 | grep "A U Thor"
> +	git cat-file commit $HASH2 >actual &&
> +	test_grep "author A U Thor" actual &&
> +	git show $HASH2 >actual &&
> +	test_grep "A U Thor" actual
>   '
>   
>   cat >tag.sig <<EOF
> @@ -148,14 +160,18 @@ test_expect_success 'repack, clone and fetch work' '
>   	git clone --no-hardlinks . clone_dir &&
>   	(
>   		cd clone_dir &&
> -		git show HEAD~5 | grep "A U Thor" &&
> -		git show $HASH2 | grep "A U Thor" &&
> +		git show HEAD~5 >actual &&
> +		test_grep "A U Thor" actual &&
> +		git show $HASH2 >actual &&
> +		test_grep "A U Thor" actual &&
>   		git cat-file commit $R &&
>   		git repack -a -d &&
>   		test_must_fail git cat-file commit $R &&
>   		git fetch ../ "refs/replace/*:refs/replace/*" &&
> -		git show HEAD~5 | grep "O Thor" &&
> -		git show $HASH2 | grep "O Thor" &&
> +		git show HEAD~5 >actual &&
> +		test_grep "O Thor" actual &&
> +		git show $HASH2 >actual &&
> +		test_grep "O Thor" actual &&
>   		git cat-file commit $R
>   	)
>   '
> @@ -169,13 +185,15 @@ test_expect_success '"git replace" listing and deleting' '
>   	test_must_fail git replace --delete &&
>   	test_must_fail git replace -l -d $HASH2 &&
>   	git replace -d $HASH2 &&
> -	git show $HASH2 | grep "A U Thor" &&
> +	git show $HASH2 >actual &&
> +	test_grep "A U Thor" actual &&
>   	test -z "$(git replace -l)"
>   '
>   
>   test_expect_success '"git replace" replacing' '
>   	git replace $HASH2 $R &&
> -	git show $HASH2 | grep "O Thor" &&
> +	git show $HASH2 >actual &&
> +	test_grep "O Thor" actual &&
>   	test_must_fail git replace $HASH2 $R &&
>   	git replace -f $HASH2 $R &&
>   	test_must_fail git replace -f &&
> @@ -186,7 +204,8 @@ test_expect_success '"git replace" resolves sha1' '
>   	SHORTHASH2=$(git rev-parse --short=8 $HASH2) &&
>   	git replace -d $SHORTHASH2 &&
>   	git replace $SHORTHASH2 $R &&
> -	git show $HASH2 | grep "O Thor" &&
> +	git show $HASH2 >actual &&
> +	test_grep "O Thor" actual &&
>   	test_must_fail git replace $HASH2 $R &&
>   	git replace -f $HASH2 $R &&
>   	test_must_fail git replace --force &&
> @@ -209,10 +228,12 @@ test_expect_success '"git replace" resolves sha1' '
>   #
>   test_expect_success 'create parallel branch without the bug' '
>   	git replace -d $HASH2 &&
> -	git show $HASH2 | grep "A U Thor" &&
> +	git show $HASH2 >actual &&
> +	test_grep "A U Thor" actual &&
>   	git checkout $HASH1 &&
>   	git cherry-pick $HASH2 &&
> -	git show $HASH5 | git apply &&
> +	git show $HASH5 >actual &&
> +	git apply actual &&
>   	git commit --amend -m "hello: 4 more lines WITHOUT the bug" hello &&
>   	PARA2=$(git rev-parse --verify HEAD) &&
>   	git cherry-pick $HASH3 &&
> @@ -225,7 +246,8 @@ test_expect_success 'create parallel branch without the bug' '
>   	git checkout main &&
>   	cur=$(git rev-parse --verify HEAD) &&
>   	test "$cur" = "$HASH7" &&
> -	git log --pretty=oneline | grep $PARA2 &&
> +	git log --pretty=oneline >actual &&
> +	test_grep $PARA2 actual &&
>   	git remote add cloned ./clone_dir
>   '
>   
> @@ -234,23 +256,30 @@ test_expect_success 'push to cloned repo' '
>   	(
>   		cd clone_dir &&
>   		git checkout parallel &&
> -		git log --pretty=oneline | grep $PARA2
> +		git log --pretty=oneline >actual &&
> +		test_grep $PARA2 actual
>   	)
>   '
>   
>   test_expect_success 'push branch with replacement' '
> -	git cat-file commit $PARA3 | grep "author A U Thor" &&
> -	S=$(git cat-file commit $PARA3 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
> -	git cat-file commit $S | grep "author O Thor" &&
> +	git cat-file commit $PARA3 >actual &&
> +	test_grep "author A U Thor" actual &&
> +	S=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
> +	git cat-file commit $S >actual &&
> +	test_grep "author O Thor" actual &&
>   	git replace $PARA3 $S &&
> -	git show $HASH6~2 | grep "O Thor" &&
> -	git show $PARA3 | grep "O Thor" &&
> +	git show $HASH6~2 >actual &&
> +	test_grep "O Thor" actual &&
> +	git show $PARA3 >actual &&
> +	test_grep "O Thor" actual &&
>   	git push cloned $HASH6^:refs/heads/parallel2 &&
>   	(
>   		cd clone_dir &&
>   		git checkout parallel2 &&
> -		git log --pretty=oneline | grep $PARA3 &&
> -		git show $PARA3 | grep "A U Thor"
> +		git log --pretty=oneline >actual &&
> +		test_grep $PARA3 actual &&
> +		git show $PARA3 >actual &&
> +		test_grep "A U Thor" actual
>   	)
>   '
>   
> @@ -260,14 +289,14 @@ test_expect_success 'fetch branch with replacement' '
>   		cd clone_dir &&
>   		git fetch origin refs/heads/tofetch:refs/heads/parallel3 &&
>   		git log --pretty=oneline parallel3 >output.txt &&
> -		! grep $PARA3 output.txt &&
> +		test_grep ! $PARA3 output.txt &&
>   		git show $PARA3 >para3.txt &&
> -		grep "A U Thor" para3.txt &&
> +		test_grep "A U Thor" para3.txt &&
>   		git fetch origin "refs/replace/*:refs/replace/*" &&
>   		git log --pretty=oneline parallel3 >output.txt &&
> -		grep $PARA3 output.txt &&
> +		test_grep $PARA3 output.txt &&
>   		git show $PARA3 >para3.txt &&
> -		grep "O Thor" para3.txt
> +		test_grep "O Thor" para3.txt
>   	)
>   '
>   
> @@ -284,8 +313,8 @@ test_expect_success 'bisect and replacements' '
>   '
>   
>   test_expect_success 'index-pack and replacements' '
> -	git --no-replace-objects rev-list --objects HEAD |
> -	git --no-replace-objects pack-objects test- &&
> +	git --no-replace-objects rev-list --objects HEAD >actual &&
> +	git --no-replace-objects pack-objects test- <actual &&
>   	git index-pack test-*.pack
>   '
>   
> @@ -319,7 +348,8 @@ test_expect_success '-f option bypasses the type check' '
>   '
>   
>   test_expect_success 'git cat-file --batch works on replace objects' '
> -	git replace | grep $PARA3 &&
> +	git replace >actual &&
> +	test_grep $PARA3 actual &&
>   	echo $PARA3 | git cat-file --batch
>   '
>   
> @@ -344,7 +374,8 @@ test_expect_success 'test --format medium' '
>   		echo "$PARA3 -> $S" &&
>   		echo "$MYTAG -> $HASH1"
>   	} | sort >expected &&
> -	git replace -l --format medium | sort >actual &&
> +	git replace -l --format medium >output &&
> +	sort output >actual &&
>   	test_cmp expected actual
>   '
>   
> @@ -356,7 +387,8 @@ test_expect_success 'test --format long' '
>   		echo "$PARA3 (commit) -> $S (commit)" &&
>   		echo "$MYTAG (tag) -> $HASH1 (commit)"
>   	} | sort >expected &&
> -	git replace --format=long | sort >actual &&
> +	git replace --format=long >output &&
> +	sort output >actual &&
>   	test_cmp expected actual
>   '
>   
> @@ -374,12 +406,16 @@ test_expect_success 'setup fake editors' '
>   test_expect_success '--edit with and without already replaced object' '
>   	test_must_fail env GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
>   	GIT_EDITOR=./fakeeditor git replace --force --edit "$PARA3" &&
> -	git replace -l | grep "$PARA3" &&
> -	git cat-file commit "$PARA3" | grep "A fake Thor" &&
> +	git replace -l >actual &&
> +	test_grep "$PARA3" actual &&
> +	git cat-file commit "$PARA3" >actual &&
> +	test_grep "A fake Thor" actual &&
>   	git replace -d "$PARA3" &&
>   	GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
> -	git replace -l | grep "$PARA3" &&
> -	git cat-file commit "$PARA3" | grep "A fake Thor"
> +	git replace -l >actual &&
> +	test_grep "$PARA3" actual &&
> +	git cat-file commit "$PARA3" >actual &&
> +	test_grep "A fake Thor" actual
>   '
>   
>   test_expect_success '--edit and change nothing or command failed' '
> @@ -387,8 +423,10 @@ test_expect_success '--edit and change nothing or command failed' '
>   	test_must_fail env GIT_EDITOR=true git replace --edit "$PARA3" &&
>   	test_must_fail env GIT_EDITOR="./failingfakeeditor" git replace --edit "$PARA3" &&
>   	GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
> -	git replace -l | grep "$PARA3" &&
> -	git cat-file commit "$PARA3" | grep "A fake Thor"
> +	git replace -l >actual &&
> +	test_grep "$PARA3" actual &&
> +	git cat-file commit "$PARA3" >actual &&
> +	test_grep "A fake Thor" actual
>   '
>   
>   test_expect_success 'replace ref cleanup' '
> @@ -468,7 +506,8 @@ test_expect_success GPG 'set up a merge commit with a mergetag' '
>   	git checkout main &&
>   	git merge -s ours test_tag &&
>   	HASH10=$(git rev-parse --verify HEAD) &&
> -	git cat-file commit $HASH10 | grep "^mergetag object"
> +	git cat-file commit $HASH10 >actual &&
> +	test_grep "^mergetag object" actual
>   '
>   
>   test_expect_success GPG '--graft on a commit with a mergetag' '
Chizoba ODINAKA· Oct 14, 2024, 15:27 UTC · re: Phillip Wood · lore

Re: [Outreachy][PATCH v5] t6050: avoid pipes with upstream Git commands

On Mon, 14 Oct 2024 at 15:00, Phillip Wood <phillip.wood123@gmail.com> wrote:
Show 21 quoted lines
>
> Hi Chizoba
>
> On 12/10/2024 07:21, chizobajames21@gmail.com wrote:
> > From: Chizoba ODINAKA <chizobajames21@gmail.com>
> >
> > In pipes, the exit code of a chain of commands is determined by
> > the final command. In order not to miss the exit code of a failed
> > Git command, avoid pipes instead write output of Git commands
> > into a file.
> > For better debugging experience, instances of "grep" were changed
> > to "test_grep". "test_grep" provides more context in case of a
> > failed "grep".
> >
> > Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com>
> > Changes in v5:
> > - replace 'sort -o' which might not be supported by some platforms
> >    to the more conventional output redirection, > .
>
> It's really helpful to list the changes like this, but please do so
> below the "---" line so it does not become part of the commit message
Fixed.
Show 7 quoted lines
> when the patch is applied. The range-diff looks good, with a fixed up
> commit message I think we can declare victory for this microproject.
>
> Best Wishes
>
> Phillip
>
Thanks Philip
Show 323 quoted lines
> > ---
> > Range-diff against v4:
> > 1:  28cab4c752 ! 1:  b339b1f550 t6050: avoid pipes with upstream Git commands
> >      @@ Commit message
> >           failed "grep".
> >
> >           Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com>
> >      -    Changes in v4:
> >      -    - Correct the subject
> >      -    - fixed some ommitted output option in sorts that where writing to
> >      -      stdout
> >      +    Changes in v5:
> >      +    - replace 'sort -o' which might not be supported by some platforms
> >      +      to the more conventional output redirection, > .
> >
> >        ## t/t6050-replace.sh ##
> >       @@ t/t6050-replace.sh: test_expect_success 'set up buggy branch' '
> >      @@ t/t6050-replace.sh: test_expect_success 'test --format medium' '
> >                       echo "$MYTAG -> $HASH1"
> >               } | sort >expected &&
> >       -       git replace -l --format medium | sort >actual &&
> >      -+       git replace -l --format medium >actual &&
> >      -+       sort actual -o actual &&
> >      ++       git replace -l --format medium >output &&
> >      ++       sort output >actual &&
> >               test_cmp expected actual
> >        '
> >
> >      @@ t/t6050-replace.sh: test_expect_success 'test --format long' '
> >                       echo "$MYTAG (tag) -> $HASH1 (commit)"
> >               } | sort >expected &&
> >       -       git replace --format=long | sort >actual &&
> >      -+       git replace --format=long >actual &&
> >      -+       sort actual -o actual &&
> >      ++       git replace --format=long >output &&
> >      ++       sort output >actual &&
> >               test_cmp expected actual
> >        '
> >
> >
> >   t/t6050-replace.sh | 133 +++++++++++++++++++++++++++++----------------
> >   1 file changed, 86 insertions(+), 47 deletions(-)
> >
> > diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
> > index d7702fc756..37cb6547b6 100755
> > --- a/t/t6050-replace.sh
> > +++ b/t/t6050-replace.sh
> > @@ -98,30 +98,42 @@ test_expect_success 'set up buggy branch' '
> >   '
> >
> >   test_expect_success 'replace the author' '
> > -     git cat-file commit $HASH2 | grep "author A U Thor" &&
> > -     R=$(git cat-file commit $HASH2 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
> > -     git cat-file commit $R | grep "author O Thor" &&
> > +     git cat-file commit $HASH2 >actual &&
> > +     test_grep "author A U Thor" actual &&
> > +     R=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
> > +     git cat-file commit $R >actual &&
> > +     test_grep "author O Thor" actual &&
> >       git update-ref refs/replace/$HASH2 $R &&
> > -     git show HEAD~5 | grep "O Thor" &&
> > -     git show $HASH2 | grep "O Thor"
> > +     git show HEAD~5 >actual &&
> > +     test_grep "O Thor" actual &&
> > +     git show $HASH2 >actual &&
> > +     test_grep "O Thor" actual
> >   '
> >
> >   test_expect_success 'test --no-replace-objects option' '
> > -     git cat-file commit $HASH2 | grep "author O Thor" &&
> > -     git --no-replace-objects cat-file commit $HASH2 | grep "author A U Thor" &&
> > -     git show $HASH2 | grep "O Thor" &&
> > -     git --no-replace-objects show $HASH2 | grep "A U Thor"
> > +     git cat-file commit $HASH2 >actual &&
> > +     test_grep "author O Thor" actual &&
> > +     git --no-replace-objects cat-file commit $HASH2 >actual &&
> > +     test_grep "author A U Thor" actual &&
> > +     git show $HASH2 >actual &&
> > +     test_grep "O Thor" actual &&
> > +     git --no-replace-objects show $HASH2 >actual &&
> > +     test_grep "A U Thor" actual
> >   '
> >
> >   test_expect_success 'test GIT_NO_REPLACE_OBJECTS env variable' '
> > -     GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 | grep "author A U Thor" &&
> > -     GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 | grep "A U Thor"
> > +     GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 >actual &&
> > +     test_grep "author A U Thor"  actual &&
> > +     GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 >actual &&
> > +     test_grep "A U Thor" actual
> >   '
> >
> >   test_expect_success 'test core.usereplacerefs config option' '
> >       test_config core.usereplacerefs false &&
> > -     git cat-file commit $HASH2 | grep "author A U Thor" &&
> > -     git show $HASH2 | grep "A U Thor"
> > +     git cat-file commit $HASH2 >actual &&
> > +     test_grep "author A U Thor" actual &&
> > +     git show $HASH2 >actual &&
> > +     test_grep "A U Thor" actual
> >   '
> >
> >   cat >tag.sig <<EOF
> > @@ -148,14 +160,18 @@ test_expect_success 'repack, clone and fetch work' '
> >       git clone --no-hardlinks . clone_dir &&
> >       (
> >               cd clone_dir &&
> > -             git show HEAD~5 | grep "A U Thor" &&
> > -             git show $HASH2 | grep "A U Thor" &&
> > +             git show HEAD~5 >actual &&
> > +             test_grep "A U Thor" actual &&
> > +             git show $HASH2 >actual &&
> > +             test_grep "A U Thor" actual &&
> >               git cat-file commit $R &&
> >               git repack -a -d &&
> >               test_must_fail git cat-file commit $R &&
> >               git fetch ../ "refs/replace/*:refs/replace/*" &&
> > -             git show HEAD~5 | grep "O Thor" &&
> > -             git show $HASH2 | grep "O Thor" &&
> > +             git show HEAD~5 >actual &&
> > +             test_grep "O Thor" actual &&
> > +             git show $HASH2 >actual &&
> > +             test_grep "O Thor" actual &&
> >               git cat-file commit $R
> >       )
> >   '
> > @@ -169,13 +185,15 @@ test_expect_success '"git replace" listing and deleting' '
> >       test_must_fail git replace --delete &&
> >       test_must_fail git replace -l -d $HASH2 &&
> >       git replace -d $HASH2 &&
> > -     git show $HASH2 | grep "A U Thor" &&
> > +     git show $HASH2 >actual &&
> > +     test_grep "A U Thor" actual &&
> >       test -z "$(git replace -l)"
> >   '
> >
> >   test_expect_success '"git replace" replacing' '
> >       git replace $HASH2 $R &&
> > -     git show $HASH2 | grep "O Thor" &&
> > +     git show $HASH2 >actual &&
> > +     test_grep "O Thor" actual &&
> >       test_must_fail git replace $HASH2 $R &&
> >       git replace -f $HASH2 $R &&
> >       test_must_fail git replace -f &&
> > @@ -186,7 +204,8 @@ test_expect_success '"git replace" resolves sha1' '
> >       SHORTHASH2=$(git rev-parse --short=8 $HASH2) &&
> >       git replace -d $SHORTHASH2 &&
> >       git replace $SHORTHASH2 $R &&
> > -     git show $HASH2 | grep "O Thor" &&
> > +     git show $HASH2 >actual &&
> > +     test_grep "O Thor" actual &&
> >       test_must_fail git replace $HASH2 $R &&
> >       git replace -f $HASH2 $R &&
> >       test_must_fail git replace --force &&
> > @@ -209,10 +228,12 @@ test_expect_success '"git replace" resolves sha1' '
> >   #
> >   test_expect_success 'create parallel branch without the bug' '
> >       git replace -d $HASH2 &&
> > -     git show $HASH2 | grep "A U Thor" &&
> > +     git show $HASH2 >actual &&
> > +     test_grep "A U Thor" actual &&
> >       git checkout $HASH1 &&
> >       git cherry-pick $HASH2 &&
> > -     git show $HASH5 | git apply &&
> > +     git show $HASH5 >actual &&
> > +     git apply actual &&
> >       git commit --amend -m "hello: 4 more lines WITHOUT the bug" hello &&
> >       PARA2=$(git rev-parse --verify HEAD) &&
> >       git cherry-pick $HASH3 &&
> > @@ -225,7 +246,8 @@ test_expect_success 'create parallel branch without the bug' '
> >       git checkout main &&
> >       cur=$(git rev-parse --verify HEAD) &&
> >       test "$cur" = "$HASH7" &&
> > -     git log --pretty=oneline | grep $PARA2 &&
> > +     git log --pretty=oneline >actual &&
> > +     test_grep $PARA2 actual &&
> >       git remote add cloned ./clone_dir
> >   '
> >
> > @@ -234,23 +256,30 @@ test_expect_success 'push to cloned repo' '
> >       (
> >               cd clone_dir &&
> >               git checkout parallel &&
> > -             git log --pretty=oneline | grep $PARA2
> > +             git log --pretty=oneline >actual &&
> > +             test_grep $PARA2 actual
> >       )
> >   '
> >
> >   test_expect_success 'push branch with replacement' '
> > -     git cat-file commit $PARA3 | grep "author A U Thor" &&
> > -     S=$(git cat-file commit $PARA3 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
> > -     git cat-file commit $S | grep "author O Thor" &&
> > +     git cat-file commit $PARA3 >actual &&
> > +     test_grep "author A U Thor" actual &&
> > +     S=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
> > +     git cat-file commit $S >actual &&
> > +     test_grep "author O Thor" actual &&
> >       git replace $PARA3 $S &&
> > -     git show $HASH6~2 | grep "O Thor" &&
> > -     git show $PARA3 | grep "O Thor" &&
> > +     git show $HASH6~2 >actual &&
> > +     test_grep "O Thor" actual &&
> > +     git show $PARA3 >actual &&
> > +     test_grep "O Thor" actual &&
> >       git push cloned $HASH6^:refs/heads/parallel2 &&
> >       (
> >               cd clone_dir &&
> >               git checkout parallel2 &&
> > -             git log --pretty=oneline | grep $PARA3 &&
> > -             git show $PARA3 | grep "A U Thor"
> > +             git log --pretty=oneline >actual &&
> > +             test_grep $PARA3 actual &&
> > +             git show $PARA3 >actual &&
> > +             test_grep "A U Thor" actual
> >       )
> >   '
> >
> > @@ -260,14 +289,14 @@ test_expect_success 'fetch branch with replacement' '
> >               cd clone_dir &&
> >               git fetch origin refs/heads/tofetch:refs/heads/parallel3 &&
> >               git log --pretty=oneline parallel3 >output.txt &&
> > -             ! grep $PARA3 output.txt &&
> > +             test_grep ! $PARA3 output.txt &&
> >               git show $PARA3 >para3.txt &&
> > -             grep "A U Thor" para3.txt &&
> > +             test_grep "A U Thor" para3.txt &&
> >               git fetch origin "refs/replace/*:refs/replace/*" &&
> >               git log --pretty=oneline parallel3 >output.txt &&
> > -             grep $PARA3 output.txt &&
> > +             test_grep $PARA3 output.txt &&
> >               git show $PARA3 >para3.txt &&
> > -             grep "O Thor" para3.txt
> > +             test_grep "O Thor" para3.txt
> >       )
> >   '
> >
> > @@ -284,8 +313,8 @@ test_expect_success 'bisect and replacements' '
> >   '
> >
> >   test_expect_success 'index-pack and replacements' '
> > -     git --no-replace-objects rev-list --objects HEAD |
> > -     git --no-replace-objects pack-objects test- &&
> > +     git --no-replace-objects rev-list --objects HEAD >actual &&
> > +     git --no-replace-objects pack-objects test- <actual &&
> >       git index-pack test-*.pack
> >   '
> >
> > @@ -319,7 +348,8 @@ test_expect_success '-f option bypasses the type check' '
> >   '
> >
> >   test_expect_success 'git cat-file --batch works on replace objects' '
> > -     git replace | grep $PARA3 &&
> > +     git replace >actual &&
> > +     test_grep $PARA3 actual &&
> >       echo $PARA3 | git cat-file --batch
> >   '
> >
> > @@ -344,7 +374,8 @@ test_expect_success 'test --format medium' '
> >               echo "$PARA3 -> $S" &&
> >               echo "$MYTAG -> $HASH1"
> >       } | sort >expected &&
> > -     git replace -l --format medium | sort >actual &&
> > +     git replace -l --format medium >output &&
> > +     sort output >actual &&
> >       test_cmp expected actual
> >   '
> >
> > @@ -356,7 +387,8 @@ test_expect_success 'test --format long' '
> >               echo "$PARA3 (commit) -> $S (commit)" &&
> >               echo "$MYTAG (tag) -> $HASH1 (commit)"
> >       } | sort >expected &&
> > -     git replace --format=long | sort >actual &&
> > +     git replace --format=long >output &&
> > +     sort output >actual &&
> >       test_cmp expected actual
> >   '
> >
> > @@ -374,12 +406,16 @@ test_expect_success 'setup fake editors' '
> >   test_expect_success '--edit with and without already replaced object' '
> >       test_must_fail env GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
> >       GIT_EDITOR=./fakeeditor git replace --force --edit "$PARA3" &&
> > -     git replace -l | grep "$PARA3" &&
> > -     git cat-file commit "$PARA3" | grep "A fake Thor" &&
> > +     git replace -l >actual &&
> > +     test_grep "$PARA3" actual &&
> > +     git cat-file commit "$PARA3" >actual &&
> > +     test_grep "A fake Thor" actual &&
> >       git replace -d "$PARA3" &&
> >       GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
> > -     git replace -l | grep "$PARA3" &&
> > -     git cat-file commit "$PARA3" | grep "A fake Thor"
> > +     git replace -l >actual &&
> > +     test_grep "$PARA3" actual &&
> > +     git cat-file commit "$PARA3" >actual &&
> > +     test_grep "A fake Thor" actual
> >   '
> >
> >   test_expect_success '--edit and change nothing or command failed' '
> > @@ -387,8 +423,10 @@ test_expect_success '--edit and change nothing or command failed' '
> >       test_must_fail env GIT_EDITOR=true git replace --edit "$PARA3" &&
> >       test_must_fail env GIT_EDITOR="./failingfakeeditor" git replace --edit "$PARA3" &&
> >       GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
> > -     git replace -l | grep "$PARA3" &&
> > -     git cat-file commit "$PARA3" | grep "A fake Thor"
> > +     git replace -l >actual &&
> > +     test_grep "$PARA3" actual &&
> > +     git cat-file commit "$PARA3" >actual &&
> > +     test_grep "A fake Thor" actual
> >   '
> >
> >   test_expect_success 'replace ref cleanup' '
> > @@ -468,7 +506,8 @@ test_expect_success GPG 'set up a merge commit with a mergetag' '
> >       git checkout main &&
> >       git merge -s ours test_tag &&
> >       HASH10=$(git rev-parse --verify HEAD) &&
> > -     git cat-file commit $HASH10 | grep "^mergetag object"
> > +     git cat-file commit $HASH10 >actual &&
> > +     test_grep "^mergetag object" actual
> >   '
> >
> >   test_expect_success GPG '--graft on a commit with a mergetag' '
>
chizobajames21@gmail.com· Oct 14, 2024, 15:24 UTC · re: chizobajames21@gmail.com · lore

[Outreachy][PATCH v6] t6050: avoid pipes with upstream Git commands

From: Chizoba ODINAKA <chizobajames21@gmail.com>

In pipes, the exit code of a chain of commands is determined by the final command. In order not to miss the exit code of a failed Git command, avoid pipes instead write output of Git commands into a file. For better debugging experience, instances of "grep" were changed to "test_grep". "test_grep" provides more context in case of a failed "grep".

Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com>
---
Change in v6:
- fixed a typographical error in the commit message
Range-diff against v5:
1:  b339b1f550 ! 1:  aad95a1b96 t6050: avoid pipes with upstream Git commands
    @@ Commit message
         failed "grep".
     
         Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com>
    -    Changes in v5:
    -    - replace 'sort -o' which might not be supported by some platforms
    -      to the more conventional output redirection, > .
     
      ## t/t6050-replace.sh ##
     @@ t/t6050-replace.sh: test_expect_success 'set up buggy branch' '
 t/t6050-replace.sh | 133 +++++++++++++++++++++++++++++----------------
 1 file changed, 86 insertions(+), 47 deletions(-)
Show changes to t/t6050-replace.sh +86 −47
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index d7702fc756..37cb6547b6 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -98,30 +98,42 @@ test_expect_success 'set up buggy branch' '
 '
 
 test_expect_success 'replace the author' '
-	git cat-file commit $HASH2 | grep "author A U Thor" &&
-	R=$(git cat-file commit $HASH2 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
-	git cat-file commit $R | grep "author O Thor" &&
+	git cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor" actual &&
+	R=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
+	git cat-file commit $R >actual &&
+	test_grep "author O Thor" actual &&
 	git update-ref refs/replace/$HASH2 $R &&
-	git show HEAD~5 | grep "O Thor" &&
-	git show $HASH2 | grep "O Thor"
+	git show HEAD~5 >actual &&
+	test_grep "O Thor" actual &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual
 '
 
 test_expect_success 'test --no-replace-objects option' '
-	git cat-file commit $HASH2 | grep "author O Thor" &&
-	git --no-replace-objects cat-file commit $HASH2 | grep "author A U Thor" &&
-	git show $HASH2 | grep "O Thor" &&
-	git --no-replace-objects show $HASH2 | grep "A U Thor"
+	git cat-file commit $HASH2 >actual &&
+	test_grep "author O Thor" actual &&
+	git --no-replace-objects cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor" actual &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual &&
+	git --no-replace-objects show $HASH2 >actual &&
+	test_grep "A U Thor" actual
 '
 
 test_expect_success 'test GIT_NO_REPLACE_OBJECTS env variable' '
-	GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 | grep "author A U Thor" &&
-	GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 | grep "A U Thor"
+	GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor"  actual &&
+	GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 >actual &&
+	test_grep "A U Thor" actual
 '
 
 test_expect_success 'test core.usereplacerefs config option' '
 	test_config core.usereplacerefs false &&
-	git cat-file commit $HASH2 | grep "author A U Thor" &&
-	git show $HASH2 | grep "A U Thor"
+	git cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor" actual &&
+	git show $HASH2 >actual &&
+	test_grep "A U Thor" actual
 '
 
 cat >tag.sig <<EOF
@@ -148,14 +160,18 @@ test_expect_success 'repack, clone and fetch work' '
 	git clone --no-hardlinks . clone_dir &&
 	(
 		cd clone_dir &&
-		git show HEAD~5 | grep "A U Thor" &&
-		git show $HASH2 | grep "A U Thor" &&
+		git show HEAD~5 >actual &&
+		test_grep "A U Thor" actual &&
+		git show $HASH2 >actual &&
+		test_grep "A U Thor" actual &&
 		git cat-file commit $R &&
 		git repack -a -d &&
 		test_must_fail git cat-file commit $R &&
 		git fetch ../ "refs/replace/*:refs/replace/*" &&
-		git show HEAD~5 | grep "O Thor" &&
-		git show $HASH2 | grep "O Thor" &&
+		git show HEAD~5 >actual &&
+		test_grep "O Thor" actual &&
+		git show $HASH2 >actual &&
+		test_grep "O Thor" actual &&
 		git cat-file commit $R
 	)
 '
@@ -169,13 +185,15 @@ test_expect_success '"git replace" listing and deleting' '
 	test_must_fail git replace --delete &&
 	test_must_fail git replace -l -d $HASH2 &&
 	git replace -d $HASH2 &&
-	git show $HASH2 | grep "A U Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "A U Thor" actual &&
 	test -z "$(git replace -l)"
 '
 
 test_expect_success '"git replace" replacing' '
 	git replace $HASH2 $R &&
-	git show $HASH2 | grep "O Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual &&
 	test_must_fail git replace $HASH2 $R &&
 	git replace -f $HASH2 $R &&
 	test_must_fail git replace -f &&
@@ -186,7 +204,8 @@ test_expect_success '"git replace" resolves sha1' '
 	SHORTHASH2=$(git rev-parse --short=8 $HASH2) &&
 	git replace -d $SHORTHASH2 &&
 	git replace $SHORTHASH2 $R &&
-	git show $HASH2 | grep "O Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual &&
 	test_must_fail git replace $HASH2 $R &&
 	git replace -f $HASH2 $R &&
 	test_must_fail git replace --force &&
@@ -209,10 +228,12 @@ test_expect_success '"git replace" resolves sha1' '
 #
 test_expect_success 'create parallel branch without the bug' '
 	git replace -d $HASH2 &&
-	git show $HASH2 | grep "A U Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "A U Thor" actual &&
 	git checkout $HASH1 &&
 	git cherry-pick $HASH2 &&
-	git show $HASH5 | git apply &&
+	git show $HASH5 >actual &&
+	git apply actual &&
 	git commit --amend -m "hello: 4 more lines WITHOUT the bug" hello &&
 	PARA2=$(git rev-parse --verify HEAD) &&
 	git cherry-pick $HASH3 &&
@@ -225,7 +246,8 @@ test_expect_success 'create parallel branch without the bug' '
 	git checkout main &&
 	cur=$(git rev-parse --verify HEAD) &&
 	test "$cur" = "$HASH7" &&
-	git log --pretty=oneline | grep $PARA2 &&
+	git log --pretty=oneline >actual &&
+	test_grep $PARA2 actual &&
 	git remote add cloned ./clone_dir
 '
 
@@ -234,23 +256,30 @@ test_expect_success 'push to cloned repo' '
 	(
 		cd clone_dir &&
 		git checkout parallel &&
-		git log --pretty=oneline | grep $PARA2
+		git log --pretty=oneline >actual &&
+		test_grep $PARA2 actual
 	)
 '
 
 test_expect_success 'push branch with replacement' '
-	git cat-file commit $PARA3 | grep "author A U Thor" &&
-	S=$(git cat-file commit $PARA3 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
-	git cat-file commit $S | grep "author O Thor" &&
+	git cat-file commit $PARA3 >actual &&
+	test_grep "author A U Thor" actual &&
+	S=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
+	git cat-file commit $S >actual &&
+	test_grep "author O Thor" actual &&
 	git replace $PARA3 $S &&
-	git show $HASH6~2 | grep "O Thor" &&
-	git show $PARA3 | grep "O Thor" &&
+	git show $HASH6~2 >actual &&
+	test_grep "O Thor" actual &&
+	git show $PARA3 >actual &&
+	test_grep "O Thor" actual &&
 	git push cloned $HASH6^:refs/heads/parallel2 &&
 	(
 		cd clone_dir &&
 		git checkout parallel2 &&
-		git log --pretty=oneline | grep $PARA3 &&
-		git show $PARA3 | grep "A U Thor"
+		git log --pretty=oneline >actual &&
+		test_grep $PARA3 actual &&
+		git show $PARA3 >actual &&
+		test_grep "A U Thor" actual
 	)
 '
 
@@ -260,14 +289,14 @@ test_expect_success 'fetch branch with replacement' '
 		cd clone_dir &&
 		git fetch origin refs/heads/tofetch:refs/heads/parallel3 &&
 		git log --pretty=oneline parallel3 >output.txt &&
-		! grep $PARA3 output.txt &&
+		test_grep ! $PARA3 output.txt &&
 		git show $PARA3 >para3.txt &&
-		grep "A U Thor" para3.txt &&
+		test_grep "A U Thor" para3.txt &&
 		git fetch origin "refs/replace/*:refs/replace/*" &&
 		git log --pretty=oneline parallel3 >output.txt &&
-		grep $PARA3 output.txt &&
+		test_grep $PARA3 output.txt &&
 		git show $PARA3 >para3.txt &&
-		grep "O Thor" para3.txt
+		test_grep "O Thor" para3.txt
 	)
 '
 
@@ -284,8 +313,8 @@ test_expect_success 'bisect and replacements' '
 '
 
 test_expect_success 'index-pack and replacements' '
-	git --no-replace-objects rev-list --objects HEAD |
-	git --no-replace-objects pack-objects test- &&
+	git --no-replace-objects rev-list --objects HEAD >actual &&
+	git --no-replace-objects pack-objects test- <actual &&
 	git index-pack test-*.pack
 '
 
@@ -319,7 +348,8 @@ test_expect_success '-f option bypasses the type check' '
 '
 
 test_expect_success 'git cat-file --batch works on replace objects' '
-	git replace | grep $PARA3 &&
+	git replace >actual &&
+	test_grep $PARA3 actual &&
 	echo $PARA3 | git cat-file --batch
 '
 
@@ -344,7 +374,8 @@ test_expect_success 'test --format medium' '
 		echo "$PARA3 -> $S" &&
 		echo "$MYTAG -> $HASH1"
 	} | sort >expected &&
-	git replace -l --format medium | sort >actual &&
+	git replace -l --format medium >output &&
+	sort output >actual &&
 	test_cmp expected actual
 '
 
@@ -356,7 +387,8 @@ test_expect_success 'test --format long' '
 		echo "$PARA3 (commit) -> $S (commit)" &&
 		echo "$MYTAG (tag) -> $HASH1 (commit)"
 	} | sort >expected &&
-	git replace --format=long | sort >actual &&
+	git replace --format=long >output &&
+	sort output >actual &&
 	test_cmp expected actual
 '
 
@@ -374,12 +406,16 @@ test_expect_success 'setup fake editors' '
 test_expect_success '--edit with and without already replaced object' '
 	test_must_fail env GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --force --edit "$PARA3" &&
-	git replace -l | grep "$PARA3" &&
-	git cat-file commit "$PARA3" | grep "A fake Thor" &&
+	git replace -l >actual &&
+	test_grep "$PARA3" actual &&
+	git cat-file commit "$PARA3" >actual &&
+	test_grep "A fake Thor" actual &&
 	git replace -d "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
-	git replace -l | grep "$PARA3" &&
-	git cat-file commit "$PARA3" | grep "A fake Thor"
+	git replace -l >actual &&
+	test_grep "$PARA3" actual &&
+	git cat-file commit "$PARA3" >actual &&
+	test_grep "A fake Thor" actual
 '
 
 test_expect_success '--edit and change nothing or command failed' '
@@ -387,8 +423,10 @@ test_expect_success '--edit and change nothing or command failed' '
 	test_must_fail env GIT_EDITOR=true git replace --edit "$PARA3" &&
 	test_must_fail env GIT_EDITOR="./failingfakeeditor" git replace --edit "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
-	git replace -l | grep "$PARA3" &&
-	git cat-file commit "$PARA3" | grep "A fake Thor"
+	git replace -l >actual &&
+	test_grep "$PARA3" actual &&
+	git cat-file commit "$PARA3" >actual &&
+	test_grep "A fake Thor" actual
 '
 
 test_expect_success 'replace ref cleanup' '
@@ -468,7 +506,8 @@ test_expect_success GPG 'set up a merge commit with a mergetag' '
 	git checkout main &&
 	git merge -s ours test_tag &&
 	HASH10=$(git rev-parse --verify HEAD) &&
-	git cat-file commit $HASH10 | grep "^mergetag object"
+	git cat-file commit $HASH10 >actual &&
+	test_grep "^mergetag object" actual
 '
 
 test_expect_success GPG '--graft on a commit with a mergetag' '

-- 
2.34.1
Taylor Blau· Oct 14, 2024, 21:57 UTC · re: chizobajames21@gmail.com · lore

Re: [Outreachy][PATCH v6] t6050: avoid pipes with upstream Git commands

On Mon, Oct 14, 2024 at 04:24:11PM +0100, chizobajames21@gmail.com wrote:
Show 5 quoted lines
>  test_expect_success 'test GIT_NO_REPLACE_OBJECTS env variable' '
> -	GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 | grep "author A U Thor" &&
> -	GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 | grep "A U Thor"
> +	GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 >actual &&
> +	test_grep "author A U Thor"  actual &&
Nit: it looks like there is an extra space between the closing '"' quote
character and the filename 'actual'.
Show 10 quoted lines
> @@ -284,8 +313,8 @@ test_expect_success 'bisect and replacements' '
>  '
>
>  test_expect_success 'index-pack and replacements' '
> -	git --no-replace-objects rev-list --objects HEAD |
> -	git --no-replace-objects pack-objects test- &&
> +	git --no-replace-objects rev-list --objects HEAD >actual &&
> +	git --no-replace-objects pack-objects test- <actual &&
>  	git index-pack test-*.pack
>  '

Hmm. In other instances, actual seems like an OK name choice, but here I wonder if 'in' would be more appropriate, since we're feeding it as input to another Git command.

Other than those couple of comments, these all look pretty reasonable to me.

Thanks, Taylor

Chizoba ODINAKA· Oct 15, 2024, 11:07 UTC · re: Taylor Blau · lore

Re: [Outreachy][PATCH v6] t6050: avoid pipes with upstream Git commands

Hi Taylor
On Mon, 14 Oct 2024 at 22:57, Taylor Blau <me@ttaylorr.com> wrote:
Show 11 quoted lines
>
> On Mon, Oct 14, 2024 at 04:24:11PM +0100, chizobajames21@gmail.com wrote:
> >  test_expect_success 'test GIT_NO_REPLACE_OBJECTS env variable' '
> > -     GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 | grep "author A U Thor" &&
> > -     GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 | grep "A U Thor"
> > +     GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 >actual &&
> > +     test_grep "author A U Thor"  actual &&
>
> Nit: it looks like there is an extra space between the closing '"' quote
> character and the filename 'actual'.
>
Thanks for spotting this out. I will fix this up in the next patch.
Show 15 quoted lines
> > @@ -284,8 +313,8 @@ test_expect_success 'bisect and replacements' '
> >  '
> >
> >  test_expect_success 'index-pack and replacements' '
> > -     git --no-replace-objects rev-list --objects HEAD |
> > -     git --no-replace-objects pack-objects test- &&
> > +     git --no-replace-objects rev-list --objects HEAD >actual &&
> > +     git --no-replace-objects pack-objects test- <actual &&
> >       git index-pack test-*.pack
> >  '
>
> Hmm. In other instances, actual seems like an OK name choice, but here I
> wonder if 'in' would be more appropriate, since we're feeding it as
> input to another Git command.
>

Given that it's actually (puns intended) an output from the Git command in the preceding line, isn't a neutral name, like 'actual' a better choice than 'in'.

Show 5 quoted lines
> Other than those couple of comments, these all look pretty reasonable to
> me.
>
> Thanks,
> Taylor

Thanks for the comments. Chizoba

chizobajames21@gmail.com· Oct 15, 2024, 11:26 UTC · re: Chizoba ODINAKA · lore

[Outreachy][PATCH v7] t6050: avoid pipes with upstream Git commands

From: Chizoba ODINAKA <chizobajames21@gmail.com>

In pipes, the exit code of a chain of commands is determined by the final command. In order not to miss the exit code of a failed Git command, avoid pipes instead write output of Git commands into a file. For better debugging experience, instances of "grep" were changed to "test_grep". "test_grep" provides more context in case of a failed "grep".

Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com>
---
What changed:
- a typography error, excess whitespace
Range-diff against v6:
1:  aad95a1b96 ! 1:  14c677dd80 t6050: avoid pipes with upstream Git commands
    @@ t/t6050-replace.sh: test_expect_success 'set up buggy branch' '
     -	GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 | grep "author A U Thor" &&
     -	GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 | grep "A U Thor"
     +	GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 >actual &&
    -+	test_grep "author A U Thor"  actual &&
    ++	test_grep "author A U Thor" actual &&
     +	GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 >actual &&
     +	test_grep "A U Thor" actual
      '
 t/t6050-replace.sh | 133 +++++++++++++++++++++++++++++----------------
 1 file changed, 86 insertions(+), 47 deletions(-)
Show changes to t/t6050-replace.sh +86 −47
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index d7702fc756..deba3ca430 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -98,30 +98,42 @@ test_expect_success 'set up buggy branch' '
 '
 
 test_expect_success 'replace the author' '
-	git cat-file commit $HASH2 | grep "author A U Thor" &&
-	R=$(git cat-file commit $HASH2 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
-	git cat-file commit $R | grep "author O Thor" &&
+	git cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor" actual &&
+	R=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
+	git cat-file commit $R >actual &&
+	test_grep "author O Thor" actual &&
 	git update-ref refs/replace/$HASH2 $R &&
-	git show HEAD~5 | grep "O Thor" &&
-	git show $HASH2 | grep "O Thor"
+	git show HEAD~5 >actual &&
+	test_grep "O Thor" actual &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual
 '
 
 test_expect_success 'test --no-replace-objects option' '
-	git cat-file commit $HASH2 | grep "author O Thor" &&
-	git --no-replace-objects cat-file commit $HASH2 | grep "author A U Thor" &&
-	git show $HASH2 | grep "O Thor" &&
-	git --no-replace-objects show $HASH2 | grep "A U Thor"
+	git cat-file commit $HASH2 >actual &&
+	test_grep "author O Thor" actual &&
+	git --no-replace-objects cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor" actual &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual &&
+	git --no-replace-objects show $HASH2 >actual &&
+	test_grep "A U Thor" actual
 '
 
 test_expect_success 'test GIT_NO_REPLACE_OBJECTS env variable' '
-	GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 | grep "author A U Thor" &&
-	GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 | grep "A U Thor"
+	GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor" actual &&
+	GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 >actual &&
+	test_grep "A U Thor" actual
 '
 
 test_expect_success 'test core.usereplacerefs config option' '
 	test_config core.usereplacerefs false &&
-	git cat-file commit $HASH2 | grep "author A U Thor" &&
-	git show $HASH2 | grep "A U Thor"
+	git cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor" actual &&
+	git show $HASH2 >actual &&
+	test_grep "A U Thor" actual
 '
 
 cat >tag.sig <<EOF
@@ -148,14 +160,18 @@ test_expect_success 'repack, clone and fetch work' '
 	git clone --no-hardlinks . clone_dir &&
 	(
 		cd clone_dir &&
-		git show HEAD~5 | grep "A U Thor" &&
-		git show $HASH2 | grep "A U Thor" &&
+		git show HEAD~5 >actual &&
+		test_grep "A U Thor" actual &&
+		git show $HASH2 >actual &&
+		test_grep "A U Thor" actual &&
 		git cat-file commit $R &&
 		git repack -a -d &&
 		test_must_fail git cat-file commit $R &&
 		git fetch ../ "refs/replace/*:refs/replace/*" &&
-		git show HEAD~5 | grep "O Thor" &&
-		git show $HASH2 | grep "O Thor" &&
+		git show HEAD~5 >actual &&
+		test_grep "O Thor" actual &&
+		git show $HASH2 >actual &&
+		test_grep "O Thor" actual &&
 		git cat-file commit $R
 	)
 '
@@ -169,13 +185,15 @@ test_expect_success '"git replace" listing and deleting' '
 	test_must_fail git replace --delete &&
 	test_must_fail git replace -l -d $HASH2 &&
 	git replace -d $HASH2 &&
-	git show $HASH2 | grep "A U Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "A U Thor" actual &&
 	test -z "$(git replace -l)"
 '
 
 test_expect_success '"git replace" replacing' '
 	git replace $HASH2 $R &&
-	git show $HASH2 | grep "O Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual &&
 	test_must_fail git replace $HASH2 $R &&
 	git replace -f $HASH2 $R &&
 	test_must_fail git replace -f &&
@@ -186,7 +204,8 @@ test_expect_success '"git replace" resolves sha1' '
 	SHORTHASH2=$(git rev-parse --short=8 $HASH2) &&
 	git replace -d $SHORTHASH2 &&
 	git replace $SHORTHASH2 $R &&
-	git show $HASH2 | grep "O Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual &&
 	test_must_fail git replace $HASH2 $R &&
 	git replace -f $HASH2 $R &&
 	test_must_fail git replace --force &&
@@ -209,10 +228,12 @@ test_expect_success '"git replace" resolves sha1' '
 #
 test_expect_success 'create parallel branch without the bug' '
 	git replace -d $HASH2 &&
-	git show $HASH2 | grep "A U Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "A U Thor" actual &&
 	git checkout $HASH1 &&
 	git cherry-pick $HASH2 &&
-	git show $HASH5 | git apply &&
+	git show $HASH5 >actual &&
+	git apply actual &&
 	git commit --amend -m "hello: 4 more lines WITHOUT the bug" hello &&
 	PARA2=$(git rev-parse --verify HEAD) &&
 	git cherry-pick $HASH3 &&
@@ -225,7 +246,8 @@ test_expect_success 'create parallel branch without the bug' '
 	git checkout main &&
 	cur=$(git rev-parse --verify HEAD) &&
 	test "$cur" = "$HASH7" &&
-	git log --pretty=oneline | grep $PARA2 &&
+	git log --pretty=oneline >actual &&
+	test_grep $PARA2 actual &&
 	git remote add cloned ./clone_dir
 '
 
@@ -234,23 +256,30 @@ test_expect_success 'push to cloned repo' '
 	(
 		cd clone_dir &&
 		git checkout parallel &&
-		git log --pretty=oneline | grep $PARA2
+		git log --pretty=oneline >actual &&
+		test_grep $PARA2 actual
 	)
 '
 
 test_expect_success 'push branch with replacement' '
-	git cat-file commit $PARA3 | grep "author A U Thor" &&
-	S=$(git cat-file commit $PARA3 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
-	git cat-file commit $S | grep "author O Thor" &&
+	git cat-file commit $PARA3 >actual &&
+	test_grep "author A U Thor" actual &&
+	S=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
+	git cat-file commit $S >actual &&
+	test_grep "author O Thor" actual &&
 	git replace $PARA3 $S &&
-	git show $HASH6~2 | grep "O Thor" &&
-	git show $PARA3 | grep "O Thor" &&
+	git show $HASH6~2 >actual &&
+	test_grep "O Thor" actual &&
+	git show $PARA3 >actual &&
+	test_grep "O Thor" actual &&
 	git push cloned $HASH6^:refs/heads/parallel2 &&
 	(
 		cd clone_dir &&
 		git checkout parallel2 &&
-		git log --pretty=oneline | grep $PARA3 &&
-		git show $PARA3 | grep "A U Thor"
+		git log --pretty=oneline >actual &&
+		test_grep $PARA3 actual &&
+		git show $PARA3 >actual &&
+		test_grep "A U Thor" actual
 	)
 '
 
@@ -260,14 +289,14 @@ test_expect_success 'fetch branch with replacement' '
 		cd clone_dir &&
 		git fetch origin refs/heads/tofetch:refs/heads/parallel3 &&
 		git log --pretty=oneline parallel3 >output.txt &&
-		! grep $PARA3 output.txt &&
+		test_grep ! $PARA3 output.txt &&
 		git show $PARA3 >para3.txt &&
-		grep "A U Thor" para3.txt &&
+		test_grep "A U Thor" para3.txt &&
 		git fetch origin "refs/replace/*:refs/replace/*" &&
 		git log --pretty=oneline parallel3 >output.txt &&
-		grep $PARA3 output.txt &&
+		test_grep $PARA3 output.txt &&
 		git show $PARA3 >para3.txt &&
-		grep "O Thor" para3.txt
+		test_grep "O Thor" para3.txt
 	)
 '
 
@@ -284,8 +313,8 @@ test_expect_success 'bisect and replacements' '
 '
 
 test_expect_success 'index-pack and replacements' '
-	git --no-replace-objects rev-list --objects HEAD |
-	git --no-replace-objects pack-objects test- &&
+	git --no-replace-objects rev-list --objects HEAD >actual &&
+	git --no-replace-objects pack-objects test- <actual &&
 	git index-pack test-*.pack
 '
 
@@ -319,7 +348,8 @@ test_expect_success '-f option bypasses the type check' '
 '
 
 test_expect_success 'git cat-file --batch works on replace objects' '
-	git replace | grep $PARA3 &&
+	git replace >actual &&
+	test_grep $PARA3 actual &&
 	echo $PARA3 | git cat-file --batch
 '
 
@@ -344,7 +374,8 @@ test_expect_success 'test --format medium' '
 		echo "$PARA3 -> $S" &&
 		echo "$MYTAG -> $HASH1"
 	} | sort >expected &&
-	git replace -l --format medium | sort >actual &&
+	git replace -l --format medium >output &&
+	sort output >actual &&
 	test_cmp expected actual
 '
 
@@ -356,7 +387,8 @@ test_expect_success 'test --format long' '
 		echo "$PARA3 (commit) -> $S (commit)" &&
 		echo "$MYTAG (tag) -> $HASH1 (commit)"
 	} | sort >expected &&
-	git replace --format=long | sort >actual &&
+	git replace --format=long >output &&
+	sort output >actual &&
 	test_cmp expected actual
 '
 
@@ -374,12 +406,16 @@ test_expect_success 'setup fake editors' '
 test_expect_success '--edit with and without already replaced object' '
 	test_must_fail env GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --force --edit "$PARA3" &&
-	git replace -l | grep "$PARA3" &&
-	git cat-file commit "$PARA3" | grep "A fake Thor" &&
+	git replace -l >actual &&
+	test_grep "$PARA3" actual &&
+	git cat-file commit "$PARA3" >actual &&
+	test_grep "A fake Thor" actual &&
 	git replace -d "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
-	git replace -l | grep "$PARA3" &&
-	git cat-file commit "$PARA3" | grep "A fake Thor"
+	git replace -l >actual &&
+	test_grep "$PARA3" actual &&
+	git cat-file commit "$PARA3" >actual &&
+	test_grep "A fake Thor" actual
 '
 
 test_expect_success '--edit and change nothing or command failed' '
@@ -387,8 +423,10 @@ test_expect_success '--edit and change nothing or command failed' '
 	test_must_fail env GIT_EDITOR=true git replace --edit "$PARA3" &&
 	test_must_fail env GIT_EDITOR="./failingfakeeditor" git replace --edit "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
-	git replace -l | grep "$PARA3" &&
-	git cat-file commit "$PARA3" | grep "A fake Thor"
+	git replace -l >actual &&
+	test_grep "$PARA3" actual &&
+	git cat-file commit "$PARA3" >actual &&
+	test_grep "A fake Thor" actual
 '
 
 test_expect_success 'replace ref cleanup' '
@@ -468,7 +506,8 @@ test_expect_success GPG 'set up a merge commit with a mergetag' '
 	git checkout main &&
 	git merge -s ours test_tag &&
 	HASH10=$(git rev-parse --verify HEAD) &&
-	git cat-file commit $HASH10 | grep "^mergetag object"
+	git cat-file commit $HASH10 >actual &&
+	test_grep "^mergetag object" actual
 '
 
 test_expect_success GPG '--graft on a commit with a mergetag' '

-- 
2.34.1
chizobajames21@gmail.com· Oct 22, 2024, 01:27 UTC · re: chizobajames21@gmail.com · lore

[Outreachy][PATCH v7] t6050: avoid pipes with upstream Git commands

From: Chizoba ODINAKA <chizobajames21@gmail.com>

In pipes, the exit code of a chain of commands is determined by the final command. In order not to miss the exit code of a failed Git command, avoid pipes instead write output of Git commands into a file. For better debugging experience, instances of "grep" were changed to "test_grep". "test_grep" provides more context in case of a failed "grep".

Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com>
---
What changed:
- a typography error, excess whitespace
Range-diff against v6:
1:  aad95a1b96 ! 1:  14c677dd80 t6050: avoid pipes with upstream Git commands
    @@ t/t6050-replace.sh: test_expect_success 'set up buggy branch' '
     -	GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 | grep "author A U Thor" &&
     -	GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 | grep "A U Thor"
     +	GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 >actual &&
    -+	test_grep "author A U Thor"  actual &&
    ++	test_grep "author A U Thor" actual &&
     +	GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 >actual &&
     +	test_grep "A U Thor" actual
      '
 t/t6050-replace.sh | 133 +++++++++++++++++++++++++++++----------------
 1 file changed, 86 insertions(+), 47 deletions(-)
Show changes to t/t6050-replace.sh +86 −47
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index d7702fc756..deba3ca430 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -98,30 +98,42 @@ test_expect_success 'set up buggy branch' '
 '
 
 test_expect_success 'replace the author' '
-	git cat-file commit $HASH2 | grep "author A U Thor" &&
-	R=$(git cat-file commit $HASH2 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
-	git cat-file commit $R | grep "author O Thor" &&
+	git cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor" actual &&
+	R=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
+	git cat-file commit $R >actual &&
+	test_grep "author O Thor" actual &&
 	git update-ref refs/replace/$HASH2 $R &&
-	git show HEAD~5 | grep "O Thor" &&
-	git show $HASH2 | grep "O Thor"
+	git show HEAD~5 >actual &&
+	test_grep "O Thor" actual &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual
 '
 
 test_expect_success 'test --no-replace-objects option' '
-	git cat-file commit $HASH2 | grep "author O Thor" &&
-	git --no-replace-objects cat-file commit $HASH2 | grep "author A U Thor" &&
-	git show $HASH2 | grep "O Thor" &&
-	git --no-replace-objects show $HASH2 | grep "A U Thor"
+	git cat-file commit $HASH2 >actual &&
+	test_grep "author O Thor" actual &&
+	git --no-replace-objects cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor" actual &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual &&
+	git --no-replace-objects show $HASH2 >actual &&
+	test_grep "A U Thor" actual
 '
 
 test_expect_success 'test GIT_NO_REPLACE_OBJECTS env variable' '
-	GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 | grep "author A U Thor" &&
-	GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 | grep "A U Thor"
+	GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor" actual &&
+	GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 >actual &&
+	test_grep "A U Thor" actual
 '
 
 test_expect_success 'test core.usereplacerefs config option' '
 	test_config core.usereplacerefs false &&
-	git cat-file commit $HASH2 | grep "author A U Thor" &&
-	git show $HASH2 | grep "A U Thor"
+	git cat-file commit $HASH2 >actual &&
+	test_grep "author A U Thor" actual &&
+	git show $HASH2 >actual &&
+	test_grep "A U Thor" actual
 '
 
 cat >tag.sig <<EOF
@@ -148,14 +160,18 @@ test_expect_success 'repack, clone and fetch work' '
 	git clone --no-hardlinks . clone_dir &&
 	(
 		cd clone_dir &&
-		git show HEAD~5 | grep "A U Thor" &&
-		git show $HASH2 | grep "A U Thor" &&
+		git show HEAD~5 >actual &&
+		test_grep "A U Thor" actual &&
+		git show $HASH2 >actual &&
+		test_grep "A U Thor" actual &&
 		git cat-file commit $R &&
 		git repack -a -d &&
 		test_must_fail git cat-file commit $R &&
 		git fetch ../ "refs/replace/*:refs/replace/*" &&
-		git show HEAD~5 | grep "O Thor" &&
-		git show $HASH2 | grep "O Thor" &&
+		git show HEAD~5 >actual &&
+		test_grep "O Thor" actual &&
+		git show $HASH2 >actual &&
+		test_grep "O Thor" actual &&
 		git cat-file commit $R
 	)
 '
@@ -169,13 +185,15 @@ test_expect_success '"git replace" listing and deleting' '
 	test_must_fail git replace --delete &&
 	test_must_fail git replace -l -d $HASH2 &&
 	git replace -d $HASH2 &&
-	git show $HASH2 | grep "A U Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "A U Thor" actual &&
 	test -z "$(git replace -l)"
 '
 
 test_expect_success '"git replace" replacing' '
 	git replace $HASH2 $R &&
-	git show $HASH2 | grep "O Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual &&
 	test_must_fail git replace $HASH2 $R &&
 	git replace -f $HASH2 $R &&
 	test_must_fail git replace -f &&
@@ -186,7 +204,8 @@ test_expect_success '"git replace" resolves sha1' '
 	SHORTHASH2=$(git rev-parse --short=8 $HASH2) &&
 	git replace -d $SHORTHASH2 &&
 	git replace $SHORTHASH2 $R &&
-	git show $HASH2 | grep "O Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "O Thor" actual &&
 	test_must_fail git replace $HASH2 $R &&
 	git replace -f $HASH2 $R &&
 	test_must_fail git replace --force &&
@@ -209,10 +228,12 @@ test_expect_success '"git replace" resolves sha1' '
 #
 test_expect_success 'create parallel branch without the bug' '
 	git replace -d $HASH2 &&
-	git show $HASH2 | grep "A U Thor" &&
+	git show $HASH2 >actual &&
+	test_grep "A U Thor" actual &&
 	git checkout $HASH1 &&
 	git cherry-pick $HASH2 &&
-	git show $HASH5 | git apply &&
+	git show $HASH5 >actual &&
+	git apply actual &&
 	git commit --amend -m "hello: 4 more lines WITHOUT the bug" hello &&
 	PARA2=$(git rev-parse --verify HEAD) &&
 	git cherry-pick $HASH3 &&
@@ -225,7 +246,8 @@ test_expect_success 'create parallel branch without the bug' '
 	git checkout main &&
 	cur=$(git rev-parse --verify HEAD) &&
 	test "$cur" = "$HASH7" &&
-	git log --pretty=oneline | grep $PARA2 &&
+	git log --pretty=oneline >actual &&
+	test_grep $PARA2 actual &&
 	git remote add cloned ./clone_dir
 '
 
@@ -234,23 +256,30 @@ test_expect_success 'push to cloned repo' '
 	(
 		cd clone_dir &&
 		git checkout parallel &&
-		git log --pretty=oneline | grep $PARA2
+		git log --pretty=oneline >actual &&
+		test_grep $PARA2 actual
 	)
 '
 
 test_expect_success 'push branch with replacement' '
-	git cat-file commit $PARA3 | grep "author A U Thor" &&
-	S=$(git cat-file commit $PARA3 | sed -e "s/A U/O/" | git hash-object -t commit --stdin -w) &&
-	git cat-file commit $S | grep "author O Thor" &&
+	git cat-file commit $PARA3 >actual &&
+	test_grep "author A U Thor" actual &&
+	S=$(sed -e "s/A U/O/" actual | git hash-object -t commit --stdin -w) &&
+	git cat-file commit $S >actual &&
+	test_grep "author O Thor" actual &&
 	git replace $PARA3 $S &&
-	git show $HASH6~2 | grep "O Thor" &&
-	git show $PARA3 | grep "O Thor" &&
+	git show $HASH6~2 >actual &&
+	test_grep "O Thor" actual &&
+	git show $PARA3 >actual &&
+	test_grep "O Thor" actual &&
 	git push cloned $HASH6^:refs/heads/parallel2 &&
 	(
 		cd clone_dir &&
 		git checkout parallel2 &&
-		git log --pretty=oneline | grep $PARA3 &&
-		git show $PARA3 | grep "A U Thor"
+		git log --pretty=oneline >actual &&
+		test_grep $PARA3 actual &&
+		git show $PARA3 >actual &&
+		test_grep "A U Thor" actual
 	)
 '
 
@@ -260,14 +289,14 @@ test_expect_success 'fetch branch with replacement' '
 		cd clone_dir &&
 		git fetch origin refs/heads/tofetch:refs/heads/parallel3 &&
 		git log --pretty=oneline parallel3 >output.txt &&
-		! grep $PARA3 output.txt &&
+		test_grep ! $PARA3 output.txt &&
 		git show $PARA3 >para3.txt &&
-		grep "A U Thor" para3.txt &&
+		test_grep "A U Thor" para3.txt &&
 		git fetch origin "refs/replace/*:refs/replace/*" &&
 		git log --pretty=oneline parallel3 >output.txt &&
-		grep $PARA3 output.txt &&
+		test_grep $PARA3 output.txt &&
 		git show $PARA3 >para3.txt &&
-		grep "O Thor" para3.txt
+		test_grep "O Thor" para3.txt
 	)
 '
 
@@ -284,8 +313,8 @@ test_expect_success 'bisect and replacements' '
 '
 
 test_expect_success 'index-pack and replacements' '
-	git --no-replace-objects rev-list --objects HEAD |
-	git --no-replace-objects pack-objects test- &&
+	git --no-replace-objects rev-list --objects HEAD >actual &&
+	git --no-replace-objects pack-objects test- <actual &&
 	git index-pack test-*.pack
 '
 
@@ -319,7 +348,8 @@ test_expect_success '-f option bypasses the type check' '
 '
 
 test_expect_success 'git cat-file --batch works on replace objects' '
-	git replace | grep $PARA3 &&
+	git replace >actual &&
+	test_grep $PARA3 actual &&
 	echo $PARA3 | git cat-file --batch
 '
 
@@ -344,7 +374,8 @@ test_expect_success 'test --format medium' '
 		echo "$PARA3 -> $S" &&
 		echo "$MYTAG -> $HASH1"
 	} | sort >expected &&
-	git replace -l --format medium | sort >actual &&
+	git replace -l --format medium >output &&
+	sort output >actual &&
 	test_cmp expected actual
 '
 
@@ -356,7 +387,8 @@ test_expect_success 'test --format long' '
 		echo "$PARA3 (commit) -> $S (commit)" &&
 		echo "$MYTAG (tag) -> $HASH1 (commit)"
 	} | sort >expected &&
-	git replace --format=long | sort >actual &&
+	git replace --format=long >output &&
+	sort output >actual &&
 	test_cmp expected actual
 '
 
@@ -374,12 +406,16 @@ test_expect_success 'setup fake editors' '
 test_expect_success '--edit with and without already replaced object' '
 	test_must_fail env GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --force --edit "$PARA3" &&
-	git replace -l | grep "$PARA3" &&
-	git cat-file commit "$PARA3" | grep "A fake Thor" &&
+	git replace -l >actual &&
+	test_grep "$PARA3" actual &&
+	git cat-file commit "$PARA3" >actual &&
+	test_grep "A fake Thor" actual &&
 	git replace -d "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
-	git replace -l | grep "$PARA3" &&
-	git cat-file commit "$PARA3" | grep "A fake Thor"
+	git replace -l >actual &&
+	test_grep "$PARA3" actual &&
+	git cat-file commit "$PARA3" >actual &&
+	test_grep "A fake Thor" actual
 '
 
 test_expect_success '--edit and change nothing or command failed' '
@@ -387,8 +423,10 @@ test_expect_success '--edit and change nothing or command failed' '
 	test_must_fail env GIT_EDITOR=true git replace --edit "$PARA3" &&
 	test_must_fail env GIT_EDITOR="./failingfakeeditor" git replace --edit "$PARA3" &&
 	GIT_EDITOR=./fakeeditor git replace --edit "$PARA3" &&
-	git replace -l | grep "$PARA3" &&
-	git cat-file commit "$PARA3" | grep "A fake Thor"
+	git replace -l >actual &&
+	test_grep "$PARA3" actual &&
+	git cat-file commit "$PARA3" >actual &&
+	test_grep "A fake Thor" actual
 '
 
 test_expect_success 'replace ref cleanup' '
@@ -468,7 +506,8 @@ test_expect_success GPG 'set up a merge commit with a mergetag' '
 	git checkout main &&
 	git merge -s ours test_tag &&
 	HASH10=$(git rev-parse --verify HEAD) &&
-	git cat-file commit $HASH10 | grep "^mergetag object"
+	git cat-file commit $HASH10 >actual &&
+	test_grep "^mergetag object" actual
 '
 
 test_expect_success GPG '--graft on a commit with a mergetag' '

-- 
2.34.1
Chizoba ODINAKA· Oct 22, 2024, 01:37 UTC · re: chizobajames21@gmail.com · lore

Re: [Outreachy][PATCH v7] t6050: avoid pipes with upstream Git commands

Hi
On Tue, 22 Oct 2024 at 02:27, <chizobajames21@gmail.com> wrote:
Show 16 quoted lines
>
> From: Chizoba ODINAKA <chizobajames21@gmail.com>
>
> In pipes, the exit code of a chain of commands is determined by
> the final command. In order not to miss the exit code of a failed
> Git command, avoid pipes instead write output of Git commands
> into a file.
> For better debugging experience, instances of "grep" were changed
> to "test_grep". "test_grep" provides more context in case of a
> failed "grep".
>
> Signed-off-by: Chizoba ODINAKA <chizobajames21@gmail.com>
> ---
> What changed:
> - a typography error, excess whitespace
>

This patch version has implemented all recommended changes. Thanks Chizoba

Patrick Steinhardt· Oct 22, 2024, 05:02 UTC · re: chizobajames21@gmail.com · lore

Re: [Outreachy][PATCH v7] t6050: avoid pipes with upstream Git commands

On Tue, Oct 22, 2024 at 02:27:01AM +0100, chizobajames21@gmail.com wrote:
Show 9 quoted lines
> From: Chizoba ODINAKA <chizobajames21@gmail.com>
> 
> In pipes, the exit code of a chain of commands is determined by
> the final command. In order not to miss the exit code of a failed
> Git command, avoid pipes instead write output of Git commands
> into a file.
> For better debugging experience, instances of "grep" were changed
> to "test_grep". "test_grep" provides more context in case of a
> failed "grep".
Thanks, this version looks good to me!
Patrick
Taylor Blau· Oct 22, 2024, 16:48 UTC · re: Patrick Steinhardt · lore

Re: [Outreachy][PATCH v7] t6050: avoid pipes with upstream Git commands

On Tue, Oct 22, 2024 at 07:02:41AM +0200, Patrick Steinhardt wrote:
Show 12 quoted lines
> On Tue, Oct 22, 2024 at 02:27:01AM +0100, chizobajames21@gmail.com wrote:
> > From: Chizoba ODINAKA <chizobajames21@gmail.com>
> >
> > In pipes, the exit code of a chain of commands is determined by
> > the final command. In order not to miss the exit code of a failed
> > Git command, avoid pipes instead write output of Git commands
> > into a file.
> > For better debugging experience, instances of "grep" were changed
> > to "test_grep". "test_grep" provides more context in case of a
> > failed "grep".
>
> Thanks, this version looks good to me!
Thanks, both. Let's start merging this one down.

Thanks, Taylor

← back to recent threads