Closed
Bug 1253233
Opened 8 years ago
Closed 8 years ago
[e10s] Add remote browser coverage in mochitest-chrome tests for find toolbar
Categories
(Toolkit :: Find Toolbar, defect)
Toolkit
Find Toolbar
Tracking
()
People
(Reporter: mikedeboer, Assigned: mikedeboer)
References
(Blocks 1 open bug)
Details
Attachments
(5 files)
2.40 KB,
patch
|
Felipe
:
review+
|
Details | Diff | Splinter Review |
10.48 KB,
patch
|
Felipe
:
review+
|
Details | Diff | Splinter Review |
5.33 KB,
patch
|
Felipe
:
review+
|
Details | Diff | Splinter Review |
5.15 KB,
patch
|
Felipe
:
review+
|
Details | Diff | Splinter Review |
32.39 KB,
patch
|
Felipe
:
review+
|
Details | Diff | Splinter Review |
test_findbar.xul, test_findbar_events.xul, test_bug263683.xul, test_bug304188.xul and test_bug331215.xul need work to work in e10s mode as well.
Flags: qe-verify-
Flags: firefox-backlog+
Updated•8 years ago
|
Blocks: e10s-tests
Assignee | ||
Updated•8 years ago
|
Summary: [e10s] Re-enable mochitest-chrome tests for find toolbar → [e10s] Add remote browser coverage in mochitest-chrome tests for find toolbar
Assignee | ||
Comment 1•8 years ago
|
||
Up-ing the patches whilst pondering who I should ask to review them...
Assignee | ||
Comment 2•8 years ago
|
||
Assignee | ||
Comment 3•8 years ago
|
||
Assignee | ||
Comment 4•8 years ago
|
||
Assignee | ||
Comment 5•8 years ago
|
||
Assignee | ||
Comment 6•8 years ago
|
||
Comment on attachment 8729653 [details] [diff] [review] Patch 1: make preparations to allow testing of remote browser elements Felipe, I'm asking you to review all the patches here, if you don't mind! Each test conversion is done with the same pattern, so that duplication should make things appear less bulky to review... I hope!
Attachment #8729653 -
Flags: review?(felipc)
Assignee | ||
Updated•8 years ago
|
Attachment #8729655 -
Flags: review?(felipc)
Assignee | ||
Updated•8 years ago
|
Attachment #8729657 -
Flags: review?(felipc)
Assignee | ||
Updated•8 years ago
|
Attachment #8729658 -
Flags: review?(felipc)
Assignee | ||
Comment 7•8 years ago
|
||
Comment on attachment 8729659 [details] [diff] [review] Patch 5: refactor findbar_window.xul to run the test on remote browsers as well This is one badass test. The 'TODO' parts I added are meant to be filed as follow-up bugs. Especially to figure out why `testFindAgainNotFound()` is failing no a remote browser element, because that's likely a bug in the findbar code.
Attachment #8729659 -
Flags: review?(felipc)
Comment 8•8 years ago
|
||
Comment on attachment 8729655 [details] [diff] [review] Patch 2: refactor bug263683_window.xul to run the test on remote browsers as well Review of attachment 8729655 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/content/tests/chrome/bug263683_window.xul @@ +19,5 @@ > <script type="application/javascript"><![CDATA[ > + const {interfaces: Ci, classes: Cc, results: Cr, utils: Cu} = Components; > + Cu.import("resource://gre/modules/Task.jsm"); > + Cu.import("resource://testing-common/ContentTask.jsm"); > + ContentTask.setTestScope(window.opener.wrappedJSObject); I don't understand what is this for, can you explain?
Assignee | ||
Comment 9•8 years ago
|
||
Comment on attachment 8729655 [details] [diff] [review] Patch 2: refactor bug263683_window.xul to run the test on remote browsers as well Review of attachment 8729655 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/content/tests/chrome/bug263683_window.xul @@ +19,5 @@ > <script type="application/javascript"><![CDATA[ > + const {interfaces: Ci, classes: Cc, results: Cr, utils: Cu} = Components; > + Cu.import("resource://gre/modules/Task.jsm"); > + Cu.import("resource://testing-common/ContentTask.jsm"); > + ContentTask.setTestScope(window.opener.wrappedJSObject); I assume all three lines? The two imports are for convenience objects I'm using below. (Task and ContentTask, which usually make the code flow in unit tests easier to follow and reason about - as you know). `ContentTask.setTestScope` is invoked to set the scope where it can find the `ok()` and `info()` methods. This makes it possible to use the whole range of Assert.jsm assertion methods _inside_ a ContentTask. Super convenient. Does that answer your question?
Updated•8 years ago
|
Attachment #8729653 -
Flags: review?(felipc) → review+
Updated•8 years ago
|
Attachment #8729655 -
Flags: review?(felipc) → review+
Updated•8 years ago
|
Attachment #8729657 -
Flags: review?(felipc) → review+
Updated•8 years ago
|
Attachment #8729658 -
Flags: review?(felipc) → review+
Updated•8 years ago
|
Attachment #8729659 -
Flags: review?(felipc) → review+
Assignee | ||
Comment 10•8 years ago
|
||
w00t! Thanks, Felipe!
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 11•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/77f49b774291 https://hg.mozilla.org/integration/fx-team/rev/f6c028e4f604 https://hg.mozilla.org/integration/fx-team/rev/0b9ba3cfe5ce https://hg.mozilla.org/integration/fx-team/rev/a498165cef0c
Keywords: checkin-needed
Comment 13•8 years ago
|
||
backed out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=8193581&repo=fx-team
Flags: needinfo?(mdeboer)
Comment 14•8 years ago
|
||
Backout: https://hg.mozilla.org/integration/fx-team/rev/7a3f8d12197f https://hg.mozilla.org/integration/fx-team/rev/e044d0a616db https://hg.mozilla.org/integration/fx-team/rev/efe7d026ac64
Updated•8 years ago
|
Keywords: leave-open
Comment 15•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/77f49b774291 https://hg.mozilla.org/mozilla-central/rev/2aad094c7cf8
Assignee | ||
Comment 16•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2da4f4e5fd11
Assignee | ||
Comment 17•8 years ago
|
||
Well, this is fun; test_bug263683.xul permafails on OSX 10.6 opt ONLY(!?!), which caused the backout. Hrmpf. (The other two patches were part of the same patch, thus were backed out as well - understandably.)
Flags: needinfo?(mdeboer)
Assignee | ||
Comment 18•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7554aa1e66bf
Assignee | ||
Comment 19•8 years ago
|
||
(In reply to Mike de Boer [:mikedeboer] from comment #17) *part of the same push
Assignee | ||
Comment 20•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/fe2112d79fc1178e0b34f7674e61309f7de082ab Bug 1253233: refactor bug263683_window.xul to run the test on remote browsers as well. r=felipe https://hg.mozilla.org/integration/fx-team/rev/912ee4b6a03385ac8e5bd80f06cbb57cc9b594c1 Bug 1253233: refactor bug304188_window.xul to run the test on remote browsers as well. r=felipe https://hg.mozilla.org/integration/fx-team/rev/8f944086e8a3cd97549e815e7fbb463d76717e83 Bug 1253233: refactor bug331215_window.xul to run the test on remote browsers as well. r=felipe
Assignee | ||
Updated•8 years ago
|
Keywords: leave-open
Comment 21•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/fe2112d79fc1 https://hg.mozilla.org/mozilla-central/rev/912ee4b6a033 https://hg.mozilla.org/mozilla-central/rev/8f944086e8a3
Status: ASSIGNED → 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
•