On Fri, 27 Mar 2020, Rémi Bernon wrote: [...]
IIUC the fix also involves updating the failing tests message to mark some information as irrelevant to the failure itself. In this case, why not just remove that information from the message?
Sometimes it's needed to diagnose the failure.
Because the test failure messages are used to identify individual tests and track their results in time they should probably only include meaningful information and if some additional information is still useful for debugging, add a trace right before the test to print it.
That makes the tests more complicated: lots of ok() calls will need an associate trace() which will either push the WineTest reports above the 1.5 MB limit, or will require only tracing the value if the test fails:
else ok(!strcmp(buff, time), "Expected %s, got %s\n", time, buff);
-> else { ok(!strcmp(buff, time), "Unexpected LOCALE_USER_DEFAULT date / format\n"); trace("Expected %s, got %s\n", time, buff); }
or else if (strcmp(buff, time)) { ok(0, "Unexpected LOCALE_USER_DEFAULT date / format\n"); trace("Expected %s, got %s\n", time, buff); }
I guess one could cheat by putting a linefeed before the variable parts.
else ok(!strcmp(buff, time), "Unexpected date / format\nExpected %s, got %s\n", time, buff);
It's kind of ugly and requires formulating the failure message in a specific way but it would work I guess.
[...]
Maybe using the surrounding function name and the test line number relative to it would be a good unique identifier instead. I think it would reduce the possible false positives to when tests within a function are added or reordered, which would be a good occasion to fix the failing tests.
This has some drawbacks: * It makes it hard for developers to jump to the right line based on the failure message.
* It would be even harder for test.winehq.org to link to the right line in the source.
* The relative line numbers would change precisely when the test function is patched, which is when we would like to avoid false positives.
That would require some wrapper around test functions as well to track relative line numbers, but it's maybe not as bad as going over every test message to check and remove irrelevant information.
A quick and dirty PoC: https://gcc.godbolt.org/z/3TELVD
(I haven't thought a lot about it and there's maybe some obvious traps I missed.)
We have cases where test_other_function() calls test_some_function() so declare_winetest() would need to handle nesting. And some functions, but not all, take a line number as a parameter. See for instance check_unicode_string_() in advapi32:lsa. I suspect the combination of both cases could be tricky to handle.