From: Patrick Steinhardt <ps@pks•im>
To: chizobajames21@gmail•com
Cc: git@vger•kernel.org, phillip.wood@dunelm•org.uk
Subject: Re: [Outreachy][PATCH] t6050: avoid pipes in git related commands
Date: Wed, 9 Oct 2024 09:28:33 +0200 [thread overview]
Message-ID: <ZwYwm2-ixmyYVqo8@pks.im> (raw)
In-Reply-To: <20241008162117.6452-1-chizobajames21@gmail.com>
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
next prev parent reply other threads:[~2024-10-09 7:28 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-08 16:21 [Outreachy][PATCH] t6050: avoid pipes in git related commands chizobajames21
2024-10-09 7:28 ` Patrick Steinhardt [this message]
2024-10-10 7:26 ` Chizoba ODINAKA
2024-10-10 6:39 ` [Outreachy][PATCH v2] t6050: avoid pipes with downstream Git commands chizobajames21
2024-10-10 14:08 ` Phillip Wood
2024-10-10 18:51 ` Chizoba ODINAKA
2024-10-11 9:17 ` phillip.wood123
2024-10-11 15:45 ` [Outreachy][PATCH v3] " chizobajames21
2024-10-11 16:02 ` Junio C Hamano
2024-10-11 16:03 ` Eric Sunshine
2024-10-11 17:49 ` Junio C Hamano
2024-10-11 23:59 ` [Outreachy][PATCH v4] t6050: avoid pipes with upstream " chizobajames21
2024-10-12 5:35 ` Eric Sunshine
2024-10-12 6:28 ` Chizoba ODINAKA
2024-10-12 6:21 ` [Outreachy][PATCH v5] " chizobajames21
2024-10-14 14:00 ` Phillip Wood
2024-10-14 15:27 ` Chizoba ODINAKA
2024-10-14 15:24 ` [Outreachy][PATCH v6] " chizobajames21
2024-10-14 21:57 ` Taylor Blau
2024-10-15 11:07 ` Chizoba ODINAKA
2024-10-15 11:26 ` [Outreachy][PATCH v7] " chizobajames21
2024-10-22 1:27 ` chizobajames21
2024-10-22 1:37 ` Chizoba ODINAKA
2024-10-22 5:02 ` Patrick Steinhardt
2024-10-22 16:48 ` Taylor Blau
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZwYwm2-ixmyYVqo8@pks.im \
--to=ps@pks$(echo .)im \
--cc=chizobajames21@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=phillip.wood@dunelm$(echo .)org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox