Subject: Re: [PATCH] tests: drop use of 'tee' that hides exit status

From: Junio C Hamano <[email-masked]>
Date: Tue, 13 Aug 2024 10:23:22 -0700

Johannes Schindelin <[email-masked]> 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.