On 07/05/2025 07:52, Patrick Steinhardt wrote:
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
