Closed
Bug 1292660
Opened 8 years ago
Closed 8 years ago
Running |mach mochitest| on android busted because kwargs['app'] is None
Categories
(Testing :: Mochitest, defect)
Tracking
(firefox51 fixed)
RESOLVED
FIXED
mozilla51
Tracking | Status | |
---|---|---|
firefox51 | --- | fixed |
People
(Reporter: ahal, Assigned: ahal)
References
Details
Attachments
(1 file)
This is a regression from bug 1288827. Over there I moved the options.app resolving logic a little later on to the 'run_test_harness' method. But there is a spot in the mochitest android mach command that requires it before then: https://dxr.mozilla.org/mozilla-central/source/testing/mochitest/mach_commands.py#480 I suspect we need to move grant_runtime_permissions into the harness proper. Geoff, do you know why we call that function in the mach command but not in the test harness (i.e in continuous integration)?
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(gbrown)
Assignee | ||
Comment 1•8 years ago
|
||
Looks like we also might not *need* to pass in kwargs['app'] to grant_runtime_permissions. We can probably also get that out of build_obj.substs. I'll need to do a fennec build to test it.
Comment 2•8 years ago
|
||
(In reply to Andrew Halberstadt [:ahal] from comment #0) > I suspect we need to move grant_runtime_permissions into the harness proper. > Geoff, do you know why we call that function in the mach command but not in > the test harness (i.e in continuous integration)? mozharness jobs (and autophone too) use "install -g" to allow all permissions: https://dxr.mozilla.org/mozilla-central/rev/0ba72e8027cfcbcbf3426770ac264a7ade2af090/testing/mozharness/scripts/android_emulator_unittest.py#385. I think that is a generally safer approach for handling runtime permissions. However, mobile developers do not want permissions granted silently at install time so we grant specific permissions via grant_runtime_permissions when running tests via mach. It would be great if we could use build_obj.substs instead of the 'app' option.
Flags: needinfo?(gbrown)
Assignee | ||
Comment 3•8 years ago
|
||
This fixes a regression from bug 1288827. It happened because I moved the logic that finds the application path a little later on in the test harness. But there was an instance where it was being used in the android mach command before that point. As it turned out, we don't really *need* that value there. This patch grabs the same value from build_obj.substs which is already an argument to the function. Review commit: https://reviewboard.mozilla.org/r/69748/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/69748/
Attachment #8778446 -
Flags: review?(gbrown)
Assignee | ||
Comment 4•8 years ago
|
||
I *think* this should work. But I'm having problems getting my fennec build working, so haven't been able to test it out. George, Geoff, I'll try some more to get my build going, but in the meantime if either of you want to test this out, we could probably land the fix a bit sooner.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Comment 5•8 years ago
|
||
That patch seems to be working here.
Comment 6•8 years ago
|
||
Comment on attachment 8778446 [details] Bug 1292660 - Fix |mach mochitest| kwargs['app'] is None error on android, https://reviewboard.mozilla.org/r/69748/#review66846 Verified this works for me too (Android 6.0 emulator, robocop).
Attachment #8778446 -
Flags: review?(gbrown) → review+
Comment 7•8 years ago
|
||
Oh, actually, there's another grant_runtime_permissions: https://dxr.mozilla.org/mozilla-central/rev/0ba72e8027cfcbcbf3426770ac264a7ade2af090/layout/tools/reftest/mach_commands.py#254 Better include that too!
Comment hidden (mozreview-request) |
Pushed by ahalberstadt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/66de56fb04fb Fix |mach mochitest| kwargs['app'] is None error on android, r=gbrown
Comment 10•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/66de56fb04fb
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in
before you can comment on or make changes to this bug.
Description
•