Closed
Bug 1396182
Opened 7 years ago
Closed 7 years ago
Intermittent devtools/client/styleeditor/test/browser_styleeditor_opentab.js | The menu item is disabled - Got false, expected true
Categories
(DevTools :: Style Editor, defect, P2)
DevTools
Style Editor
Tracking
(firefox57 fixed)
RESOLVED
FIXED
Firefox 57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: intermittent-bug-filer, Assigned: pbro)
References
(Blocks 1 open bug)
Details
(Keywords: intermittent-failure, Whiteboard: [stockwell fixed:other])
Attachments
(1 file)
Filed by: philringnalda [at] gmail.com https://treeherder.mozilla.org/logviewer.html#?job_id=127890719&repo=autoland https://queue.taskcluster.net/v1/task/NPuk063wT_W8zLOKBH3nbw/runs/0/artifacts/public/logs/live_backing.log
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Comment 5•7 years ago
|
||
This started 9 days ago. I have a hunch this started with bug 1379099, because in that bug we added an animation in the sidebar, where the context menu is supposed to be displayed. So maybe this introduced a timing issue when the test tries to show the menu.
Priority: P5 → P2
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 8•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c7ca00783d64f929b0c5354a714bd1af064e256&group_state=expanded
Comment hidden (Intermittent Failures Robot) |
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8906517 [details] Bug 1396182 - Wait for the styleditor animations to be done before running tests; https://reviewboard.mozilla.org/r/178268/#review183848 ::: devtools/client/styleeditor/test/head.js:91 (Diff revision 2) > + // animation is done. So we listen to it here so tests don't have to and can't miss it. > + let onMediaListChanged = ui.once("media-list-changed"); > + > + // The stylesheet list appears with an animation. Let this animation finish. > + let animations = ui._root.getAnimations({subtree: true}); > + yield Promise.all(animations.map(a => a.finished)); Isn't that related to this async code: http://searchfox.org/mozilla-central/source/devtools/client/styleeditor/StyleSheetEditor.jsm#129-130 this.cssSheet.getMediaRules().then(this._onMediaRulesChanged, console.error); Then, shouldn't we instead wait for it to finish by introducing StyleSheetEditor.init method returning a promise and we would wait for it to finish from here: http://searchfox.org/mozilla-central/source/devtools/client/styleeditor/StyleEditorUI.jsm#392 Like we do for fetchSource(). This would allow tracking this on DAMP, and also benefit to any code relying on showToolbox("styleditor").
Attachment #8906517 -
Flags: review?(poirot.alex) → review+
Comment 11•7 years ago
|
||
r+ as it looks like a reasonable fix just for test, but I would really like us fix showToolbox/DAMP and really reflect what users see. These animations are definitely part of the load time and should be tracked for perf regression.
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Comment 14•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8906517 [details] Bug 1396182 - Wait for the styleditor animations to be done before running tests; https://reviewboard.mozilla.org/r/178268/#review183848 > Isn't that related to this async code: > http://searchfox.org/mozilla-central/source/devtools/client/styleeditor/StyleSheetEditor.jsm#129-130 > this.cssSheet.getMediaRules().then(this._onMediaRulesChanged, console.error); > > Then, shouldn't we instead wait for it to finish by introducing StyleSheetEditor.init method returning a promise and we would wait for it to finish from here: > http://searchfox.org/mozilla-central/source/devtools/client/styleeditor/StyleEditorUI.jsm#392 > Like we do for fetchSource(). > > This would allow tracking this on DAMP, and also benefit to any code relying on showToolbox("styleditor"). True. Let's land this test fix as the intermittent is quite frequent. I'll file the next bug to take care of showToolbox being more realistic.
Comment 15•7 years ago
|
||
Pushed by pbrosset@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2b4360780665 Wait for the styleditor animations to be done before running tests; r=ochameau
Updated•7 years ago
|
Whiteboard: [stockwell fixed:other]
Comment 16•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2b4360780665
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•