From d9d24cf7d7c20990f16154043f80e23d4c92ac4d Mon Sep 17 00:00:00 2001 From: jbudorick Date: Tue, 19 May 2015 13:08:10 -0700 Subject: [PATCH] Revert of [Android] Refactor the native test wrappers. (patchset #7 id:120001 of https://codereview.chromium.org/1126543009/) Reason for revert: broke http://build.chromium.org/p/chromium.mojo/builders/Chromium%20Mojo%20Android e.g. http://build.chromium.org/p/chromium.mojo/builders/Chromium%20Mojo%20Android/builds/1922 Original issue's description: > [Android] Refactor the native test wrappers. > > BUG=476410 > > Committed: https://crrev.com/581d25e509e4a2b1e8927c6d8accc0c90ea86090 > Cr-Commit-Position: refs/heads/master@{#330531} TBR=jochen@chromium.org,cjhopman@chromium.org,jaekyun@chromium.org,perezju@chromium.org,tedchoc@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=476410 Review URL: https://codereview.chromium.org/1138993009 Cr-Original-Commit-Position: refs/heads/master@{#330591} Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src Cr-Mirrored-Commit: 6cdb69279cb4f83fd8d1f31f58eb0edf44d2bec3 --- android/pylib/constants/__init__.py | 6 +- android/pylib/gtest/gtest_test_instance.py | 19 ++--- android/pylib/gtest/local_device_gtest_run.py | 34 +++------ android/pylib/gtest/setup.py | 4 +- android/pylib/gtest/test_package_apk.py | 10 ++- android/pylib/gtest/test_runner.py | 1 - .../instrumentation_test_instance.py | 12 +-- android/pylib/utils/apk_helper.py | 73 ++++--------------- apk_browsertest.gypi | 43 ----------- 9 files changed, 56 insertions(+), 146 deletions(-) delete mode 100644 apk_browsertest.gypi diff --git a/android/pylib/constants/__init__.py b/android/pylib/constants/__init__.py index 4523188e8..cb5f8995e 100644 --- a/android/pylib/constants/__init__.py +++ b/android/pylib/constants/__init__.py @@ -102,7 +102,7 @@ PACKAGE_INFO = { 'org.chromium.android_webview.test'), 'gtest': PackageInfo( 'org.chromium.native_test', - 'org.chromium.native_test.NativeUnitTestActivity', + 'org.chromium.native_test.NativeTestActivity', '/data/local/tmp/chrome-native-tests-command-line', None, None), @@ -110,13 +110,13 @@ PACKAGE_INFO = { 'org.chromium.components_browsertests_apk', ('org.chromium.components_browsertests_apk' + '.ComponentsBrowserTestsActivity'), - '/data/local/tmp/chrome-native-tests-command-line', + '/data/local/tmp/components-browser-tests-command-line', None, None), 'content_browsertests': PackageInfo( 'org.chromium.content_browsertests_apk', 'org.chromium.content_browsertests_apk.ContentBrowserTestsActivity', - '/data/local/tmp/chrome-native-tests-command-line', + '/data/local/tmp/content-browser-tests-command-line', None, None), 'chromedriver_webview_shell': PackageInfo( diff --git a/android/pylib/gtest/gtest_test_instance.py b/android/pylib/gtest/gtest_test_instance.py index cea50160f..b6f83b31b 100644 --- a/android/pylib/gtest/gtest_test_instance.py +++ b/android/pylib/gtest/gtest_test_instance.py @@ -17,12 +17,6 @@ sys.path.append(os.path.join( import unittest_util -BROWSER_TEST_SUITES = [ - 'components_browsertests', - 'content_browsertests', -] - - # 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. @@ -98,9 +92,16 @@ class GtestTestInstance(test_instance.TestInstance): raise ValueError('Platform mode currently supports only 1 gtest suite') self._suite = args.suite_name[0] - self._apk_path = os.path.join( - constants.GetOutDirectory(), '%s_apk' % self._suite, - '%s-debug.apk' % self._suite) + if (self._suite == 'content_browsertests' or + self._suite == 'components_browsertests'): + error_func('%s are not currently supported ' + 'in platform mode.' % self._suite) + self._apk_path = os.path.join( + constants.GetOutDirectory(), 'apks', '%s.apk' % self._suite) + else: + self._apk_path = os.path.join( + constants.GetOutDirectory(), '%s_apk' % self._suite, + '%s-debug.apk' % self._suite) self._exe_path = os.path.join(constants.GetOutDirectory(), self._suite) if not os.path.exists(self._apk_path): diff --git a/android/pylib/gtest/local_device_gtest_run.py b/android/pylib/gtest/local_device_gtest_run.py index 15a58a459..4241e8521 100644 --- a/android/pylib/gtest/local_device_gtest_run.py +++ b/android/pylib/gtest/local_device_gtest_run.py @@ -24,9 +24,6 @@ _EXTRA_COMMAND_LINE_FILE = ( 'org.chromium.native_test.NativeTestActivity.CommandLineFile') _EXTRA_COMMAND_LINE_FLAGS = ( 'org.chromium.native_test.NativeTestActivity.CommandLineFlags') -_EXTRA_NATIVE_TEST_ACTIVITY = ( - 'org.chromium.native_test.NativeTestInstrumentationTestRunner' - '.NativeTestActivity') _MAX_SHARD_SIZE = 256 @@ -40,11 +37,8 @@ _SUITE_REQUIRES_TEST_SERVER_SPAWNER = [ class _ApkDelegate(object): def __init__(self, apk): self._apk = apk - - helper = apk_helper.ApkHelper(self._apk) - self._activity = helper.GetActivityName() - self._package = helper.GetPackageName() - self._runner = helper.GetInstrumentationName() + self._package = apk_helper.GetPackageName(self._apk) + self._runner = apk_helper.GetInstrumentationName(self._apk) self._component = '%s/%s' % (self._package, self._runner) self._enable_test_server_spawner = False @@ -57,7 +51,6 @@ class _ApkDelegate(object): extras = { _EXTRA_COMMAND_LINE_FILE: command_line_file.name, - _EXTRA_NATIVE_TEST_ACTIVITY: self._activity, } return device.StartInstrumentation( @@ -139,7 +132,7 @@ class LocalDeviceGtestRun(local_device_test_run.LocalDeviceTestRun): #override def TestPackage(self): - return self._test_instance.suite + return self._test_instance._suite #override def SetUp(self): @@ -173,16 +166,13 @@ class LocalDeviceGtestRun(local_device_test_run.LocalDeviceTestRun): #override def _CreateShards(self, tests): - if self._test_instance.suite in gtest_test_instance.BROWSER_TEST_SUITES: - return tests - else: - device_count = len(self._env.devices) - shards = [] - for i in xrange(0, device_count): - unbounded_shard = tests[i::device_count] - shards += [unbounded_shard[j:j+_MAX_SHARD_SIZE] - for j in xrange(0, len(unbounded_shard), _MAX_SHARD_SIZE)] - return [':'.join(s) for s in shards] + device_count = len(self._env.devices) + shards = [] + for i in xrange(0, device_count): + unbounded_shard = tests[i::device_count] + shards += [unbounded_shard[j:j+_MAX_SHARD_SIZE] + for j in xrange(0, len(unbounded_shard), _MAX_SHARD_SIZE)] + return [':'.join(s) for s in shards] #override def _GetTests(self): @@ -195,8 +185,8 @@ class LocalDeviceGtestRun(local_device_test_run.LocalDeviceTestRun): #override def _RunTest(self, device, test): # Run the test. - output = self._delegate.RunWithFlags( - device, '--gtest_filter=%s' % test, timeout=900, retries=0) + output = self._delegate.RunWithFlags(device, '--gtest_filter=%s' % test, + timeout=900, retries=0) for s in self._servers[str(device)]: s.Reset() self._delegate.Clear(device) diff --git a/android/pylib/gtest/setup.py b/android/pylib/gtest/setup.py index 8ba9a8d7c..d51b90ecf 100644 --- a/android/pylib/gtest/setup.py +++ b/android/pylib/gtest/setup.py @@ -15,7 +15,6 @@ from pylib.base import base_setup from pylib.base import base_test_result from pylib.base import test_dispatcher from pylib.device import device_utils -from pylib.gtest import gtest_test_instance from pylib.gtest import test_package_apk from pylib.gtest import test_package_exe from pylib.gtest import test_runner @@ -241,7 +240,8 @@ def Setup(test_options, devices): tests = unittest_util.FilterTestNames(tests, test_options.gtest_filter) # Coalesce unit tests into a single test per device - if test_options.suite_name not in gtest_test_instance.BROWSER_TEST_SUITES: + if (test_options.suite_name != 'content_browsertests' and + test_options.suite_name != 'components_browsertests'): num_devices = len(devices) tests = [':'.join(tests[i::num_devices]) for i in xrange(num_devices)] tests = [t for t in tests if t] diff --git a/android/pylib/gtest/test_package_apk.py b/android/pylib/gtest/test_package_apk.py index 64478209e..9672f7aec 100644 --- a/android/pylib/gtest/test_package_apk.py +++ b/android/pylib/gtest/test_package_apk.py @@ -30,14 +30,18 @@ class TestPackageApk(TestPackage): suite_name: Name of the test suite (e.g. base_unittests). """ TestPackage.__init__(self, suite_name) - self.suite_path = os.path.join( - constants.GetOutDirectory(), '%s_apk' % suite_name, - '%s-debug.apk' % suite_name) if suite_name == 'content_browsertests': + self.suite_path = os.path.join( + constants.GetOutDirectory(), 'apks', '%s.apk' % suite_name) self._package_info = constants.PACKAGE_INFO['content_browsertests'] elif suite_name == 'components_browsertests': + self.suite_path = os.path.join( + constants.GetOutDirectory(), 'apks', '%s.apk' % suite_name) self._package_info = constants.PACKAGE_INFO['components_browsertests'] else: + self.suite_path = os.path.join( + constants.GetOutDirectory(), '%s_apk' % suite_name, + '%s-debug.apk' % suite_name) self._package_info = constants.PACKAGE_INFO['gtest'] def _CreateCommandLineFileOnDevice(self, device, options): diff --git a/android/pylib/gtest/test_runner.py b/android/pylib/gtest/test_runner.py index 6d01990c2..49263888c 100644 --- a/android/pylib/gtest/test_runner.py +++ b/android/pylib/gtest/test_runner.py @@ -11,7 +11,6 @@ from pylib import ports from pylib.base import base_test_result from pylib.base import base_test_runner from pylib.device import device_errors -from pylib.gtest import gtest_test_instance from pylib.local import local_test_server_spawner from pylib.perf import perf_control diff --git a/android/pylib/instrumentation/instrumentation_test_instance.py b/android/pylib/instrumentation/instrumentation_test_instance.py index f9957b089..0633f1466 100644 --- a/android/pylib/instrumentation/instrumentation_test_instance.py +++ b/android/pylib/instrumentation/instrumentation_test_instance.py @@ -201,9 +201,8 @@ class InstrumentationTestInstance(test_instance.TestInstance): if not os.path.exists(self._test_jar): error_func('Unable to find test JAR: %s' % self._test_jar) - apk = apk_helper.ApkHelper(self.test_apk) - self._test_package = apk.GetPackageName() - self._test_runner = apk.GetInstrumentationName() + self._test_package = apk_helper.GetPackageName(self.test_apk) + self._test_runner = apk_helper.GetInstrumentationName(self.test_apk) self._package_info = None for package_info in constants.PACKAGE_INFO.itervalues(): @@ -275,9 +274,10 @@ class InstrumentationTestInstance(test_instance.TestInstance): constants.GetOutDirectory(), constants.SDK_BUILD_APKS_DIR, 'OnDeviceInstrumentationDriver.apk') if os.path.exists(self._driver_apk): - driver_apk = apk_helper.ApkHelper(self._driver_apk) - self._driver_package = driver_apk.GetPackageName() - self._driver_name = driver_apk.GetInstrumentationName() + self._driver_package = apk_helper.GetPackageName( + self._driver_apk) + self._driver_name = apk_helper.GetInstrumentationName( + self._driver_apk) else: self._driver_apk = None diff --git a/android/pylib/utils/apk_helper.py b/android/pylib/utils/apk_helper.py index 6dd209e3d..f5e9cd386 100644 --- a/android/pylib/utils/apk_helper.py +++ b/android/pylib/utils/apk_helper.py @@ -19,14 +19,14 @@ _MANIFEST_ELEMENT_RE = re.compile(r'\s*(?:E|N): (\S*) .*$') def GetPackageName(apk_path): """Returns the package name of the apk.""" - return ApkHelper(apk_path).GetPackageName() - - -# TODO(jbudorick): Deprecate and remove this function once callers have been -# converted to ApkHelper.GetInstrumentationName -def GetInstrumentationName(apk_path): - """Returns the name of the Instrumentation in the apk.""" - return ApkHelper(apk_path).GetInstrumentationName() + aapt_cmd = [_AAPT_PATH, 'dump', 'badging', apk_path] + aapt_output = cmd_helper.GetCmdOutput(aapt_cmd).split('\n') + package_name_re = re.compile(r'package: .*name=\'(\S*)\'') + for line in aapt_output: + m = package_name_re.match(line) + if m: + return m.group(1) + raise Exception('Failed to determine package name of %s' % apk_path) def _ParseManifestFromApk(apk_path): @@ -65,53 +65,12 @@ def _ParseManifestFromApk(apk_path): return parsed_manifest -class ApkHelper(object): - def __init__(self, apk_path): - self._apk_path = apk_path - self._manifest = None - self._package_name = None - - def GetActivityName(self): - """Returns the name of the Activity in the apk.""" - manifest_info = self._GetManifest() - try: - activity = ( - manifest_info['manifest']['application']['activity'] - ['android:name'][0]) - except KeyError: - return None - if '.' not in activity: - activity = '%s.%s' % (self.GetPackageName(), activity) - elif activity.startswith('.'): - activity = '%s%s' % (self.GetPackageName(), activity) - return activity - - def GetInstrumentationName( - self, default='android.test.InstrumentationTestRunner'): - """Returns the name of the Instrumentation in the apk.""" - manifest_info = self._GetManifest() - try: - return manifest_info['manifest']['instrumentation']['android:name'][0] - except KeyError: - return default - - def GetPackageName(self): - """Returns the package name of the apk.""" - if self._package_name: - return self._package_name - - aapt_cmd = [_AAPT_PATH, 'dump', 'badging', self._apk_path] - aapt_output = cmd_helper.GetCmdOutput(aapt_cmd).split('\n') - package_name_re = re.compile(r'package: .*name=\'(\S*)\'') - for line in aapt_output: - m = package_name_re.match(line) - if m: - self._package_name = m.group(1) - return self._package_name - raise Exception('Failed to determine package name of %s' % self._apk_path) - - def _GetManifest(self): - if not self._manifest: - self._manifest = _ParseManifestFromApk(self._apk_path) - return self._manifest +def GetInstrumentationName( + apk_path, default='android.test.InstrumentationTestRunner'): + """Returns the name of the Instrumentation in the apk.""" + try: + manifest_info = _ParseManifestFromApk(apk_path) + return manifest_info['manifest']['instrumentation']['android:name'][0] + except KeyError: + return default diff --git a/apk_browsertest.gypi b/apk_browsertest.gypi deleted file mode 100644 index 316f52fb8..000000000 --- a/apk_browsertest.gypi +++ /dev/null @@ -1,43 +0,0 @@ -# Copyright 2015 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. - -# This file is meant to be included into a target to provide a rule -# to build APK-based browser test suites. -# -# To use this, create a gyp target with the following form: -# { -# 'target_name': 'test_suite_name_apk', -# 'type': 'none', -# 'variables': { -# 'test_suite_name': 'test_suite_name', # string -# 'java_in_dir': 'path/to/java/dir', -# }, -# 'includes': ['path/to/this/gypi/file'], -# } -# - -{ - 'dependencies': [ - '<(DEPTH)/base/base.gyp:base_java', - '<(DEPTH)/build/android/pylib/device/commands/commands.gyp:chromium_commands', - '<(DEPTH)/build/android/pylib/remote/device/dummy/dummy.gyp:remote_device_dummy_apk', - '<(DEPTH)/testing/android/appurify_support.gyp:appurify_support_java', - '<(DEPTH)/testing/android/native_test.gyp:native_test_java', - '<(DEPTH)/tools/android/android_tools.gyp:android_tools', - ], - 'conditions': [ - ['OS == "android"', { - 'variables': { - # These are used to configure java_apk.gypi included below. - 'apk_name': '<(test_suite_name)', - 'intermediate_dir': '<(PRODUCT_DIR)/<(test_suite_name)_apk', - 'final_apk_path': '<(intermediate_dir)/<(test_suite_name)-debug.apk', - 'native_lib_target': 'lib<(test_suite_name)', - # TODO(yfriedman, cjhopman): Support managed installs for gtests. - 'gyp_managed_install': 0, - }, - 'includes': [ 'java_apk.gypi' ], - }], # 'OS == "android" - ], # conditions -}