From 1590c4ca9e93750011a1dcbe95ec065e8e31f696 Mon Sep 17 00:00:00 2001 From: agrieve Date: Mon, 4 Apr 2016 19:03:45 -0700 Subject: [PATCH] Reland 2 of GN: Make breakpad_unittests & sandbox_linux_unittests use test() This simplifies build rules for native tests, and allows us to get rid of ${target}_deps targets (once recipes are updated). This change fixes the generated wrapper scripts, which didn't work. TBR=jbudorick BUG=589318 Review URL: https://codereview.chromium.org/1854233002 Cr-Original-Commit-Position: refs/heads/master@{#385084} Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src Cr-Mirrored-Commit: 62ab0028a0165eecd60568783531d1ad3d031a26 --- android/native_app_dependencies.gypi | 9 +++ android/pylib/gtest/gtest_test_instance.py | 21 ++++--- .../local/device/local_device_gtest_run.py | 32 ++++------ android/test_runner.py | 3 + config/android/internal_rules.gni | 20 +++++-- config/android/rules.gni | 60 +++++++------------ 6 files changed, 76 insertions(+), 69 deletions(-) diff --git a/android/native_app_dependencies.gypi b/android/native_app_dependencies.gypi index f74e7aee8..4651ac36e 100644 --- a/android/native_app_dependencies.gypi +++ b/android/native_app_dependencies.gypi @@ -27,6 +27,7 @@ { 'variables': { 'include_main_binary%': 1, + 'extra_files%': [], }, 'conditions': [ ['android_must_copy_system_libraries == 1', { @@ -55,6 +56,14 @@ }, ], }], + ['extra_files!=[]', { + 'copies': [ + { + 'destination': '<(output_dir)', + 'files': [ '<@(extra_files)' ], + } + ], + }], ['include_main_binary==1', { 'copies': [ { diff --git a/android/pylib/gtest/gtest_test_instance.py b/android/pylib/gtest/gtest_test_instance.py index f5b2c69ad..32911168e 100644 --- a/android/pylib/gtest/gtest_test_instance.py +++ b/android/pylib/gtest/gtest_test_instance.py @@ -144,9 +144,18 @@ class GtestTestInstance(test_instance.TestInstance): self._shard_timeout = args.shard_timeout self._skip_clear_data = args.skip_clear_data self._suite = args.suite_name[0] + self._exe_dist_dir = None - self._exe_path = os.path.join(constants.GetOutDirectory(), - self._suite) + # GYP: + if args.executable_dist_dir: + self._exe_dist_dir = os.path.abspath(args.executable_dist_dir) + else: + # TODO(agrieve): Remove auto-detection once recipes pass flag explicitly. + exe_dist_dir = os.path.join(constants.GetOutDirectory(), + '%s__dist' % self._suite) + + if os.path.exists(exe_dist_dir): + self._exe_dist_dir = exe_dist_dir incremental_part = '' if args.test_apk_incremental_install_script: @@ -171,9 +180,7 @@ class GtestTestInstance(test_instance.TestInstance): self._extras[EXTRA_SHARD_NANO_TIMEOUT] = int(1e9 * self._shard_timeout) self._shard_timeout = 900 - if not os.path.exists(self._exe_path): - self._exe_path = None - if not self._apk_helper and not self._exe_path: + if not self._apk_helper and not self._exe_dist_dir: error_func('Could not find apk or executable for %s' % self._suite) self._data_deps = [] @@ -234,8 +241,8 @@ class GtestTestInstance(test_instance.TestInstance): return self._app_data_files @property - def exe(self): - return self._exe_path + def exe_dist_dir(self): + return self._exe_dist_dir @property def extras(self): diff --git a/android/pylib/local/device/local_device_gtest_run.py b/android/pylib/local/device/local_device_gtest_run.py index a745751d7..8172790fb 100644 --- a/android/pylib/local/device/local_device_gtest_run.py +++ b/android/pylib/local/device/local_device_gtest_run.py @@ -161,26 +161,18 @@ class _ApkDelegate(object): class _ExeDelegate(object): - def __init__(self, tr, exe): - self._exe_host_path = exe - self._exe_file_name = os.path.split(exe)[-1] - self._exe_device_path = '%s/%s' % ( - constants.TEST_EXECUTABLE_DIR, self._exe_file_name) - deps_host_path = self._exe_host_path + '_deps' - if os.path.exists(deps_host_path): - self._deps_host_path = deps_host_path - self._deps_device_path = self._exe_device_path + '_deps' - else: - self._deps_host_path = None + def __init__(self, tr, dist_dir): + self._host_dist_dir = dist_dir + self._exe_file_name = os.path.basename(dist_dir)[:-len('__dist')] + self._device_dist_dir = posixpath.join( + constants.TEST_EXECUTABLE_DIR, os.path.basename(dist_dir)) self._test_run = tr def Install(self, device): # TODO(jbudorick): Look into merging this with normal data deps pushing if # executables become supported on nonlocal environments. - host_device_tuples = [(self._exe_host_path, self._exe_device_path)] - if self._deps_host_path: - host_device_tuples.append((self._deps_host_path, self._deps_device_path)) - device.PushChangedFiles(host_device_tuples) + device.PushChangedFiles([(self._host_dist_dir, self._device_dist_dir)], + delete_device_stale=True) def Run(self, test, device, flags=None, **kwargs): tool = self._test_run.GetTool(device).GetTestWrapper() @@ -188,17 +180,17 @@ class _ExeDelegate(object): cmd = [tool] else: cmd = [] - cmd.append(self._exe_device_path) + cmd.append(posixpath.join(self._device_dist_dir, self._exe_file_name)) if test: cmd.append('--gtest_filter=%s' % ':'.join(test)) if flags: + # TODO(agrieve): This won't work if multiple flags are passed. cmd.append(flags) cwd = constants.TEST_EXECUTABLE_DIR env = { - 'LD_LIBRARY_PATH': - '%s/%s_deps' % (constants.TEST_EXECUTABLE_DIR, self._exe_file_name), + 'LD_LIBRARY_PATH': self._device_dist_dir } try: gcov_strip_depth = os.environ['NATIVE_COVERAGE_DEPTH_STRIP'] @@ -228,8 +220,8 @@ class LocalDeviceGtestRun(local_device_test_run.LocalDeviceTestRun): if self._test_instance.apk: self._delegate = _ApkDelegate(self._test_instance) - elif self._test_instance.exe: - self._delegate = _ExeDelegate(self, self._test_instance.exe) + elif self._test_instance.exe_dist_dir: + self._delegate = _ExeDelegate(self, self._test_instance.exe_dist_dir) self._crashes = set() self._servers = collections.defaultdict(list) diff --git a/android/test_runner.py b/android/test_runner.py index e55f8c0cd..5edc6da49 100755 --- a/android/test_runner.py +++ b/android/test_runner.py @@ -249,6 +249,9 @@ def AddGTestOptions(parser): group.add_argument('-s', '--suite', dest='suite_name', nargs='+', metavar='SUITE_NAME', required=True, help='Executable name of the test suite to run.') + group.add_argument('--executable-dist-dir', + help="Path to executable's dist directory for native" + " (non-apk) tests.") group.add_argument('--test-apk-incremental-install-script', help='Path to install script for the test apk.') group.add_argument('--gtest_also_run_disabled_tests', diff --git a/config/android/internal_rules.gni b/config/android/internal_rules.gni index 06dbfb77f..65c42427c 100644 --- a/config/android/internal_rules.gni +++ b/config/android/internal_rules.gni @@ -2070,22 +2070,32 @@ template("test_runner_script") { "//build/android:test_runner_py", ] + test_runner_args = [ + _test_type, + "--output-directory", + rebase_path(root_build_dir, root_build_dir), + ] + # apk_target is not used for native executable tests # (e.g. breakpad_unittests). if (defined(invoker.apk_target)) { + assert(!defined(invoker.executable_dist_dir)) deps += [ "${invoker.apk_target}__build_config" ] _apk_build_config = get_label_info(invoker.apk_target, "target_gen_dir") + "/" + get_label_info(invoker.apk_target, "name") + ".build_config" _rebased_apk_build_config = rebase_path(_apk_build_config, root_build_dir) assert(_rebased_apk_build_config != "") # Mark as used. + } else if (_test_type == "gtest") { + assert( + defined(invoker.executable_dist_dir), + "Must define either apk_target or executable_dist_dir for test_runner_script()") + test_runner_args += [ + "--executable-dist-dir", + rebase_path(invoker.executable_dist_dir, root_build_dir), + ] } - test_runner_args = [ - _test_type, - "--output-directory", - rebase_path(root_build_dir, root_build_dir), - ] if (_test_type == "gtest") { assert(defined(invoker.test_suite)) test_runner_args += [ diff --git a/config/android/rules.gni b/config/android/rules.gni index 14896fcda..ed6317250 100644 --- a/config/android/rules.gni +++ b/config/android/rules.gni @@ -2234,7 +2234,7 @@ template("android_aidl") { # dist_dir: Directory for the exe and libraries. Everything in this directory # will be deleted before copying in the exe and libraries. # binary: Path to (stripped) executable. -# include_main_binary: Whether |binary| should be copied to |dist_dir|. +# extra_files: List of extra files to copy in (optional). # # Example # create_native_executable_dist("foo_dist") { @@ -2243,34 +2243,27 @@ template("android_aidl") { # deps = [ ":the_thing_that_makes_foo" ] # } template("create_native_executable_dist") { - set_sources_assignment_filter([]) forward_variables_from(invoker, [ "testonly" ]) - dist_dir = invoker.dist_dir - binary = invoker.binary - template_name = target_name + _libraries_list = "${target_gen_dir}/${target_name}_library_dependencies.list" - libraries_list = - "${target_gen_dir}/${template_name}_library_dependencies.list" + _find_deps_target_name = "${target_name}__find_library_dependencies" - find_deps_target_name = "${template_name}__find_library_dependencies" - copy_target_name = "${template_name}__copy_libraries_and_exe" - - action(find_deps_target_name) { + # TODO(agrieve): Extract dependent libs from GN rather than readelf. + action(_find_deps_target_name) { forward_variables_from(invoker, [ "deps" ]) - visibility = [ ":$copy_target_name" ] script = "//build/android/gyp/write_ordered_libraries.py" depfile = "$target_gen_dir/$target_name.d" inputs = [ - binary, + invoker.binary, android_readelf, ] outputs = [ depfile, - libraries_list, + _libraries_list, ] - rebased_binaries = rebase_path([ binary ], root_build_dir) + rebased_binaries = rebase_path([ invoker.binary ], root_build_dir) args = [ "--depfile", rebase_path(depfile, root_build_dir), @@ -2278,47 +2271,40 @@ template("create_native_executable_dist") { "--libraries-dir", rebase_path(root_shlib_dir, root_build_dir), "--output", - rebase_path(libraries_list, root_build_dir), + rebase_path(_libraries_list, root_build_dir), "--readelf", rebase_path(android_readelf, root_build_dir), ] } - copy_ex(copy_target_name) { - visibility = [ ":$template_name" ] - + copy_ex(target_name) { clear_dir = true inputs = [ - libraries_list, + _libraries_list, + invoker.binary, ] - if (defined(invoker.include_main_binary) && invoker.include_main_binary) { - inputs += [ binary ] - } - dest = dist_dir + dest = invoker.dist_dir - rebased_libraries_list = rebase_path(libraries_list, root_build_dir) - args = [ "--files=@FileArg($rebased_libraries_list:lib_paths)" ] - if (defined(invoker.include_main_binary) && invoker.include_main_binary) { - rebased_binaries_list = rebase_path([ binary ], root_build_dir) - args += [ "--files=$rebased_binaries_list" ] + _rebased_libraries_list = rebase_path(_libraries_list, root_build_dir) + _rebased_binaries_list = rebase_path([ invoker.binary ], root_build_dir) + args = [ + "--files=@FileArg($_rebased_libraries_list:lib_paths)", + "--files=$_rebased_binaries_list", + ] + if (defined(invoker.extra_files)) { + _rebased_extra_files = rebase_path(invoker.extra_files, root_build_dir) + args += [ "--files=$_rebased_extra_files" ] } deps = [ - ":$find_deps_target_name", + ":$_find_deps_target_name", ] if (defined(invoker.deps)) { deps += invoker.deps } } - - group(template_name) { - forward_variables_from(invoker, [ "visibility" ]) - public_deps = [ - ":$copy_target_name", - ] - } } # Compile a protocol buffer to java.