Skip to content

GitLab

T
tangled
Commit d053575d authored by Sylvain Defresne's avatar Sylvain Defresne Committed by Commit Bot
Browse files
Options

Revert "generate_wrapper targets for iOS' non-EG/EG2 tests, EG, and EG2 tests" 

This reverts commit 4ed2234c.

Reason for revert: causes `gn gen --ide=xcode` to fails with error code 250 when targeting iOS (this is caused by an assertion failure in gn in debug mode resulting in a crash in release),

Original change's description:
> generate_wrapper targets for iOS' non-EG/EG2 tests, EG, and EG2 tests
> 
> The existing iOS recipe utilizes an isolate template, and an
> isolated.gen.json file to fill the template in per test with
> config-variables and path-variables. This isolate template contains
> the command arguments to invoke the iOS test runner.
> 
> To support the iOS recipe migration to the Chromium recipe, I use GN to
> generate a script per test target instead of the isolate template. The
> script is pre-baked with arguments to the test runner (run.py). The
> given Swarming task is expected to execute the test by invoking the
> generated script.
> 
> Invoking the generated script requires changes to gn_isolate_map.pyl
> for each iOS test target, and thus, these changes are only additive.
> 
> * "ios_test_runner_wrapper" template wraps the generate_wrapper target.
>  It bakes iOS specific arguments into the script, such that it can
>  launch run.py with the given args.
> * ios_eg2_test, ios_eg_test, and test templates have
>  been updated to utilize the ios_test_runner_wrapper for iOS targets.
> * mb.py to skip generated_directory check when writing isolates.
>  View crbug/ in comments.
> 
> Change-Id: I941d37dfdff0bd1ead1baff70907a197fee3fe64
> Bug: 1055328,912681
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2023748
> Commit-Queue: Jeff Yoon <jeffyoon@chromium.org>
> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
> Reviewed-by: John Budorick <jbudorick@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#745105}

TBR=rohitrao@chromium.org,sdefresne@chromium.org,eugenebut@chromium.org,jbudorick@chromium.org,jeffyoon@chromium.org

Change-Id: I649d1051b276738c87ea54633e447ebdadad539f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1055328, 912681
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2078260Reviewed-by: default avatarSylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745289}
parent c9bd5e63
Hide whitespace changes
Inline Side-by-side
Showing with 8 additions and 303 deletions
build/config/ios/ios_test_runner_config.gni deleted 100644 → 0
View file @ c9bd5e63
# Copyright 2020 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.
declare_args() {
# Controls what version of xcode to use for iOS testers. Note that this
# version should be in alignment with the swarming named caches used by tests,
# defined under src/testing/buildbot/ to avoid performance issues.
test_runner_xcode_build_version = "11c29"
}
build/config/ios/ios_test_runner_wrapper.gni deleted 100644 → 0
View file @ c9bd5e63
# Copyright 2020 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.
import("//build/config/ios/ios_sdk.gni")
import("//build/config/ios/ios_test_runner_config.gni")
import("//build/util/generate_wrapper.gni")
# Invokes generate_wrapper to create an executable script wrapping iOS'
# run.py with baked in arguments. Only takes effect when test entry in
# gn_isolate_map.pyl is updated to type="generated_script" with script
# set to the wrapper output path.
#
# Arguments:
#
# data
# (optional, default [ "//ios/build/bots/scripts/" ]) list of files or
# directories required to run target
#
# data_deps
# (optional) list of target non-linked labels
#
# deps
# (optional) list of files or directories required to run target
#
# executable_args
# (optional) a list of string arguments to pass to run.py
#
# retries
# (optional, default 3) number of retry attempts
#
# shards
# (optional, default 1) number of shards to execute tests in parallel. not
# the same as swamring shards.
#
# wrapper_output_name
# (optional, default "run_${target_name}") name of the wrapper script
#
template("ios_test_runner_wrapper") {
generate_wrapper(target_name) {
forward_variables_from(invoker,
[
"data",
"data_deps",
"deps",
"executable_args",
"retries",
"shards",
"wrapper_output_name",
])
testonly = true
# iOS main test runner
executable = "//ios/build/bots/scripts/run.py"
# arguments passed to run.py
if (!defined(executable_args)) {
executable_args = []
}
_rebased_mac_toolchain = rebase_path("//mac_toolchain", root_build_dir)
_rebased_xcode_path = rebase_path("//Xcode.app", root_build_dir)
# --out-dir argument is specified in gn_isolate_map.pyl because
# ${ISOLATED_OUTDIR} doesn't get resolved through this wrapper.
executable_args += [
"--xcode-path",
"@WrappedPath(${_rebased_xcode_path})",
"--mac-toolchain-cmd",
"@WrappedPath(${_rebased_mac_toolchain})",
]
executable_args += [
"--xcode-build-version",
test_runner_xcode_build_version,
]
# Default retries to 3
if (!defined(retries)) {
retries = 3
}
executable_args += [
"--retries",
"${retries}",
]
# Default shards to 1
if (!defined(shards)) {
shards = 1
}
executable_args += [
"--shards",
"${shards}",
]
# test runner relies on iossim if use_ios_simulator (defined in ios_sdk.gni)
if (use_ios_simulator) {
_rebased_root_build_dir = rebase_path("${root_build_dir}", root_build_dir)
if (!defined(data_deps)) {
data_deps = []
}
data_deps += [ "//testing/iossim" ]
executable_args += [
"--iossim",
"@WrappedPath(${_rebased_root_build_dir}/iossim)",
]
}
# wrapper script output name and path
if (!defined(wrapper_output_name)) {
_wrapper_output_name = "run_${target_name}"
} else {
_wrapper_output_name = wrapper_output_name
}
wrapper_script = "${root_build_dir}/bin/${_wrapper_output_name}"
# ios/build/bot/scripts/*.py needs to be present for test runner
if (!defined(data)) {
data = []
}
data += [ "//ios/build/bots/scripts/" ]
}
}
build/config/ios/rules.gni
View file @ d053575d
...@@ -1971,14 +1971,10 @@ template("ios_xcuitest_test") { ...@@ -1971,14 +1971,10 @@ template("ios_xcuitest_test") {
"xcode_test_application_name must be defined for $target_name") "xcode_test_application_name must be defined for $target_name")
_xcuitest_target = target_name _xcuitest_target = target_name
if (defined(invoker.output_name)) {
_xcuitest_target = invoker.output_name
}
_xcuitest_runner_target = _xcuitest_target + "_runner" _xcuitest_runner_target = _xcuitest_target + "_runner"
_xcuitest_module_target = _xcuitest_target + "_module" _xcuitest_module_target = _xcuitest_target + "_module"
group(target_name) { group(_xcuitest_target) {
testonly = true testonly = true
deps = [ ":$_xcuitest_runner_target" ] deps = [ ":$_xcuitest_runner_target" ]
......
ios/chrome/test/earl_grey2/chrome_ios_eg2_test.gni
View file @ d053575d
...@@ -137,12 +137,9 @@ template("chrome_ios_eg2_test") { ...@@ -137,12 +137,9 @@ template("chrome_ios_eg2_test") {
ios_eg2_test(target_name) { ios_eg2_test(target_name) {
forward_variables_from(invoker, forward_variables_from(invoker,
[ [
"xcode_test_application_name",
"deps", "deps",
"data_deps", "data_deps",
"executable_args",
"retries",
"shards",
"xcode_test_application_name",
]) ])
xctest_bundle_principal_class = "ChromeEGTestBundleMain" xctest_bundle_principal_class = "ChromeEGTestBundleMain"
......
ios/third_party/earl_grey/ios_eg_test.gni
View file @ d053575d
...@@ -2,25 +2,14 @@ ...@@ -2,25 +2,14 @@
# Use of this source code is governed by a BSD-style license that can be # Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file. # found in the LICENSE file.
import("//build/config/ios/ios_test_runner_wrapper.gni")
import("//build/config/ios/rules.gni") import("//build/config/ios/rules.gni")
import("//ios/build/chrome_build.gni") import("//ios/build/chrome_build.gni")
import("//ios/public/provider/chrome/browser/build_config.gni") import("//ios/public/provider/chrome/browser/build_config.gni")
# EarlGrey tests are just XCTests that also depends on EarlGrey. # EarlGrey tests are just XCTests that also depends on EarlGrey.
template("ios_eg_test") { template("ios_eg_test") {
_target_name = target_name ios_xctest_test(target_name) {
_test_target = "${target_name}_test" forward_variables_from(invoker, "*")
ios_xctest_test(_test_target) {
forward_variables_from(invoker,
"*",
[
"executable_args",
"retries",
"shards",
"xctest",
"xcode_parallelization",
])
if (!defined(bundle_deps)) { if (!defined(bundle_deps)) {
bundle_deps = [] bundle_deps = []
} }
...@@ -39,55 +28,6 @@ template("ios_eg_test") { ...@@ -39,55 +28,6 @@ template("ios_eg_test") {
"//ios/third_party/earl_grey:earl_grey+link", "//ios/third_party/earl_grey:earl_grey+link",
"//ios/third_party/ochamcrest:ochamcrest+link", "//ios/third_party/ochamcrest:ochamcrest+link",
] ]
# TODO(crbug.com/1056328) Because we change the target name, the subnodes
# are going to append with the _test in the naming, which won't be backwards
# compatible during migration from iOS recipe to Chromium.
output_name = "${_target_name}"
}
ios_test_runner_wrapper(target_name) {
forward_variables_from(invoker,
[
"data",
"data_deps",
"deps",
"executable_args",
"retries",
"shards",
"xctest",
"xcode_parallelization",
])
_root_build_dir = rebase_path("${root_build_dir}", root_build_dir)
# include the test target above as data_deps
if (!defined(data_deps)) {
data_deps = []
}
data_deps += [ ":${_test_target}" ]
if (!defined(executable_args)) {
executable_args = []
}
# EG test apps are *.app format. No host, but required xctest and may need
# xcode-parallelization
executable_args += [
"--app",
"@WrappedPath(${_root_build_dir}/${target_name}.app)",
]
# Default xctest to true for EG tests. If set, only set arg if value = true
if (!defined(xctest) || (defined(xctest) && xctest)) {
executable_args += [ "--xctest" ]
}
# Default xcode_parallelization to true for EG tests.
if (!defined(xcode_parallelization) ||
(defined(xcode_parallelization) && xcode_parallelization)) {
executable_args += [ "--xcode-parallelization" ]
}
} }
} }
......
ios/third_party/earl_grey2/ios_eg2_test.gni
View file @ d053575d
...@@ -2,7 +2,6 @@ ...@@ -2,7 +2,6 @@
# Use of this source code is governed by a BSD-style license that can be # Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file. # found in the LICENSE file.
import("//build/config/ios/ios_test_runner_wrapper.gni")
import("//build/config/ios/rules.gni") import("//build/config/ios/rules.gni")
import("//ios/build/chrome_build.gni") import("//ios/build/chrome_build.gni")
import("//ios/public/provider/chrome/browser/build_config.gni") import("//ios/public/provider/chrome/browser/build_config.gni")
...@@ -64,9 +63,7 @@ template("ios_eg2_test") { ...@@ -64,9 +63,7 @@ template("ios_eg2_test") {
defined(invoker.deps), defined(invoker.deps),
"deps must be defined for $target_name to include at least one earl grey test file.") "deps must be defined for $target_name to include at least one earl grey test file.")
_target_name = target_name ios_xcuitest_test(target_name) {
_test_target = "${target_name}_test"
ios_xcuitest_test(_test_target) {
forward_variables_from(invoker, forward_variables_from(invoker,
[ [
"xcode_test_application_name", "xcode_test_application_name",
...@@ -80,51 +77,6 @@ template("ios_eg2_test") { ...@@ -80,51 +77,6 @@ template("ios_eg2_test") {
deps = [] deps = []
} }
deps += [ "//ios/third_party/earl_grey2:test_lib" ] deps += [ "//ios/third_party/earl_grey2:test_lib" ]
# TODO(crbug.com/1056328) Because we change the target name, the subnodes
# are going to append with the _test in the naming, which won't be backwards
# compatible during migration from iOS recipe to Chromium.
output_name = "${_target_name}"
}
ios_test_runner_wrapper(target_name) {
forward_variables_from(invoker,
[
"data",
"data_deps",
"deps",
"executable_args",
"retries",
"shards",
"xcode_test_application_name",
])
_root_build_dir = rebase_path("${root_build_dir}", root_build_dir)
if (!defined(data_deps)) {
data_deps = []
}
# Include the top ios_eg2_test target, and the host app
data_deps += [ ":${_test_target}" ]
if (!defined(executable_args)) {
executable_args = []
}
# EG2 tests app are bundled as *-Runner.app, while the host app is bundled
# as *.app.
executable_args += [
"--app",
"@WrappedPath(${_root_build_dir}/${target_name}-Runner.app)",
]
executable_args += [
"--host-app",
"@WrappedPath(${_root_build_dir}/${xcode_test_application_name}.app)",
]
# All EG2 tests require xcode-parallelization. EG2 doesn't use xctest, so we
# leave undefined
executable_args += [ "--xcode-parallelization" ]
} }
} }
......
testing/iossim/BUILD.gn
View file @ d053575d
...@@ -2,7 +2,6 @@ ...@@ -2,7 +2,6 @@
# Use of this source code is governed by a BSD-style license that can be # Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file. # found in the LICENSE file.
import("//build/config/ios/ios_sdk.gni")
import("//build/config/mac/mac_sdk.gni") import("//build/config/mac/mac_sdk.gni")
if (current_toolchain == host_toolchain) { if (current_toolchain == host_toolchain) {
...@@ -17,8 +16,4 @@ if (current_toolchain == host_toolchain) { ...@@ -17,8 +16,4 @@ if (current_toolchain == host_toolchain) {
sources = [ get_label_info(":iossim($host_toolchain)", "root_out_dir") + sources = [ get_label_info(":iossim($host_toolchain)", "root_out_dir") +
"/iossim" ] "/iossim" ]
} }
} else {
group("iossim") {
public_deps = [ ":iossim($default_toolchain)" ]
}
} }
testing/test.gni
View file @ d053575d
...@@ -23,12 +23,6 @@ if (is_chromeos) { ...@@ -23,12 +23,6 @@ if (is_chromeos) {
import("//build/config/chromeos/rules.gni") import("//build/config/chromeos/rules.gni")
} }
if (is_ios) {
import("//build/config/ios/ios_sdk.gni")
import("//build/config/ios/ios_test_runner_wrapper.gni")
import("//build/config/ios/rules.gni")
}
# Define a test as an executable (or apk on Android) with the "testonly" flag # Define a test as an executable (or apk on Android) with the "testonly" flag
# set. # set.
# Variable: # Variable:
...@@ -233,6 +227,9 @@ template("test") { ...@@ -233,6 +227,9 @@ template("test") {
output_name = _exec_target output_name = _exec_target
} }
} else if (is_ios) { } else if (is_ios) {
import("//build/config/ios/ios_sdk.gni")
import("//build/config/ios/rules.gni")
declare_args() { declare_args() {
# Keep the unittest-as-xctest functionality defaulted to off until the # Keep the unittest-as-xctest functionality defaulted to off until the
# bots are updated to handle it properly. # bots are updated to handle it properly.
...@@ -242,31 +239,6 @@ template("test") { ...@@ -242,31 +239,6 @@ template("test") {
} }
_test_target = target_name _test_target = target_name
_wrapper_output_name = "run_${target_name}"
ios_test_runner_wrapper(_wrapper_output_name) {
forward_variables_from(invoker,
[
"data",
"data_deps",
"deps",
"executable_args",
"retries",
"shards",
])
_root_build_dir = rebase_path("${root_build_dir}", root_build_dir)
if (!defined(executable_args)) {
executable_args = []
}
executable_args += [
"--app",
"@WrappedPath(${_root_build_dir}/${_test_target}.app)",
]
wrapper_output_name = "${_wrapper_output_name}"
}
_resources_bundle_data = target_name + "_resources_bundle_data" _resources_bundle_data = target_name + "_resources_bundle_data"
bundle_data(_resources_bundle_data) { bundle_data(_resources_bundle_data) {
...@@ -309,13 +281,6 @@ template("test") { ...@@ -309,13 +281,6 @@ template("test") {
bundle_deps = [] bundle_deps = []
} }
bundle_deps += [ ":$_resources_bundle_data" ] bundle_deps += [ ":$_resources_bundle_data" ]
if (!defined(data_deps)) {
data_deps = []
}
# Include the generate_wrapper as part of data_deps
data_deps += [ ":${_wrapper_output_name}" ]
} }
} else if (is_chromeos && cros_board != "") { } else if (is_chromeos && cros_board != "") {
# Building for a cros board (ie: not linux-chromeos). # Building for a cros board (ie: not linux-chromeos).
......
tools/mb/mb.py
View file @ d053575d
...@@ -1310,7 +1310,6 @@ class MetaBuildWrapper(object): ...@@ -1310,7 +1310,6 @@ class MetaBuildWrapper(object):
vals.get('cros_passthrough', False)) vals.get('cros_passthrough', False))
is_mac = self.platform == 'darwin' is_mac = self.platform == 'darwin'
is_msan = 'is_msan=true' in vals['gn_args'] is_msan = 'is_msan=true' in vals['gn_args']
is_ios = 'target_os="ios"' in vals['gn_args']
err = '' err = ''
for f in files: for f in files:
...@@ -1320,11 +1319,6 @@ class MetaBuildWrapper(object): ...@@ -1320,11 +1319,6 @@ class MetaBuildWrapper(object):
if is_android: if is_android:
break break
# iOS has generated directories in gn data items.
# Skipping for iOS instead of listing all apps.
if is_ios:
break
# Skip a few existing violations that need to be cleaned up. Each of # Skip a few existing violations that need to be cleaned up. Each of
# these will lead to incorrect incremental builds if their directory # these will lead to incorrect incremental builds if their directory
# contents change. Do not add to this list. # contents change. Do not add to this list.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment