[android] Switch test scripts to use last modified time instead of md5sum when checking dependencie…

build/android/pylib/android_commands.py

 # Copyright (c) 2012 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 an interface to communicate with the device via the adb command.
 
 Assumes adb binary is currently on system path.
 """
 
 import collections
@@ skipping 43 matching lines @@
 
 # Keycode "enum" suitable for passing to AndroidCommands.SendKey().
 KEYCODE_HOME = 3
 KEYCODE_BACK = 4
 KEYCODE_DPAD_UP = 19
 KEYCODE_DPAD_DOWN = 20
 KEYCODE_DPAD_RIGHT = 22
 KEYCODE_ENTER = 66
 KEYCODE_MENU = 82
 
-MD5SUM_DEVICE_FOLDER = constants.TEST_EXECUTABLE_DIR + '/md5sum/'
-MD5SUM_DEVICE_PATH = MD5SUM_DEVICE_FOLDER + 'md5sum_bin'
-MD5SUM_LD_LIBRARY_PATH = 'LD_LIBRARY_PATH=%s' % MD5SUM_DEVICE_FOLDER
-
 def GetEmulators():
   """Returns a list of emulators.  Does not filter by status (e.g. offline).
 
   Both devices starting with 'emulator' will be returned in below output:
 
     * daemon not running. starting it now on port 5037 *
     * daemon started successfully *
     List of devices attached
     027c10494100b4d7        device
     emulator-5554   offline
@@ skipping 86 matching lines @@
       if isinstance(utc_offset, str) and len(utc_offset) == 5:
         utc_delta = datetime.timedelta(hours=int(utc_offset[1:3]),
                                        minutes=int(utc_offset[3:5]))
         if utc_offset[0:1] == '-':
           utc_delta = -utc_delta
         lastmod -= utc_delta
       files[filename] = (int(file_match.group('size')), lastmod)
   return files
 
 
-def _ComputeFileListHash(md5sum_output):
-  """Returns a list of MD5 strings from the provided md5sum output."""
-  return [line.split('  ')[0] for line in md5sum_output]
+def _ComputeFileListTimesModified(timemodified_output):
+  """Returns a list of tuples from the provided timemodified output.
+
+  Args:
+    timemodified_output: output directly from timemodified binary.
+
+  Returns:
+    List of tuples of the form (time_modified, file_path).
+  """
+  return [line.split('  ')[:2] for line in timemodified_output]

[frankf, 2013/07/02 18:45:29]

Is this number of spaces important. Can you just do line.split()?

[craigdh, 2013/07/02 20:34:15]

Yep, important as discussed offline. Added an assert.

 
 
 def _HasAdbPushSucceeded(command_output):
   """Returns whether adb push has succeeded from the provided output."""
   if not command_output:
     return False
   # Success looks like this: "3035 KB/s (12512056 bytes in 4.025s)"
   # Errors look like this: "failed to copy  ... "
   if not re.search('^[0-9]', command_output.splitlines()[-1]):
     logging.critical('PUSH FAILED: ' + command_output)
@@ skipping 26 matching lines @@
       os.environ['PATH'] += os.pathsep + adb_dir
     self._adb = adb_interface.AdbInterface()
     if device:
       self._adb.SetTargetSerial(device)
     self._device = device
     self._logcat = None
     self.logcat_process = None
     self._logcat_tmpoutfile = None
     self._pushed_files = []
     self._device_utc_offset = self.RunShellCommand('date +%z')[0]
-    self._md5sum_build_dir = ''
+    self._timemodified_build_dir = ''
     self._external_storage = ''
     self._util_wrapper = ''
 
   def _LogShell(self, cmd):
     """Logs the adb shell command."""
     if self._device:
       device_repr = self._device[-4:]
     else:
       device_repr = '????'
     logging.info('[%s]> %s', device_repr, cmd)
@@ skipping 455 matching lines @@
       self.RunShellCommand('pm clear ' + package)
 
   def SendKeyEvent(self, keycode):
     """Sends keycode to the device.
 
     Args:
       keycode: Numeric keycode to send (see "enum" at top of file).
     """
     self.RunShellCommand('input keyevent %d' % keycode)
 
-  def CheckMd5Sum(self, local_path, device_path, ignore_paths=False):
-    """Compares the md5sum of a local path against a device path.
+  def AreFilesUnsynchronized(self, local_path, device_path):
+    """Compares the time modified of a local path against a device path.
 
     Args:
       local_path: Path (file or directory) on the host.
       device_path: Path on the device.

[frankf, 2013/07/02 18:45:29]

file or directory?

[craigdh, 2013/07/02 20:34:15]

either. clarified.

-      ignore_paths: If False, both the md5sum and the relative paths/names of
-          files must match. If True, only the md5sum must match.
 
     Returns:
-      True if the md5sums match.
+      True if the time the files were modified match.
     """
+    TIMEMODIFIED_DEVICE_DIRECTORY = (constants.TEST_EXECUTABLE_DIR +
+                                     '/timemodified/')
+    TIMEMODIFIED_DEVICE_PATH = (TIMEMODIFIED_DEVICE_DIRECTORY +
+                                'timemodified_bin')
+    TIMEMODIFIED_LD_LIBRARY_PATH = ('LD_LIBRARY_PATH=%s' %
+                                    TIMEMODIFIED_DEVICE_DIRECTORY)
     assert os.path.exists(local_path), 'Local path not found %s' % local_path
 
-    if not self._md5sum_build_dir:
+    if not self._timemodified_build_dir:
       default_build_type = os.environ.get('BUILD_TYPE', 'Debug')
       build_dir = '%s/%s/' % (
           cmd_helper.OutDirectory().get(), default_build_type)
-      md5sum_dist_path = '%s/md5sum_dist' % build_dir
-      if not os.path.exists(md5sum_dist_path):
+      host_timemodified_dist_path = '%s/timemodified_dist' % build_dir

[frankf, 2013/07/02 18:45:29]

Why is this not a constant like the device counterparts?

[craigdh, 2013/07/02 20:34:15]

Because it is determined dynamically at runtime.

+      if not os.path.exists(host_timemodified_dist_path):
         build_dir = '%s/Release/' % cmd_helper.OutDirectory().get()
-        md5sum_dist_path = '%s/md5sum_dist' % build_dir
-        assert os.path.exists(md5sum_dist_path), 'Please build md5sum.'
-      command = 'push %s %s' % (md5sum_dist_path, MD5SUM_DEVICE_FOLDER)
+        host_timemodified_dist_path = '%s/timemodified_dist' % build_dir
+        assert os.path.exists(host_timemodified_dist_path), 'Build timemodified'
+      command = 'push %s %s' % (host_timemodified_dist_path,
+                                TIMEMODIFIED_DEVICE_DIRECTORY)
       assert _HasAdbPushSucceeded(self._adb.SendCommand(command))
-      self._md5sum_build_dir = build_dir
+      self._timemodified_build_dir = build_dir
 
     self._pushed_files.append(device_path)
-    hashes_on_device = _ComputeFileListHash(
-        self.RunShellCommand(MD5SUM_LD_LIBRARY_PATH + ' ' + self._util_wrapper +
-            ' ' + MD5SUM_DEVICE_PATH + ' ' + device_path))
+    device_timemodified_output = self.RunShellCommand(
+        TIMEMODIFIED_LD_LIBRARY_PATH + ' ' + self._util_wrapper + ' ' +
+        TIMEMODIFIED_DEVICE_PATH + ' ' + device_path)
+    device_time_tuples = _ComputeFileListTimesModified(
+        device_timemodified_output)
     assert os.path.exists(local_path), 'Local path not found %s' % local_path
-    md5sum_output = cmd_helper.GetCmdOutput(
-        ['%s/md5sum_bin_host' % self._md5sum_build_dir, local_path])
-    hashes_on_host = _ComputeFileListHash(md5sum_output.splitlines())
+    host_timemodified_output = cmd_helper.GetCmdOutput(
+        ['%s/timemodified_bin_host' % self._timemodified_build_dir, local_path])
+    host_time_tuples = _ComputeFileListTimesModified(

[frankf, 2013/07/02 18:45:29]

Can you use named tuples instead of p[0]/[1]

[craigdh, 2013/07/02 20:34:15]

Done.

+        host_timemodified_output.splitlines())
 
-    if ignore_paths:
-      hashes_on_device = [h.split()[0] for h in hashes_on_device]
-      hashes_on_host = [h.split()[0] for h in hashes_on_host]
+    if len(device_time_tuples) < len(host_time_tuples):
+      return True
 
-    return hashes_on_device == hashes_on_host
+    # Ignore extra files on the device.

[frankf, 2013/07/02 18:45:29]

Add a TODO to clean up old data on the device.

[craigdh, 2013/07/02 20:34:15]

Done.

+    device_time_tuples = [t for t in device_time_tuples if t[1] != '.']

[frankf, 2013/07/02 18:45:29]

Why are listing this?

[craigdh, 2013/07/02 20:34:15]

Already removed in the patch I uploaded while you were reviewing.

+    if len(device_time_tuples) > len(host_time_tuples):
+      host_files = [os.path.relpath(os.path.normpath(p[1]),
+                    os.path.normpath(local_path)) for p in host_time_tuples]
+      def _host_has(fname):

[frankf, 2013/07/02 18:45:29]

Use blank lines to improve readability

[craigdh, 2013/07/02 20:34:15]

Done.

+        return any(p in fname for p in host_files)
+      times_on_device = [p[0] for p in device_time_tuples if _host_has(p[1])]
+    else:
+      times_on_device = [t[0] for t in device_time_tuples]
+
+    # Compare timestamps between host and device files.
+    times_on_host = [t[0] for t in host_time_tuples]
+    times_on_device.sort()
+    times_on_host.sort()
+    return any(int(dtime) < int(htime) for dtime, htime in
+        zip(times_on_device, times_on_host))
 
   def PushIfNeeded(self, local_path, device_path):
     """Pushes |local_path| to |device_path|.
 
     Works for files and directories. This method skips copying any paths in
-    |test_data_paths| that already exist on the device with the same hash.
+    |test_data_paths| that already exist on the device with the same modified
+    time.
 
     All pushed files can be removed by calling RemovePushedFiles().
     """
-    if self.CheckMd5Sum(local_path, device_path):
+    if not self.AreFilesUnsynchronized(local_path, device_path):
       return
 
     # They don't match, so remove everything first and then create it.
     if os.path.isdir(local_path):
       self.RunShellCommand('rm -r %s' % device_path, timeout_time=2 * 60)
       self.RunShellCommand('mkdir -p %s' % device_path)
 
     # NOTE: We can't use adb_interface.Push() because it hardcodes a timeout of
     # 60 seconds which isn't sufficient for a lot of users of this method.
     push_command = 'push %s %s' % (local_path, device_path)
     self._LogShell(push_command)
     output = self._adb.SendCommand(push_command, timeout_time=30 * 60)
     assert _HasAdbPushSucceeded(output)
 
-
   def GetFileContents(self, filename, log_result=False):
     """Gets contents from the file specified by |filename|."""
     return self.RunShellCommand('cat "%s" 2>/dev/null' % filename,
                                 log_result=log_result)
 
   def SetFileContents(self, filename, contents):
     """Writes |contents| to the file specified by |filename|."""
     with tempfile.NamedTemporaryFile() as f:
       f.write(contents)
       f.flush()
@@ skipping 492 matching lines @@
     host_dir = os.path.dirname(host_file)
     if not os.path.exists(host_dir):
       os.makedirs(host_dir)
     device_file = '%s/screenshot.png' % self.GetExternalStorage()
     self.RunShellCommand('/system/bin/screencap -p %s' % device_file)
     assert self._adb.Pull(device_file, host_file)
     assert os.path.exists(host_file)
 
   def SetUtilWrapper(self, util_wrapper):
     """Sets a wrapper prefix to be used when running a locally-built
-    binary on the device (ex.: md5sum_bin).
+    binary on the device (ex.: timemodified_bin).
     """
     self._util_wrapper = util_wrapper
 
   def RunInstrumentationTest(self, test, test_package, instr_args, timeout):
     """Runs a single instrumentation test.
 
     Args:
       test: Test class/method.
       test_package: Package name of test apk.
       instr_args: Extra key/value to pass to am instrument.
@@ skipping 49 matching lines @@
   """
   def __init__(self, output):
     self._output = output
 
   def write(self, data):
     data = data.replace('\r\r\n', '\n')
     self._output.write(data)
 
   def flush(self):
     self._output.flush()