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

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

[chizobajames21]

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(-)

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

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

[Patrick Steinhardt]

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.

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.

> 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

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

[Chizoba ODINAKA]

On Wed, 9 Oct 2024 at 08:28, Patrick Steinhardt <ps@pks.im> wrote:
>
> 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.

> > 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

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

[chizobajames21]

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(-)

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

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

[Phillip Wood]

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

>   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.

Best Wishes

Phillip

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

[Chizoba ODINAKA]

Hi Philip,
On Thu, 10 Oct 2024 at 15:08, Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> 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.
> > 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.

> 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

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

[phillip.wood123]

Hi Chizoba

On 10/10/2024 19:51, Chizoba ODINAKA wrote:
> 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

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

[chizobajames21]

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(-)

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

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

[Junio C Hamano]

chizobajames21@gmail.com writes:

> 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.

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

[Eric Sunshine]

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`.

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

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> 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.

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

[chizobajames21]

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(-)

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

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

[Eric Sunshine]

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 &&

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

[Chizoba ODINAKA]

On Sat, 12 Oct 2024 at 06:35, Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> 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.

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

[chizobajames21]

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(-)

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

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

[Phillip Wood]

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 
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

> ---
> 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' '

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

[Chizoba ODINAKA]

On Mon, 14 Oct 2024 at 15:00, Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> 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.

> 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

> > ---
> > 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' '
>

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

[chizobajames21]

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(-)

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

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

[Taylor Blau]

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'.

> @@ -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

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

[Chizoba ODINAKA]

Hi Taylor

On Mon, 14 Oct 2024 at 22:57, Taylor Blau <me@ttaylorr.com> wrote:
>
> 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.
> > @@ -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'.
> Other than those couple of comments, these all look pretty reasonable to
> me.
>
> Thanks,
> Taylor
Thanks for the comments.
Chizoba

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

[chizobajames21]

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(-)

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

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

[chizobajames21]

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(-)

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

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

[Chizoba ODINAKA]

Hi

On Tue, 22 Oct 2024 at 02:27, <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>
> ---
> What changed:
> - a typography error, excess whitespace
>
This patch version has implemented all recommended changes.
Thanks
Chizoba

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

[Patrick Steinhardt]

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!

Patrick

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

[Taylor Blau]

On Tue, Oct 22, 2024 at 07:02:41AM +0200, Patrick Steinhardt wrote:
> 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