Only consider Android devices with more than 30% battery remaining for layout tests

Only consider Android devices with more than 30% battery remaining for layout tests

Running layout tests drains devices faster than it can charge them. Devices with
less than 30% battery remaining should be ignored when running layout tests in
order to make sure that they don't die, which requires a manual restart.

BUG=232044
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=153106

[Peter Beverloo, 2013-06-21 15:22:51]

Please take a look.

This also fixes the issue where we re-install ContentShell on devices after a
test fails, because we need to store the _has_setup() status per device instead
of per driver.

[DaleCurtis, 2013-06-21 18:06:23]

https://codereview.chromium.org/16917006/diff/2001/Tools/Scripts/webkitpy/lay...
File Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py (right):

https://codereview.chromium.org/16917006/diff/2001/Tools/Scripts/webkitpy/lay...
Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:304: # How many
percentages of battery power does a device need to have left in order
Rephrase as a statement.  Something like "The minimum battery level required to
run layout tests."

https://codereview.chromium.org/16917006/diff/2001/Tools/Scripts/webkitpy/lay...
Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:308: # A list to
keep track of which devices have been prepared.
Instead of a global variable w/ static accessors you could have
ChromiumAndroidPort pass in AndroidDevices to ChromiumAndroidDriver like you do
with AndroidCommands.  If globals are common style here, feel to ignore me.

https://codereview.chromium.org/16917006/diff/2001/Tools/Scripts/webkitpy/lay...
Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:320:
self._usable_devices = [AndroidCommands(executive, self._default_device)]
Just set this in __init__() ? You don't need self._default_device then.

https://codereview.chromium.org/16917006/diff/2001/Tools/Scripts/webkitpy/lay...
Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:323: re_device =
re.compile('^([a-zA-Z0-9_:.-]+)\tdevice$', re.MULTILINE)
Providing an example in comment is good practice when writing a regex.

https://codereview.chromium.org/16917006/diff/2001/Tools/Scripts/webkitpy/lay...
Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:353: return
device_index in AndroidDevices._prepared_devices
device_index is an integer above, but you're calling this method with a serial. 
maybe device_serial instead?

https://codereview.chromium.org/16917006/diff/2001/Tools/Scripts/webkitpy/lay...
Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:356: def
set_device_prepared(device_index):
Ditto.

[Dirk Pranke, 2013-06-21 23:02:17]

lgtm

https://codereview.chromium.org/16917006/diff/2001/Tools/Scripts/webkitpy/lay...
File Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py (right):

https://codereview.chromium.org/16917006/diff/2001/Tools/Scripts/webkitpy/lay...
Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:308: # A list to
keep track of which devices have been prepared.
On 2013/06/21 18:06:23, DaleCurtis wrote:
> Instead of a global variable w/ static accessors you could have
> ChromiumAndroidPort pass in AndroidDevices to ChromiumAndroidDriver like you
do
> with AndroidCommands.  If globals are common style here, feel to ignore me.

globals are not common. I'm also not clear on what we think we're gaining by
using globals here. It is quite uncommon to create more than one port object per
process.

[Peter Beverloo, 2013-06-24 17:12:17]

Thank you for the comments!  Dale, I'll await your lgtm before landing -- does
this seem better to you?

https://codereview.chromium.org/16917006/diff/2001/Tools/Scripts/webkitpy/lay...
File Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py (right):

https://codereview.chromium.org/16917006/diff/2001/Tools/Scripts/webkitpy/lay...
Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:304: # How many
percentages of battery power does a device need to have left in order
On 2013/06/21 18:06:23, DaleCurtis wrote:
> Rephrase as a statement.  Something like "The minimum battery level required
to
> run layout tests."

Done.

https://codereview.chromium.org/16917006/diff/2001/Tools/Scripts/webkitpy/lay...
Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:308: # A list to
keep track of which devices have been prepared.
Agreed, changed.

https://codereview.chromium.org/16917006/diff/2001/Tools/Scripts/webkitpy/lay...
Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:320:
self._usable_devices = [AndroidCommands(executive, self._default_device)]
By using this in the constructor, the AndroidCommands class will attempt to
interact with the filesystem to find the path to the ADB tool (and may error out
when it can't be found).  In general we're trying to keep the constructors free
of that, and initialize paths like that lazily.

The default_device argument is necessary to support the argument to
run_webkit_tests.py allowing a specific device to be used for testing.

https://codereview.chromium.org/16917006/diff/2001/Tools/Scripts/webkitpy/lay...
Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:323: re_device =
re.compile('^([a-zA-Z0-9_:.-]+)\tdevice$', re.MULTILINE)
On 2013/06/21 18:06:23, DaleCurtis wrote:
> Providing an example in comment is good practice when writing a regex.

Done.

https://codereview.chromium.org/16917006/diff/2001/Tools/Scripts/webkitpy/lay...
Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:353: return
device_index in AndroidDevices._prepared_devices
On 2013/06/21 18:06:23, DaleCurtis wrote:
> device_index is an integer above, but you're calling this method with a
serial. 
> maybe device_serial instead?
> 

Done.

https://codereview.chromium.org/16917006/diff/2001/Tools/Scripts/webkitpy/lay...
Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:356: def
set_device_prepared(device_index):
On 2013/06/21 18:06:23, DaleCurtis wrote:
> Ditto.

Done.

[DaleCurtis, 2013-06-24 17:49:31]

lgtm % regex Q + nit.

https://codereview.chromium.org/16917006/diff/7001/Tools/Scripts/webkitpy/lay...
File Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py (right):

https://codereview.chromium.org/16917006/diff/7001/Tools/Scripts/webkitpy/lay...
Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:305: # to
participate in running the layout tests?
No more question mark.

https://codereview.chromium.org/16917006/diff/7001/Tools/Scripts/webkitpy/lay...
Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:324: re_device =
re.compile('^([a-zA-Z0-9_:.-]+)\tdevice$', re.MULTILINE)
Why _:.- then?

[Peter Beverloo, 2013-06-26 16:40:44]

https://codereview.chromium.org/16917006/diff/7001/Tools/Scripts/webkitpy/lay...
File Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py (right):

https://codereview.chromium.org/16917006/diff/7001/Tools/Scripts/webkitpy/lay...
Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:305: # to
participate in running the layout tests?
On 2013/06/24 17:49:31, DaleCurtis wrote:
> No more question mark.

Done.

https://codereview.chromium.org/16917006/diff/7001/Tools/Scripts/webkitpy/lay...
Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:324: re_device =
re.compile('^([a-zA-Z0-9_:.-]+)\tdevice$', re.MULTILINE)
This is the regular expression used elsewhere as well (Chromium test-suites)
which we shouldn't diverge from.  I vaguely recall that the Ids are different
for emulators, too.

[commit-bot: I haz the power, 2013-06-26 16:44:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/peter@chromium.org/16917006/19001

[commit-bot: I haz the power, 2013-06-26 16:57:06]

Retried try job too often on blink_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=blink_pres...

[commit-bot: I haz the power, 2013-06-26 17:03:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/peter@chromium.org/16917006/19001

[commit-bot: I haz the power, 2013-06-26 17:13:41]

Retried try job too often on blink_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=blink_pres...

[commit-bot: I haz the power, 2013-06-26 22:41:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/peter@chromium.org/16917006/26001

[commit-bot: I haz the power, 2013-06-27 00:14:25]

Change committed as 153106