Closed Bug 1133833 Opened 9 years ago Closed 9 years ago

Android 4.3 emulator tests

Categories

(Release Engineering :: General, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: gbrown, Assigned: kmoir)

References

Details

Attachments

(13 files, 5 obsolete files)

1.76 KB, patch
jlund
: review+
kmoir
: checked-in+
Details | Diff | Splinter Review
848 bytes, patch
jlund
: review+
kmoir
: checked-in+
Details | Diff | Splinter Review
27.66 KB, patch
jlund
: review+
kmoir
: checked-in+
Details | Diff | Splinter Review
6.57 KB, text/plain
Details
8.72 KB, patch
jlund
: review+
kmoir
: checked-in+
Details | Diff | Splinter Review
1.68 KB, patch
jlund
: review+
kmoir
: checked-in+
Details | Diff | Splinter Review
1.14 KB, patch
jlund
: review+
kmoir
: checked-in+
Details | Diff | Splinter Review
13.64 KB, text/plain
Details
1.04 KB, patch
kmoir
: checked-in+
Details | Diff | Splinter Review
1.85 KB, patch
jlund
: review+
kmoir
: checked-in+
Details | Diff | Splinter Review
1.08 KB, text/plain
jlund
: review+
Details
2.45 KB, patch
Callek
: review+
kmoir
: checked-in+
Details | Diff | Splinter Review
54.77 KB, text/plain
Details
The Firefox for Android team wants to run unit tests in an emulator running Android 4.4, or some other fairly new version of Android. These would be very much like our existing tests on the Android 2.3 emulator, but on a more modern version of Android. This will give us better test coverage on newer versions of Android and put us in a better position to consider reducing our reliance on Pandaboards.

Experimental work to this end is being pursued in bug 1062365 and is not complete. I don't have anything to hand-off to Release Engineering yet, but am filing this bug now to give some notice of what is coming.

The main rel-eng tasks that I anticipate are:
 - deploying the new avd to tooltool
 - deploying a new emulator/sdk, maybe to tooltool
 - scheduling a new set of tests for Android 4.4, using the apk from the "Android 4.0 API11+ opt" build, running on all trunk trees and try and riding the trains
 - trychooser update
Depends on: 1133836
Depends on: 1134236
Assignee: nobody → kmoir
Android 4.4 is troublesome; we'll likely use 4.3 instead.
Summary: Android 4.4 emulator tests → Android 4.3 emulator tests
Depends on: 1136264
Depends on: 1137513
Depends on: 1140148
Depends on: 1142075
Depends on: 1144144
All the pieces are in place now. We could start running 4.3 test suites on try, hidden and off by default now...or we can wait to finish "greening" in bug 1140148 and then go straight to trunk.
To verify the tests should run on the same class of AWS instance that the 2.3 emulator tests run on?  And the build we should use to run the tests is the Android 4.0 API11+ one?
Flags: needinfo?(gbrown)
Absolutely correct!
Flags: needinfo?(gbrown)
Depends on: 1148401
Another point of clarification. When you were testing these - did you have to run the reftests, crashtests, jsreftest etc on the more powerful AWS instance types like we do for 2.3 tests?
Flags: needinfo?(gbrown)
Yes -- I have been using the same aws instance types for 4.3 as for 2.3, and the run times seem appropriate.

In https://treeherder.mozilla.org/#/jobs?repo=try&revision=92c69392f2a2, the tests for "Android 2.3 API9 opt" are actually 4.3 (they use the API11+ binaries and run on Android 4.3, via hacks in my mozharness user repo). Most test times seem similar to what we see on 2.3.
Flags: needinfo?(gbrown)
Attached patch buildbot-configs (obsolete) — Splinter Review
Attached patch buildbot-configs (obsolete) — Splinter Review
Attachment #8584728 - Attachment is obsolete: true
Attached file bug1133833builder.diff (obsolete) —
Attached patch cloud toolsSplinter Review
Attachment #8584732 - Attachment is obsolete: true
Attached file bug1133833builder.diff
Attachment #8584733 - Attachment is obsolete: true
Attachment #8584739 - Flags: review?(jlund)
Attachment #8584746 - Flags: review?(jlund)
Attachment #8584772 - Flags: review?(jlund)
Comment on attachment 8584772 [details] [diff] [review]
bug1133833-2.patch

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

looks good overall. My biggest concern here is the substantial load this will add to try and our linux pool, especially if these tests are not green yet. Won't this add a number of tests to any commit that's -p all or android-api-11 and includes unittests? I don't think we can make tests non-try-by-default

the naming of ubuntu64_vm_armv7_mobile and ANDROID_4_3_AWS_DICT stand out at me as not obvious they signify a different weaker aws instance type. Although it appears like this is someone the pattern we had before.

I guess we will need a slave health patch too? What about trychooser? and looking at tools' production-masters.json, I wonder if we will need a patch there as I think production-masters.json will say to use the 'panda' masters for these: http://mxr.mozilla.org/build/source/tools/buildfarm/maintenance/production-masters.json#1316 not sure what issues that will cause or if we want these to be aws ec2 based masters.

I'll r+ so I don't block but I'd like to sanity check wrt possible try load

::: mozilla-tests/mobile_config.py
@@ +2361,5 @@
> +        ANDROID_4_3_C3_DICT['opt_unittest_suites'].append(suite)
> +    elif suite[0].startswith('jsreftest'):
> +        ANDROID_4_3_C3_DICT['opt_unittest_suites'].append(suite)
> +    else:
> +        ANDROID_4_3_AWS_DICT['opt_unittest_suites'].append(suite)      

nit: whitespace
Attachment #8584772 - Flags: review?(jlund) → review+
Attachment #8584739 - Flags: review?(jlund) → review+
Attachment #8584746 - Flags: review?(jlund) → review+
Thanks for the reviews Jordan, as for the insight on the impact on our Linux capacity. I'm not sure what do do wrt capacity. As you say, you can't disable by default on mobile on try (with current code). The platform we are trying to replace is testing on pandas so turning that off on try at the same time that we enable on linux emulators won't help the situation.

My understanding from bug 1140148 is that the tests are now on green on try.  I think they are already included in trychooser syntax by "Android api 11+".  As for production-masters.json, I'll write a patch. For slave health, I think this is covered by tst-linux64 and tst-emulator64.
run api-11 tests on linux32 and linux64 aws masters
Attachment #8587483 - Flags: review?(jlund)
Comment on attachment 8587483 [details] [diff] [review]
bug1133833tools.patch

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

I think this is fine. I'm a little worried about what effect this will have when we have panda-masters and aws-linux-masters being able to do the same builders.
Attachment #8587483 - Flags: review?(jlund) → review+
Attachment #8584739 - Flags: checked-in+
Attachment #8584772 - Flags: checked-in+
Attachment #8587483 - Flags: checked-in+
:gbrown, as these are enabled on different branches is the plan to disable the corresponding tests on pandas at the same time?
Flags: needinfo?(gbrown)
Yes.

I would not mind having some overlap: It might be re-assuring to see Android 4.0 and Android 4.3 running side-by-side for a short time. But there's not much benefit to that, and we want to reduce our panda usage as soon as possible, so if convenient, I think it will be fine to disable the corresponding tests on panda as soon as 4.3 is up.
Flags: needinfo?(gbrown)
Attachment #8584746 - Flags: checked-in+
patch for slave health
Attachment #8592341 - Flags: review?(jlund)
Attachment #8592341 - Flags: review?(jlund) → review+
Attachment #8592341 - Flags: checked-in+
So the jobs are running but I notice in treeherder that they don't appear correctly.

This is because treeherder is looking for "android 4\.3 armv7 api 11" (bug 1136264) and the builder names are Android 4.3 Emulator. Is "android 4\.3 armv7 api 11" is the preferred name? If so, I'll rename the builders.
Flags: needinfo?(ryanvm)
We should use the same convention we're using for 2.3/4.0.
Flags: needinfo?(ryanvm)
Which was an oh so helpful reply when we use "Android armv7 API 9..." for 2.3 and "Android 4.0 armv7 API 11+..." for 4.0...

And this is why we can't have nice things.
But either way, adding "emulator" to the name is a departure, so we should drop that. I think "Android 4.3 armv7 API 11+..." is what we want here since we're running the same build on different versions.
rename builder name to reflect treeherder name, will attach builder diff
Attachment #8592376 - Flags: review?(jlund)
Attached file bug1133833builder.diff
builder diff for renamed builders
Attachment #8592378 - Attachment is patch: false
Attached patch bug1133833slavehealth-2.patch (obsolete) — Splinter Review
slave health rename
Attachment #8592382 - Flags: review?(jlund)
Attachment #8592376 - Flags: review?(jlund) → review+
Attached patch bug1133833renamect.patch (obsolete) — Splinter Review
rename cloud tools
Attachment #8592400 - Flags: review?(jlund)
Comment on attachment 8592382 [details] [diff] [review]
bug1133833slavehealth-2.patch

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

::: js/slave_health.js
@@ +112,2 @@
>                  slavetype = "tst-emulator64-spot";
> +            } else if (pending.match(/^Android 4.3 armv7 API 11+ .+ test (?!(plain-reftest|crashtest|jsreftest))/)) {

I think we may need to escape the '+' (API 11\+) otherwise we would only match 11, 111, 1111, etc and not 11+.
Attachment #8592382 - Flags: review?(jlund) → review-
Comment on attachment 8592400 [details] [diff] [review]
bug1133833renamect.patch

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

::: configs/watch_pending.cfg
@@ +28,5 @@
>          "^Android 2.3( Armv6)? Emulator.*(?:plain-reftest|crashtest|jsreftest)": "tst-emulator64",
>          "^Android armv7 API 9.*test (?:(?!plain-reftest|crashtest|jsreftest).)*$": "tst-linux64",
>          "^Android armv7 API 9.*test .*(?:plain-reftest|crashtest|jsreftest)": "tst-emulator64",
> +        "^Android 4.3 armv7 API 11+.*test (?:(?!plain-reftest|crashtest|jsreftest).)*$": "tst-linux64",
> +        "^Android 4.3 armv7 API 11+.*test ((plain-reftest|crashtest|jsreftest))": "tst-emulator64",

unlike https://bugzilla.mozilla.org/attachment.cgi?id=8592382&action=edit this will work since we do not have a space after the 11+. However, to be more accurate, we should probably escape the '+' here too so we end up with `API 11\+.*test`
Attachment #8592400 - Flags: review?(jlund) → review+
Attachment #8592400 - Attachment is obsolete: true
Attachment #8592382 - Attachment is obsolete: true
Attachment #8592587 - Flags: review?(jlund)
I noticed a few new-ish test failures on 4.3 Opt, so updated some test manifests:

https://hg.mozilla.org/integration/mozilla-inbound/rev/51eab3556dba
Keywords: leave-open
From irc

tldr: after renaming is complete, enable opt tests on Trunk, disable corresponding 4.0 tests.

gbrown	okay. thanks. I'm thinking that once kim's job renaming is in effect, we can start running 4.3 Opt on central.
	RyanVM|sheriffduty	sgtm
	|<--	armenzg has left moznet (Quit: Leaving)
	-->|	armenzg (armenzg@moz-of8gca.home3.cgocable.net) has joined #ateam
	kmoir	gbrown: so you by run on central, do you mean run on trunk or just central?	
	gbrown	kmoir: trunk
	kmoir	k
	gbrown	kmoir: are you going to turn off the corresponding 4.0 tests at the same time, or wait a bit?
	kmoir	gbrown: I was planning on turning off corresponding 4.0 tests at the same time but I can do either	
	gbrown	ok. sgtm.
Attachment #8592587 - Flags: review?(jlund) → review+
Attachment #8592587 - Flags: checked-in+
Comment on attachment 8592376 [details] [diff] [review]
bug1133833renamebuilders.patch

was checked in a while ago
Attachment #8592376 - Flags: checked-in+
Attachment #8592586 - Flags: checked-in+
because strings, python, json, and because we can't have nice things

r+ from nthomas

https://github.com/mozilla/build-cloud-tools/commit/a01879a2914b2631c8bc80cfd9f5495af58f7876
Attachment #8593768 - Flags: review+
context: watch_pending was borked since: https://github.com/mozilla/build-cloud-tools/commit/52a5ae6919677f624d9f2a4504eeb00fdc498233

which was my bad since I suggested escaping the + in the first place :\

#### log from aws-manager2:/var/log/messages
Apr 16 22:35:03 aws-manager2.srv.releng.scl3.mozilla.com aws_watch_pending.py: Traceback (most recent call last):
Apr 16 22:35:03 aws-manager2.srv.releng.scl3.mozilla.com aws_watch_pending.py:   File "aws_watch_pending.py", line 505, in <module>
Apr 16 22:35:03 aws-manager2.srv.releng.scl3.mozilla.com aws_watch_pending.py:     main()
Apr 16 22:35:03 aws-manager2.srv.releng.scl3.mozilla.com aws_watch_pending.py:   File "aws_watch_pending.py", line 471, in main
Apr 16 22:35:03 aws-manager2.srv.releng.scl3.mozilla.com aws_watch_pending.py:     config = json.load(args.config)
Apr 16 22:35:03 aws-manager2.srv.releng.scl3.mozilla.com aws_watch_pending.py:   File "/builds/aws_manager/lib/python2.7/site-packages/simplejson/__init__.py", line 431, in load
Apr 16 22:35:03 aws-manager2.srv.releng.scl3.mozilla.com aws_watch_pending.py:     use_decimal=use_decimal, **kw)
Apr 16 22:35:03 aws-manager2.srv.releng.scl3.mozilla.com aws_watch_pending.py:   File "/builds/aws_manager/lib/python2.7/site-packages/simplejson/__init__.py", line 488, in loads
Apr 16 22:35:03 aws-manager2.srv.releng.scl3.mozilla.com aws_watch_pending.py:     return _default_decoder.decode(s)
Apr 16 22:35:03 aws-manager2.srv.releng.scl3.mozilla.com aws_watch_pending.py:   File "/builds/aws_manager/lib/python2.7/site-packages/simplejson/decoder.py", line 370, in decode
Apr 16 22:35:03 aws-manager2.srv.releng.scl3.mozilla.com aws_watch_pending.py:     obj, end = self.raw_decode(s)
Apr 16 22:35:03 aws-manager2.srv.releng.scl3.mozilla.com aws_watch_pending.py:   File "/builds/aws_manager/lib/python2.7/site-packages/simplejson/decoder.py", line 389, in raw_decode
all trees closed for this at 23:20pm Pacific for job backlog
trees reopen
enable Android 4.3 opt on trunk, disable corresponding 4.0 tests
Attachment #8594814 - Flags: review?(jlund)
builder diff
Depends on: 1154419
Attachment #8594814 - Flags: review?(jlund) → review?(bugspam.Callek)
Attachment #8594814 - Flags: review?(bugspam.Callek) → review+
Attachment #8594814 - Flags: checked-in+
Now that these are enabled on trunk, I think the only remaining work is to enable these on debug when they are green on Try and relevant changes have been uplifted. Also at the same time, to disable the corresponding debug tests on 4.0.
Android 4.0 API11+ opt unit tests are still running on Try. They should probably be turned off, or at least hidden, now that they are not running on trunk trees.
:gbrown: see bug 1159365 for disabling these tests on try
Blocks: 1186615
No longer depends on: 1148401
Depends on: 1216226
Depends on: 1213661
whee! j7 debug is resolved and we can close this bug!
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.