Closed Bug 1507207 Opened 3 years ago Closed 3 years ago

Make mochitest and reftest harnesses know TestRunnerActivity is using e10s


(GeckoView :: General, defect, P1)



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

Tracking Status
geckoview64 --- wontfix
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- fixed


(Reporter: snorp, Assigned: gbrown)




(2 files, 2 obsolete files)

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.
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.
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
Attached 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 - | 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:
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} but that is for another day.

::: build/mobile/
@@ +59,4 @@
>          self.lastTestSeen = ""
>          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,


::: layout/tools/reftest/
@@ +142,4 @@
>          if not args.adb_path:
>              args.adb_path = get_adb_path(self)
> +        if 'geckoview' not in

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/
@@ -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/
@@ +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
Improve support for e10s mode testing on Android; r=bc
Skip some tests on Android/e10s; r=snorp
Attachment #9025092 - Attachment is obsolete: true
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
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.