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

From: Patrick Steinhardt <[email-masked]>
Date: Wed, 7 May 2025 08:52:25 +0200

On Tue, May 06, 2025 at 02:17:09PM +0100, Phillip Wood wrote:
> Hi Patrick
> 
> On 06/05/2025 11:59, Patrick Steinhardt wrote:
> > The TAP format does not allow arbitrary output outside of a specific
> > test case. If a test suite wants to print any such diagnostic output,
> > then this output has to be prefixed with "#" to mark it accordingly.
> > A bunch of our tests generate output outside of `test_expect_*`
> > testcases anyway without such a mark, which breaks strict TAP parsers.
> > 
> > Upon further inspection, all of the output generated by such tests is
> > rather uninteresting. Refactor them so that we don't break the TAP
> > format.
> 
> 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.

Patrick