Do not reboot emulator.

Do not reboot emulator.

BUG=261705
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=212764

[navabi, 2013-07-19 19:08:25]

On 2013/07/18 23:33:43, navabi wrote:

Ping.

[frankf, 2013-07-19 19:20:31]

https://codereview.chromium.org/19774008/diff/1/build/android/buildbot/bb_dev...
File build/android/buildbot/bb_device_steps.py (right):

https://codereview.chromium.org/19774008/diff/1/build/android/buildbot/bb_dev...
build/android/buildbot/bb_device_steps.py:87: all_devices =
android_commands.GetAttachedDevices()
Instead of adding all this extra logic here, combine Get{Emulator,
AttachedDevices, AttachedOfflineDevices) into one method with default
parameters:

GetAttachedDevices(hardware=True, emulator=True, offline=False)

[Isaac (away), 2013-07-19 21:31:29]

https://chromiumcodereview.appspot.com/19774008/diff/1/build/android/buildbot...
File build/android/buildbot/bb_device_steps.py (right):

https://chromiumcodereview.appspot.com/19774008/diff/1/build/android/buildbot...
build/android/buildbot/bb_device_steps.py:87: all_devices =
android_commands.GetAttachedDevices()
On 2013/07/19 19:20:31, frankf wrote:
> Instead of adding all this extra logic here, combine Get{Emulator,
> AttachedDevices, AttachedOfflineDevices) into one method with default
> parameters:
> 
> GetAttachedDevices(hardware=True, emulator=True, offline=False)

Yeah I agree w/ frank.  Let's push this logic into pylib.

[navabi, 2013-07-19 23:03:44]

https://codereview.chromium.org/19774008/diff/1/build/android/buildbot/bb_dev...
File build/android/buildbot/bb_device_steps.py (right):

https://codereview.chromium.org/19774008/diff/1/build/android/buildbot/bb_dev...
build/android/buildbot/bb_device_steps.py:87: all_devices =
android_commands.GetAttachedDevices()
On 2013/07/19 19:20:31, frankf wrote:
> Instead of adding all this extra logic here, combine Get{Emulator,
> AttachedDevices, AttachedOfflineDevices) into one method with default
> parameters:
> 
> GetAttachedDevices(hardware=True, emulator=True, offline=False)

Done. Good idea.

[frankf, 2013-07-19 23:19:20]

https://codereview.chromium.org/19774008/diff/8001/build/android/buildbot/bb_...
File build/android/buildbot/bb_device_steps.py (right):

https://codereview.chromium.org/19774008/diff/8001/build/android/buildbot/bb_...
build/android/buildbot/bb_device_steps.py:88: offline=False)
Don't need to specify default parameteres:
android_commands.GetAttachedDevices(emulator=False)

https://codereview.chromium.org/19774008/diff/8001/build/android/pylib/androi...
File build/android/pylib/android_commands.py (left):

https://codereview.chromium.org/19774008/diff/8001/build/android/pylib/androi...
build/android/pylib/android_commands.py:117: def GetAttachedOfflineDevices():
You need update device_status_check

https://codereview.chromium.org/19774008/diff/8001/build/android/pylib/androi...
File build/android/pylib/android_commands.py (right):

https://codereview.chromium.org/19774008/diff/8001/build/android/pylib/androi...
build/android/pylib/android_commands.py:80: the returned list. The arguments
specify what devices to include in the list.
Add Args section, describing each argument.

https://codereview.chromium.org/19774008/diff/8001/build/android/pylib/androi...
build/android/pylib/android_commands.py:95: re_device =
re.compile('^emulator-[0-9]+', re.MULTILINE)
This includes both online and offline emulators right? I think you only want
online.

[navabi, 2013-07-20 01:02:57]

https://codereview.chromium.org/19774008/diff/8001/build/android/buildbot/bb_...
File build/android/buildbot/bb_device_steps.py (right):

https://codereview.chromium.org/19774008/diff/8001/build/android/buildbot/bb_...
build/android/buildbot/bb_device_steps.py:88: offline=False)
On 2013/07/19 23:19:20, frankf wrote:
> Don't need to specify default parameteres:
> android_commands.GetAttachedDevices(emulator=False)

Done.

https://codereview.chromium.org/19774008/diff/8001/build/android/pylib/androi...
File build/android/pylib/android_commands.py (left):

https://codereview.chromium.org/19774008/diff/8001/build/android/pylib/androi...
build/android/pylib/android_commands.py:117: def GetAttachedOfflineDevices():
On 2013/07/19 23:19:20, frankf wrote:
> You need update device_status_check

device_status_check moved to buildbot/bb_device_status_check and I did not find
any calls to GetAttachedOfflineDevices. Let me know where if I missed a call to
this.

https://codereview.chromium.org/19774008/diff/8001/build/android/pylib/androi...
File build/android/pylib/android_commands.py (right):

https://codereview.chromium.org/19774008/diff/8001/build/android/pylib/androi...
build/android/pylib/android_commands.py:80: the returned list. The arguments
specify what devices to include in the list.
On 2013/07/19 23:19:20, frankf wrote:
> Add Args section, describing each argument.

Done.

https://codereview.chromium.org/19774008/diff/8001/build/android/pylib/androi...
build/android/pylib/android_commands.py:95: re_device =
re.compile('^emulator-[0-9]+', re.MULTILINE)
On 2013/07/19 23:19:20, frankf wrote:
> This includes both online and offline emulators right? I think you only want
> online.

Good catch.

[frankf, 2013-07-20 01:20:36]

lgtm with nits.

https://codereview.chromium.org/19774008/diff/4003/build/android/pylib/androi...
File build/android/pylib/android_commands.py (right):

https://codereview.chromium.org/19774008/diff/4003/build/android/pylib/androi...
build/android/pylib/android_commands.py:91: hardware: include attached actual
devices that are online
Capital first letter after ':', end with '.'.

https://codereview.chromium.org/19774008/diff/4003/build/android/pylib/androi...
build/android/pylib/android_commands.py:120: devices = devices + offline_devices
As discussed device_status_check will call this
(https://chromiumcodereview.appspot.com/19284009/), but wants only offline
devices.

https://codereview.chromium.org/19774008/diff/4003/build/android/pylib/utils/...
File build/android/pylib/utils/emulator.py (right):

https://codereview.chromium.org/19774008/diff/4003/build/android/pylib/utils/...
build/android/pylib/utils/emulator.py:50: emulators =
android_commands.GetAttachedDevices(hardware=False, emulator=True)
Just hardware=False is sufficient

[Isaac (away), 2013-07-20 01:24:55]

bb lgtm

[navabi, 2013-07-20 01:26:37]

https://codereview.chromium.org/19774008/diff/4003/build/android/pylib/androi...
File build/android/pylib/android_commands.py (right):

https://codereview.chromium.org/19774008/diff/4003/build/android/pylib/androi...
build/android/pylib/android_commands.py:91: hardware: include attached actual
devices that are online
On 2013/07/20 01:20:36, frankf wrote:
> Capital first letter after ':', end with '.'.

Done.

https://codereview.chromium.org/19774008/diff/4003/build/android/pylib/androi...
build/android/pylib/android_commands.py:120: devices = devices + offline_devices
On 2013/07/20 01:20:36, frankf wrote:
> As discussed device_status_check will call this
> (https://chromiumcodereview.appspot.com/19284009/), but wants only offline
> devices.

That's right. And given hardware=False, emulator=False, offline=True, it will
only get offline devices. The three if/else blocks above will all fail, and thus
devices=[] and then add offline devices.

https://codereview.chromium.org/19774008/diff/4003/build/android/pylib/utils/...
File build/android/pylib/utils/emulator.py (right):

https://codereview.chromium.org/19774008/diff/4003/build/android/pylib/utils/...
build/android/pylib/utils/emulator.py:50: emulators =
android_commands.GetAttachedDevices(hardware=False, emulator=True)
On 2013/07/20 01:20:36, frankf wrote:
> Just hardware=False is sufficient

Done.

[commit-bot: I haz the power, 2013-07-20 01:31:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/navabi@google.com/19774008/26001

[commit-bot: I haz the power, 2013-07-20 06:18:13]

Change committed as 212764