Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(33)

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

Created:
3 years, 5 months ago by perezju
Modified:
3 years, 5 months 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 ...
3 years, 5 months 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 ...
3 years, 5 months ago (2017-07-10 12:33:08 UTC) #7
perezju
ping :)
3 years, 5 months 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 ...
3 years, 5 months 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 ...
3 years, 5 months 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 ...
3 years, 5 months 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: > ...
3 years, 5 months 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
3 years, 5 months ago (2017-07-18 08:48:50 UTC) #17
commit-bot: I haz the power
3 years, 5 months 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 408576698