Closed
Bug 1378577
Opened 7 years ago
Closed 7 years ago
Intermittent stylo layout/style/test/chrome/test_stylesheet_clone_import_rule.html | Test timed out.
Categories
(Core :: CSS Parsing and Computation, defect, P4)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla57
People
(Reporter: intermittent-bug-filer, Assigned: bradwerth)
References
Details
(Keywords: intermittent-failure, Whiteboard: [stockwell fixed:race])
Attachments
(1 file)
Filed by: wkocher [at] mozilla.com https://treeherder.mozilla.org/logviewer.html#?job_id=111973939&repo=mozilla-inbound https://queue.taskcluster.net/v1/task/F7LDYFeZRyC8Vw7V3FJosw/runs/0/artifacts/public/logs/live_backing.log
Updated•7 years ago
|
Priority: -- → P4
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Updated•7 years ago
|
Blocks: stylo-intermittent-test-failures
Updated•7 years ago
|
No longer blocks: stylo-bustage
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment 8•7 years ago
|
||
This failure picked up in frequency on August 15th, primarily for linux64-stylo-opt (36 failures in the last 7 days). Here is a recent log: https://treeherder.mozilla.org/logviewer.html#?repo=autoland&job_id=124674692&lineNumber=2737 and a related screenshot: https://public-artifacts.taskcluster.net/f9Ms3Vc9R-KDRv4vPcmy1w/0/public/test_info//mozilla-test-fail-screenshot_FAEywg.png here is data from the above log showing the failure: [task 2017-08-21T21:11:19.132865Z] 21:11:19 INFO - TEST-START | layout/style/test/chrome/test_stylesheet_clone_import_rule.html [task 2017-08-21T21:16:46.979622Z] 21:16:46 INFO - TEST-INFO | started process screentopng [task 2017-08-21T21:16:47.263028Z] 21:16:47 INFO - TEST-INFO | screentopng: exit 0 [task 2017-08-21T21:16:47.264665Z] 21:16:47 INFO - Buffered messages logged at 21:11:19 [task 2017-08-21T21:16:47.266544Z] 21:16:47 INFO - TEST-PASS | layout/style/test/chrome/test_stylesheet_clone_import_rule.html | div begins as green. [task 2017-08-21T21:16:47.267358Z] 21:16:47 INFO - TEST-PASS | layout/style/test/chrome/test_stylesheet_clone_import_rule.html | Imported sheets exist. [task 2017-08-21T21:16:47.268015Z] 21:16:47 INFO - Buffered messages finished [task 2017-08-21T21:16:47.268778Z] 21:16:47 INFO - TEST-UNEXPECTED-FAIL | layout/style/test/chrome/test_stylesheet_clone_import_rule.html | Test timed out. [task 2017-08-21T21:16:47.269555Z] 21:16:47 INFO - reportError@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:121:7 [task 2017-08-21T21:16:47.269744Z] 21:16:47 INFO - TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:142:7 [task 2017-08-21T21:16:47.269918Z] 21:16:47 INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:163:5 [task 2017-08-21T21:16:47.270899Z] 21:16:47 INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:163:5 [task 2017-08-21T21:16:47.271105Z] 21:16:47 INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:163:5 [task 2017-08-21T21:16:47.271916Z] 21:16:47 INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:163:5 [task 2017-08-21T21:16:47.272653Z] 21:16:47 INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:163:5 [task 2017-08-21T21:16:47.272856Z] 21:16:47 INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:163:5 [task 2017-08-21T21:16:47.273512Z] 21:16:47 INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:163:5 [task 2017-08-21T21:16:47.274321Z] 21:16:47 INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:163:5 :jet, I see you are the triage owner for this component- is this something you can help find someone to look into this sometime in the next 2 weeks?
Flags: needinfo?(bugs)
Updated•7 years ago
|
Whiteboard: [stockwell needswork]
Comment hidden (Intermittent Failures Robot) |
This test also seems to fail with high frequency on Windows in my Stylo by default simulation: https://treeherder.mozilla.org/#/jobs?repo=try&revision=96721b0f705a9801aaa60803f3ec438e6acc4e63&filter-searchStr=windows%20chrome%20m(c3)
:emilio, maybe you have ideas about this @import test issue, or can redirect to someone?
Flags: needinfo?(emilio+bugs)
Blocks: stylo-nightly
Bug 1387933 was just fixed, which might be related to this, not sure.
Comment hidden (Intermittent Failures Robot) |
Comment 14•7 years ago
|
||
It may indeed be related to that, but I'm suspicious about the test. In particular: stylesheet.insertRule('@import url("import_useless2.css")', 0); stylesheet.insertRule('@import url("import_useless2.css")', 1); // Wait for two StyleRuleAdded events to be fired. await ContentTaskUtils.waitForEvent(document, "StyleRuleAdded", true); await ContentTaskUtils.waitForEvent(document, "StyleRuleAdded", true); What prevents one of the two events from fire before we call waitForEvent? I'm not familiar with ContentTaskUtils at all, but that looks slightly fishy to me...
Flags: needinfo?(emilio+bugs)
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Comment 17•7 years ago
|
||
(In reply to Emilio Cobos Álvarez [:emilio] from comment #14) > It may indeed be related to that, but I'm suspicious about the test. In > particular: > > stylesheet.insertRule('@import url("import_useless2.css")', 0); > stylesheet.insertRule('@import url("import_useless2.css")', 1); > > // Wait for two StyleRuleAdded events to be fired. > await ContentTaskUtils.waitForEvent(document, "StyleRuleAdded", true); > await ContentTaskUtils.waitForEvent(document, "StyleRuleAdded", true); > > What prevents one of the two events from fire before we call waitForEvent? > I'm not familiar with ContentTaskUtils at all, but that looks slightly fishy > to me... You're right. I missed that while constructing this test. I find a way to fix it while preserving the intent.
Assignee: nobody → bwerth
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8902042 -
Flags: review?(emilio)
Comment 20•7 years ago
|
||
mozreview-review |
Comment on attachment 8902042 [details] Bug 1378577: Fix await race condition which causes intermittent timeouts in test_stylesheet_clone_import_rule.html. https://reviewboard.mozilla.org/r/173448/#review178818 Nice solution! I didn't know about the `checkFn` argument, that's why I wasn't aware of any trivial solution to this (which is why I didn't post a patch in the first place). r=me, with the style issues double-checked with the style guide, or someone that writes more Firefox JS than I :) ::: layout/style/test/chrome/test_stylesheet_clone_import_rule.html:42 (Diff revision 1) > + let generateCountingListenerCallback = function(numberOfEventsToWaitFor) { > + let eventsReceived = 0; > + return function(event) { > + return (++eventsReceived >= numberOfEventsToWaitFor); > + } > + } nit: Not sure if you're supposed to put a semicolon here or not, please double-check. As someone used to rust, it looks slightly clearer to me, but not sure what the JS style guide is. ::: layout/style/test/chrome/test_stylesheet_clone_import_rule.html:43 (Diff revision 1) > + let eventsReceived = 0; > + return function(event) { > + return (++eventsReceived >= numberOfEventsToWaitFor); > + } > + } > + let gotAllOurStyleRuleAddedEvents = ContentTaskUtils.waitForEvent(document, nit: Indentation here looks somewhat off (though I'm not sure what the preferred style is for javascript).
Attachment #8902042 -
Flags: review?(emilio) → review+
Comment 21•7 years ago
|
||
mozreview-review |
Comment on attachment 8902042 [details] Bug 1378577: Fix await race condition which causes intermittent timeouts in test_stylesheet_clone_import_rule.html. https://reviewboard.mozilla.org/r/173448/#review178820 ::: layout/style/test/chrome/test_stylesheet_clone_import_rule.html:42 (Diff revision 1) > + let generateCountingListenerCallback = function(numberOfEventsToWaitFor) { > + let eventsReceived = 0; > + return function(event) { > + return (++eventsReceived >= numberOfEventsToWaitFor); > + } > + } Yes, I'd suggest a semicolon here. (We have ESLint to check JS style, but it's not enabled for the layout directory.) ::: layout/style/test/chrome/test_stylesheet_clone_import_rule.html:43 (Diff revision 1) > + let eventsReceived = 0; > + return function(event) { > + return (++eventsReceived >= numberOfEventsToWaitFor); > + } > + } > + let gotAllOurStyleRuleAddedEvents = ContentTaskUtils.waitForEvent(document, Not sure there's one style for this at the moment... This is probably okay!
Comment hidden (mozreview-request) |
Comment 23•7 years ago
|
||
Pushed by bwerth@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7f2bcb7a902d Fix await race condition which causes intermittent timeouts in test_stylesheet_clone_import_rule.html. r=emilio
Comment 24•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7f2bcb7a902d
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Updated•7 years ago
|
status-firefox56:
--- → affected
Comment 25•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/f105366cc45d
Whiteboard: [stockwell needswork] → [stockwell fixed]
Comment 26•7 years ago
|
||
It seems this isn't fixed for linux-stylo/debug builds, see https://treeherder.mozilla.org/#/jobs?repo=try&revision=fa2398342c71364ccac794d0e2a0928496a4d75c That commit should have included you patch here: https://hg.mozilla.org/try/file/fa2398342c71364ccac794d0e2a0928496a4d75c/layout/style/test/chrome/test_stylesheet_clone_import_rule.html
Comment 27•7 years ago
|
||
Should we reopen this bug or open another bug? I guess I cannot land bug 1387993 with this intermittent there, because it seems to be high frequent. I guess I need to either disable that test or wait for this intermittent to be completely fixed.
Flags: needinfo?(bwerth)
Assignee | ||
Comment 28•7 years ago
|
||
The error log is very weird. The last test-pass message we get is from http://searchfox.org/mozilla-central/rev/51b3d67a5ec1758bd2fe7d7b6e75ad6b6b5da223/layout/style/test/chrome/test_stylesheet_clone_import_rule.html#47. The test then times out before we get to the next test-pass a few lines down at http://searchfox.org/mozilla-central/rev/51b3d67a5ec1758bd2fe7d7b6e75ad6b6b5da223/layout/style/test/chrome/test_stylesheet_clone_import_rule.html#53. If the log is accurate, then that would seem to indicate that the test is hanging on retrieving the "cssRules" property off a stylesheet DOM object. I'll see if I can figure out what's going on. I'll open a new bug.
Flags: needinfo?(bwerth)
Updated•7 years ago
|
Whiteboard: [stockwell fixed] → [stockwell fixed:race]
Comment hidden (Intermittent Failures Robot) |
You need to log in
before you can comment on or make changes to this bug.
Description
•