Make TimeoutRetryThread's stoppable

build/android/pylib/device/device_utils.py

Index: build/android/pylib/device/device_utils.py
diff --git a/build/android/pylib/device/device_utils.py b/build/android/pylib/device/device_utils.py
index b2fa1ff3953a702f6cd3676f495db89144dcf0d7..9b42727ab942f1027bc7781273a59e51b4274d03 100644
--- a/build/android/pylib/device/device_utils.py
+++ b/build/android/pylib/device/device_utils.py
@@ -14,6 +14,7 @@ import os
 import re
 import sys
 import tempfile
+import threading
 import time
 import zipfile
 
@@ -108,10 +109,8 @@ class DeviceUtils(object):
     Raises:
       CommandTimeoutError on timeout.
     """
-    return self._IsOnlineImpl()
-
-  def _IsOnlineImpl(self):
-    return self.old_interface.IsOnline()
+    state = self.adb.GetState()
+    return state == 'device'
 
   @decorators.WithTimeoutAndRetriesFromInstance()
   def HasRoot(self, timeout=None, retries=None):
@@ -169,7 +168,7 @@ class DeviceUtils(object):
       CommandTimeoutError on timeout.
       DeviceUnreachableError on missing device.
     """
-    return self._GetPropImpl('ro.build.type') == 'user'
+    return self.build_type == 'user'
 
   @decorators.WithTimeoutAndRetriesFromInstance()
   def GetExternalStoragePath(self, timeout=None, retries=None):
@@ -203,6 +202,25 @@ class DeviceUtils(object):
     return value
 
   @decorators.WithTimeoutAndRetriesFromInstance()
+  def GetApplicationPath(self, package, timeout=None, retries=None):
+    """Get the path of the installed apk on the device for the given package.
+
+    Args:
+      package: Name of the package.
+
+    Returns:
+      Path to the apk on the device if it exists, None otherwise.
+    """
+    output = self.RunShellCommand(['pm', 'path', package], single_line=True,
+                                  check_return=True)
+    if not output:
+      return None
+    if not output.startswith('package:'):
+      raise device_errors.CommandFailedError('pm path returned: %r' % output,
+                                             str(self))
+    return output[len('package:'):]
+
+  @decorators.WithTimeoutAndRetriesFromInstance()
   def WaitUntilFullyBooted(self, wifi=False, timeout=None, retries=None):
     """Wait for the device to fully boot.
 
@@ -220,18 +238,61 @@ class DeviceUtils(object):
       CommandTimeoutError if one of the component waits times out.
       DeviceUnreachableError if the device becomes unresponsive.
     """
-    self._WaitUntilFullyBootedImpl(wifi=wifi, timeout=timeout)
+    def sd_card_ready():
+      return self.RunShellCommand(['ls', self.GetExternalStoragePath()],
+                                  single_line=True, check_return=True)
 
-  def _WaitUntilFullyBootedImpl(self, wifi=False, timeout=None):
-    if timeout is None:
-      timeout = self._default_timeout
-    self.old_interface.WaitForSystemBootCompleted(timeout)
-    self.old_interface.WaitForDevicePm()
-    self.old_interface.WaitForSdCardReady(timeout)
+    def pm_ready():
+      return self.GetApplicationPath('android')
+
+    def boot_completed():
+      return self.GetProp('sys.boot_completed') == '1'
+
+    def wifi_enabled():
+      return 'Wi-Fi is enabled' in self.RunShellCommand(['dumpsys', 'wifi'])
+
+    self.adb.WaitForDevice()
+    self._WaitFor(sd_card_ready)
+    self._WaitFor(pm_ready)
+    self._WaitFor(boot_completed)
     if wifi:
-      while not 'Wi-Fi is enabled' in (
-          self.old_interface.RunShellCommand('dumpsys wifi')):
-        time.sleep(1)
+      self._WaitFor(wifi_enabled)
+
+  def _WaitFor(self, condition, wait_period=5, max_tries=None):
+    """Wait for a condition to become true.
+
+    Repeadly call the function condition(), with no arguments, until it returns
+    a true value.
+
+    This method assumes that it is being called within a TimeoutRetryThread.
+
+    Args:
+      condition: function with the condition to check
+      wait_period: how long to wait before retrying to check the condition
+      max_tries: maximum number of checks to make, the default tries forever
+        (usually until a timeout error is raised)
+
+    Returns:
+      The true value returned by the condition, or None if the condition was
+      not met after max_tries.
+    """
+    thread = threading.current_thread()
+    while max_tries is None or max_tries > 0:
+      try:
+        r = condition()
+      except device_errors.CommandFailedError:
+        r = None

[perezju, 2014/10/28 14:31:18]

clients don't need to check whether their thread had timed-out!

+      if max_tries is not None:
+        max_tries -= 1
+      if r:
+        logging.info('(device: %s) condition %r met (%1.2fs) at %s',
+            str(self), condition.__name__, thread.ElapsedTime(), thread.name)
+        return r
+      else:
+        logging.info('(device: %s) condition %r not yet met (%1.2fs) at %s',
+            str(self), condition.__name__, thread.ElapsedTime(), thread.name)
+        self.adb.Wait(wait_period)

[perezju, 2014/10/28 14:31:18]

well, almost... time.sleep needs would need to be replaced with adb.Wait

+    return None
 
   REBOOT_DEFAULT_TIMEOUT = 10 * _DEFAULT_TIMEOUT
   REBOOT_DEFAULT_RETRIES = _DEFAULT_RETRIES
@@ -251,9 +312,14 @@ class DeviceUtils(object):
       CommandTimeoutError on timeout.
       DeviceUnreachableError on missing device.
     """
-    self.old_interface.Reboot()
+    def device_offline():
+      return not self.IsOnline()
+
+    self.adb.Reboot()
+    self._cache = {}
+    self._WaitFor(device_offline, wait_period=1, max_tries=5)
     if block:
-      self._WaitUntilFullyBootedImpl(timeout=timeout)
+      self.WaitUntilFullyBooted()
 
   INSTALL_DEFAULT_TIMEOUT = 4 * _DEFAULT_TIMEOUT
   INSTALL_DEFAULT_RETRIES = _DEFAULT_RETRIES
@@ -392,10 +458,13 @@ class DeviceUtils(object):
 
     output = output.splitlines()
     if single_line:
-      if len(output) != 1:
-        msg = 'exactly one line of output expected, but got: %s'
-        raise device_errors.CommandFailedError(msg % output)
-      return output[0]
+      if not output:
+        return ''

[perezju, 2014/10/28 14:31:18]

this is what "pm path" returns for non-existent packages, so thought it's better
to return an empty string rather than to raise a command error which might get
confused with adb or other device problems.

I also made sure that this change also makes sense for all other callers with
single_line=true (thankfully not that many yet!).

[jbudorick, 2014/10/28 17:28:46]

sgtm

+      elif len(output) == 1:
+        return output[0]
+      else:
+        msg = 'one line of output was expected, but got: %s'
+        raise device_errors.CommandFailedError(msg % output, str(self))
     else:
       return output
 
@@ -715,7 +784,7 @@ class DeviceUtils(object):
       finally:
         if zip_proc.is_alive():
           zip_proc.terminate()
-        if self._IsOnlineImpl():
+        if self.IsOnline():
           self._RunShellCommandImpl(['rm', zip_on_device], check_return=True)
 
   @staticmethod
@@ -894,13 +963,18 @@ class DeviceUtils(object):
     """
     return self.old_interface.SetJavaAssertsEnabled(enabled)
 
-  @decorators.WithTimeoutAndRetriesFromInstance()
-  def GetProp(self, property_name, timeout=None, retries=None):
+  @property
+  def build_type(self):
+    """Returns the build type of the system (e.g. userdebug)."""
+    return self.GetProp('ro.build.type', cache=True)
+
+  def GetProp(self, property_name, cache=False, timeout=None, retries=None):
     """Gets a property from the device.
 
     Args:
       property_name: A string containing the name of the property to get from
                      the device.
+      cache: A boolean indicating whether to cache the value of this property.
       timeout: timeout in seconds
       retries: number of retries
 
@@ -910,13 +984,22 @@ class DeviceUtils(object):
     Raises:
       CommandTimeoutError on timeout.
     """
-    return self._GetPropImpl(property_name)
-
-  def _GetPropImpl(self, property_name):
-    return self.old_interface.system_properties[property_name]
+    cache_key = '_prop:' + property_name
+    if cache and cache_key in self._cache:
+      return self._cache[cache_key]
+    else:
+      # timeout and retries are handled down at run shell, because we don't
+      # want to apply them in the other branch when reading from the cache
+      value = self.RunShellCommand(['getprop', property_name],
+                                   single_line=True, check_return=True,
+                                   timeout=timeout, retries=retries)
+      if cache or cache_key in self._cache:
+        self._cache[cache_key] = value
+      return value
 
   @decorators.WithTimeoutAndRetriesFromInstance()
-  def SetProp(self, property_name, value, timeout=None, retries=None):
+  def SetProp(self, property_name, value, check=False, timeout=None,
+              retries=None):
     """Sets a property on the device.
 
     Args:
@@ -924,13 +1007,25 @@ class DeviceUtils(object):
                      the device.
       value: A string containing the value to set to the property on the
              device.
+      check: A boolean indicating whether to check that the property was
+             successfully set on the device.
       timeout: timeout in seconds
       retries: number of retries
 
     Raises:
+      CommandFailedError if check is true and the property was not correctly
+        set on the device (e.g. because it is not rooted).
       CommandTimeoutError on timeout.
     """
-    self.old_interface.system_properties[property_name] = value
+    self.RunShellCommand(['setprop', property_name, value], check_return=True)
+    if property_name in self._cache:
+      del self._cache[property_name]
+    # TODO(perezju) remove the option and make the check mandatory, but using a
+    # single shell script to both set- and getprop.
+    if check and value != self.GetProp(property_name):
+      raise device_errors.CommandFailedError(
+          'Unable to set property %r on the device to %r'
+          % (property_name, value), str(self))
 
   @decorators.WithTimeoutAndRetriesFromInstance()
   def GetABI(self, timeout=None, retries=None):