Re: [PATCH 1/4] t: fix cases where output breaks TAP format

From: Phillip Wood <phillip.wood123@gmail.com>
Date: May 7, 2025, 10:12 UTC
Message-ID: <548bec93-ae39-4536-8972-5a9ec538aa3e@gmail.com>
In-Reply-To: <aBsDKayGKszubn5i@pks.im>

On 07/05/2025 07:52, Patrick Steinhardt wrote:

Show 12 quoted lines

> On Tue, May 06, 2025 at 02:17:09PM +0100, Phillip Wood wrote:
>
>> I think there is an argument that these tests are broken and we should be
>> running these commands inside test_expect_success(). However this patch
>> doesn't make things substantially worse because although we lose the output
>> from test_create_repo that probably isn't going to matter. The changes to
>> the highlighting prereq look fine too.
> 
> Yeah, agreed, our modern style when writing tests should always use
> `test_expect_success()` indeed. So an alternative to this commit would
> thus be to use `test_expect_success()` as you propose. Let me know your
> preference, I'm happy to adapt if you think this is preferable.

If you feel like re-rolling using test_expect_success() that would definitely be an improvement but I don't think your patch makes things worse than they already are so don't feel you have to.

Thanks

Phillip