Geoff Brown [:gbrown] Assignee
	Description • 9 years ago

We currently use sut devicemanager for all android emulator unit tests (Android 2.3 and Android x86), and android_emulator_unittest.py assumes that sutagent is installed and running on the emulator.

In bug 1062365, we might want to use adb instead of sut for emulators running Android 4.4 and higher.

I think we should at least have the option: use either sut or adb for tests started by android_emulator_unittest.py.

	Geoff Brown [:gbrown] Assignee
	Comment 4 • 9 years ago

This adds a "device_manager" parameter to the android emulator configuration. android_emulator_unittest.py is updated to:
 - not redirect SUT when adb is configured
 - not verify SUT connection when adb is configured
 - not pass --deviceIP and --devicePort in-tree options when adb is configured
Existing configuration files for Android 2.3 and Android x86 are configured for sut, so no change is expected to existing tests.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9bf7b007068b verifies this does no harm to existing tests. (The gl1 retry is an existing condition currently seen on mozilla-central.)

	Geoff Brown [:gbrown] Assignee
	Comment 5 • 9 years ago

Comment on attachment 8551457 [details] [diff] [review]
update to in-tree config -- make --dm-trans more flexible

This updates the in-tree configuration to allow the --dm_trans option to pick up the devicemanager configured in mozharness.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6dd050cbc39d confirms that existing tests keep running fine and using sut. 

The 2nd mochitest-1 run, http://ftp.mozilla.org/pub/mozilla.org/mobile/try-builds/gbrown@mozilla.com-6dd050cbc39d/try-android-api-9/try_ubuntu64_vm_mobile_test-mochitest-1-bm115-tests1-linux64-build165.txt.gz, uses adb -- and still works!

	Armen [:armenzg]
	Comment 6 • 9 years ago

Comment on attachment 8551876 [details] [diff] [review]
mozharness changes -- add "device_manager" to config and support in android_emulator_unittest

Review of attachment 8551876 [details] [diff] [review]:
-----------------------------------------------------------------

r- for the modification of options on the fly.
I believe we can fix this by specifying an in-tree config for Android 4.4 jobs where we specify adb and we don't specify --deviceIP and --devicePort.

::: scripts/android_emulator_unittest.py
@@ +430,5 @@
>          for option in self.tree_config["suite_definitions"][suite_category]["options"]:
> +            # remove --deviceIP and --devicePort if using adb devicemanager
> +            if not (c['device_manager'] == 'adb' and
> +                    (option.startswith('--deviceIP') or option.startswith('--devicePort'))):
> +                cmd.extend([option % str_format_values])

I'm not fond of modifying options on the fly.

I would like us to isolate Android 2.3 jobs from Android 4.4 jobs definitions.
I think we should add an in-tree config for Android 4.4 jobs where we specify to use adb.

Would this work for you?

	Geoff Brown [:gbrown] Assignee
	Comment 7 • 9 years ago

I agree that modifying options on the fly is bad.

I agree that we need to add an in-tree config for Android 4.4.

I would like to be able to change a job from sut to adb or vice versa -- and that is complicated because --deviceIP and --devicePort are tied to --dm_trans, which is tied to the sut validation behavior of android_emulator_unittest.py. We could support "--deviceIP=%(device_ip)s" but I think we would also need to come up with a way to specify, "if %(device_ip)s is null, do not add --deviceIP", which is kind-of an advanced form of modifying options on the fly!

I'll try adding in-tree config for Android 4.4 for now and see how that goes.

	Armen [:armenzg]
	Comment 10 • 9 years ago

(In reply to Geoff Brown [:gbrown] from comment #7)
> I would like to be able to change a job from sut to adb or vice versa -- and
> that is complicated because --deviceIP and --devicePort are tied to
> --dm_trans, which is tied to the sut validation behavior of
> android_emulator_unittest.py. We could support "--deviceIP=%(device_ip)s"
> but I think we would also need to come up with a way to specify, "if
> %(device_ip)s is null, do not add --deviceIP", which is kind-of an advanced
> form of modifying options on the fly!
> 
The joy of programming :)

	Geoff Brown [:gbrown] Assignee
	Comment 11 • 9 years ago

Comment on attachment 8551987 [details] [diff] [review]
in-tree config for android 4.4

Review of attachment 8551987 [details] [diff] [review]:
-----------------------------------------------------------------

Here's an in-tree config for 4.4. This is a copy of android_arm_config.py (the Android 2.3 config) with --dm_trans="adb" and --deviceIP and --devicePort arguments removed.

	Geoff Brown [:gbrown] Assignee
	Comment 12 • 9 years ago

Add a 4.4 mozharness config to reference the 4.4 in-tree config.
Add "device_manager" to android configs.
Avoid sut redirection and other sut references when adb configured.

No harm done: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e9e08efa8157

	Armen [:armenzg]
	Comment 13 • 9 years ago

Comment on attachment 8552158 [details] [diff] [review]
mozharness changes: add "device_manager" to config, add android 4.4 config

Review of attachment 8552158 [details] [diff] [review]:
-----------------------------------------------------------------

::: configs/android/androidarm_4_4.py
@@ +11,5 @@
> +    "emulator_process_name": "emulator64-arm",
> +    "emulator_cpu": "cortex-a9",
> +    "device_manager": "adb",
> +    "exes": {
> +        'adb': '/tools/android-sdk18/platform-tools/adb',

I assume this (and similar below) will eventually change.

::: scripts/android_emulator_unittest.py
@@ +428,5 @@
> +        if self.config["device_manager"] == "sut":
> +            str_format_values.extend({
> +                'device_ip': c['device_ip'],
> +                'device_port': str(emulator['sut_port1']),
> +            })

It works for me :)

	Geoff Brown [:gbrown] Assignee
	Comment 14 • 9 years ago

(In reply to Armen Zambrano - Automation & Tools Engineer (:armenzg) from comment #13)
> I assume this (and similar below) will eventually change.

Yes - I'll modify the 4.4 config incrementally in other bugs.

	Geoff Brown [:gbrown] Assignee
	Comment 15 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/447467c0b2c4

	Geoff Brown [:gbrown] Assignee
	Comment 16 • 9 years ago

https://hg.mozilla.org/build/mozharness/rev/313d91e863f5

	Geoff Brown [:gbrown] Assignee
	Comment 17 • 9 years ago

Here is one more incremental test showing Android 2.3 running with adb instead of sut: https://treeherder.mozilla.org/#/jobs?repo=try&revision=470476640967

Most tests run fine, but there are a few failures that look new and may well be related to the devicemanager change. I won't investigate these failures since we have no plans to run this configuration, but these results might be interesting once 4.4 is stood up.

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 18 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/447467c0b2c4

	Chris Cooper [:coop] (he/him)
	Comment 19 • 9 years ago

In production: https://hg.mozilla.org/build/mozharness/rev/313d91e863f5

	Chris Cooper [:coop] (he/him)
	Comment 20 • 9 years ago

(In reply to Chris Cooper [:coop] from comment #19)
> In production: https://hg.mozilla.org/build/mozharness/rev/313d91e863f5

Backed out for failures like this:

https://treeherder.mozilla.org/logviewer.html#?job_id=923324&repo=mozilla-central

	Geoff Brown [:gbrown] Assignee
	Comment 21 • 9 years ago

s/extend/update/ seems to do the trick, but I will verify (again, more carefully) on try before pushing again.

	Geoff Brown [:gbrown] Assignee
	Comment 22 • 9 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=262e7fcf08f3 looks solid.

https://hg.mozilla.org/build/mozharness/rev/cd253454cf3c

	Armen [:armenzg]
	Comment 23 • 9 years ago

https://hg.mozilla.org/build/ash-mozharness/rev/6bd3653426f8

	Jordan Lund (:jlund)
	Comment 24 • 9 years ago

a mozharness patch has from this bug is in production