Make mochitest and reftest harnesses know TestRunnerActivity is using e10s

RESOLVED FIXED in Firefox 65

Status

defect
P1
normal
RESOLVED FIXED
9 months ago
8 months ago

People

(Reporter: snorp, Assigned: gbrown)

Tracking

(Blocks 1 bug)

unspecified
mozilla65
Dependency tree / graph

Firefox Tracking Flags

(geckoview64 wontfix, firefox63 wontfix, firefox64 wontfix, firefox65 fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

TestRunnerActivity is using e10s, but the harness thinks it's not because of some Fennec stuff.
Assignee: nobody → snorp
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.
Priority: -- → P1
On second thought, I think we can handle this better -- looking into it...
Flags: needinfo?(gbrown)
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
Depends on: 1509534
Posted patch improve android e10s support (obsolete) — Splinter Review
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)
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 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+
Attachment #9025092 - Attachment is obsolete: false
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
Attachment #9025092 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/6cdb4edf3731
https://hg.mozilla.org/mozilla-central/rev/1e9f3b7f0e6d
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
status-geckoview64=wontfix
Product: Firefox for Android → GeckoView
Target Milestone: Firefox 65 → mozilla65
You need to log in before you can comment on or make changes to this bug.