Thread (6 messages) 6 messages, 4 authors, 26d ago

Re: [PATCH] t4216: fix no-op test that breaks TAP output

From: Taylor Blau <hidden>
Date: 2026-06-19 14:04:26

Hi Patrick,

A couple of thanks are owed: one to Todd for reporting this issue in the
first place, another to Peff for analyizing why it didn't appear broken
before, and a third for you for proposing a patch to fix it.

If you choose to delete this piece of test infrastructure entirely (I
think that there is an alternative direction that I would prefer, but
see below for more on why), I think the patch you wrote below is OK.

But...

On Fri, Jun 19, 2026 at 09:20:20AM +0200, Patrick Steinhardt wrote:
In t4216 we have have a prerequisite that is active in case the system's
`char` type is signed by default. This prerequisite isn't really used by
anything though: while it is used to guard one of our tests, that
specific test is essentially a no-op. So all this infrastructure does is
to provide some debugging hint to a reader that pays a lot of attention.
I don't think that this is guarding nothing, but I agree that the test
as written is strange. As I recall, this was to sanity check the v1
Bloom values, but allow failures on platforms where the `char` type is
unsigned by default.

I don't feel that strongly about whether or not we check the exact
value of the filter, but I think there are a couple of arguments in
favor of doing so. Most compelling would be that we know that our
murmur3 implementation is correct (in at least one case) and that we
don't regress that case in the future. We do have these checks for v2
changed-path Bloom filters where the signed-ness of `char` is
irrelevant.
Besides that, the way we set up the prerequisite also results in broken
TAP output on systems where `char` is unsigned by default: we use
`test_cmp()` to diff two files outside of of any test body, and if the
files differ we enable the prerequisite. If so, the call to `test_cmp()`
would also print output, and that output is of course not valid TAP
output.
Given this and the above, I would probably err on the side of
designating this as 'test_lazy_prereq' or otherwise silencing the output
of 'test_cmp' so that this does not taint the TAP output.

Thanks,
Taylor
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help