Re: [PATCH v2] t4253-am-keep-cr-dos: avoid using pipes
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 7, 2019, 09:04 UTC
- Message-ID
- <xmqqy33iljo8.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20190505081633.41157-1-liboxuan@connect.hku.hk>
Boxuan Li <liboxuan@connect.hku.hk> writes:
> The exit code of the upstream in a pipe is ignored thus we should avoid > using it. By writing out the output of the git command to a file, we can > test the exit codes of both the commands.
We are trying to catch breakages in "am" in these two tests (see the title of the test file), and we rely on format-patch to correctly produce its output---if we assume that the command being tested, i.e. "am", could be fed garbage, the tests become pointless.
And once we decide to rely on the correctness of format-patch in these two tests, there no longer is a reason to fear that invocations of it upstream of a pipe would lose the exit status.
So while the patch is not wrong per-se, but these two changes are borderline Meh.
Show 30 quoted lines
> Signed-off-by: Boxuan Li <liboxuan@connect.hku.hk> > --- > Thanks to Eric Sunshine's review, I've removed spaces after '>' > and changed 'actual' into 'output'. > --- > t/t4253-am-keep-cr-dos.sh | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/t/t4253-am-keep-cr-dos.sh b/t/t4253-am-keep-cr-dos.sh > index 553fe3e88e..6e1b73ec3a 100755 > --- a/t/t4253-am-keep-cr-dos.sh > +++ b/t/t4253-am-keep-cr-dos.sh > @@ -51,14 +51,16 @@ test_expect_success 'am with dos files without --keep-cr' ' > > test_expect_success 'am with dos files with --keep-cr' ' > git checkout -b dosfiles-keep-cr initial && > - git format-patch -k --stdout initial..master | git am --keep-cr -k -3 && > + git format-patch -k --stdout initial..master >output && > + git am --keep-cr -k -3 output && > git diff --exit-code master > ' > > test_expect_success 'am with dos files config am.keepcr' ' > git config am.keepcr 1 && > git checkout -b dosfiles-conf-keepcr initial && > - git format-patch -k --stdout initial..master | git am -k -3 && > + git format-patch -k --stdout initial..master >output && > + git am -k -3 output && > git diff --exit-code master > '