On Wed, 9 Oct 2024 at 08:28, Patrick Steinhardt [off-list ref] wrote:

On Tue, Oct 08, 2024 at 05:21:17PM +0100, chizobajames21@gmail.com wrote:

quoted

From: Chizoba ODINAKA <redacted>

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.

quoted

Signed-off-by: Chizoba ODINAKA <redacted>
---
 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