Changeset 136158 in webkit

Timestamp:

Nov 29, 2012, 1:27:32 PM (14 years ago)

Author:

peter@chromium.org

Message:

run-perf-tests --chromium-android should not require adb in my path
​https://bugs.webkit.org/show_bug.cgi?id=103581

Reviewed by Eric Seidel.

Remove the need to have "adb" available in the path for Layout and Performance
tests. We'll determine the versions of the "adb" version in path (if any) and
the one provided in the Chromium Android checkout. Unless the "adb" available
in the path is newer, the provided version will be used.

Some other minor nits addressed:

• The path_to_forwarder/path_to_md5sum should not be in the "private overrides" section, as they're not overriding anything and are used by the driver.
• Make _restart_adb_as_root slightly more robust by waiting for the device to come back online regardless of the output.

• Scripts/webkitpy/layout_tests/port/chromium_android.py:

(ChromiumAndroidPort.init):
(ChromiumAndroidPort.check_build):
(ChromiumAndroidPort.path_to_adb):
(ChromiumAndroidPort):
(ChromiumAndroidPort.path_to_forwarder):
(ChromiumAndroidPort.path_to_md5sum):
(ChromiumAndroidPort._path_to_helper):
(ChromiumAndroidPort._determine_adb_version):
(ChromiumAndroidPort._get_devices):
(ChromiumAndroidDriver.init):
(ChromiumAndroidDriver._setup_md5sum_and_push_data_if_needed):
(ChromiumAndroidDriver._push_executable):
(ChromiumAndroidDriver._restart_adb_as_root):

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/layout_tests/port/chromium_android.py (modified) (11 diffs)

trunk/Tools/ChangeLog

@@ +1 @@
+2012-11-29  Peter Beverloo  <peter@chromium.org>
+
+        run-perf-tests --chromium-android should not require adb in my path
+        https://bugs.webkit.org/show_bug.cgi?id=103581
+
+        Reviewed by Eric Seidel.
+
+        Remove the need to have "adb" available in the path for Layout and Performance
+        tests. We'll determine the versions of the "adb" version in path (if any) and
+        the one provided in the Chromium Android checkout. Unless the "adb" available
+        in the path is newer, the provided version will be used.
+
+        Some other minor nits addressed:
+        - The path_to_forwarder/path_to_md5sum should not be in the "private overrides"
+          section, as they're not overriding anything and are used by the driver.
+        - Make _restart_adb_as_root slightly more robust by waiting for the device
+          to come back online regardless of the output.
+
+        * Scripts/webkitpy/layout_tests/port/chromium_android.py:
+        (ChromiumAndroidPort.__init__):
+        (ChromiumAndroidPort.check_build):
+        (ChromiumAndroidPort.path_to_adb):
+        (ChromiumAndroidPort):
+        (ChromiumAndroidPort.path_to_forwarder):
+        (ChromiumAndroidPort.path_to_md5sum):
+        (ChromiumAndroidPort._path_to_helper):
+        (ChromiumAndroidPort._determine_adb_version):
+        (ChromiumAndroidPort._get_devices):
+        (ChromiumAndroidDriver.__init__):
+        (ChromiumAndroidDriver._setup_md5sum_and_push_data_if_needed):
+        (ChromiumAndroidDriver._push_executable):
+        (ChromiumAndroidDriver._restart_adb_as_root):
+
 2012-11-29  Martin Robinson  <mrobinson@igalia.com>
 

trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py

@@ -33 +33 @@
 import re
 import subprocess
+import sys
 import threading
 import time
@@ -43 +44 @@
 
 _log = logging.getLogger(__name__)
-
 
 # The root directory for test resources, which has the same structure as the
@@ -156 +156 @@
     port_name = 'chromium-android'
 
+    # Avoid initializing the adb path [worker count]+1 times by storing it as a static member.
+    _adb_path = None
+
     FALLBACK_PATHS = [
         'chromium-android',
@@ -209 +212 @@
     def check_build(self, needs_http):
         result = super(ChromiumAndroidPort, self).check_build(needs_http)
-        result = self._check_file_exists(self._path_to_md5sum(), 'md5sum utility') and result
-        result = self._check_file_exists(self._path_to_forwarder(), 'forwarder utility') and result
+        result = self._check_file_exists(self.path_to_md5sum(), 'md5sum utility') and result
+        result = self._check_file_exists(self.path_to_forwarder(), 'forwarder utility') and result
         if not result:
             _log.error('For complete Android build requirements, please see:')
@@ -261 +264 @@
         return self.create_driver(0)._drt_cmd_line(self.get_option('pixel_tests'), [])
 
+    def path_to_adb(self):
+        if ChromiumAndroidPort._adb_path:
+            return ChromiumAndroidPort._adb_path
+
+        provided_adb_path = self.path_from_chromium_base('third_party', 'android_tools', 'sdk', 'platform-tools', 'adb')
+
+        path_version = self._determine_adb_version('adb')
+        provided_version = self._determine_adb_version(provided_adb_path)
+        assert provided_version, 'The checked in Android SDK is missing. Are you sure you ran update-webkit --chromium-android?'
+
+        if not path_version:
+            ChromiumAndroidPort._adb_path = provided_adb_path
+        elif provided_version > path_version:
+            # FIXME: The Printer isn't initialized when this is called, so using _log would just show an unitialized logger error.
+            print >> sys.stderr, 'The "adb" version in your path is older than the one checked in, consider updating your local Android SDK. Using the checked in one.'
+            ChromiumAndroidPort._adb_path = provided_adb_path
+        else:
+            ChromiumAndroidPort._adb_path = 'adb'
+
+        return ChromiumAndroidPort._adb_path
+
+    def path_to_forwarder(self):
+        return self._build_path('forwarder')
+
+    def path_to_md5sum(self):
+        return self._build_path(MD5SUM_DEVICE_FILE_NAME)
+
     # Overridden private functions.
 
@@ -281 +311 @@
         return None
 
-    def _path_to_forwarder(self):
-        return self._build_path('forwarder')
-
-    def _path_to_md5sum(self):
-        return self._build_path(MD5SUM_DEVICE_FILE_NAME)
-
     def _path_to_image_diff(self):
         return self._host_port._path_to_image_diff()
@@ -309 +333 @@
 
     # Local private functions.
+
+    def _determine_adb_version(self, adb_path):
+        re_version = re.compile('^.*version ([\d\.]+)$')
+        try:
+            output = self._executive.run_command([adb_path, 'version'], error_handler=self._executive.ignore_error)
+        except OSError:
+            return None
+        result = re_version.match(output)
+        if not output or not result:
+            return None
+        return [int(n) for n in result.group(1).split('.')]
 
     def _get_devices(self):
         if not self._devices:
             re_device = re.compile('^([a-zA-Z0-9_:.-]+)\tdevice$', re.MULTILINE)
-            result = self._executive.run_command(['adb', 'devices'], error_handler=self._executive.ignore_error)
+            result = self._executive.run_command([self.path_to_adb(), 'devices'], error_handler=self._executive.ignore_error)
             self._devices = re_device.findall(result)
             if not self._devices:
@@ -339 +374 @@
         self._original_governors = {}
         self._device_serial = port._get_device_serial(worker_number)
-        self._adb_command = ['adb', '-s', self._device_serial]
+        self._adb_command = [port.path_to_adb(), '-s', self._device_serial]
 
     def __del__(self):
@@ -346 +381 @@
 
     def _setup_md5sum_and_push_data_if_needed(self):
-        self._md5sum_path = self._port._path_to_md5sum()
+        self._md5sum_path = self._port.path_to_md5sum()
         if not self._file_exists_on_device(MD5SUM_DEVICE_PATH):
             if not self._push_to_device(self._md5sum_path, MD5SUM_DEVICE_PATH):
@@ -404 +439 @@
 
     def _push_executable(self):
-        self._push_file_if_needed(self._port._path_to_forwarder(), DEVICE_FORWARDER_PATH)
+        self._push_file_if_needed(self._port.path_to_forwarder(), DEVICE_FORWARDER_PATH)
         self._push_file_if_needed(self._port._build_path('DumpRenderTree.pak'), DEVICE_DRT_DIR + 'DumpRenderTree.pak')
         self._push_file_if_needed(self._port._build_path('DumpRenderTree_resources'), DEVICE_DRT_DIR + 'DumpRenderTree_resources')
@@ -434 +469 @@
         if 'adbd is already running as root' in output:
             return
-        elif 'restarting adbd as root' in output:
-            self._run_adb_command(['wait-for-device'])
-        else:
+        elif not 'restarting adbd as root' in output:
             self._log_error('Unrecognized output from adb root: %s' % output)
+
+        # Regardless the output, give the device a moment to come back online.
+        self._run_adb_command(['wait-for-device'])
 
     def _run_adb_command(self, cmd, ignore_error=False):