Closed
Bug 1263784
Opened 8 years ago
Closed 8 years ago
Fix test_modal_prompts.html and test_modal_select.html to run under E10S
Categories
(Toolkit :: General, defect)
Toolkit
General
Tracking
()
RESOLVED
FIXED
mozilla48
People
(Reporter: Dolske, Assigned: Dolske)
References
(Blocks 1 open bug)
Details
Attachments
(5 files)
Followup to bug 1252723, fix test_modal_prompts.html and test_modal_select.html to run under E10S.
Assignee | ||
Comment 1•8 years ago
|
||
Move all actions to be performed on a prompt (e.g. clicking a button, checking a checkbox) into an "action" object. Review commit: https://reviewboard.mozilla.org/r/45621/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/45621/
Attachment #8740206 -
Flags: review?(adw)
Attachment #8740207 -
Flags: review?(adw)
Attachment #8740208 -
Flags: review?(adw)
Attachment #8740209 -
Flags: review?(adw)
Attachment #8740210 -
Flags: review?(adw)
Assignee | ||
Comment 2•8 years ago
|
||
Move all expected-ui state checks into state object. Review commit: https://reviewboard.mozilla.org/r/45623/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/45623/
Assignee | ||
Comment 3•8 years ago
|
||
Make expected-state object independent from ui object. Review commit: https://reviewboard.mozilla.org/r/45625/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/45625/
Assignee | ||
Comment 4•8 years ago
|
||
Move state/action objects out of handleDialog, into main runTest() body. Review commit: https://reviewboard.mozilla.org/r/45627/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/45627/
Assignee | ||
Comment 5•8 years ago
|
||
Use chromeScript and message manager to fetch state of prompt and dismiss it. Enable tests in E10S. Review commit: https://reviewboard.mozilla.org/r/45629/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/45629/
Assignee | ||
Comment 6•8 years ago
|
||
A few comments, to hopefully make things easier/clearer: * This review looks bigger than it really is, a lot of it is just mindless moving things around. :) The basic idea is that the main test is still running in content, and right before triggering a prompt it passes an "action" object to the parent (telling it to start watching for a prompt, and how to interact with / dismiss it). The test already had a "state" object describing what it was expecting in each prompt when shown (title, button labels, etc) -- the parent now gathers a similar object describing what it sees in the actual prompt, and passes that back to the child (which compares that with what it expected). * The first 3 patches are evolving the test towards cleaner state/action usage. * The 4th patch looks huge, but should basically be a rubber-stamp review. ("Move state/action objects out of handleDialog, into main runTest() body.") It's just moving all the state/action objects. Previously each subtest was split between runTest (where the API call was setup and returned values checked) and handleDialog (where the expected state/action was checked), which was really kind of confusing. With action/state moved, handleDialog() basically became a 1-line call to checkExpectedState() with the current (global) state/action. * I'll note reviewboard's diffs look weird for that 4th patch, because it thinks state/action are staying in place and all the code around them is moving. The unified diff might be clearer to look at here, since it more clearly shows just those objects moving. * The 5th test is where the actual E10Sification happens. chromeScript.js gains functions to gather prompt state and performs the requested actions, the checkPromptState() is altered to check the returned state instead of the UI directly, the main test's startCallbackTimer() is converted to handlePrompt() which messages the parent, and returns a promise that's resolved when the parent returns the prompt state / dismisses the prompt. I'm tempted to clean up some of the existing tests to use this same general pattern... But not in this bug. :)
Updated•8 years ago
|
Blocks: e10s-tests
tracking-e10s:
--- → +
Assignee | ||
Comment 7•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=fed95327db6a
Assignee | ||
Comment 8•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=01a7eb03d01d
Assignee | ||
Comment 9•8 years ago
|
||
Oops, had a few test failures on not-OSX because I forgot to handle one thing. This simple fix should get them all green, will fold into patches post-first-review. testfix: https://hg.mozilla.org/try/rev/01a7eb03d01d
Updated•8 years ago
|
Attachment #8740206 -
Flags: review?(adw) → review+
Comment 10•8 years ago
|
||
Comment on attachment 8740206 [details] MozReview Request: Bug 1263784 - Fix test_modal_prompts.html and test_modal_select.html to run under E10S. r=adw https://reviewboard.mozilla.org/r/45621/#review43197
Comment 11•8 years ago
|
||
Comment on attachment 8740207 [details] MozReview Request: Bug 1263784 - Fix test_modal_prompts.html and test_modal_select.html to run under E10S. r=adw https://reviewboard.mozilla.org/r/45623/#review43199
Attachment #8740207 -
Flags: review?(adw) → review+
Updated•8 years ago
|
Attachment #8740208 -
Flags: review?(adw) → review+
Comment 12•8 years ago
|
||
Comment on attachment 8740208 [details] MozReview Request: Bug 1263784 - Fix test_modal_prompts.html and test_modal_select.html to run under E10S. r=adw https://reviewboard.mozilla.org/r/45625/#review43201
Comment 13•8 years ago
|
||
Comment on attachment 8740209 [details] MozReview Request: Bug 1263784 - Fix test_modal_prompts.html and test_modal_select.html to run under E10S. r=adw https://reviewboard.mozilla.org/r/45627/#review43203
Attachment #8740209 -
Flags: review?(adw) → review+
Updated•8 years ago
|
Attachment #8740210 -
Flags: review?(adw) → review+
Comment 14•8 years ago
|
||
Comment on attachment 8740210 [details] MozReview Request: Bug 1263784 - Fix test_modal_prompts.html and test_modal_select.html to run under E10S. r=adw https://reviewboard.mozilla.org/r/45629/#review43205
Assignee | ||
Comment 15•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2023a7df0d34
Assignee | ||
Comment 16•8 years ago
|
||
(In reply to Justin Dolske [:Dolske] from comment #9) > Oops, had a few test failures on not-OSX because I forgot to handle one > thing. This simple fix should get them all green, will fold into patches > post-first-review. > > testfix: https://hg.mozilla.org/try/rev/01a7eb03d01d This fixed everything, except Linux-E10S is now reporting that the <browser> is sometimes the focused element instead of the expected element in the test. I suspect this is the same issue as the existing Linux ignore-focus workaround in the test. So I filed bug 1265077 to look into this, and will land this test rewrite in the meantime.
Assignee | ||
Comment 17•8 years ago
|
||
Comment on attachment 8740206 [details] MozReview Request: Bug 1263784 - Fix test_modal_prompts.html and test_modal_select.html to run under E10S. r=adw Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45621/diff/1-2/
Attachment #8740206 -
Attachment description: MozReview Request: Bug 1263784 - Fix test_modal_prompts.html and test_modal_select.html to run under E10S. r?adw → MozReview Request: Bug 1263784 - Fix test_modal_prompts.html and test_modal_select.html to run under E10S. r=adw
Attachment #8740207 -
Attachment description: MozReview Request: Bug 1263784 - Fix test_modal_prompts.html and test_modal_select.html to run under E10S. r?adw → MozReview Request: Bug 1263784 - Fix test_modal_prompts.html and test_modal_select.html to run under E10S. r=adw
Attachment #8740208 -
Attachment description: MozReview Request: Bug 1263784 - Fix test_modal_prompts.html and test_modal_select.html to run under E10S. r?adw → MozReview Request: Bug 1263784 - Fix test_modal_prompts.html and test_modal_select.html to run under E10S. r=adw
Attachment #8740209 -
Attachment description: MozReview Request: Bug 1263784 - Fix test_modal_prompts.html and test_modal_select.html to run under E10S. r?adw → MozReview Request: Bug 1263784 - Fix test_modal_prompts.html and test_modal_select.html to run under E10S. r=adw
Attachment #8740210 -
Attachment description: MozReview Request: Bug 1263784 - Fix test_modal_prompts.html and test_modal_select.html to run under E10S. r?adw → MozReview Request: Bug 1263784 - Fix test_modal_prompts.html and test_modal_select.html to run under E10S. r=adw
Assignee | ||
Comment 18•8 years ago
|
||
Comment on attachment 8740207 [details] MozReview Request: Bug 1263784 - Fix test_modal_prompts.html and test_modal_select.html to run under E10S. r=adw Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45623/diff/1-2/
Assignee | ||
Comment 19•8 years ago
|
||
Comment on attachment 8740208 [details] MozReview Request: Bug 1263784 - Fix test_modal_prompts.html and test_modal_select.html to run under E10S. r=adw Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45625/diff/1-2/
Assignee | ||
Comment 20•8 years ago
|
||
Comment on attachment 8740209 [details] MozReview Request: Bug 1263784 - Fix test_modal_prompts.html and test_modal_select.html to run under E10S. r=adw Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45627/diff/1-2/
Assignee | ||
Comment 21•8 years ago
|
||
Comment on attachment 8740210 [details] MozReview Request: Bug 1263784 - Fix test_modal_prompts.html and test_modal_select.html to run under E10S. r=adw Review request updated; see interdiff: https://reviewboard.mozilla.org/r/45629/diff/1-2/
Comment 22•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/0fa2d35a99fa https://hg.mozilla.org/integration/mozilla-inbound/rev/f17f999971a8 https://hg.mozilla.org/integration/mozilla-inbound/rev/6d466790be9a https://hg.mozilla.org/integration/mozilla-inbound/rev/9295efcd0881 https://hg.mozilla.org/integration/mozilla-inbound/rev/c8bc5838fca3
Comment 23•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0fa2d35a99fa https://hg.mozilla.org/mozilla-central/rev/f17f999971a8 https://hg.mozilla.org/mozilla-central/rev/6d466790be9a https://hg.mozilla.org/mozilla-central/rev/9295efcd0881 https://hg.mozilla.org/mozilla-central/rev/c8bc5838fca3
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
•