On Wed, 9 Oct 2024 at 08:28, Patrick Steinhardt <ps@xxxxxx> wrote: > > On Tue, Oct 08, 2024 at 05:21:17PM +0100, chizobajames21@xxxxxxxxx wrote: > > From: Chizoba ODINAKA <chizobajames21@xxxxxxxxx> > > > > 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@xxxxxxxxx> > > --- > > 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
