Closed
Bug 1265194
Opened 8 years ago
Closed 8 years ago
Refactor prompter tests
Categories
(Toolkit :: General, defect)
Toolkit
General
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: Dolske, Assigned: Dolske)
References
(Depends on 1 open bug)
Details
Attachments
(4 files)
After the changes we made for E10S in bug 1252723 and bug 1263784, I figured I'd clean up the tests here a bit more. * Move the handlePrompt() and checkPromptState() code from test_modal_prompts.html to prompt_common.js (in preparation for using it elsewhere) * Rename test_bug861605.html to test_dom_prompts.html (for more general tests of window.alert et al). I briefly considered folding some of the other tests here into it, but figured they were specialized enough to not bother. * Move the chrome script (gChromeScript) loading into prompt_common.js, and add a automatic cleanup to destroy it. * Refactor test_dom_prompts.html to use the new prompt_common.js code, with state/action objects. * Refactor test_bug619644.html, test_bug620145.html, and test_bug625187.html -- ditto. Also, it wasn't strictly needed here, but in fixing test_bug625187.html that tabprompts.xml and commonDialog.xul (tab / window modal, respectively) hide the "info.title" string differently. I went ahead and changed commonDialog.xul to use hidden=true instead of display:none so they're consistent.
Assignee | ||
Comment 1•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/46925/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/46925/
Attachment #8742085 -
Flags: review?(adw)
Attachment #8742086 -
Flags: review?(adw)
Attachment #8742087 -
Flags: review?(adw)
Assignee | ||
Comment 2•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/46927/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/46927/
Assignee | ||
Comment 3•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/46929/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/46929/
Assignee | ||
Comment 4•8 years ago
|
||
Surprisingly, my preliminary Try push was all green. https://treeherder.mozilla.org/#/jobs?repo=try&revision=26b2d4058484
Updated•8 years ago
|
tracking-e10s:
--- → ?
Assignee | ||
Comment 5•8 years ago
|
||
There's no need to track this for E10S. The tests were already made to work in E10S, this is just a code cleanup.
tracking-e10s:
? → ---
Comment 6•8 years ago
|
||
Comment on attachment 8742085 [details] MozReview Request: Bug 1265194 - Move handlePrompt() and checkPromptState() to prompt_common.js, and rename a test. r?adw https://reviewboard.mozilla.org/r/46925/#review45207
Attachment #8742085 -
Flags: review?(adw) → review+
Comment 7•8 years ago
|
||
Comment on attachment 8742086 [details] MozReview Request: Bug 1265194 - use the hidden attribute in commonDialog.xul for consistency with tabPrompts.xml. r?adw https://reviewboard.mozilla.org/r/46927/#review45209
Attachment #8742086 -
Flags: review?(adw) → review+
Comment 8•8 years ago
|
||
Comment on attachment 8742087 [details] MozReview Request: Bug 1265194 - Refactor prompt tests to use state/action objects and new common helpers. r?adw https://reviewboard.mozilla.org/r/46929/#review45211
Attachment #8742087 -
Flags: review?(adw) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/755257b3f436 https://hg.mozilla.org/integration/mozilla-inbound/rev/b9346277145b https://hg.mozilla.org/integration/mozilla-inbound/rev/5a90b42789cd
Comment 10•8 years ago
|
||
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/de2a25e8925636c5491901f78256c779372e2719 Linux: https://treeherder.mozilla.org/logviewer.html#?job_id=26468121&repo=mozilla-inbound Android: https://treeherder.mozilla.org/logviewer.html#?job_id=26467029&repo=mozilla-inbound
Assignee | ||
Comment 11•8 years ago
|
||
Linux failure is: TEST-UNEXPECTED-FAIL | toolkit/components/prompts/test/test_bug625187.html | TypeError: target is null That's the |target| in dispatchMouseEvent(), which is just a button in an iframe... Oh, this test used to have runTest() invoked from <body onload>, so I probably need to add code to ensure the iframes have loaded. (test_bug620145.html also used to do this, but it's not loading any subresources so should still be ok.) Android failure is: TEST-UNEXPECTED-FAIL | toolkit/components/prompts/test/test_dom_prompts.html | uncaught exception - uncaught exception: Error getting pref 'prompts.tab_modal.enabled' at undefined Another dumb mistake, I thought this pref was defined everywhere but it apparently isn't.
Assignee | ||
Comment 12•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=94b456621874
Assignee | ||
Comment 13•8 years ago
|
||
(Attaching a plain new patch because I'm not sure how ReviewBoard handles that.) Obvious patch per comment 11. I wasn't sure if the try push in comment 12 did the right thing (I used mach try to run just the tests in the prompts dir), so I also did another push running the M5 suite a few times without this test failing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2b2fd03f1390
Assignee | ||
Comment 14•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/508c0bee76f7bd6a4fa3a35cd8a0edcedd4b262f Bug 1265194 - Move handlePrompt() and checkPromptState() to prompt_common.js, and rename a test. r=adw https://hg.mozilla.org/integration/fx-team/rev/80b01ead9803b251a97eed873fbb096b469a77c2 Bug 1265194 - use the hidden attribute in commonDialog.xul for consistency with tabPrompts.xml. r=adw https://hg.mozilla.org/integration/fx-team/rev/7f54642144c27146b981a6d4c632396c7cd1d6c8 Bug 1265194 - Refactor prompt tests to use state/action objects and new common helpers. r=adw https://hg.mozilla.org/integration/fx-team/rev/13cbfc180fe6ea8ba7ad99eb5da126e1b5b7fdf2 Bug 1265194 - Fix test bustage.
Assignee | ||
Comment 15•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/7e35addd4309d83144512aa27eccb15d72ec1291 Bug 1265194 - Android test is now hanging, so disable it. Bug 1267092 to look at why it and others in this dir are problematic.
Comment 16•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/508c0bee76f7 https://hg.mozilla.org/mozilla-central/rev/80b01ead9803 https://hg.mozilla.org/mozilla-central/rev/7f54642144c2 https://hg.mozilla.org/mozilla-central/rev/13cbfc180fe6 https://hg.mozilla.org/mozilla-central/rev/7e35addd4309
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•