Closed Bug 1439988 Opened 7 years ago Closed 7 years ago

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

Categories

(Toolkit :: XUL Widgets, defect)

Unspecified
Windows
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: ahal, Assigned: away)

References

Details

Attachments

(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: https://treeherder.mozilla.org/#/jobs?repo=try&revision=60c9dea32ffd420b8e1290a54af506db6e829c7c Example log: https://treeherder.mozilla.org/logviewer.html#?job_id=163446074&repo=try&lineNumber=3843 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: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2c6de643c86977706798662ac050ede540c40e95&group_state=expanded 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: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6bb88232e3b212a36674c18920628e972a1aad86&filter-tier=1&filter-tier=2&filter-tier=3&group_state=expanded&filter-searchStr=reftest 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 dmajor@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/67fc44096e91 Use a full progress meter to avoid flakiness from animation. r=enndeakin
Status: NEW → RESOLVED
Closed: 7 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.

Attachment

General

Creator:
Created:
Updated:
Size: