Make web_tests runner support the full build dir path on cmd line
When running inside an arbitrarily-named build dir, the web_tests runner only supports the "--build-directory" and "--target" args, with the former pointing to "out/" and the latter pointing to the dir immediately after "out/". But embedding just the name of the second dir breaks the deterministic bot since it builds in two different directories, then diffs them, eg: https://ci.chromium.org/ui/p/chromium/builders/ci/Deterministic%20Linux/50482/infra The answer to that issue for most everything is to use wrapper scripts' templates' "@WrappedPath()" mechanism. This embeds the string "@WrappedPath()" in the generated wrappers, then at test-execution time it gets replaced with the actual build dir. However, those templates don't really support _just_ passing the 2nd dir in the build dir path, which web_tests would need. So instead, this changes the web_tests runner's "--build-directory" arg to point to the full build dir, rather than just "out/". This also updates the args that get bound in the generated wrapper scripts for the web_tests targets. Consequently, anyone running the tests via generated wrapper scripts (ie: bin/run_*) won't notice the change. But anyone running the harness script directly/manually will need to change their cmd-line invocation. This will allow the infra experiment rolled in https://crrev.com/c/5980565 to roll fwd again. Bug: 377545420, 355218109 Change-Id: I054c0e8394bb15cf733ae2866c084941cd1fc722 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5992459 Reviewed-by: Dirk Pranke <dpranke@google.com> Commit-Queue: Ben Pastene <bpastene@chromium.org> Reviewed-by: Weizhong Xia <weizhong@google.com> Cr-Commit-Position: refs/heads/main@{#1382468}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
255ff8556c
commit
8661004b51
13
BUILD.gn
13
BUILD.gn
@ -1132,6 +1132,8 @@ if (use_blink && !is_cronet_build) {
|
||||
_common_web_test_options = [
|
||||
"--no-show-results",
|
||||
"--zero-tests-executed-ok",
|
||||
"--build-directory",
|
||||
"@WrappedPath(.)",
|
||||
]
|
||||
if (is_debug) {
|
||||
_common_web_test_options += [ "--debug" ]
|
||||
@ -1161,15 +1163,6 @@ if (use_blink && !is_cronet_build) {
|
||||
"--out-dir",
|
||||
"@WrappedPath(.)",
|
||||
]
|
||||
} else {
|
||||
base_out_dir = rebase_path(get_path_info(root_build_dir, "dir"), ".")
|
||||
out_dir = get_path_info(root_build_dir, "name")
|
||||
_common_web_test_options += [
|
||||
"--build-directory",
|
||||
base_out_dir,
|
||||
"--target",
|
||||
out_dir,
|
||||
]
|
||||
}
|
||||
|
||||
if (!is_chromeos_ash && !is_ios && !is_fuchsia && !is_android && !is_castos) {
|
||||
@ -1381,6 +1374,8 @@ if (use_blink && !is_cronet_build) {
|
||||
"4",
|
||||
"--debug-rwt-logging",
|
||||
"--clobber-old-results",
|
||||
"--build-directory",
|
||||
"@WrappedPath(.)",
|
||||
]
|
||||
|
||||
# https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_tests.md
|
||||
|
@ -99,20 +99,14 @@ script_test("system_webview_crx_smoke_tests") {
|
||||
_common_web_test_options = [
|
||||
"--no-show-results",
|
||||
"--zero-tests-executed-ok",
|
||||
"--build-directory",
|
||||
"@WrappedPath(.)",
|
||||
]
|
||||
if (is_debug) {
|
||||
_common_web_test_options += [ "--debug" ]
|
||||
} else {
|
||||
_common_web_test_options += [ "--release" ]
|
||||
}
|
||||
base_out_dir = rebase_path(get_path_info(root_build_dir, "dir"), "../../.")
|
||||
out_dir = get_path_info(root_build_dir, "name")
|
||||
_common_web_test_options += [
|
||||
"--build-directory",
|
||||
base_out_dir,
|
||||
"--target",
|
||||
out_dir,
|
||||
]
|
||||
|
||||
script_test("system_webview_wpt") {
|
||||
script = "//third_party/blink/tools/run_wpt_tests.py"
|
||||
|
@ -3362,20 +3362,14 @@ if (current_toolchain == default_toolchain) {
|
||||
_common_web_test_options = [
|
||||
"--no-show-results",
|
||||
"--zero-tests-executed-ok",
|
||||
"--build-directory",
|
||||
"@WrappedPath(.)",
|
||||
]
|
||||
if (is_debug) {
|
||||
_common_web_test_options += [ "--debug" ]
|
||||
} else {
|
||||
_common_web_test_options += [ "--release" ]
|
||||
}
|
||||
base_out_dir = rebase_path(get_path_info(root_build_dir, "dir"), "../../.")
|
||||
out_dir = get_path_info(root_build_dir, "name")
|
||||
_common_web_test_options += [
|
||||
"--build-directory",
|
||||
base_out_dir,
|
||||
"--target",
|
||||
out_dir,
|
||||
]
|
||||
|
||||
script_test("chrome_public_wpt") {
|
||||
script = "//third_party/blink/tools/run_wpt_tests.py"
|
||||
|
@ -2950,10 +2950,12 @@ class Port(object):
|
||||
|
||||
def build_path(self, *comps: str, target: Optional[str] = None):
|
||||
"""Returns a path from the build directory."""
|
||||
return self._filesystem.join(
|
||||
self._path_from_chromium_base(),
|
||||
self.get_option('build_directory') or 'out', target
|
||||
or self._options.target, *comps)
|
||||
build_path = self.get_option('build_directory')
|
||||
if build_path:
|
||||
return self._filesystem.join(self._filesystem.abspath(build_path),
|
||||
*comps)
|
||||
return self._filesystem.join(self._path_from_chromium_base(), 'out',
|
||||
target or self._options.target, *comps)
|
||||
|
||||
def _check_driver_build_up_to_date(self, target):
|
||||
# FIXME: We should probably get rid of this check altogether as it has
|
||||
|
@ -1727,8 +1727,10 @@ class PortTest(LoggingTestCase):
|
||||
# Test for a protected method - pylint: disable=protected-access
|
||||
# Test that optional paths are used regardless of whether they exist.
|
||||
options = optparse.Values({
|
||||
'configuration': 'Release',
|
||||
'build_directory': 'xcodebuild'
|
||||
'configuration':
|
||||
'Release',
|
||||
'build_directory':
|
||||
'/mock-checkout/xcodebuild/Release'
|
||||
})
|
||||
self.assertEqual(
|
||||
self.make_port(options=options).build_path(),
|
||||
|
@ -272,9 +272,10 @@ def add_results_options_group(parser: argparse.ArgumentParser,
|
||||
'--build-directory',
|
||||
metavar='PATH',
|
||||
default='out',
|
||||
help=(
|
||||
'Path to the directory where build files are kept, not including '
|
||||
'configuration. In general this will be "out".'))
|
||||
help=('Full path to the directory where build files are generated. '
|
||||
'Likely similar to "out/some-dir-name/". If not specified, will '
|
||||
'look for a dir under out/ of the same name as the value passed '
|
||||
'to --target.'))
|
||||
results_group.add_argument(
|
||||
'--clobber-old-results',
|
||||
action='store_true',
|
||||
@ -830,13 +831,14 @@ def _update_configuration_and_target(host, options):
|
||||
|
||||
def _read_configuration_from_gn(fs, options):
|
||||
"""Returns the configuration to used based on args.gn, if possible."""
|
||||
build_directory = getattr(options, 'build_directory', 'out')
|
||||
target = options.target
|
||||
build_directory = getattr(options, 'build_directory', None)
|
||||
finder = PathFinder(fs)
|
||||
path = fs.join(finder.chromium_base(), build_directory, target, 'args.gn')
|
||||
if not build_directory:
|
||||
build_directory = fs.join(finder.chromium_base(), 'out',
|
||||
options.target)
|
||||
path = fs.join(build_directory, 'args.gn')
|
||||
if not fs.exists(path):
|
||||
path = fs.join(finder.chromium_base(), build_directory, target,
|
||||
'toolchain.ninja')
|
||||
path = fs.join(build_directory, 'toolchain.ninja')
|
||||
if not fs.exists(path):
|
||||
# This does not appear to be a GN-based build directory, so we don't know
|
||||
# how to interpret it.
|
||||
|
@ -106,7 +106,11 @@ class FactoryTest(unittest.TestCase):
|
||||
factory.PortFactory(host).get(options=options)
|
||||
self.assertEqual(options, optparse.Values())
|
||||
|
||||
def get_port(self, target=None, configuration=None, files=None):
|
||||
def get_port(self,
|
||||
target=None,
|
||||
configuration=None,
|
||||
build_directory=None,
|
||||
files=None):
|
||||
host = MockHost()
|
||||
finder = PathFinder(host.filesystem)
|
||||
files = files or {}
|
||||
@ -114,8 +118,13 @@ class FactoryTest(unittest.TestCase):
|
||||
host.filesystem.write_text_file(
|
||||
finder.path_from_chromium_base(path), contents)
|
||||
options = optparse.Values({
|
||||
'target': target,
|
||||
'configuration': configuration
|
||||
'target':
|
||||
target,
|
||||
'configuration':
|
||||
configuration,
|
||||
'build_directory':
|
||||
finder.path_from_chromium_base(build_directory)
|
||||
if build_directory else None,
|
||||
})
|
||||
return factory.PortFactory(host).get(options=options)
|
||||
|
||||
@ -150,38 +159,45 @@ class FactoryTest(unittest.TestCase):
|
||||
self.assertEqual(port._options.target, 'Release_x64')
|
||||
|
||||
def test_release_args_gn(self):
|
||||
port = self.get_port(
|
||||
target='foo', files={'out/foo/args.gn': 'is_debug = false'})
|
||||
port = self.get_port(target='foo',
|
||||
files={'out/foo/args.gn': 'is_debug = false'},
|
||||
build_directory='out/foo')
|
||||
self.assertEqual(port._options.configuration, 'Release')
|
||||
self.assertEqual(port._options.target, 'foo')
|
||||
|
||||
# Also test that we handle multi-line args files properly.
|
||||
port = self.get_port(
|
||||
target='foo',
|
||||
files={'out/foo/args.gn': 'is_debug = false\nfoo = bar\n'})
|
||||
files={'out/foo/args.gn': 'is_debug = false\nfoo = bar\n'},
|
||||
build_directory='out/foo')
|
||||
self.assertEqual(port._options.configuration, 'Release')
|
||||
self.assertEqual(port._options.target, 'foo')
|
||||
|
||||
port = self.get_port(
|
||||
target='foo',
|
||||
files={'out/foo/args.gn': 'foo=bar\nis_debug=false\n'})
|
||||
files={'out/foo/args.gn': 'foo=bar\nis_debug=false\n'},
|
||||
build_directory='out/foo')
|
||||
self.assertEqual(port._options.configuration, 'Release')
|
||||
self.assertEqual(port._options.target, 'foo')
|
||||
|
||||
def test_debug_args_gn(self):
|
||||
port = self.get_port(
|
||||
target='foo', files={'out/foo/args.gn': 'is_debug = true'})
|
||||
port = self.get_port(target='foo',
|
||||
files={'out/foo/args.gn': 'is_debug = true'},
|
||||
build_directory='out/foo')
|
||||
self.assertEqual(port._options.configuration, 'Debug')
|
||||
self.assertEqual(port._options.target, 'foo')
|
||||
|
||||
def test_default_gn_build(self):
|
||||
port = self.get_port(
|
||||
target='Default', files={'out/Default/toolchain.ninja': ''})
|
||||
port = self.get_port(target='Default',
|
||||
files={'out/Default/toolchain.ninja': ''},
|
||||
build_directory='out/Default')
|
||||
self.assertEqual(port._options.configuration, 'Debug')
|
||||
self.assertEqual(port._options.target, 'Default')
|
||||
|
||||
def test_empty_args_gn(self):
|
||||
port = self.get_port(target='foo', files={'out/foo/args.gn': ''})
|
||||
port = self.get_port(target='foo',
|
||||
files={'out/foo/args.gn': ''},
|
||||
build_directory='out/foo')
|
||||
self.assertEqual(port._options.configuration, 'Debug')
|
||||
self.assertEqual(port._options.target, 'foo')
|
||||
|
||||
@ -197,11 +213,12 @@ class FactoryTest(unittest.TestCase):
|
||||
|
||||
def test_both_configuration_and_target_is_an_error(self):
|
||||
with self.assertRaises(ValueError):
|
||||
self.get_port(
|
||||
target='Debug',
|
||||
configuration='Release',
|
||||
files={'out/Debug/toolchain.ninja': ''})
|
||||
self.get_port(target='Debug',
|
||||
configuration='Release',
|
||||
files={'out/Debug/toolchain.ninja': ''},
|
||||
build_directory='out/Debug')
|
||||
|
||||
def test_no_target_has_correct_config(self):
|
||||
port = self.get_port(files={'out/Release/args.gn': 'is_debug = true'})
|
||||
port = self.get_port(files={'out/Release/args.gn': 'is_debug = true'},
|
||||
build_directory='out/Release')
|
||||
self.assertEqual(port._options.configuration, 'Debug')
|
||||
|
@ -397,8 +397,10 @@ class WPTAdapterTest(unittest.TestCase):
|
||||
self.assertEqual(os.environ['NEW_ENV_VAR'], 'new_env_var_value')
|
||||
|
||||
def test_show_results(self):
|
||||
adapter = WPTAdapter.from_args(
|
||||
self.host, ['--product=headless_shell', '--no-manifest-update'])
|
||||
adapter = WPTAdapter.from_args(self.host, [
|
||||
'--product=headless_shell', '--no-manifest-update',
|
||||
'--build-directory=/mock-checkout/out/Release'
|
||||
])
|
||||
post_run_tasks = mock.Mock()
|
||||
self._mocks.enter_context(
|
||||
mock.patch('blinkpy.web_tests.port.base.Port.clean_up_test_run',
|
||||
|
Reference in New Issue
Block a user