Closed
Bug 1507207
Opened 6 years ago
Closed 6 years ago
Make mochitest and reftest harnesses know TestRunnerActivity is using e10s
Categories
(GeckoView :: Sandboxing, defect, P1)
GeckoView
Sandboxing
Tracking
(geckoview64 wontfix, firefox63 wontfix, firefox64 wontfix, firefox65 fixed)
RESOLVED
FIXED
mozilla65
People
(Reporter: snorp, Assigned: gbrown)
References
Details
Attachments
(2 files, 2 obsolete files)
2.28 KB,
patch
|
snorp
:
review+
|
Details | Diff | Splinter Review |
13.35 KB,
patch
|
gbrown
:
review+
|
Details | Diff | Splinter Review |
TestRunnerActivity is using e10s, but the harness thinks it's not because of some Fennec stuff.
Reporter | ||
Updated•6 years ago
|
Assignee: nobody → snorp
Reporter | ||
Comment 1•6 years ago
|
||
It would be better if we launched TestRunnerActivity to use e10s or not according to the mochitest options, but this at least makes things consistent for the common case.
Reporter | ||
Updated•6 years ago
|
Blocks: geckoview_mochitest
Updated•6 years ago
|
status-firefox63:
--- → wontfix
status-firefox64:
--- → wontfix
status-firefox65:
--- → affected
Priority: -- → P1
Assignee | ||
Comment 2•6 years ago
|
||
On second thought, I think we can handle this better -- looking into it...
Flags: needinfo?(gbrown)
Assignee | ||
Comment 3•6 years ago
|
||
Good things happening here, but more to do. Hope to have a full solution soon. https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&searchStr=android&revision=bb78bf778e3a370f1152cf43ffd4029c5212a9cd
Assignee: snorp → gbrown
Flags: needinfo?(gbrown)
Summary: Make mochitest harness know TestRunnerActivity is using e10s → Make mochitest and reftest harnesses know TestRunnerActivity is using e10s
Assignee | ||
Comment 4•6 years ago
|
||
There is no rush for review here -- some time next week is fine! We generally want to run geckoview test apps in e10s mode and fennec in non-e10s mode. geckoview test apps can also be run non-e10s on request. Before this patch, we were launching the geckoview test apps in e10s mode correctly, but - treeherder names did not reflect e10s correctly - mozharness state did not reflect e10s correctly - there was no convenient way to run geckoview test apps in non-e10s mode - mozinfo.json indicated e10s=False (so mochitests marked skip-if=e10s would be run against geckoview running in e10s mode) - similarly, the reftest sandbox contained browserIsRemote=False even when running geckoview in e10s mode You can see in current Android 7.0 x86 mochitest logs: [task 2018-11-23T22:16:13.562Z] 22:16:13 INFO - runtests.py | Running with e10s: False ... [task 2018-11-23T22:16:15.005Z] 22:16:15 INFO - adb launch_application: am start -W -n org.mozilla.geckoview.test/org.mozilla.geckoview.test.TestRunnerActivity ... --ez use_multiprocess True ... and in reftest logs: [task 2018-11-23T22:18:42.635Z] 22:18:42 INFO - REFTEST INFO | Dumping JSON representation of sandbox [task 2018-11-23T22:18:42.636Z] 22:18:42 INFO - REFTEST INFO | {... "browserIsRemote":false...} This patch adds all the plumbing to connect the taskcluster e10s config to mozharness to test harnesses all the way down to the mozdevice launch_activity call. Whew! I hope I haven't broken anything: https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&revision=6f64a29d0798d3ff8038222b2d01a0db22089c16
Attachment #9025092 -
Attachment is obsolete: true
Attachment #9027290 -
Flags: review?(bob)
Assignee | ||
Comment 5•6 years ago
|
||
I think I've done everything correctly here now, but when I do, several crashtests start failing. Here are the manifest changes required to get a green run again. OK to go ahead with that?
Attachment #9027291 -
Flags: review?(snorp)
Comment 6•6 years ago
|
||
Comment on attachment 9027290 [details] [diff] [review] improve android e10s support Review of attachment 9027290 [details] [diff] [review]: ----------------------------------------------------------------- r+ with optional nits about optional parameters again. I wish we could consolidate android_{emulator,hardware}_unittest.py but that is for another day. ::: build/mobile/remoteautomation.py @@ +59,4 @@ > self.lastTestSeen = "remoteautomation.py" > self.launchApp([cmd] + args, > self.environment(env, crashreporter=not debuggerInfo), > + e10s, **self.processArgs) ditto optional params env=env / e10s=e10s ? @@ +289,4 @@ > args = args[:-1] > if 'geckoview' in self.appName: > activity = "TestRunnerActivity" > + self.device.launch_activity(self.appName, activity, e10s=e10s, moz_env=env, activity_name=activity ::: layout/tools/reftest/mach_commands.py @@ +142,4 @@ > if not args.adb_path: > args.adb_path = get_adb_path(self) > > + if 'geckoview' not in args.app: Should we tell the developer that we are not running under e10s so they aren't misled by their command line options? ::: layout/tools/reftest/reftestcommandline.py @@ -486,5 @@ > options.httpdPath = os.path.join(options.utilityPath, "components") > > - # Disable e10s by default on Android because we don't run Android > - # e10s jobs anywhere yet. > - options.e10s = False I guess we didn't warn them before but... ::: testing/mochitest/mach_commands.py @@ +211,4 @@ > manifest.tests.extend(tests) > options.manifestFile = manifest > > + options.e10s = False Perhaps log that we aren't running robocop under e10s for geckoview?
Attachment #9027290 -
Flags: review?(bob) → review+
Assignee | ||
Comment 7•6 years ago
|
||
Updated for review comments - thanks!! https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&revision=d260560ef43bd8fa2cdc775537fa2b7f3f72d979
Attachment #9027290 -
Attachment is obsolete: true
Attachment #9027709 -
Flags: review+
Updated•6 years ago
|
Attachment #9025092 -
Attachment is obsolete: false
Reporter | ||
Updated•6 years ago
|
Attachment #9027291 -
Flags: review?(snorp) → review+
Pushed by gbrown@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/6cdb4edf3731 Improve support for e10s mode testing on Android; r=bc https://hg.mozilla.org/integration/mozilla-inbound/rev/1e9f3b7f0e6d Skip some tests on Android/e10s; r=snorp
Updated•6 years ago
|
Attachment #9025092 -
Attachment is obsolete: true
Comment 9•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6cdb4edf3731 https://hg.mozilla.org/mozilla-central/rev/1e9f3b7f0e6d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
Updated•5 years ago
|
Product: Firefox for Android → GeckoView
Updated•5 years ago
|
Target Milestone: Firefox 65 → mozilla65
Comment 11•2 years ago
|
||
Moving some e10s bugs to the new GeckoView::Sandboxing component.
Component: General → Sandboxing
You need to log in
before you can comment on or make changes to this bug.
Description
•