Closed
Bug 1342240
Opened 7 years ago
Closed 7 years ago
Permafailing android 315920-12c.html,315920-13a.html,315920-8a.html,315920-8b.html,315920-7c.html | image comparison, max difference: 229, number of differing pixels: 23,45
Categories
(Core :: Layout: Form Controls, defect)
Core
Layout: Form Controls
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox52 | --- | unaffected |
firefox-esr52 | --- | unaffected |
firefox53 | --- | unaffected |
firefox54 | --- | fixed |
People
(Reporter: intermittent-bug-filer, Unassigned)
References
(Depends on 1 open bug)
Details
(Keywords: intermittent-failure, Whiteboard: [stockwell disabled])
Attachments
(1 file)
2.24 KB,
patch
|
gbrown
:
review+
|
Details | Diff | Splinter Review |
Filed by: wkocher [at] mozilla.com https://treeherder.mozilla.org/logviewer.html#?job_id=79652956&repo=mozilla-central https://queue.taskcluster.net/v1/task/OYgxkuRVTBCkqhKrt2mxhQ/runs/0/artifacts/public/logs/live_backing.log https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=https://queue.taskcluster.net/v1/task/OYgxkuRVTBCkqhKrt2mxhQ/runs/0/artifacts/public/logs/live_backing.log&only_show_unexpected=1 This started failing on Android on a merge this morning from mozilla-inbound to mozilla-central (and then again started failing on inbound and autoland when m-c merged back around to them): https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&fromchange=32dcdde1fc64fc39a9065dc4218265dbc727673f&noautoclassify&bugfiler&selectedJob=79652956&filter-searchStr=9670c21a3ea41b4e517ccc62ab9ad9f32be7e5dc I don't see any patch that's blatantly at fault here.
The merge from autoland to m-c (which had this test still passing): https://hg.mozilla.org/mozilla-central/pushloghtml?changeset=27dade5e0c8350189eeb6495d70a9fb25ce137a9 The merge from inbound to m-c (which had the test start permafailing): https://hg.mozilla.org/mozilla-central/pushloghtml?changeset=c02dd6a7e9c193b488271eb53e3ea039042c9ed6
Looks like it expects the first checkbox to be checked, but it isn't.
Comment hidden (Intermittent Failures Robot) |
Comment 4•7 years ago
|
||
:bz -- You worked on this test, long ago. Any idea what's gone wrong?
Flags: needinfo?(bzbarsky)
Comment 5•7 years ago
|
||
Well, the obvious fail is that the checkmark in the checkbox is not drawing. The actual checked state is correct, since the text is green. It's just the drawing of the checkmark that's broken. And it's not broken in general (paints in the reference).... So presumably some sort of invalidation bug. But nothing in the pushlog jumps out at me. This was green on both parents of the merge but broken on the merge? :(
Flags: needinfo?(bzbarsky)
Comment 6•7 years ago
|
||
I mean nothing in the "inbound merge to m-c" pushlog. On the autoland side, maybe bug 1026804 messes up something in the test harness? May be worth doing a try with that turned back off. Past that no sure.
Comment 7•7 years ago
|
||
(In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #6) > maybe bug 1026804 messes up something in the test harness? May be worth > doing a try with that turned back off. Past that no sure. I'll check on that - thanks. https://treeherder.mozilla.org/#/jobs?repo=try&revision=b528fced62ea37341048d1dabcae1a858948f585
Comment 8•7 years ago
|
||
Another possibility: In all of the failed cases, 315920-12c runs as the first test in the chunk; in all of the recent tests that passed before the merge, 315920-12c ran as the second test in the chunk, after 315920-12b. OK: REFTEST SUITE-START | Running 14321 tests REFTEST INFO | Running chunk 15 out of 48 chunks. tests 4585-4883/299 REFTEST TEST-START | http://10.0.2.2:8854/tests/layout/reftests/bugs/315920-12b.html == http://10.0.2.2:8854/tests/layout/reftests/bugs/315920-12-ref.html Fail: REFTEST SUITE-START | Running 14323 tests REFTEST INFO | Running chunk 15 out of 48 chunks. tests 4586-4884/299 REFTEST TEST-START | http://10.0.2.2:8854/tests/layout/reftests/bugs/315920-12c.html == http://10.0.2.2:8854/tests/layout/reftests/bugs/315920-12-ref.html
Comment 9•7 years ago
|
||
(In reply to Geoff Brown [:gbrown] from comment #7) > (In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #6) > > maybe bug 1026804 messes up something in the test harness? May be worth > > doing a try with that turned back off. Past that no sure. > > I'll check on that - thanks. > > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=b528fced62ea37341048d1dabcae1a858948f585 No, it's not bug 1026804.
Comment hidden (Intermittent Failures Robot) |
On Android, the checkmark is an SVG image. Perhaps we're firing onload despite the image not having loaded (or maybe it loaded but hasn't gone through the whole drawing pipeline yet)? https://searchfox.org/mozilla-central/rev/c9ccb8ec1f5acda78e6344ffb87aa3a409031e3d/layout/style/res/forms.css#643
Comment 12•7 years ago
|
||
Good news, adding a couple of reftests turned the crank, and it's no longer permaorange on Android opt. Bad news, if, as that did, you put 315920-13a.html first in a chunk, it'll fail to get the checkmark loaded in time, and now Android debug's permaorange.
Summary: Permafailing android 315920-12c.html == 315920-12-ref.html | image comparison, max difference: 229, number of differing pixels: 23 → Permafailing android 315920-12c.html,315920-13a.html | image comparison, max difference: 229, number of differing pixels: 23
Comment 13•7 years ago
|
||
I got the same result on try: I removed some unrelated reftests to shift the tests so that 315920-12c no longer runs first, and all passed. https://treeherder.mozilla.org/#/jobs?repo=try&revision=209941037623eedefc0e2f281428ce22f78351db
Comment 14•7 years ago
|
||
Using MozReftestInvalidate should fix this. See bug 1334944.
See Also: → 1339062
Comment hidden (Intermittent Failures Robot) |
Comment 16•7 years ago
|
||
Yeah, running first in a chunk before anything in that session has had a chance to load the image could have this effect. After that the image would be cached and loads would complete sync.... That at least explains why this could happen on a merge.
Comment hidden (Intermittent Failures Robot) |
Comment 18•7 years ago
|
||
I saw this failure in 315920-8a.html on my try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9661df4cb88d5e5266bedb934afc61be59e303d4&selectedJob=80356303 Because I added some reftests?
Summary: Permafailing android 315920-12c.html,315920-13a.html | image comparison, max difference: 229, number of differing pixels: 23 → Permafailing android 315920-12c.html,315920-13a.html,315920-8a.html | image comparison, max difference: 229, number of differing pixels: 23,45
Comment hidden (Intermittent Failures Robot) |
(In reply to Mats Palmgren (:mats) from comment #14) > Using MozReftestInvalidate should fix this. See bug 1334944. It still seems wrong to me that this is required. Shouldn't the reftest harness be waiting for images to load (perhaps via shouldWaitForPendingPaints() in reftest-content.js calling to nsPresContext::IsDOMPaintEventPending)?
Updated•7 years ago
|
Whiteboard: [stockwell needswork]
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Updated•7 years ago
|
Summary: Permafailing android 315920-12c.html,315920-13a.html,315920-8a.html | image comparison, max difference: 229, number of differing pixels: 23,45 → Permafailing android 315920-12c.html,315920-13a.html,315920-8a.html,315920-8b.html | image comparison, max difference: 229, number of differing pixels: 23,45
Updated•7 years ago
|
Summary: Permafailing android 315920-12c.html,315920-13a.html,315920-8a.html,315920-8b.html | image comparison, max difference: 229, number of differing pixels: 23,45 → Permafailing android 315920-12c.html,315920-13a.html,315920-8a.html,315920-8b.html,315920-7c.html | image comparison, max difference: 229, number of differing pixels: 23,45
Comment 26•7 years ago
|
||
I assume the fix for this is in bug 1334944, I will push on that, with 552 failures in the last week (60%+ of the time), this is a bit more than a nuisance. If we cannot get bug 1334944 assigned and in progress by end of day tomorrow, I will disable all 315920 bugs until we have time.
Comment hidden (Intermittent Failures Robot) |
Comment 28•7 years ago
|
||
this is sort of 'disabling' the tests until bug 1334944 can get completion.
Attachment #8843438 -
Flags: review?(gbrown)
Comment 29•7 years ago
|
||
Comment on attachment 8843438 [details] [diff] [review] add fuzzy-if to frequently failing tests. Review of attachment 8843438 [details] [diff] [review]: ----------------------------------------------------------------- Maybe add a comment pointing back to this bug.
Attachment #8843438 -
Flags: review?(gbrown) → review+
Comment 30•7 years ago
|
||
Pushed by jmaher@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/1416b28a89c5 Permafailing android 315920*, add fuzzy-if. r=gbrown
Updated•7 years ago
|
Whiteboard: [stockwell needswork] → [stockwell disabled]
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment 34•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1416b28a89c5
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Comment hidden (Intermittent Failures Robot) |
Updated•7 years ago
|
status-firefox52:
--- → unaffected
status-firefox53:
--- → unaffected
status-firefox-esr52:
--- → unaffected
Comment hidden (Intermittent Failures Robot) |
Comment 37•7 years ago
|
||
This is hitting -9 on Aurora today. I'll fuzzy-if it away like we did on trunk in comment 34 I guess. https://treeherder.mozilla.org/logviewer.html#?job_id=87311362&repo=mozilla-aurora
Comment 38•7 years ago
|
||
Fuzz harder on Aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/a55f31cc0fe1 I suppose I could land this on trunk too, but I guess I'll hold out hope that we'll fix this properly somewhere else before we end up having to.
You need to log in
before you can comment on or make changes to this bug.
Description
•