Closed Bug 1439988 Opened 3 years ago Closed 3 years ago

Windows 7/10 opt failure with run-by-manifest in toolkit/content/tests/reftests/bug-442419-progressmeter-max.xul


(Toolkit :: XUL Widgets, defect)

Not set



Tracking Status
firefox61 --- fixed


(Reporter: ahal, Assigned: dmajor)




(1 file)

In bug 1353461 we are in the process of switching reftest to "run-by-manifest" mode. This means that between every manifest of tests we will restart Firefox. In general this leads to improved stability, though in a few cases existing tests that happened to depend on side-effects of previous tests can start failing. This is one of those cases.

This test is already disabled across the board on OSX and on Windows 8 (which we don't run anymore). I suspect the failure is a timing issue that run-by-manifest mode makes more reproducible on the other windows versions. It seems to only be an issue on opt (though ironically the windows 8 exclusion was specifically for debug). There are also no less than 5 intermittent bugs filed against this test (3 of which are still open), all for Windows.

Here is a push:

Example log:

We'll need to either fix or disable this test on Windows before we can land run-by-manifest. Given how flaky it already seems to be, I'd lean towards the latter if we can't find someone to work on it right away.
clang-cl makes this test start failing most of the time on Windows 7 debug 32-bit as well:

Do you think I can ignore this?
Neil (picking a random target based on blame), bug-442419-progressmeter-max.xul has a giant fails-if and random-if section, supposedly because the "progress meter animates". As far as I can tell, the test just cares that custom positions are allowed, and not what the meter actually looks like. Is there any styling magic that could give the meter a flat look and get rid of all this flakiness?
Flags: needinfo?(enndeakin)
This would be done by native theme code and I don't from glancing at this code see a means to turn the animation off. One could be added I suppose -- On Windows, this would be to the places where QueueAnimatedContentForRefresh is called.
Flags: needinfo?(enndeakin)
IMO this test doesn't deserve any product-code changes :/
Blocks: 1447727
From looking through the code, apparently we don't animate if the progress is full.

With a full progress bar, we pass everywhere without any skip-if or random-if:

Since we just want to test custom `max` values, perhaps this is sufficient? IMO it seems better than disabling it in so many places.
Attachment #8962510 - Flags: review?(enndeakin)
Comment on attachment 8962510 [details] [diff] [review]
Use a full progressmeter to avoid animation

I think that is probably ok.
Attachment #8962510 - Flags: review?(enndeakin) → review+
Pushed by
Use a full progress meter to avoid flakiness from animation. r=enndeakin
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Assignee: nobody → dmajor
You need to log in before you can comment on or make changes to this bug.