Refactor wipe option to wipe on default and have option to skip wipe.

build/android/provision_devices.py

 #!/usr/bin/env python
 #
 # Copyright (c) 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.
 
 """Provisions Android devices with settings required for bots.
 
 Usage:
   ./provision_devices.py [-d <device serial number>]
@@ skipping 111 matching lines @@
 def ProvisionDevices(options):
   # TODO(jbudorick): Parallelize provisioning of all attached devices after
   # swithcing from AndroidCommands.
   if options.device is not None:
     devices = [options.device]
   else:
     devices = android_commands.GetAttachedDevices()
   for device_serial in devices:
     device = device_utils.DeviceUtils(device_serial)
     device.old_interface.EnableAdbRoot()
-    WipeDeviceData(device)
-  try:
-    device_utils.DeviceUtils.parallel(devices).old_interface.Reboot(True)
-  except errors.DeviceUnresponsiveError:
-    pass
-  for device_serial in devices:
-    device = device_utils.DeviceUtils(device_serial)
-    device.WaitUntilFullyBooted(timeout=90)
-    device.old_interface.EnableAdbRoot()
     _ConfigureLocalProperties(device)
     device_settings.ConfigureContentSettingsDict(
         device, device_settings.DETERMINISTIC_DEVICE_SETTINGS)
     # TODO(tonyg): We eventually want network on. However, currently radios
     # can cause perfbots to drain faster than they charge.
     if 'perf' in os.environ.get('BUILDBOT_BUILDERNAME', '').lower():
       device_settings.ConfigureContentSettingsDict(
           device, device_settings.NETWORK_DISABLED_SETTINGS)
     device.old_interface.RunShellCommandWithSU('date -u %f' % time.time())
   try:
     device_utils.DeviceUtils.parallel(devices).old_interface.Reboot(True)
   except errors.DeviceUnresponsiveError:
     pass
   for device_serial in devices:
     device = device_utils.DeviceUtils(device_serial)
     device.WaitUntilFullyBooted(timeout=90)
     (_, props) = device.old_interface.GetShellCommandStatusAndOutput('getprop')
     for prop in props:
       print prop
   if options.auto_reconnect:
     PushAndLaunchAdbReboot(devices, options.target)
 
 
 def main(argv):
   logging.basicConfig(level=logging.INFO)
 
   parser = optparse.OptionParser()
-  parser.add_option('-w', '--wipe', action='store_true',
-                    help='Wipe device data from all attached devices.')
+  parser.add_option('--skip-wipe', action='store_true', default=False,
+                    help="Don't wipe device data during provisioning.")
   parser.add_option('-d', '--device',
                     help='The serial number of the device to be provisioned')
   parser.add_option('-t', '--target', default='Debug', help='The build target')
   parser.add_option(
       '-r', '--auto-reconnect', action='store_true',
       help='Push binary which will reboot the device on adb disconnections.')
   options, args = parser.parse_args(argv[1:])
   constants.SetBuildType(options.target)
 
   if args:
     print >> sys.stderr, 'Unused args %s' % args
     return 1
 
-  if options.wipe:
-    devices = android_commands.GetAttachedDevices()
+  if not options.skip_wipe:
+    if options.device is not None:
+      devices = [options.device]
+    else:
+      devices = android_commands.GetAttachedDevices()
     for device_serial in devices:
       device = device_utils.DeviceUtils(device_serial)
+      device.old_interface.EnableAdbRoot()
       WipeDeviceData(device)
     try:
-      (device_utils.DeviceUtils.parallel(devices)
-          .old_interface.Reboot(True).pFinish(None))
+      device_utils.DeviceUtils.parallel(devices).old_interface.Reboot(True)
     except errors.DeviceUnresponsiveError:
       pass
-  else:
-    ProvisionDevices(options)
+    for device_serial in devices:
+      device.WaitUntilFullyBooted(timeout=90)
+
+  ProvisionDevices(options)

[dnj, 2014/06/12 01:14:02]

Sorry, I should have noticed this last time. You are calling
'GetAttachedDevices' twice (in the 'not skip_wipe' case), which could
potentially lead to inconsistent results if a device is added or removed during
the 'wipe'. Minimally, it is extra 'adb' command-line invocation.

I'd update 'ProvisionDevices' to accept a 'devices' argument and forward the one
that you constructed here so the set of devices that you work with is
consistent. This would also save you from having to recreate the logic to build
the 'devices' array.

[jbudorick, 2014/06/12 01:20:10]

Either this or move the wiping logic into ProvisionDevices, since we're now
always calling it.

[navabi, 2014/06/12 18:17:30]

Done. Good suggestions.

 
 
 if __name__ == '__main__':
   sys.exit(main(sys.argv))