[build/android] Fix device.RunShellCommand usages

[build/android] Fix device.RunShellCommand usages

- Pass command as list of args or set shell=True when needed.
- Set check_return=True where appropriate.

BUG=chromium:628617
,catapult:#3242

Review-Url: https://codereview.chromium.org/2760923002
Cr-Commit-Position: refs/heads/master@{#458097}
Committed: https://chromium.googlesource.com/chromium/src/+/43cc0fba21372346c981396d719443b05e7a2a0c

[perezju, 2017-03-20 15:22:58]

perezju@chromium.org changed reviewers:
+ jbudorick@chromium.org, rnephew@chromium.org

[perezju, 2017-03-20 15:22:58]

As discussed offline with John, I'm ignoring:

build/android/provision_devices.py
build/android/pylib/content_settings.py
build/android/pylib/device_settings.py
build/android/pylib/valgrind_tools.py

I *think* that pylib/local/device/* files are not duplicated in catapult, so I
am modifying those already.

https://codereview.chromium.org/2760923002/diff/1/build/android/pylib/local/d...
File build/android/pylib/local/device/local_device_gtest_run.py (right):

https://codereview.chromium.org/2760923002/diff/1/build/android/pylib/local/d...
build/android/pylib/local/device/local_device_gtest_run.py:238: # fine from the
test runner's perspective; thus check_return=False.
Had to search through commit messages to figure out why this was set to False.
:)

https://codereview.chromium.org/2760923002/diff/1/build/android/pylib/local/d...
File build/android/pylib/local/device/local_device_instrumentation_test_run.py
(right):

https://codereview.chromium.org/2760923002/diff/1/build/android/pylib/local/d...
build/android/pylib/local/device/local_device_instrumentation_test_run.py:385:
check_return=True, shell=True)
This appears to be sort of common-ish, wondering whether we want some
"RemovePathContents".

[jbudorick, 2017-03-20 15:26:51]

On 2017/03/20 15:22:58, perezju wrote:
> As discussed offline with John, I'm ignoring:
> 
> build/android/provision_devices.py
> build/android/pylib/content_settings.py
> build/android/pylib/device_settings.py
> build/android/pylib/valgrind_tools.py
> 
> I *think* that pylib/local/device/* files are not duplicated in catapult, so I
> am modifying those already.

Correct, they are not duplicated in catapult.

> 
>
https://codereview.chromium.org/2760923002/diff/1/build/android/pylib/local/d...
> File build/android/pylib/local/device/local_device_gtest_run.py (right):
> 
>
https://codereview.chromium.org/2760923002/diff/1/build/android/pylib/local/d...
> build/android/pylib/local/device/local_device_gtest_run.py:238: # fine from
the
> test runner's perspective; thus check_return=False.
> Had to search through commit messages to figure out why this was set to False.
> :)
> 
>
https://codereview.chromium.org/2760923002/diff/1/build/android/pylib/local/d...
> File build/android/pylib/local/device/local_device_instrumentation_test_run.py
> (right):
> 
>
https://codereview.chromium.org/2760923002/diff/1/build/android/pylib/local/d...
> build/android/pylib/local/device/local_device_instrumentation_test_run.py:385:
> check_return=True, shell=True)
> This appears to be sort of common-ish, wondering whether we want some
> "RemovePathContents".

[jbudorick, 2017-03-20 15:27:02]

lgtm

https://codereview.chromium.org/2760923002/diff/1/build/android/pylib/local/d...
File build/android/pylib/local/device/local_device_gtest_run.py (right):

https://codereview.chromium.org/2760923002/diff/1/build/android/pylib/local/d...
build/android/pylib/local/device/local_device_gtest_run.py:238: # fine from the
test runner's perspective; thus check_return=False.
On 2017/03/20 15:22:58, perezju wrote:
> Had to search through commit messages to figure out why this was set to False.
> :)

woops, this is probably something I should've documented to begin with.

https://codereview.chromium.org/2760923002/diff/1/build/android/pylib/local/d...
File build/android/pylib/local/device/local_device_instrumentation_test_run.py
(right):

https://codereview.chromium.org/2760923002/diff/1/build/android/pylib/local/d...
build/android/pylib/local/device/local_device_instrumentation_test_run.py:385:
check_return=True, shell=True)
On 2017/03/20 15:22:58, perezju wrote:
> This appears to be sort of common-ish, wondering whether we want some
> "RemovePathContents".

Maybe an addition to
https://codesearch.chromium.org/chromium/src/third_party/catapult/devil/devil...
?

[perezju, 2017-03-20 15:48:31]

The CQ bit was checked by perezju@chromium.org

[commit-bot: I haz the power, 2017-03-20 15:49:06]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-20 16:55:12]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1490024911227400, "parent_rev":
"d15dc6a9d267c09c67d36838cba240d69bc68ab2", "commit_rev":
"43cc0fba21372346c981396d719443b05e7a2a0c"}

[commit-bot: I haz the power, 2017-03-20 16:56:47]

Description was changed from

==========
[build/android] Fix device.RunShellCommand usages

- Pass command as list of args or set shell=True when needed.
- Set check_return=True where appropriate.

BUG=chromium:628617,catapult:#3242
==========

to

==========
[build/android] Fix device.RunShellCommand usages

- Pass command as list of args or set shell=True when needed.
- Set check_return=True where appropriate.

BUG=chromium:628617,catapult:#3242

Review-Url: https://codereview.chromium.org/2760923002
Cr-Commit-Position: refs/heads/master@{#458097}
Committed:
https://chromium.googlesource.com/chromium/src/+/43cc0fba21372346c981396d7194...
==========

[commit-bot: I haz the power, 2017-03-20 16:56:48]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/43cc0fba21372346c981396d7194...