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

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Andrew Halberstadt [:ahal]]

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.

Comment 1

[(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?

Comment 2

[(Away)]

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?

Comment 3

[Neil Deakin]

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.

Comment 4

[(Away)]

IMO this test doesn't deserve any product-code changes :/

Comment 5

[(Away)]

Attachment: Use a full progressmeter to avoid animation

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.

Comment 6

[Neil Deakin]

Comment on attachment 8962510 [details] [diff] [review]
Use a full progressmeter to avoid animation

I think that is probably ok.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/67fc44096e91
Use a full progress meter to avoid flakiness from animation. r=enndeakin

Comment 8

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/67fc44096e91