Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(1)

Issue 2973293002: [devil] Set permissions on install and gracefully handle errors (Closed)

Created:
4 months, 2 weeks ago by perezju
Modified:
4 months, 1 week ago
Reviewers:
jbudorick
CC:
catapult-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
catapult
Visibility:
Public.

Description

[devil] Set permissions on install and gracefully handle errors Grant required app permissions on apk Install, regardless of whether the apk was already installed or not. This caused some issues where developers manually installed apks and then test runners would fail due to the missing permissions. Also improve the code to deal with errors while setting permissions, and blacklist some more unsettable permissions. BUG=chromium:732724, chromium:739531 Review-Url: https://codereview.chromium.org/2973293002 Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/6fc5e4196940629dc5227a4bfdeb6db9879bbe3c

Patch Set 1 : original CL #

Patch Set 2 : handle pm grant errors #

Total comments: 6

Patch Set 3 : rebase #

Patch Set 4 : single adb shell call #

Total comments: 4

Patch Set 5 : nits #

Patch Set 6 : small fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+127 lines, -46 lines) Patch
M devil/devil/android/device_utils.py View 1 2 3 4 8 chunks +60 lines, -26 lines 0 comments Download
M devil/devil/android/device_utils_test.py View 1 2 3 4 5 3 chunks +67 lines, -20 lines 0 comments Download

Messages

Total messages: 20 (11 generated)
perezju
PTAL https://codereview.chromium.org/2973293002/diff/20001/devil/devil/android/device_utils.py File devil/devil/android/device_utils.py (right): https://codereview.chromium.org/2973293002/diff/20001/devil/devil/android/device_utils.py#newcode104 devil/devil/android/device_utils.py:104: 'android.permission.RESTRICTED_VR_ACCESS', These two caused failures on chrome_public_test_vr_apk and ...
4 months, 2 weeks ago (2017-07-10 12:31:30 UTC) #6
perezju
https://codereview.chromium.org/2973293002/diff/20001/devil/devil/android/device_utils.py File devil/devil/android/device_utils.py (right): https://codereview.chromium.org/2973293002/diff/20001/devil/devil/android/device_utils.py#newcode2660 devil/devil/android/device_utils.py:2660: m = re.search(r'java\.lang\.\w+Exception: .*$', exc.output) On 2017/07/10 12:31:30, perezju ...
4 months, 2 weeks ago (2017-07-10 12:33:08 UTC) #7
perezju
ping :)
4 months, 1 week ago (2017-07-14 09:13:46 UTC) #8
jbudorick
oops, was OOO M+Tu and missed this on coming back. https://codereview.chromium.org/2973293002/diff/20001/devil/devil/android/device_utils.py File devil/devil/android/device_utils.py (right): https://codereview.chromium.org/2973293002/diff/20001/devil/devil/android/device_utils.py#newcode2657 ...
4 months, 1 week ago (2017-07-14 15:45:28 UTC) #9
perezju
On 2017/07/14 15:45:28, jbudorick wrote: > It'd be nice to not increase the number of ...
4 months, 1 week ago (2017-07-17 14:14:30 UTC) #10
jbudorick
lgtm w/ nits https://codereview.chromium.org/2973293002/diff/60001/devil/devil/android/device_utils.py File devil/devil/android/device_utils.py (right): https://codereview.chromium.org/2973293002/diff/60001/devil/devil/android/device_utils.py#newcode2686 devil/devil/android/device_utils.py:2686: res = self.RunShellCommand( nit: might be ...
4 months, 1 week ago (2017-07-17 15:31:38 UTC) #11
perezju
https://codereview.chromium.org/2973293002/diff/60001/devil/devil/android/device_utils.py File devil/devil/android/device_utils.py (right): https://codereview.chromium.org/2973293002/diff/60001/devil/devil/android/device_utils.py#newcode2686 devil/devil/android/device_utils.py:2686: res = self.RunShellCommand( On 2017/07/17 15:31:38, jbudorick wrote: > ...
4 months, 1 week ago (2017-07-17 16:08:38 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2973293002/100001
4 months, 1 week ago (2017-07-18 08:48:50 UTC) #17
commit-bot: I haz the power
4 months, 1 week ago (2017-07-18 09:16:26 UTC) #20
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...

Powered by Google App Engine
This is Rietveld efc10ee0f