On 6/1/22 15:26, Francois Gouget wrote:
Wine has many unreliable tests which makes it hard to know whether a patch causes new failures. So one proposal was to mark unreliable tests as flaky, and have a mode where flaky failures can ignored by setting $WINETEST_ALLOW_FLAKY.
So one would be able to apply a patch, run 'WINETEST_ALLOW_FLAKY=1 make test' and if that fails that means the patch introduces a new failure.
The way it works is that when a flaky test fails the message is 'Flaky failed...' instead of 'Test failed...'. I also added the count of flaky failures to the summary line. So the flaky failures are not totally swept under the rug. The main difference is that if $WINETEST_ALLOW_FLAKY is set, the exit code is 0 if the only failures are flaky ones.
That still leaves some open questions though:
- Should the message be 'Test failed' instead of 'Flaky failed' when $WINETEST_ALLOW_FLAKY is not set? I opted for the latter because it adds information that the failed test has been marked as flaky.
Imho the message should keep the 'Test failed' / 'Test succeeded' prefixes. For instance I think it would look more consistent with something like:
Test succeeded inside todo block: Test succeeded inside flaky block: Test marked todo: Test marked flaky: Test failed: Test succeeded:
On a related note the patch below has flaky_windows and flaky_wine macros. Is that too fine grained? Maybe we don't want to run the risk of marking a test as flaky_windows only to discover later that it can also fail in Wine? But conversely, wouldn't it be valuable to know that a test is only flaky in Wine and not in Windows?
Note: flaky_windows is a bit long but I was worried that flaky_win would be too similar to flaky_wine.
I decided having flaky_{windows,wine}_if() macros would be overkill.
I think flaky_if should be enough, and then there could be some shortcut global variables for `!strcmp(winetest_platform, "windows" / "wine")`, so it's not too long to write.
Cheers,