From 5b0ed2d5ac861c3d3ba6643e5a52b915699bcabe Mon Sep 17 00:00:00 2001 From: "frankf@chromium.org" Date: Fri, 12 Jul 2013 06:58:43 +0000 Subject: [PATCH] [Android] Use isolate remap instead of check. - Instead of parsing *.isolated files, use isolate remap to create a temporary dependency dir. - Add an exclusion list to filter dependecies at a finer grain than what's specified in isolate files. - Convert base_unittests and unit_tests to use isolate. This adds an additional 50MB to the dependency size due to many small directories not specified in the exclusion list. BUG=249870 Review URL: https://chromiumcodereview.appspot.com/18233018 git-svn-id: http://src.chromium.org/svn/trunk/src/build@211350 4ff67af0-8c30-449e-8e8b-ad334ec8d88c --- android/pylib/android_commands.py | 5 +- android/pylib/base/base_test_runner.py | 14 -- android/pylib/gtest/dispatch.py | 121 ++++++++++++++++- android/pylib/gtest/test_package.py | 12 +- android/pylib/gtest/test_runner.py | 129 ++++--------------- android/pylib/instrumentation/test_runner.py | 6 +- 6 files changed, 155 insertions(+), 132 deletions(-) diff --git a/android/pylib/android_commands.py b/android/pylib/android_commands.py index da8b34458..48116b88f 100644 --- a/android/pylib/android_commands.py +++ b/android/pylib/android_commands.py @@ -725,9 +725,10 @@ class AndroidCommands(object): assert _HasAdbPushSucceeded(self._adb.SendCommand(command)) self._md5sum_build_dir = build_dir + cmd = (MD5SUM_LD_LIBRARY_PATH + ' ' + self._util_wrapper + ' ' + + MD5SUM_DEVICE_PATH + ' ' + device_path) hashes_on_device = _ComputeFileListHash( - self.RunShellCommand(MD5SUM_LD_LIBRARY_PATH + ' ' + self._util_wrapper + - ' ' + MD5SUM_DEVICE_PATH + ' ' + device_path)) + self.RunShellCommand(cmd, timeout_time=2 * 60)) 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]) diff --git a/android/pylib/base/base_test_runner.py b/android/pylib/base/base_test_runner.py index 3fb6c269c..171e6fc3a 100644 --- a/android/pylib/base/base_test_runner.py +++ b/android/pylib/base/base_test_runner.py @@ -111,20 +111,6 @@ class BaseTestRunner(object): """Run once after all tests are run.""" self.ShutdownHelperToolsForTestSuite() - def CopyTestData(self, test_data_paths, dest_dir): - """Copies |test_data_paths| list of files/directories to |dest_dir|. - - Args: - test_data_paths: A list of files or directories relative to |dest_dir| - which should be copied to the device. The paths must exist in - |DIR_SOURCE_ROOT|. - dest_dir: Absolute path to copy to on the device. - """ - for p in test_data_paths: - self.adb.PushIfNeeded( - os.path.join(constants.DIR_SOURCE_ROOT, p), - os.path.join(dest_dir, p)) - def LaunchTestHttpServer(self, document_root, port=None, extra_config_contents=None): """Launches an HTTP server to serve HTTP tests. diff --git a/android/pylib/gtest/dispatch.py b/android/pylib/gtest/dispatch.py index af1a29114..3dfb2620c 100644 --- a/android/pylib/gtest/dispatch.py +++ b/android/pylib/gtest/dispatch.py @@ -6,8 +6,10 @@ import copy import fnmatch +import glob import logging import os +import shutil from pylib import android_commands from pylib import cmd_helper @@ -23,6 +25,120 @@ import gtest_config import test_runner +# TODO(frankf): Add more test targets here after making sure we don't +# blow up the dependency size (and the world). +_ISOLATE_FILE_PATHS = { + 'base_unittests': 'base/base_unittests.isolate', + 'unit_tests': 'chrome/unit_tests.isolate', +} + +# Used for filtering large data deps at a finer grain than what's allowed in +# isolate files since pushing deps to devices is expensive. +# Wildcards are allowed. +_DEPS_EXCLUSION_LIST = [ + 'chrome/test/data/extensions/api_test', + 'chrome/test/data/extensions/secure_shell', + 'chrome/test/data/firefox*', + 'chrome/test/data/gpu', + 'chrome/test/data/image_decoding', + 'chrome/test/data/import', + 'chrome/test/data/page_cycler', + 'chrome/test/data/perf', + 'chrome/test/data/pyauto_private', + 'chrome/test/data/safari_import', + 'chrome/test/data/scroll', + 'chrome/test/data/third_party', + 'third_party/hunspell_dictionaries/*.dic', +] + +_ISOLATE_SCRIPT = os.path.join( + constants.DIR_SOURCE_ROOT, 'tools', 'swarm_client', 'isolate.py') + + +def _GenerateDepsDirUsingIsolate(test_suite, build_type): + """Generate the dependency dir for the test suite using isolate. + + Args: + test_suite: The test suite basename (e.g. base_unittests). + build_type: Release/Debug + + Returns: + If an isolate file exists, returns path to dependency dir on the host. + Otherwise, returns False. + """ + product_dir = os.path.join(cmd_helper.OutDirectory.get(), build_type) + assert os.path.isabs(product_dir) + isolate_rel_path = _ISOLATE_FILE_PATHS.get(test_suite) + if not isolate_rel_path: + return False + + isolate_abs_path = os.path.join(constants.DIR_SOURCE_ROOT, isolate_rel_path) + isolated_abs_path = os.path.join( + product_dir, '%s.isolated' % test_suite) + assert os.path.exists(isolate_abs_path) + deps_dir = os.path.join(product_dir, 'isolate_deps_dir') + if os.path.isdir(deps_dir): + shutil.rmtree(deps_dir) + isolate_cmd = [ + 'python', _ISOLATE_SCRIPT, + 'remap', + '--isolate', isolate_abs_path, + '--isolated', isolated_abs_path, + '-V', 'PRODUCT_DIR=%s' % product_dir, + '-V', 'OS=android', + '--outdir', deps_dir, + ] + assert not cmd_helper.RunCmd(isolate_cmd) + + # We're relying on the fact that timestamps are preserved + # by the remap command (hardlinked). Otherwise, all the data + # will be pushed to the device once we move to using time diff + # instead of md5sum. Perform a sanity check here. + for root, _, filenames in os.walk(deps_dir): + if filenames: + linked_file = os.path.join(root, filenames[0]) + orig_file = os.path.join( + constants.DIR_SOURCE_ROOT, + os.path.relpath(linked_file, deps_dir)) + if os.stat(linked_file).st_ino == os.stat(orig_file).st_ino: + break + else: + raise Exception('isolate remap command did not use hardlinks.') + + # Delete excluded files as defined by _DEPS_EXCLUSION_LIST. + old_cwd = os.getcwd() + try: + os.chdir(deps_dir) + excluded_paths = [x for y in _DEPS_EXCLUSION_LIST for x in glob.glob(y)] + if excluded_paths: + logging.info('Excluding the following from dependency list: %s', + excluded_paths) + for p in excluded_paths: + if os.path.isdir(p): + shutil.rmtree(p) + else: + os.remove(p) + finally: + os.chdir(old_cwd) + + # On Android, all pak files need to be in the top-level 'paks' directory. + paks_dir = os.path.join(deps_dir, 'paks') + os.mkdir(paks_dir) + for root, _, filenames in os.walk(os.path.join(deps_dir, 'out')): + for filename in fnmatch.filter(filenames, '*.pak'): + shutil.move(os.path.join(root, filename), paks_dir) + + # Move everything in PRODUCT_DIR to top level. + deps_product_dir = os.path.join(deps_dir, 'out', build_type) + if os.path.isdir(deps_product_dir): + for p in os.listdir(deps_product_dir): + shutil.move(os.path.join(deps_product_dir, p), deps_dir) + os.rmdir(deps_product_dir) + os.rmdir(os.path.join(deps_dir, 'out')) + + return deps_dir + + def _FullyQualifiedTestSuites(exe, option_test_suite, build_type): """Get a list of absolute paths to test suite targets. @@ -150,6 +266,8 @@ def _RunATestSuite(options, suite_name): if not ports.ResetTestServerPortAllocation(): raise Exception('Failed to reset test server port.') + deps_dir = _GenerateDepsDirUsingIsolate(suite_name, options.build_type) + # Constructs a new TestRunner with the current options. def RunnerFactory(device, shard_index): return test_runner.TestRunner( @@ -164,7 +282,8 @@ def _RunATestSuite(options, suite_name): options.push_deps, constants.GTEST_TEST_PACKAGE_NAME, constants.GTEST_TEST_ACTIVITY_NAME, - constants.GTEST_COMMAND_LINE_FILE) + constants.GTEST_COMMAND_LINE_FILE, + deps_dir=deps_dir) # Get tests and split them up based on the number of devices. if options.test_filter: diff --git a/android/pylib/gtest/test_package.py b/android/pylib/gtest/test_package.py index d5504a3ff..f903e52b6 100644 --- a/android/pylib/gtest/test_package.py +++ b/android/pylib/gtest/test_package.py @@ -89,23 +89,13 @@ class TestPackage(object): def PushDataAndPakFiles(self): external_storage = self.adb.GetExternalStorage() - if (self.test_suite_basename == 'ui_unittests' or - self.test_suite_basename == 'unit_tests'): + if (self.test_suite_basename == 'ui_unittests'): self.adb.PushIfNeeded( self.test_suite_dirname + '/chrome.pak', external_storage + '/paks/chrome.pak') self.adb.PushIfNeeded( self.test_suite_dirname + '/locales/en-US.pak', external_storage + '/paks/en-US.pak') - if self.test_suite_basename == 'unit_tests': - self.adb.PushIfNeeded( - self.test_suite_dirname + '/resources.pak', - external_storage + '/paks/resources.pak') - self.adb.PushIfNeeded( - self.test_suite_dirname + '/chrome_100_percent.pak', - external_storage + '/paks/chrome_100_percent.pak') - self.adb.PushIfNeeded(self.test_suite_dirname + '/test_data', - external_storage + '/test_data') if self.test_suite_basename in ('content_unittests', 'components_unittests'): self.adb.PushIfNeeded( diff --git a/android/pylib/gtest/test_runner.py b/android/pylib/gtest/test_runner.py index 4e989ba43..4efb9ae33 100644 --- a/android/pylib/gtest/test_runner.py +++ b/android/pylib/gtest/test_runner.py @@ -2,15 +2,11 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. -import glob import logging import os -import sys from pylib import android_commands -from pylib import cmd_helper from pylib import constants -from pylib import perf_tests_helper from pylib.android_commands import errors from pylib.base import base_test_result from pylib.base import base_test_runner @@ -19,109 +15,22 @@ from pylib.utils import run_tests_helper import test_package_apk import test_package_executable -sys.path.insert( - 0, os.path.join(constants.DIR_SOURCE_ROOT, 'tools', 'swarm_client')) -import run_isolated - -_ISOLATE_FILE_PATHS = { - #'base_unittests': 'base/base_unittests.isolate', - #'net_unittests': 'net/net_unittests.isolate', - #'unit_tests': 'chrome/unit_tests.isolate', - #'content_browsertests': 'content/content_browsertests.isolate', - #'content_unittests': 'content/content_unittests.isolate', - } -_ISOLATE_SCRIPT = os.path.join( - constants.DIR_SOURCE_ROOT, 'tools', 'swarm_client', 'isolate.py') - - -def _GetDataFilesForTestSuite(product_dir, test_suite_basename): +# We're moving to using isolate files instead of harcoding +# dependencies here. Look at the TODO in dispatch.py. +def _GetDataFilesForTestSuite(test_suite_basename): """Returns a list of data files/dirs needed by the test suite. Args: - product_dir: Absolute path to product directory (e.g. /../out/Debug). test_suite_basename: The test suite basename (e.g. base_unittests). Returns: A list of test file and directory paths. """ - # TODO(frankf): *.isolated should be generated by gyp using foo_tests_run - # targets. This is a stop-gap solution as currently there are no such - # targets for content_unittests and content_browsertests. - isolate_rel_path = _ISOLATE_FILE_PATHS.get(test_suite_basename) - if isolate_rel_path: - isolate_abs_path = os.path.join(constants.DIR_SOURCE_ROOT, isolate_rel_path) - isolated_abs_path = os.path.join( - product_dir, '%s.isolated' % test_suite_basename) - assert os.path.exists(isolate_abs_path) - isolate_cmd = [ - 'python', _ISOLATE_SCRIPT, - 'check', - '--isolate=%s' % isolate_abs_path, - '--isolated=%s' % isolated_abs_path, - '-V', 'PRODUCT_DIR=%s' % product_dir, - '-V', 'OS=android', - '--outdir=%s' % product_dir, - ] - assert not cmd_helper.RunCmd(isolate_cmd) - with open(isolated_abs_path) as f: - isolated_content = run_isolated.load_isolated(f.read(), - os_flavor='android') - assert isolated_content['os'] == 'android' - return isolated_content['files'].keys() - - logging.info('Did not find an isolate file for the test suite.') # Ideally, we'd just push all test data. However, it has >100MB, and a lot # of the files are not relevant (some are used for browser_tests, others for # features not supported, etc..). - if test_suite_basename == 'base_unittests': - return [ - 'base/test/data/', - ] - elif test_suite_basename == 'unit_tests': - test_files = [ - 'base/test/data/', - 'chrome/test/data/download-test1.lib', - 'chrome/test/data/extensions/bad_magic.crx', - 'chrome/test/data/extensions/good.crx', - 'chrome/test/data/extensions/icon1.png', - 'chrome/test/data/extensions/icon2.png', - 'chrome/test/data/extensions/icon3.png', - 'chrome/test/data/extensions/allow_silent_upgrade/', - 'chrome/test/data/extensions/app/', - 'chrome/test/data/extensions/bad/', - 'chrome/test/data/extensions/effective_host_permissions/', - 'chrome/test/data/extensions/empty_manifest/', - 'chrome/test/data/extensions/good/Extensions/', - 'chrome/test/data/extensions/manifest_tests/', - 'chrome/test/data/extensions/page_action/', - 'chrome/test/data/extensions/permissions/', - 'chrome/test/data/extensions/script_and_capture/', - 'chrome/test/data/extensions/unpacker/', - 'chrome/test/data/bookmarks/', - 'chrome/test/data/components/', - 'chrome/test/data/extensions/json_schema_test.js', - 'chrome/test/data/History/', - 'chrome/test/data/json_schema_validator/', - 'chrome/test/data/pref_service/', - 'chrome/test/data/simple_open_search.xml', - 'chrome/test/data/top_sites/', - 'chrome/test/data/web_app_info/', - 'chrome/test/data/webui/', - 'chrome/third_party/mock4js/', - 'components/test/data/web_database', - 'net/data/ssl/certificates', - 'third_party/accessibility-developer-tools/gen/axs_testing.js', - 'third_party/zlib/google/test/data', - ] - # The following are spell check data. Now only list the data under - # third_party/hunspell_dictionaries which are used by unit tests. - old_cwd = os.getcwd() - os.chdir(constants.DIR_SOURCE_ROOT) - test_files += glob.glob('third_party/hunspell_dictionaries/*.bdic') - os.chdir(old_cwd) - return test_files - elif test_suite_basename == 'media_unittests': + if test_suite_basename == 'media_unittests': return [ 'media/test/data', ] @@ -257,17 +166,19 @@ class TestRunner(base_test_runner.BaseTestRunner): test_apk_package_name: Apk package name for tests running in APKs. test_activity_name: Test activity to invoke for APK tests. command_line_file: Filename to use to pass arguments to tests. + deps_dir: The path to the dependency dir on the host to push to the device. """ def __init__(self, device, test_suite, test_arguments, timeout, cleanup_test_files, tool_name, build_type, in_webkit_checkout, push_deps, test_apk_package_name=None, - test_activity_name=None, command_line_file=None): + test_activity_name=None, command_line_file=None, deps_dir=None): super(TestRunner, self).__init__(device, tool_name, build_type, push_deps) self._running_on_emulator = self.device.startswith('emulator') self._test_arguments = test_arguments self.in_webkit_checkout = in_webkit_checkout self._cleanup_test_files = cleanup_test_files + self._deps_dir = deps_dir logging.warning('Test suite: ' + test_suite) if os.path.splitext(test_suite)[1] == '.apk': @@ -301,16 +212,25 @@ class TestRunner(base_test_runner.BaseTestRunner): #override def PushDataDeps(self): + self.adb.WaitForSdCardReady(20) self.test_package.PushDataAndPakFiles() self.tool.CopyFiles() - test_data = _GetDataFilesForTestSuite(self.test_package.test_suite_dirname, - self.test_package.test_suite_basename) - if test_data: - # Make sure SD card is ready. - self.adb.WaitForSdCardReady(20) - self.CopyTestData(test_data, self.adb.GetExternalStorage()) if self.test_package.test_suite_basename == 'webkit_unit_tests': self.PushWebKitUnitTestsData() + return + + if not self._deps_dir: + logging.info('Did not find an isolate file for the test suite.') + for p in _GetDataFilesForTestSuite(self.test_package.test_suite_basename): + self.adb.PushIfNeeded( + os.path.join(constants.DIR_SOURCE_ROOT, p), + os.path.join(self.adb.GetExternalStorage(), p)) + return + + for p in os.listdir(self._deps_dir): + self.adb.PushIfNeeded( + os.path.join(self._deps_dir, p), + os.path.join(self.adb.GetExternalStorage(), p)) def PushWebKitUnitTestsData(self): """Pushes the webkit_unit_tests data files to the device. @@ -328,6 +248,11 @@ class TestRunner(base_test_runner.BaseTestRunner): os.path.join( self.adb.GetExternalStorage(), 'third_party/WebKit/Source/WebKit/chromium/tests/data')) + self.adb.PushIfNeeded( + os.path.join(constants.DIR_SOURCE_ROOT, + 'third_party/hyphen/hyph_en_US.dic'), + os.path.join(self.adb.GetExternalStorage(), + 'third_party/hyphen/hyph_en_US.dic')) # TODO(craigdh): There is no reason for this to be part of TestRunner. def GetDisabledTests(self): diff --git a/android/pylib/instrumentation/test_runner.py b/android/pylib/instrumentation/test_runner.py index 60000bf71..0a30da6d9 100644 --- a/android/pylib/instrumentation/test_runner.py +++ b/android/pylib/instrumentation/test_runner.py @@ -104,8 +104,10 @@ class TestRunner(base_test_runner.BaseTestRunner): if test_data: # Make sure SD card is ready. self.adb.WaitForSdCardReady(20) - for data in test_data: - self.CopyTestData([data], self.adb.GetExternalStorage()) + for p in test_data: + self.adb.PushIfNeeded( + os.path.join(constants.DIR_SOURCE_ROOT, p), + os.path.join(self.adb.GetExternalStorage(), p)) # TODO(frankf): Specify test data in this file as opposed to passing # as command-line.