Closed Bug 1265194 Opened 9 years ago Closed 9 years ago

Refactor prompter tests

Categories

(Toolkit :: General, defect)

Product:
Toolkit
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla48
Tracking Flags:
Tracking Status
firefox48 --- fixed

People

(Reporter: Dolske, Assigned: Dolske)

References

(Depends on 1 open bug)

Details

Attachments

(4 files)

MozReview Request: Bug 1265194 - Move handlePrompt() and checkPromptState() to prompt_common.js, and rename a test. r?adw
58 bytes, text/x-review-board-request
adw
: review+
Details
MozReview Request: Bug 1265194 - use the hidden attribute in commonDialog.xul for consistency with tabPrompts.xml. r?adw
58 bytes, text/x-review-board-request
adw
: review+
Details
MozReview Request: Bug 1265194 - Refactor prompt tests to use state/action objects and new common helpers. r?adw
58 bytes, text/x-review-board-request
adw
: review+
Details
Fix test
2.91 KB, patch
Details | Diff | Splinter Review
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.
Surprisingly, my preliminary Try push was all green. https://treeherder.mozilla.org/#/jobs?repo=try&revision=26b2d4058484
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 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 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 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+
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.
Attached patch Fix test — — Splinter Review
(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
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.
Depends on: 1267092
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.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: