From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] tests: drop use of 'tee' that hides exit status
Date: Tue, 13 Aug 2024 10:23:22 -0700 [thread overview]
Message-ID: <xmqqcymc5oc5.fsf@gitster.g> (raw)
In-Reply-To: <f5d0510c-d455-5e80-08b7-e08c81df4adb@gmx.de> (Johannes Schindelin's message of "Tue, 13 Aug 2024 14:22:11 +0200 (CEST)")
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Hi Junio,
>
> On Thu, 8 Aug 2024, Junio C Hamano wrote:
>
>> diff --git c/t/t1001-read-tree-m-2way.sh w/t/t1001-read-tree-m-2way.sh
>> index 88c524f655..48a1550371 100755
>> --- c/t/t1001-read-tree-m-2way.sh
>> +++ w/t/t1001-read-tree-m-2way.sh
>> @@ -397,7 +397,7 @@ test_expect_success 'a/b vs a, plus c/d case setup.' '
>>
>> test_expect_success 'a/b vs a, plus c/d case test.' '
>> read_tree_u_must_succeed -u -m "$treeH" "$treeM" &&
>> - git ls-files --stage | tee >treeMcheck.out &&
>> + git ls-files --stage >treeMcheck.out &&
>
> While this obviously fixes the bug where the test case was incorrectly
> allowed to continue after a failing `git ls-files --stage` call, I will
> note that I interpret the intention of the `| tee` as showing the output
> in the logs in addition to redirecting it to a file for the benefit of
> additional checks in the same test case.
If we really want to do that, we'd do
git ls-files --stage >treeMcheck.out &&
cat treeMcheck.out &&
instead. You wouldn't lose the exit status, and the output would
show the contents just like "tee" would.
We however tend to also remove a leftover debugging "cat", so the
"cat" is likely to be removed during such a clean-up.
> So, what to do here? I don't really know. The easiest option that most
> other people would likely be happy with would be to go with the `| tee`
> dropping.
As long as we treat the CI as a "batch" environment, the postmortem
tarball of the trash directory is probably the best we can do I can
think of.
prev parent reply other threads:[~2024-08-13 17:23 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-08 21:19 [PATCH] tests: drop use of 'tee' that hides exit status Junio C Hamano
2024-08-09 0:37 ` Rubén Justo
2024-08-09 1:25 ` Junio C Hamano
2024-08-13 12:22 ` Johannes Schindelin
2024-08-13 12:59 ` Jeff King
2024-08-13 17:23 ` Junio C Hamano [this message]
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=xmqqcymc5oc5.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).