Closed Bug 1261233 Opened 9 years ago Closed 9 years ago

Intermittent test_bug331215.xul | application timed out after 330 seconds with no output

Categories

(Toolkit :: General, defect, P3)

Product:
Toolkit
Component:
General
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla54
Tracking Flags:
Tracking Status
firefox52 --- fixed
firefox-esr52 --- fixed
firefox53 --- fixed
firefox54 --- fixed

People

(Reporter: KWierso, Assigned: mikedeboer)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

Bug 1261233 - prevent test_bug331215.xul from intermittently timing out.
59 bytes, text/x-review-board-request
Gijs
: review+
Details
Summary: Intermittent test_bug331215.xul | application timed out after 330 seconds with no output | application crashed [@ mach_msg_trap + 0xa] → Intermittent test_bug331215.xul | application timed out after 330 seconds with no output
Bulk assigning P3 to all open intermittent bugs without a priority set in Firefox components per bug 1298978.
Priority: -- → P3
Mike, this and bug 1259339 seem to be hitting again. Can you please take a look?
Flags: needinfo?(mdeboer)
See Also: → 1259339
Assignee: nobody → mdeboer
Status: NEW → ASSIGNED
Flags: needinfo?(mdeboer)
Comment on attachment 8835079 [details] Bug 1261233 - prevent test_bug331215.xul from intermittently timing out. https://reviewboard.mozilla.org/r/110796/#review112190 Looks good to me, thanks for making our tests better. Some suggestions below. :-) ::: toolkit/content/tests/chrome/bug331215_window.xul:51 (Diff revision 1) > > function* startTestWithBrowser(browserId) { > info("Starting test with browser '" + browserId + "'"); > gBrowser = document.getElementById(browserId); > gFindBar.browser = gBrowser; > - let promise = ContentTask.spawn(gBrowser, null, function* () { > + let promise = BrowserTestUtils.browserLoaded(gBrowser); I think you want gBrowser.selectedBrowser. Not sure if it actually works if you pass tabbrowser, but might as well be precise. :-) ::: toolkit/content/tests/chrome/bug331215_window.xul:66 (Diff revision 1) > .getControllerForCommand("cmd_moveTop") > .doCommand("cmd_moveTop"); > - yield enterStringIntoFindField("l"); > + yield promiseEnterStringIntoFindField("l"); > ok(gFindBar._findField.getAttribute("status") == "notfound", > "Findfield status attribute should have been 'notfound'" + > " after entering ltest"); Nit: can we fix the trailing ws here while we're here? ::: toolkit/content/tests/chrome/bug331215_window.xul:74 (Diff revision 1) > } > > - function* enterStringIntoFindField(aString) { > + function promiseEnterStringIntoFindField(aString) { > - for (let i = 0; i < aString.length; i++) { > - let event = document.createEvent("KeyboardEvent"); > - let promise = new Promise(resolve => { > + let promise = new Promise(resolve => { Nit: just use: ```js return new Promise(resolve => { ... }); ``` ::: toolkit/content/tests/chrome/bug331215_window.xul:86 (Diff revision 1) > - } > + } > - }; > + }; > - gFindBar.browser.finder.addResultListener(listener); > + gFindBar.browser.finder.addResultListener(listener); > - }); > + }); > + > + for (let i = 0; i < aString.length; i++) { Nit: trailing ws Also, you can just actually use a: ```js for (let c of aString) { ``` style loop (yeah! iterate over characters!) Also, can we use a new KeyEvent/KeyboardEvent() instead of using `createEvent` and would that allow us to just pass the string instead of the charCode? That'd be simpler. The constructor should definitely be simpler than the initKeyEvent method. :-)
Attachment #8835079 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8835079 [details] Bug 1261233 - prevent test_bug331215.xul from intermittently timing out. https://reviewboard.mozilla.org/r/110796/#review112190 > I think you want gBrowser.selectedBrowser. Not sure if it actually works if you pass tabbrowser, but might as well be precise. :-) I understand the confusement; gBrowser is used differently than in browser/ Fx code and mochitest-browser tests - here it's simply used as the holder of the current browser element. > Nit: trailing ws > > Also, you can just actually use a: > > ```js > for (let c of aString) { > ``` > style loop (yeah! iterate over characters!) > > Also, can we use a new KeyEvent/KeyboardEvent() instead of using `createEvent` and would that allow us to just pass the string instead of the charCode? That'd be simpler. The constructor should definitely be simpler than the initKeyEvent method. :-) Excellent review comments, thanks!
Pushed by mdeboer@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8f1fa60cbe6a prevent test_bug331215.xul from intermittently timing out. r=Gijs
https://hg.mozilla.org/mozilla-central/rev/8f1fa60cbe6a
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: