Closed Bug 1284874 Opened 8 years ago Closed 8 years ago

Run Marionette tests for Fennec on Android 4.3 API15+ debug in TaskCluster, tier 3

Categories

(Firefox Build System :: Task Configuration, task)

task
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla51

People

(Reporter: impossibus, Assigned: impossibus)

References

Details

Attachments

(8 files)

58 bytes, text/x-review-board-request
ahal
: review+
Details
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
58 bytes, text/x-review-board-request
gbrown
: review+
Details
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
58 bytes, text/x-review-board-request
dustin
: review+
gbrown
: review+
Details
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
58 bytes, text/x-review-board-request
automatedtester
: review+
Details
58 bytes, text/x-review-board-request
kmoir
: review+
Details
Now that we have Marionette support for Fennec, let's schedule a job in TC. In addition to task configuration in yml, this requires adapting either the android_emulator_test.py or marionette.py mozharness script to the new suite.
If you start with the mozharness refactoring, you may be able to land after bug 1281004, which will save you a headache of writing yml files, and me a headache of reading, reimplementing, and deleting them :)
I'm running into automation-only problems with Marionette Test Runner launching Fennec. https://treeherder.mozilla.org/#/jobs?repo=try&revision=adab3fd4f365&selectedJob=24403689 The following command fails with 'Error: Unknown option: -p', which I guess could point to a problem with using '-profile'? /home/worker/workspace/build/android-sdk18/platform-tools/adb -s emulator-5554 shell am start -a android.activity.MAIN -n org.mozilla.fennec/org.mozilla.gecko.BrowserApp --es args "-no-remote -profile /storage/sdcard/tests/profile -marionette" --es env0 MOZ_CRASHREPORTER=1 ... However, my |mach marionette-test| implementation runs Fennec in the same way as above without issue. In that case it's using the mozemulator-4.3 AVD retrieved by ./mach android-emulator, and it's on OS X. The mochitest android jobs also launch Fennec with -profile, so my wild guess is probably wrong. Based on the log output, I'm not sure whether the error is from adb, am or Fennec itself; I've not been able to debug this interactively. Geoff, any ideas?
Flags: needinfo?(gbrown)
Your command line *looks* right but I think there must be something wrong with it. I can reproduce with a local device: gbrown@mozpad:~/src$ adb shell am start -a android.activity.MAIN -n org.mozilla.fennec_gbrown/org.mozilla.gecko.BrowserApp --es args "-no-remote -profile /mnt/sdcard/tests/profile" Error: Unknown option: -p java.lang.NullPointerException at com.android.commands.am.Am.runStart(Am.java:556) at com.android.commands.am.Am.onRun(Am.java:232) at com.android.internal.os.BaseCommand.run(BaseCommand.java:47) at com.android.commands.am.Am.main(Am.java:75) at com.android.internal.os.RuntimeInit.nativeFinishInit(Native Method) at com.android.internal.os.RuntimeInit.main(RuntimeInit.java:235) at dalvik.system.NativeStart.main(Native Method) I don't understand where '-p' is coming from. It looks like 'am' is not parsing its command line correctly (or at least the way we want it to!).
Pardon my wild guess, but does adb not properly pass on the quoting?
It is all about the quoting! Try this: adb shell 'am start ... --es args "-no-remote -profile ..."' (quotes around the argument to adb shell). That seems to work for me.
Flags: needinfo?(gbrown)
Thanks for the replies, that helped so much! In the end, adding single quotes around 'am start... ' didn't quite do the trick, but I figured it out. https://treeherder.mozilla.org/#/jobs?repo=try&revision=2aeaa53a6d44&selectedJob=24490111
When running the command for starting Fennec, quotation marks aren't processed properly when the 'am' portion of the command is represented with one string token per argument; the args must be joined into one string instead. Also add log message about command being run in BaseRunner. Review commit: https://reviewboard.mozilla.org/r/67028/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/67028/
Attachment #8774567 - Flags: review?(ahalberstadt)
Attachment #8774568 - Flags: review?(dburns)
Attachment #8774569 - Flags: review?(gbrown)
Attachment #8774570 - Flags: review?(gbrown)
Attachment #8774570 - Flags: review?(dustin)
This is useful for overriding the computed Android package name in automation. Review commit: https://reviewboard.mozilla.org/r/67030/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/67030/
Attachment #8774568 - Flags: review?(dburns) → review+
Comment on attachment 8774569 [details] Bug 1284874 - Add marionette test-suite to android_emulator_unittest.py; https://reviewboard.mozilla.org/r/67032/#review64074 ::: testing/mozharness/scripts/android_emulator_unittest.py:534 (Diff revision 1) > try_tests)) > > + if self.test_suite == 'marionette': > + manifest = os.path.join(dirs['abs_marionette_tests_dir'], > + self.config['marionette_test_manifest']) > + cmd.append(manifest) This is okay, but I think you could roll this into the configuration, in the "marionette" "options".
Attachment #8774569 - Flags: review?(gbrown) → review+
Comment on attachment 8774570 [details] Bug 1284874 - Skip irrelevant Marionette tests on Fennec; https://reviewboard.mozilla.org/r/67034/#review64078 I noticed failed tests in the try run. What's the plan for dealing with those? ::: taskcluster/ci/android-test/tests.yml:79 (Diff revision 1) > + treeherder-symbol: tc(Mn) > + instance-size: xlarge > + loopback-video: true > + e10s: false > + tier: 2 > + max-run-time: 10800 Do you need such a large max-run-time? Could you run as multiple chunks instead?
Attachment #8774570 - Flags: review?(gbrown) → review+
Comment on attachment 8774570 [details] Bug 1284874 - Skip irrelevant Marionette tests on Fennec; https://reviewboard.mozilla.org/r/67034/#review64222
Attachment #8774570 - Flags: review?(dustin) → review+
Comment on attachment 8774567 [details] Bug 1284874 - Represent am command as one string in FennecRunner; https://reviewboard.mozilla.org/r/67028/#review64828 Lgtm!
Attachment #8774567 - Flags: review?(ahalberstadt) → review+
Depends on: 1294427
No longer depends on: 1294427
New try run, now with chunks! https://treeherder.mozilla.org/#/jobs?repo=try&revision=09f1d03165175954c9504cbde0d25db8882f10d8 Note that all the tests are failing because of regression in Bug 1294427, awaiting review.
Comment on attachment 8774569 [details] Bug 1284874 - Add marionette test-suite to android_emulator_unittest.py; https://reviewboard.mozilla.org/r/67032/#review64074 > This is okay, but I think you could roll this into the configuration, in the "marionette" "options". Thanks, I've implemented your suggestion.
Comment on attachment 8774570 [details] Bug 1284874 - Skip irrelevant Marionette tests on Fennec; https://reviewboard.mozilla.org/r/67034/#review64078 We expect some tests to fail because they're not relevant to Fennec. I've included a patch to disable the ones I'm aware of. Otherwise the plan is to run these as Tier-2 until we fix/disable any others that come to our attention. > Do you need such a large max-run-time? Could you run as multiple chunks instead? Done.
Comment on attachment 8781601 [details] Bug 1284874 - Run Marionette tests for Fennec on Android 4.3 API15+ debug in TaskCluster, tier 3; https://reviewboard.mozilla.org/r/71994/#review69500 Consider adding a comment in test-sets.yml reminding folks why this is debug-only. Even though you are turning these on as tier 2, I hope they will be mostly green and stable, to minimize developer confusion on try runs, etc. ::: taskcluster/ci/android-test/test-sets.yml:24 (Diff revision 1) > - mochitest-gpu > - mochitest-media > - mochitest-webgl > - reftest > - xpcshell > + - marionette The rest of the list is in alphabetic order; I would prefer to see that order preserved.
Attachment #8781601 - Flags: review?(gbrown) → review+
Comment on attachment 8774570 [details] Bug 1284874 - Skip irrelevant Marionette tests on Fennec; https://reviewboard.mozilla.org/r/67034/#review69548
Attachment #8774570 - Flags: review?(dburns) → review+
Comment on attachment 8781601 [details] Bug 1284874 - Run Marionette tests for Fennec on Android 4.3 API15+ debug in TaskCluster, tier 3; https://reviewboard.mozilla.org/r/71994/#review69500 > The rest of the list is in alphabetic order; I would prefer to see that order preserved. Alpha order and comment re debug added.
Comment on attachment 8781601 [details] Bug 1284874 - Run Marionette tests for Fennec on Android 4.3 API15+ debug in TaskCluster, tier 3; https://reviewboard.mozilla.org/r/71994/#review70224 Minor tweaks, otherwise :thumbsup: ::: taskcluster/ci/android-test/test-sets.yml:16 (Diff revision 2) > > debug-tests: > - cppunit > - crashtest > - jsreftest > + # Marionette is only enabled on Fennec debug builds This is true for everything in `debug-tests` but not in `opt-tests`, so probably not worth being called out specially. ::: taskcluster/ci/android-test/tests.yml:74 (Diff revision 2) > - --test-suite=jsreftest > > +marionette: > + description: "Marionette unittest run" > + suite: marionette > + treeherder-symbol: tc-Mn() On desktop, this is tc(Mn), so for consistency that should probably be the choice here, too (which means you don't need the addition to GROUP_NAMES). https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/desktop-test/tests.yml#139
Attachment #8781601 - Flags: review?(dustin) → review+
Comment on attachment 8783019 [details] Bug 1284874 - Add regex for Marionette summary to TinderBoxPrintRe; https://reviewboard.mozilla.org/r/73006/#review70762
Attachment #8783019 - Flags: review?(kmoir) → review+
While disabling more irrelevant tests, I noticed that the results summary wasn't being parsed correctly by the mozharness script. Fixed now: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d613b73232b3bb6897487065cf886d5a756d4160
Comment on attachment 8783017 [details] Bug 1284874 - Add Fennec support to Marionette test skip decorators; https://reviewboard.mozilla.org/r/73002/#review70990 ::: testing/marionette/harness/marionette/marionette_test.py:92 (Diff revision 1) > return wrapper > > def skip_if_desktop(target): > def wrapper(self, *args, **kwargs): > - if self.marionette.session_capabilities.get('b2g') is None: > + browser = self.marionette.session_capabilities.get('browserName') > + if (self.marionette.session_capabilities.get('b2g') is None and Let's have this conditional just check for desktop instead of the 2 part conditional
Attachment #8783017 - Flags: review?(dburns) → review-
Comment on attachment 8783018 [details] Bug 1284874 - Update skip decorators and remove dead code in TestScreenOrientation; https://reviewboard.mozilla.org/r/73004/#review70992
Attachment #8783018 - Flags: review?(dburns) → review+
Comment on attachment 8783017 [details] Bug 1284874 - Add Fennec support to Marionette test skip decorators; https://reviewboard.mozilla.org/r/73002/#review71184
Attachment #8783017 - Flags: review?(dburns) → review+
Summary: Run Marionette tests for Fennec on Android 4.3 API15+ debug in TaskCluster, tier 2 → Run Marionette tests for Fennec on Android 4.3 API15+ debug in TaskCluster, tier 3
Pushed by mjzffr@gmail.com: https://hg.mozilla.org/integration/autoland/rev/b7e4a7997738 Represent am command as one string in FennecRunner; r=ahal https://hg.mozilla.org/integration/autoland/rev/a11299475290 Add 'package' argument to Marionette Test Runner; r=automatedtester https://hg.mozilla.org/integration/autoland/rev/ad6eee9bb1d4 Add Fennec support to Marionette test skip decorators; r=automatedtester https://hg.mozilla.org/integration/autoland/rev/06e5caf3fae3 Update skip decorators and remove dead code in TestScreenOrientation; r=automatedtester https://hg.mozilla.org/integration/autoland/rev/03d5eb3c540b Skip irrelevant Marionette tests on Fennec; r=automatedtester,dustin,gbrown https://hg.mozilla.org/integration/autoland/rev/b5f3b359d429 Add regex for Marionette summary to TinderBoxPrintRe; r=kmoir https://hg.mozilla.org/integration/autoland/rev/7d2c4f7e006d Add marionette test-suite to android_emulator_unittest.py; r=gbrown https://hg.mozilla.org/integration/autoland/rev/70e8a2680183 Run Marionette tests for Fennec on Android 4.3 API15+ debug in TaskCluster, tier 3; r=dustin,gbrown
Here's a happy try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9ecc8cb8e09d86704c69984372c4fc722af86f4a There were two issues to fix: optional config parameters broke xpcshell tests, and some marionette tests were unconditionally (and therefore unintentionally) skipped, which caused failures due to Bug 1259055. I'm going to double-check a couple more things, then land.
Flags: needinfo?(mjzffr)
Pushed by mjzffr@gmail.com: https://hg.mozilla.org/integration/autoland/rev/41045c609d55 Represent am command as one string in FennecRunner; r=ahal https://hg.mozilla.org/integration/autoland/rev/10c6cff03f92 Add 'package' argument to Marionette Test Runner; r=automatedtester https://hg.mozilla.org/integration/autoland/rev/872b8f719431 Add Fennec support to Marionette test skip decorators; r=automatedtester https://hg.mozilla.org/integration/autoland/rev/06ca7be5f7e4 Update skip decorators and remove dead code in TestScreenOrientation; r=automatedtester https://hg.mozilla.org/integration/autoland/rev/48b86c7b2779 Skip irrelevant Marionette tests on Fennec; r=automatedtester,dustin,gbrown https://hg.mozilla.org/integration/autoland/rev/8e3a7630d0e6 Add regex for Marionette summary to TinderBoxPrintRe; r=kmoir https://hg.mozilla.org/integration/autoland/rev/0a15d2d023ff Add marionette test-suite to android_emulator_unittest.py; r=gbrown https://hg.mozilla.org/integration/autoland/rev/000140429082 Run Marionette tests for Fennec on Android 4.3 API15+ debug in TaskCluster, tier 3; r=dustin,gbrown
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: