Fix force stop using wrong "ps" cmd.

Fix force stop using wrong "ps" cmd.

ps command was changed for devices with sdk level greater than
Nougat N-MR1. Fixing the usage of ps in ForceStop for these
devices.

BUG=chromium:707319
Review-Url: https://codereview.chromium.org/2815573003
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/8cea3a22656a0b60d48e530cc5f7259dd762c873

[mikecase (-- gone --), 2017-04-11 23:36:56]

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

[jbudorick, 2017-04-11 23:47:24]

https://codereview.chromium.org/2815573003/diff/20001/devil/devil/android/dev...
File devil/devil/android/device_utils.py (right):

https://codereview.chromium.org/2815573003/diff/20001/devil/devil/android/dev...
devil/devil/android/device_utils.py:1184: cmd = ('p=%s;if [[ "$(%s)" = *$p* ]];
then am force-stop $p; fi' %
I'm wondering if maintaining the hotrolled script here is worth it vs switching
to

  if self.GetPids(process_name=package):
    self.RunShellCommand(['am', 'force-stop', package], check_return=True)

[mikecase (-- gone --), 2017-04-12 00:09:32]

Briefly experimented with...

## Current way
time (adb shell '(p=chrome; if [[ "$(ps)" = *$p* ]]; then am force-stop blargs;
fi; echo $?)')
>>


## Your suggested simplification
time (adb shell '(ps -e | grep -F chrome; echo $?)' && adb shell '(am force-stop
blargs; echo $?)')


Not sure if those would really be the exact commands being run, but anyway, they
ran within like .02 seconds of eachother. Basically, no time difference at all
it seems. I think Ill go ahead and make the simplification.

[jbudorick, 2017-04-12 00:13:28]

jbudorick@chromium.org changed reviewers:
+ agrieve@chromium.org

[jbudorick, 2017-04-12 00:13:29]

+agrieve in case he has opinions about un-hotrolling this.

[agrieve, 2017-04-12 00:27:29]

On 2017/04/12 00:13:29, jbudorick wrote:
> +agrieve in case he has opinions about un-hotrolling this.

What fun. As far as what I think about "un-hotrolling", the PersistentShell
class that was added looks like a much better solution than all the script
snippets.

[perezju, 2017-04-12 09:13:20]

https://codereview.chromium.org/2815573003/diff/20001/devil/devil/android/dev...
File devil/devil/android/device_utils.py (right):

https://codereview.chromium.org/2815573003/diff/20001/devil/devil/android/dev...
devil/devil/android/device_utils.py:1184: cmd = ('p=%s;if [[ "$(%s)" = *$p* ]];
then am force-stop $p; fi' %
On 2017/04/11 23:47:24, jbudorick wrote:
> I'm wondering if maintaining the hotrolled script here is worth it vs
switching
> to
> 
>   if self.GetPids(process_name=package):
>     self.RunShellCommand(['am', 'force-stop', package], check_return=True)

+1

[mikecase (-- gone --), 2017-04-12 16:33:44]

updated to remove hand-rolled script. I didnt use PersistentShell (for now)
since all of our functions in device_utils (such as GetPids and ForceStop) don't
work with persistent shell as far as I can tell (and I didnt want to reimplement
them).

Would be nice if you were inside a PersistentShell context for us to somehow
swap out calls to RunShellCommand with pshell.RunCommand. Im not familiar with
PersistentShell at all, so not sure if what I said makes sense xD

[jbudorick, 2017-04-12 16:38:56]

lgtm

+shenghuazhang (cc) is starting to look at using PersistentShell more broadly
for performance reasons, but doing so worries me (particularly in bot
configurations). I'm not sure how well adb will tolerate multiple long-lived
shells. (alexandermont wrote it for tracing clock sync because they needed very
low latency between the host and device markers, iiuc, and that's the only
scenario in which it's used at the moment.)

[perezju, 2017-04-12 16:40:24]

lgtm

[mikecase (-- gone --), 2017-04-12 16:41:38]

The CQ bit was checked by mikecase@chromium.org

[commit-bot: I haz the power, 2017-04-12 16:41:47]

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-04-12 17:12:14]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1492015298836220,
"parent_rev": "13f9e31b07042399c8874d55c764afdbcfb3de42", "commit_rev":
"8cea3a22656a0b60d48e530cc5f7259dd762c873"}

[commit-bot: I haz the power, 2017-04-12 17:12:19]

Description was changed from

==========
Fix force stop using wrong "ps" cmd.

ps command was changed for devices with sdk level greater than
Nougat N-MR1. Fixing the usage of ps in ForceStop for these
devices.

BUG=chromium:707319
==========

to

==========
Fix force stop using wrong "ps" cmd.

ps command was changed for devices with sdk level greater than
Nougat N-MR1. Fixing the usage of ps in ForceStop for these
devices.

BUG=chromium:707319

Review-Url: https://codereview.chromium.org/2815573003
Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2017-04-12 17:12:20]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...