Refactor test scripts for android to improve performance

build/android/pylib/device/device_utils.py

 # Copyright 2014 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.
 
 """Provides a variety of device interactions based on adb.
 
 Eventually, this will be based on adb_wrapper.
 """
 # pylint: disable=W0613
 
@@ skipping 108 matching lines @@
       self.old_interface = pylib.android_commands.AndroidCommands(str(device))
     elif isinstance(device, pylib.android_commands.AndroidCommands):
       self.adb = adb_wrapper.AdbWrapper(device.GetDevice())
       self.old_interface = device
     else:
       raise ValueError('Unsupported device value: %r' % device)
     self._commands_installed = None
     self._default_timeout = default_timeout
     self._default_retries = default_retries
     self._cache = {}
+    self._cache['application_paths'] = {}
     assert hasattr(self, decorators.DEFAULT_TIMEOUT_ATTR)
     assert hasattr(self, decorators.DEFAULT_RETRIES_ATTR)
 
   @decorators.WithTimeoutAndRetriesFromInstance()
   def IsOnline(self, timeout=None, retries=None):
     """Checks whether the device is online.
 
     Args:
       timeout: timeout in seconds
       retries: number of retries
@@ skipping 133 matching lines @@
     Args:
       package: Name of the package.
 
     Returns:
       Path to the apk on the device if it exists, None otherwise.
     """
     # 'pm path' is liable to incorrectly exit with a nonzero number starting
     # in Lollipop.
     # TODO(jbudorick): Check if this is fixed as new Android versions are
     # released to put an upper bound on this.
+    application_paths = self._cache['application_paths']

[jbudorick, 2015/01/21 22:54:09]

If we stay with the cache, please move this above the comment. It's explaining
the lines that follow.

+    if package in application_paths:
+      return application_paths[package]
+
     should_check_return = (self.build_version_sdk <

[jbudorick, 2015/01/21 22:54:09]

w.r.t. my tuning suggestion: we already have some version-dependent code here.

                            constants.ANDROID_SDK_VERSION_CODES.LOLLIPOP)
     output = self.RunShellCommand(['pm', 'path', package], single_line=True,
                                   check_return=should_check_return)
     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:'):]
+    application_paths[package] = output[len('package:'):]
+    return application_paths[package]
 
   @decorators.WithTimeoutAndRetriesFromInstance()
   def WaitUntilFullyBooted(self, wifi=False, timeout=None, retries=None):
     """Wait for the device to fully boot.
 
     This means waiting for the device to boot, the package manager to be
     available, and the SD card to be ready. It can optionally mean waiting
     for wifi to come up, too.
 
     Args:
@@ skipping 50 matching lines @@
 
     Raises:
       CommandTimeoutError on timeout.
       DeviceUnreachableError on missing device.
     """
     def device_offline():
       return not self.IsOnline()
 
     self.adb.Reboot()
     self._cache = {}
+    self._cache['application_paths'] = {}
     timeout_retry.WaitFor(device_offline, wait_period=1)
     if block:
       self.WaitUntilFullyBooted()
 
   INSTALL_DEFAULT_TIMEOUT = 4 * _DEFAULT_TIMEOUT
   INSTALL_DEFAULT_RETRIES = _DEFAULT_RETRIES
 
   @decorators.WithTimeoutAndRetriesDefaults(
       INSTALL_DEFAULT_TIMEOUT,
       INSTALL_DEFAULT_RETRIES)
@@ skipping 17 matching lines @@
     device_path = self.GetApplicationPath(package_name)
     if device_path is not None:
       should_install = bool(self._GetChangedFilesImpl(apk_path, device_path))
       if should_install and not reinstall:
         self.adb.Uninstall(package_name)
     else:
       should_install = True
     if should_install:
       self.adb.Install(apk_path, reinstall=reinstall)
 
+    application_paths = self._cache['application_paths']
+    if package_name in application_paths:
+      del application_paths[package_name]
+
   @decorators.WithTimeoutAndRetriesFromInstance()
   def RunShellCommand(self, cmd, check_return=False, cwd=None, env=None,
                       as_root=False, single_line=False,
                       timeout=None, retries=None):
     """Run an ADB shell command.
 
     The command to run |cmd| should be a sequence of program arguments or else
     a single string.
 
     When |cmd| is a sequence, it is assumed to contain the name of the command
     to run followed by its arguments. In this case, arguments are passed to the
     command exactly as given, without any further processing by the shell. This
     allows to easily pass arguments containing spaces or special characters
     without having to worry about getting quoting right. Whenever possible, it
     is recomended to pass |cmd| as a sequence.
 
     When |cmd| is given as a string, it will be interpreted and run by the
     shell on the device.
 
     This behaviour is consistent with that of command runners in cmd_helper as
     well as Python's own subprocess.Popen.
 
     TODO(perezju) Change the default of |check_return| to True when callers
       have switched to the new behaviour.
 
+    TODO(jaekyun) Should clear some of non-RO cache items if this command
+      affects them.
+
     Args:
       cmd: A string with the full command to run on the device, or a sequence
         containing the command and its arguments.
       check_return: A boolean indicating whether or not the return code should
         be checked.
       cwd: The device directory in which the command should be run.
       env: The environment variables with which the command should be run.
       as_root: A boolean indicating whether the shell command should be run
         with root privileges.
       single_line: A boolean indicating if only a single line of output is
@@ skipping 15 matching lines @@
       DeviceUnreachableError on missing device.
     """
     def env_quote(key, value):
       if not DeviceUtils._VALID_SHELL_VARIABLE.match(key):
         raise KeyError('Invalid shell variable name %r' % key)
       # using double quotes here to allow interpolation of shell variables
       return '%s=%s' % (key, cmd_helper.DoubleQuote(value))
 
     if not isinstance(cmd, basestring):
       cmd = ' '.join(cmd_helper.SingleQuote(s) for s in cmd)
+
+    if cmd.startswith('pm install') or cmd.startswith('pm uninstall'):

[jbudorick, 2015/01/21 22:54:09]

O_O

This looks _extremely_ dangerous / flaky / terrifying / high-maintenance / etc.

+      self._cache['application_paths'] = {}
+
     if env:
       env = ' '.join(env_quote(k, v) for k, v in env.iteritems())
       cmd = '%s %s' % (env, cmd)
     if cwd:
       cmd = 'cd %s && %s' % (cmd_helper.SingleQuote(cwd), cmd)
     if as_root and self.NeedsSU():
       # "su -c sh -c" allows using shell features in |cmd|
       cmd = 'su -c sh -c %s' % cmd_helper.SingleQuote(cmd)
     if timeout is None:
       timeout = self._default_timeout
@@ skipping 816 matching lines @@
     Returns:
       A Parallelizer operating over |devices|.
     """
     if not devices:
       devices = adb_wrapper.AdbWrapper.GetDevices()
     devices = [d if isinstance(d, cls) else cls(d) for d in devices]
     if async:
       return parallelizer.Parallelizer(devices)
     else:
       return parallelizer.SyncParallelizer(devices)