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

RESOLVED FIXED in Firefox 61

Status

()

defect
RESOLVED FIXED
Last year
10 months ago

People

(Reporter: ahal, Assigned: dmajor)

Tracking

unspecified
mozilla61
Unspecified
Windows
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox61 fixed)

Details

Attachments

(1 attachment)

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
https://hg.mozilla.org/mozilla-central/rev/67fc44096e91
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Assignee: nobody → dmajor
You need to log in before you can comment on or make changes to this bug.