Intelligently handle a device with critically low battery.

build/android/buildbot/bb_device_status_check.py

 #!/usr/bin/env python
 #
 # Copyright 2013 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """A class to keep track of devices across builds and report state."""
 import json
 import logging
 import optparse
@@ skipping 359 matching lines @@
   if options.json_output:
     with open(options.json_output, 'wb') as f:
       f.write(json.dumps({
         'online_devices': devices,
         'offline_devices': offline_devices,
         'expected_devices': expected_devices,
         'unique_types': unique_types,
         'unique_builds': unique_builds,
       }))
 
-  if False in fail_step_lst:
-    # TODO(navabi): Build fails on device status check step if there exists any
-    # devices with critically low battery. Remove those devices from testing,
-    # allowing build to continue with good devices.
+  index = 0
+  num_failed_devs = 0
+  for fail_status in fail_step_lst:
+    if not fail_status:
+      device_blacklist.ExtendBlacklist([str(devices[index])])

[jbudorick, 2014/10/23 01:19:32]

We may want to do something like what provision_devices does + try to enable
charging if we can.

[navabi, 2014/10/23 01:27:27]

I'm actually not sure I like what provision_devices does. It waits around to
charge the device. If you're waiting for a build to go through, you have no idea
how long you need to wait.

And if a device has problems charging, it could never charge enough (since the
requirement is 95%). WDYT, John? Do you like the way provision waits around for
the devices to charge?

This way the device is not used for the build, all the while it's charging, and
every build will remove the bad_devices file and give it another chance.

[jbudorick, 2014/10/23 14:05:27]

I don't like it, but I can understand why the perf bots do it. There are
probably better ways to achieve what they're doing, but those would be out of
the scope of this review.

I should've been more clear in my original comment. I think we should enable
charging if possible before adding the device to the blacklist. I'm fine with
blacklisting instead of waiting around for the device to charge. 

(basically, this if block:
https://code.google.com/p/chromium/codesearch#chromium/src/build/android/prov...
but not the containing while loop)

+      num_failed_devs = num_failed_devs + 1

[jbudorick, 2014/10/23 01:19:31]

nit: num_failed_devs += 1

[navabi, 2014/10/23 01:27:27]

Done.

+    index = index + 1

[jbudorick, 2014/10/23 01:19:32]

nit: index += 1

[navabi, 2014/10/23 01:27:27]

Done.

+
+  if num_failed_devs == len(devices):
     return 2
 
   if not devices:
     return 1
 
 
 if __name__ == '__main__':
   sys.exit(main())