remove stale test data on the device

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 c8b471831e62aec343c705afb12bd3095bd8ea24..35395e24f9d25b82cc66758e68aa9cf00561296b 100644
--- a/build/android/pylib/device/device_utils.py
+++ b/build/android/pylib/device/device_utils.py
@@ -463,7 +463,9 @@ class DeviceUtils(object):
     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:
@@ -810,8 +812,9 @@ class DeviceUtils(object):
       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
@@ -820,6 +823,7 @@ class DeviceUtils(object):
         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.
@@ -828,14 +832,73 @@ class DeviceUtils(object):
     """
 
     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)
@@ -866,43 +929,9 @@ class DeviceUtils(object):
           ['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: