Allow android emulator tests to use adb devicemanager

RESOLVED FIXED

Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: gbrown, Assigned: gbrown)

Tracking

Trunk
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 5 obsolete attachments)

(Assignee)

Description

4 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.
(Assignee)

Comment 1

4 years ago
Created attachment 8551454 [details] [diff] [review]
wip - mozharness changes
(Assignee)

Comment 2

4 years ago
Created attachment 8551455 [details] [diff] [review]
wip - in-tree config update
(Assignee)

Comment 3

4 years ago
Created attachment 8551457 [details] [diff] [review]
update to in-tree config -- make --dm-trans more flexible
Attachment #8551455 - Attachment is obsolete: true
(Assignee)

Comment 4

4 years ago
Created attachment 8551876 [details] [diff] [review]
mozharness changes -- add "device_manager" to config and support in android_emulator_unittest

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.)
Attachment #8551454 - Attachment is obsolete: true
Attachment #8551876 - Flags: review?(armenzg)
(Assignee)

Comment 5

4 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!
Attachment #8551457 - Attachment description: wip - in-tree config update → update to in-tree config -- make --dm-trans more flexible
Attachment #8551457 - Flags: review?(armenzg)

Comment 6

4 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?
Attachment #8551876 - Flags: review?(armenzg) → review-

Updated

4 years ago
Attachment #8551457 - Flags: review?(armenzg)
(Assignee)

Comment 7

4 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.
(Assignee)

Comment 8

4 years ago
Created attachment 8551987 [details] [diff] [review]
in-tree config for android 4.4
Attachment #8551457 - Attachment is obsolete: true
(Assignee)

Comment 9

4 years ago
Created attachment 8551995 [details] [diff] [review]
wip - mozharness changes: add "device_manager" to config, add android 4.4 config
Attachment #8551876 - Attachment is obsolete: true

Comment 10

4 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 :)
(Assignee)

Comment 11

4 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.
Attachment #8551987 - Flags: review?(armenzg)
(Assignee)

Updated

4 years ago
Attachment #8551987 - Attachment description: wip - in-tree config for android 4.4 → in-tree config for android 4.4
(Assignee)

Comment 12

4 years ago
Created attachment 8552158 [details] [diff] [review]
mozharness changes: add "device_manager" to config, add android 4.4 config

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
Attachment #8551995 - Attachment is obsolete: true
Attachment #8552158 - Flags: review?(armenzg)

Updated

4 years ago
Attachment #8551987 - Flags: review?(armenzg) → review+

Comment 13

4 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 :)
Attachment #8552158 - Flags: review?(armenzg) → review+
(Assignee)

Comment 14

4 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.
(Assignee)

Comment 17

4 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.
(Assignee)

Comment 21

4 years ago
s/extend/update/ seems to do the trick, but I will verify (again, more carefully) on try before pushing again.
a mozharness patch has from this bug is in production
(Assignee)

Updated

4 years ago
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Keywords: leave-open
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.