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)

Version 3
defect
Not set
normal

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)?
Flags: needinfo?(gbrown)
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.
(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)
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)
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: nobody → ahalberstadt
Status: NEW → ASSIGNED
That patch seems to be working here.
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+
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
https://hg.mozilla.org/mozilla-central/rev/66de56fb04fb
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: