remove stale test data on the device

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=unused-argument
 
@@ skipping 445 matching lines @@
       retries: number of retries
 
     Raises:
       CommandFailedError if the installation fails.
       CommandTimeoutError if the installation times out.
       DeviceUnreachableError on missing device.
     """
     package_name = apk_helper.GetPackageName(apk_path)
     device_path = self.GetApplicationPath(package_name)
     if device_path is not None:
-      should_install = bool(self._GetChangedFilesImpl(apk_path, device_path))
+      (files_to_push, _) = self._GetChangedAndStaleFiles(
+          apk_path, device_path)
+      should_install = bool(files_to_push)
       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)
 
   @decorators.WithTimeoutAndRetriesFromInstance()
   def RunShellCommand(self, cmd, check_return=False, cwd=None, env=None,
                       as_root=False, single_line=False, large_output=False,
@@ skipping 326 matching lines @@
     self.RunShellCommand(['input', 'keyevent', format(keycode, 'd')],
                          check_return=True)
 
   PUSH_CHANGED_FILES_DEFAULT_TIMEOUT = 10 * _DEFAULT_TIMEOUT
   PUSH_CHANGED_FILES_DEFAULT_RETRIES = _DEFAULT_RETRIES
 
   @decorators.WithTimeoutAndRetriesDefaults(
       PUSH_CHANGED_FILES_DEFAULT_TIMEOUT,
       PUSH_CHANGED_FILES_DEFAULT_RETRIES)
   def PushChangedFiles(self, host_device_tuples, timeout=None,
-                       retries=None):
+                       retries=None, delete_device_stale=False):
     """Push files to the device, skipping files that don't need updating.
+    If delete_device_stale option is True, delete stale files on device.

[perezju, 2015/06/10 09:24:23]

Maybe explain in a bit more detail:

When pushing a directory, it is traversed recursively on the host and all files
found are pushed to the device as needed. Additionally, if delete_device_stale
option is True, files on the device that don't exist on the host are deleted.

(also nit: add space between title doc line and additional notes.)

[Menglin, 2015/06/10 21:22:16]

Done.

 
     Args:
       host_device_tuples: A list of (host_path, device_path) tuples, where
         |host_path| is an absolute path of a file or directory on the host
         that should be minimially pushed to the device, and |device_path| is
         an absolute path of the destination on the device.
       timeout: timeout in seconds
       retries: number of retries
+      delete_device_stale: option to delete stale files on device
 
     Raises:
       CommandFailedError on failure.
       CommandTimeoutError on timeout.
       DeviceUnreachableError on missing device.
     """
 
     files = []
+    stale_files_on_device = []
     for h, d in host_device_tuples:
       if os.path.isdir(h):
         self.RunShellCommand(['mkdir', '-p', d], check_return=True)
-      files += self._GetChangedFilesImpl(h, d)
+      (file_tuples_to_push, stale_files) = self._GetChangedAndStaleFiles(h, d)
+      files += file_tuples_to_push
+      stale_files_on_device += stale_files

[perezju, 2015/06/10 09:24:23]

nit: maybe rename these variables to something like changed_files/stale_files
for the local variable within each iteration, and
all_changed_files/all_stale_files for the aggregates.

[Menglin, 2015/06/10 21:22:15]

Done.

+
+    if delete_device_stale:
+      self._DeleteStaleFiles(stale_files_on_device)
 
     if not files:
       return
 
+    self._PushFilesImpl(host_device_tuples, files)
+
+  def _GetChangedAndStaleFiles(self, host_path, device_path):
+    """Get files to push and delete
+
+    Args:
+      host_path: an absolute path of a file or directory on the host
+      device_path: an absolute path of a file or directory on the device
+
+    Returns:
+      a two-element tuple
+      1st element: a list of (host_files_path, device_files_path) tuples to push
+      2nd element: a list of stale files under device_path
+    """
+    real_host_path = os.path.realpath(host_path)
+    try:
+      real_device_path = self.RunShellCommand(
+          ['realpath', device_path], single_line=True, check_return=True)
+    except device_errors.CommandFailedError:
+      real_device_path = None
+    if not real_device_path:
+      return ([(host_path, device_path)], [])
+
+    try:
+      host_checksums = md5sum.CalculateHostMd5Sums([real_host_path])
+      device_checksums = md5sum.CalculateDeviceMd5Sums(
+        [real_device_path], self)
+    except EnvironmentError as e:
+      logging.warning('Error calculating md5: %s', e)
+      return ([(host_path, device_path)], [])
+
+    if os.path.isfile(host_path):
+      host_checksum = host_checksums.get(real_host_path)
+      device_checksum = device_checksums.get(real_device_path)
+      if host_checksum != device_checksum:
+        return ([(host_path, device_path)], [])
+      else:
+        return ([], [])
+    else:
+      to_push = []
+      to_delete = []
+      for host_abs_path, host_checksum in host_checksums.iteritems():
+        device_abs_path = '%s/%s' % (
+            real_device_path, os.path.relpath(host_abs_path, real_host_path))
+        device_checksum = device_checksums.get(device_abs_path)
+        if device_abs_path in device_checksums:
+          del device_checksums[device_abs_path]

[perezju, 2015/06/10 09:24:22]

I think you can replace the last three lines with:

device_checksum = device_checksums.pop(device_abs_path, None)

[Menglin, 2015/06/10 21:22:15]

Done.

+        if ((device_checksum) != host_checksum):

[perezju, 2015/06/10 09:24:23]

nit: drop all the parentheses

[Menglin, 2015/06/10 21:22:15]

Done.

+          to_push.append((host_abs_path, device_abs_path))
+      to_delete = [p for p in device_checksums.iterkeys()]

[perezju, 2015/06/10 09:24:22]

nit: to_delete = device_checksums.keys()

[Menglin, 2015/06/10 21:22:15]

Done.

+      return (to_push, to_delete)
+
+  def _PushFilesImpl(self, host_device_tuples, files):
     size = sum(host_utils.GetRecursiveDiskUsage(h) for h, _ in files)
     file_count = len(files)
     dir_size = sum(host_utils.GetRecursiveDiskUsage(h)
                    for h, _ in host_device_tuples)
     dir_file_count = 0
     for h, _ in host_device_tuples:
       if os.path.isdir(h):
         dir_file_count += sum(len(f) for _r, _d, f in os.walk(h))
       else:
         dir_file_count += 1
@@ skipping 10 matching lines @@
         dir_push_duration < zip_duration or not self._commands_installed):
       self._PushChangedFilesIndividually(host_device_tuples)
     elif push_duration < zip_duration or not self._commands_installed:
       self._PushChangedFilesIndividually(files)
     else:
       self._PushChangedFilesZipped(files)
       self.RunShellCommand(
           ['chmod', '-R', '777'] + [d for _, d in host_device_tuples],
           as_root=True, check_return=True)
 
-  def _GetChangedFilesImpl(self, host_path, device_path):
-    real_host_path = os.path.realpath(host_path)
-    try:
-      real_device_path = self.RunShellCommand(
-          ['realpath', device_path], single_line=True, check_return=True)
-    except device_errors.CommandFailedError:
-      real_device_path = None
-    if not real_device_path:
-      return [(host_path, device_path)]
-
-    try:
-      host_checksums = md5sum.CalculateHostMd5Sums([real_host_path])
-      device_paths_to_md5 = (
-          real_device_path if os.path.isfile(real_host_path)
-          else ('%s/%s' % (real_device_path, os.path.relpath(p, real_host_path))
-                for p in host_checksums.iterkeys()))
-      device_checksums = md5sum.CalculateDeviceMd5Sums(
-          device_paths_to_md5, self)
-    except EnvironmentError as e:
-      logging.warning('Error calculating md5: %s', e)
-      return [(host_path, device_path)]
-
-    if os.path.isfile(host_path):
-      host_checksum = host_checksums.get(real_host_path)
-      device_checksum = device_checksums.get(real_device_path)
-      if host_checksum != device_checksum:
-        return [(host_path, device_path)]
-      else:
-        return []
-    else:
-      to_push = []
-      for host_abs_path, host_checksum in host_checksums.iteritems():
-        device_abs_path = '%s/%s' % (
-            real_device_path, os.path.relpath(host_abs_path, real_host_path))
-        if (device_checksums.get(device_abs_path) != host_checksum):
-          to_push.append((host_abs_path, device_abs_path))
-      return to_push
+  def _DeleteStaleFiles(self, stale_files):
+    for stale_file_path in stale_files:
+      self.RunShellCommand(['rm', stale_file_path])

[perezju, 2015/06/10 09:24:22]

do we really need this method? maybe just inline within PushChangedFiles

[Menglin, 2015/06/10 21:22:15]

Done.

 
   def _InstallCommands(self):
     if self._commands_installed is None:
       try:
         if not install_commands.Installed(self):
           install_commands.InstallCommands(self)
         self._commands_installed = True
       except Exception as e:
         logging.warning('unzip not available: %s' % str(e))
         self._commands_installed = False
@@ skipping 663 matching lines @@
     blacklist = device_blacklist.ReadBlacklist()
     def blacklisted(adb):
       if adb.GetDeviceSerial() in blacklist:
         logging.warning('Device %s is blacklisted.', adb.GetDeviceSerial())
         return True
       return False
 
     return [cls(adb) for adb in adb_wrapper.AdbWrapper.Devices()
             if not blacklisted(adb)]