Revert "Switch the generators to generate TestSpecs."
This reverts commit 12a75338de3b4d4af67963ef4ffece4c06889441.
Reason for revert: Breaks perf builders e.g. https://ci.chromium.org/p/chrome/builders/ci/mac-10_13_laptop_high_end-perf/11136
Original change's description:
> Switch the generators to generate TestSpecs.
>
> Generating TestSpecs instead of tests directly will allow for comparing
> the test specs that are created from the source side specs with those
> declared within a BotSpec in order to track and verify the migration of
> tests to the source side specs.
>
> Bug: 1128746
> Change-Id: I0d8c3479c54aa8a791cca8bb13cec7b68522f910
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/build/+/2454499
> Commit-Queue: Garrett Beaty <gbeaty@chromium.org>
> Auto-Submit: Garrett Beaty <gbeaty@chromium.org>
> Reviewed-by: Stephen Martinis <martiniss@chromium.org>
TBR=martiniss@chromium.org,gbeaty@chromium.org,infra-scoped@luci-project-accounts.iam.gserviceaccount.com
Change-Id: Ia94b1ae11de5e859243f6420eb18e04ca30ca86c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1128746
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/build/+/2462016
Reviewed-by: Garrett Beaty <gbeaty@chromium.org>
Commit-Queue: Garrett Beaty <gbeaty@chromium.org>
diff --git a/recipes/recipe_modules/chromium_tests/api.py b/recipes/recipe_modules/chromium_tests/api.py
index 7e1112c..a6d0dbf 100644
--- a/recipes/recipe_modules/chromium_tests/api.py
+++ b/recipes/recipe_modules/chromium_tests/api.py
@@ -245,19 +245,18 @@
scripts_compile_targets_fn,
bot_update_step):
tests = [s.get_test() for s in bot_spec.test_specs]
- # TODO(https://crbug.com/1128746): Once all test specs are migrated to the
- # source side spec files, switch the generators to return tests directly
+ # TODO(phajdan.jr): Switch everything to scripts generators and simplify.
for generator in generators.ALL_GENERATORS:
- test_specs = generator(
- self.m,
- self,
- builder_group,
- buildername,
- source_side_spec,
- bot_update_step,
- swarming_dimensions=swarming_dimensions,
- scripts_compile_targets_fn=scripts_compile_targets_fn)
- tests.extend(s.get_test() for s in test_specs)
+ tests.extend(
+ generator(
+ self.m,
+ self,
+ builder_group,
+ buildername,
+ source_side_spec,
+ bot_update_step,
+ swarming_dimensions=swarming_dimensions,
+ scripts_compile_targets_fn=scripts_compile_targets_fn))
return tuple(tests)
def read_source_side_spec(self, source_side_spec_file):
diff --git a/recipes/recipe_modules/chromium_tests/generators.py b/recipes/recipe_modules/chromium_tests/generators.py
index 7442206..ce7330b 100644
--- a/recipes/recipe_modules/chromium_tests/generators.py
+++ b/recipes/recipe_modules/chromium_tests/generators.py
@@ -5,11 +5,8 @@
import string
import textwrap
-from . import bot_spec
from . import steps
-from RECIPE_MODULES.build.attr_utils import attrib, attrs
-
def get_args_for_test(api, chromium_tests_api, test_spec, bot_update_step):
"""Gets the argument list for a dynamically generated test, as
@@ -97,7 +94,7 @@
return [string.Template(arg).safe_substitute(substitutions) for arg in args]
-def generator_common(api, raw_test_spec, swarming_delegate, local_delegate,
+def generator_common(api, spec, swarming_delegate, local_delegate,
swarming_dimensions):
"""Common logic for generating tests from JSON specs.
@@ -107,46 +104,28 @@
local_delgate: function to call to create a local test.
Yields:
- instances of bot_spec.TestSpec.
+ instances of Test.
"""
+ tests = []
kwargs = {}
- target_name = raw_test_spec.get('test') or raw_test_spec.get('isolate_name')
- name = raw_test_spec.get('name', target_name)
-
- wrap_test_spec = lambda s: s
- experiment_percentage = raw_test_spec.get('experiment_percentage')
- if experiment_percentage is not None:
- experiment_percentage = int(experiment_percentage)
-
- @attrs()
- class ExperimentalTestSpec(object):
-
- _test_spec = attrib(bot_spec.TestSpec)
- _experiment_percentage = attrib(int)
-
- def get_test(self):
- return steps.ExperimentalTest(self._test_spec.get_test(),
- self._experiment_percentage, api)
-
- wrap_test_spec = lambda s: ExperimentalTestSpec(
- test_spec=s, experiment_percentage=experiment_percentage)
+ target_name = spec.get('test') or spec.get('isolate_name')
+ name = spec.get('name', target_name)
kwargs['target_name'] = target_name
# TODO(crbug.com/1074033): Remove full_test_target.
- kwargs['full_test_target'] = raw_test_spec.get('test_target')
- kwargs['test_id_prefix'] = raw_test_spec.get('test_id_prefix')
- kwargs['resultdb'] = raw_test_spec.get('resultdb')
+ kwargs['full_test_target'] = spec.get('test_target')
+ kwargs['test_id_prefix'] = spec.get('test_id_prefix')
kwargs['name'] = name
# Enables resultdb if the build is picked for the experiment.
# TODO(crbug.com/1108016): Enable resultdb globally.
is_sink_exp = ('chromium.resultdb.result_sink' in
api.buildbucket.build.input.experiments)
- kwargs['resultdb'] = raw_test_spec.get('resultdb') if is_sink_exp else None
+ kwargs['resultdb'] = spec.get('resultdb') if is_sink_exp else None
- set_up = list(raw_test_spec.get('setup', []))
+ set_up = list(spec.get('setup', []))
processed_set_up = []
for set_up_step in set_up:
set_up_step_script = set_up_step.get('script')
@@ -169,7 +148,7 @@
as_log='details')
kwargs['set_up'] = processed_set_up
- tear_down = list(raw_test_spec.get('teardown', []))
+ tear_down = list(spec.get('teardown', []))
processed_tear_down = []
for tear_down_step in tear_down:
tear_down_step_script = tear_down_step.get('script')
@@ -192,7 +171,7 @@
as_log='details')
kwargs['tear_down'] = processed_tear_down
- swarming_spec = raw_test_spec.get('swarming', {})
+ swarming_spec = spec.get('swarming', {})
if swarming_spec.get('can_use_on_swarming_builders'):
swarming_dimension_sets = swarming_spec.get('dimension_sets')
swarming_optional_dimensions = swarming_spec.get('optional_dimensions')
@@ -220,7 +199,7 @@
if service_account:
kwargs['service_account'] = service_account
- merge = dict(raw_test_spec.get('merge', {}))
+ merge = dict(spec.get('merge', {}))
if merge:
merge_script = merge.get('script')
if merge_script:
@@ -241,7 +220,7 @@
as_log='details')
kwargs['merge'] = merge
- trigger_script = dict(raw_test_spec.get('trigger_script', {}))
+ trigger_script = dict(spec.get('trigger_script', {}))
if trigger_script:
trigger_script_path = trigger_script.get('script')
if trigger_script_path:
@@ -272,17 +251,24 @@
# Also, add in optional dimensions.
kwargs['optional_dimensions'] = swarming_optional_dimensions
- yield wrap_test_spec(swarming_delegate(raw_test_spec, **kwargs))
+ tests.append(swarming_delegate(spec, **kwargs))
else:
- yield wrap_test_spec(local_delegate(raw_test_spec, **kwargs))
+ tests.append(local_delegate(spec, **kwargs))
+
+ experiment_percentage = spec.get('experiment_percentage')
+ for t in tests:
+ if experiment_percentage is not None:
+ yield steps.ExperimentalTest(t, experiment_percentage, api)
+ else:
+ yield t
def generate_gtests(api,
chromium_tests_api,
builder_group,
buildername,
- source_side_spec,
+ test_spec,
bot_update_step,
swarming_dimensions=None,
scripts_compile_targets_fn=None):
@@ -302,7 +288,7 @@
del api
return [
canonicalize_test(t)
- for t in source_side_spec.get(buildername, {}).get('gtest_tests', [])
+ for t in test_spec.get(buildername, {}).get('gtest_tests', [])
]
for spec in get_tests(api):
@@ -317,22 +303,21 @@
def generate_gtests_from_one_spec(api, chromium_tests_api, builder_group,
- buildername, raw_test_spec, bot_update_step,
+ buildername, spec, bot_update_step,
swarming_dimensions):
- def gtest_delegate_common(raw_test_spec, **kwargs):
+ def gtest_delegate_common(spec, **kwargs):
del kwargs
common_gtest_kwargs = {}
- args = get_args_for_test(api, chromium_tests_api, raw_test_spec,
- bot_update_step)
- if raw_test_spec['shard_index'] != 0 or raw_test_spec['total_shards'] != 1:
+ args = get_args_for_test(api, chromium_tests_api, spec, bot_update_step)
+ if spec['shard_index'] != 0 or spec['total_shards'] != 1:
args.extend([
- '--test-launcher-shard-index=%d' % raw_test_spec['shard_index'],
- '--test-launcher-total-shards=%d' % raw_test_spec['total_shards']
+ '--test-launcher-shard-index=%d' % spec['shard_index'],
+ '--test-launcher-total-shards=%d' % spec['total_shards']
])
common_gtest_kwargs['args'] = args
- common_gtest_kwargs['override_compile_targets'] = raw_test_spec.get(
+ common_gtest_kwargs['override_compile_targets'] = spec.get(
'override_compile_targets', None)
common_gtest_kwargs['waterfall_builder_group'] = builder_group
@@ -340,27 +325,25 @@
return common_gtest_kwargs
- def gtest_swarming_delegate(raw_test_spec, **kwargs):
- kwargs.update(gtest_delegate_common(raw_test_spec, **kwargs))
+ def gtest_swarming_delegate(spec, **kwargs):
+ kwargs.update(gtest_delegate_common(spec, **kwargs))
kwargs['isolate_profile_data'] = (
- raw_test_spec.get('isolate_coverage_data') or
- raw_test_spec.get('isolate_profile_data'))
- kwargs['ignore_task_failure'] = raw_test_spec.get('ignore_task_failure',
- False)
+ spec.get('isolate_coverage_data') or spec.get('isolate_profile_data'))
+ kwargs['ignore_task_failure'] = spec.get('ignore_task_failure', False)
# Enables resultdb if the build is picked for the experiment.
# TODO(crbug.com/1108016): Enable resultdb globally.
if not kwargs.get('resultdb') and ('chromium.resultdb.result_sink' in
api.buildbucket.build.input.experiments):
kwargs['resultdb'] = {'enable': True, 'result_format': 'gtest'}
- return bot_spec.TestSpec.create(steps.SwarmingGTestTest, **kwargs)
+ return steps.SwarmingGTestTest(**kwargs)
- def gtest_local_delegate(raw_test_spec, **kwargs):
- kwargs.update(gtest_delegate_common(raw_test_spec, **kwargs))
- kwargs['use_xvfb'] = raw_test_spec.get('use_xvfb', True)
- return bot_spec.TestSpec.create(steps.LocalGTestTest, **kwargs)
+ def gtest_local_delegate(spec, **kwargs):
+ kwargs.update(gtest_delegate_common(spec, **kwargs))
+ kwargs['use_xvfb'] = spec.get('use_xvfb', True)
+ return steps.LocalGTestTest(**kwargs)
- for t in generator_common(api, raw_test_spec, gtest_swarming_delegate,
+ for t in generator_common(api, spec, gtest_swarming_delegate,
gtest_local_delegate, swarming_dimensions):
yield t
@@ -369,15 +352,14 @@
chromium_tests_api,
builder_group,
buildername,
- source_side_spec,
+ test_spec,
bot_update_step,
swarming_dimensions=None,
scripts_compile_targets_fn=None):
del api, chromium_tests_api, bot_update_step
del swarming_dimensions, scripts_compile_targets_fn
- for test in source_side_spec.get(buildername, {}).get('junit_tests', []):
- yield bot_spec.TestSpec.create(
- steps.AndroidJunitTest,
+ for test in test_spec.get(buildername, {}).get('junit_tests', []):
+ yield steps.AndroidJunitTest(
test.get('name', test['test']),
target_name=test['test'],
additional_args=test.get('args'),
@@ -389,16 +371,15 @@
chromium_tests_api,
builder_group,
buildername,
- source_side_spec,
+ test_spec,
bot_update_step,
swarming_dimensions=None,
scripts_compile_targets_fn=None):
# Unused arguments
del api, chromium_tests_api, bot_update_step, swarming_dimensions
- for script_spec in source_side_spec.get(buildername, {}).get('scripts', []):
- yield bot_spec.TestSpec.create(
- steps.ScriptTest,
+ for script_spec in test_spec.get(buildername, {}).get('scripts', []):
+ yield steps.ScriptTest(
str(script_spec['name']),
script_spec['script'],
scripts_compile_targets_fn(),
@@ -412,13 +393,13 @@
chromium_tests_api,
builder_group,
buildername,
- source_side_spec,
+ test_spec,
bot_update_step,
swarming_dimensions=None,
scripts_compile_targets_fn=None):
del scripts_compile_targets_fn
- for spec in source_side_spec.get(buildername, {}).get('isolated_scripts', []):
+ for spec in test_spec.get(buildername, {}).get('isolated_scripts', []):
for test in generate_isolated_script_tests_from_one_spec(
api, chromium_tests_api, builder_group, buildername, spec,
bot_update_step, swarming_dimensions):
@@ -427,11 +408,10 @@
def generate_isolated_script_tests_from_one_spec(api, chromium_tests_api,
builder_group, buildername,
- source_side_spec,
- bot_update_step,
+ spec, bot_update_step,
swarming_dimensions):
- def isolated_script_delegate_common(raw_test_spec, name=None, **kwargs):
+ def isolated_script_delegate_common(test, name=None, **kwargs):
del kwargs
common_kwargs = {}
@@ -439,21 +419,20 @@
# The variable substitution and precommit/non-precommit arguments
# could be supported for the other test types too, but that wasn't
# desired at the time of this writing.
- common_kwargs['args'] = get_args_for_test(api, chromium_tests_api,
- raw_test_spec, bot_update_step)
+ common_kwargs['args'] = get_args_for_test(api, chromium_tests_api, test,
+ bot_update_step)
# This features is only needed for the cases in which the *_run compile
# target is needed to generate isolate files that contains dynamically libs.
# TODO(nednguyen, kbr): Remove this once all the GYP builds are converted
# to GN.
- common_kwargs['override_compile_targets'] = raw_test_spec.get(
+ common_kwargs['override_compile_targets'] = test.get(
'override_compile_targets', None)
common_kwargs['isolate_profile_data'] = (
- raw_test_spec.get('isolate_coverage_data') or
- raw_test_spec.get('isolate_profile_data'))
+ test.get('isolate_coverage_data') or test.get('isolate_profile_data'))
# TODO(tansell): Remove this once custom handling of results is no longer
# needed.
- results_handler_name = raw_test_spec.get('results_handler', 'default')
+ results_handler_name = test.get('results_handler', 'default')
try:
common_kwargs['results_handler'] = {
'default': lambda: None,
@@ -472,10 +451,10 @@
return common_kwargs
- def isolated_script_swarming_delegate(raw_test_spec, **kwargs):
- kwargs.update(isolated_script_delegate_common(raw_test_spec, **kwargs))
+ def isolated_script_swarming_delegate(spec, **kwargs):
+ kwargs.update(isolated_script_delegate_common(spec, **kwargs))
- swarming_spec = raw_test_spec.get('swarming', {})
+ swarming_spec = spec.get('swarming', {})
kwargs['ignore_task_failure'] = swarming_spec.get('ignore_task_failure',
False)
@@ -497,14 +476,13 @@
kwargs['resultdb'][
'test_location_base'] = '//third_party/webgl/src/sdk/tests/'
- return bot_spec.TestSpec.create(steps.SwarmingIsolatedScriptTest, **kwargs)
+ return steps.SwarmingIsolatedScriptTest(**kwargs)
- def isolated_script_local_delegate(raw_test_spec, **kwargs):
- kwargs.update(isolated_script_delegate_common(raw_test_spec, **kwargs))
- return bot_spec.TestSpec.create(steps.LocalIsolatedScriptTest, **kwargs)
+ def isolated_script_local_delegate(spec, **kwargs):
+ kwargs.update(isolated_script_delegate_common(spec, **kwargs))
+ return steps.LocalIsolatedScriptTest(**kwargs)
- for t in generator_common(api, source_side_spec,
- isolated_script_swarming_delegate,
+ for t in generator_common(api, spec, isolated_script_swarming_delegate,
isolated_script_local_delegate,
swarming_dimensions):
yield t
diff --git a/recipes/recipe_modules/chromium_tests/steps.py b/recipes/recipe_modules/chromium_tests/steps.py
index 266e30f..bb09eae 100644
--- a/recipes/recipe_modules/chromium_tests/steps.py
+++ b/recipes/recipe_modules/chromium_tests/steps.py
@@ -2357,7 +2357,7 @@
if self.raw_cmd:
pre_args += ['--raw-cmd']
- args = _merge_args_and_test_options(self, self.raw_cmd + list(self._args),
+ args = _merge_args_and_test_options(self, self.raw_cmd + self._args,
test_options)
# TODO(nednguyen, kbr): define contract with the wrapper script to rerun
# a subset of the tests. (crbug.com/533481)
diff --git a/recipes/recipe_modules/chromium_tests/tests/steps/generate_fuchsia_test.py b/recipes/recipe_modules/chromium_tests/tests/steps/generate_fuchsia_test.py
index f5362a8..28c878c 100644
--- a/recipes/recipe_modules/chromium_tests/tests/steps/generate_fuchsia_test.py
+++ b/recipes/recipe_modules/chromium_tests/tests/steps/generate_fuchsia_test.py
@@ -30,16 +30,15 @@
update_step = api.bot_update.ensure_checkout()
single_spec = api.properties.get('single_spec')
- source_side_spec = {
+ test_spec = {
'test_buildername': {
'gtest_tests': [single_spec] if single_spec else [],
}
}
- for test_spec in generators.generate_gtests(api, api.chromium_tests,
- 'test_group', 'test_buildername',
- source_side_spec, update_step):
- test = test_spec.get_test()
+ for test in generators.generate_gtests(api, api.chromium_tests, 'test_group',
+ 'test_buildername', test_spec,
+ update_step):
test.run(api, '')
diff --git a/recipes/recipe_modules/chromium_tests/tests/steps/generate_gtest.py b/recipes/recipe_modules/chromium_tests/tests/steps/generate_gtest.py
index 9401b0e..7ba22e8 100644
--- a/recipes/recipe_modules/chromium_tests/tests/steps/generate_gtest.py
+++ b/recipes/recipe_modules/chromium_tests/tests/steps/generate_gtest.py
@@ -37,16 +37,15 @@
update_step = api.bot_update.ensure_checkout()
single_spec = api.properties.get('single_spec')
- source_side_spec = {
+ test_spec = {
'test_buildername': {
'gtest_tests': [single_spec] if single_spec else [],
}
}
- for test_spec in generators.generate_gtests(api, api.chromium_tests,
- 'test_group', 'test_buildername',
- source_side_spec, update_step):
- test = test_spec.get_test()
+ for test in generators.generate_gtests(api, api.chromium_tests, 'test_group',
+ 'test_buildername', test_spec,
+ update_step):
try:
test.pre_run(api, '')
test.run(api, '')
diff --git a/recipes/recipe_modules/chromium_tests/tests/steps/generate_isolated_script.py b/recipes/recipe_modules/chromium_tests/tests/steps/generate_isolated_script.py
index 306ed62..17784f4 100644
--- a/recipes/recipe_modules/chromium_tests/tests/steps/generate_isolated_script.py
+++ b/recipes/recipe_modules/chromium_tests/tests/steps/generate_isolated_script.py
@@ -37,16 +37,16 @@
api.chromium_swarming.set_default_dimension('pool', 'foo')
single_spec = api.properties.get('single_spec')
- source_side_spec = {
+ test_spec = {
'test_buildername': {
'isolated_scripts': [single_spec] if single_spec else [],
}
}
- for test_spec in generators.generate_isolated_script_tests(
- api, api.chromium_tests, 'test_group', 'test_buildername',
- source_side_spec, update_step):
- test = test_spec.get_test()
+ for test in generators.generate_isolated_script_tests(api, api.chromium_tests,
+ 'test_group',
+ 'test_buildername',
+ test_spec, update_step):
try:
test.pre_run(api, '')
test.run(api, '')
diff --git a/recipes/recipe_modules/chromium_tests/tests/steps/generate_junit_test.py b/recipes/recipe_modules/chromium_tests/tests/steps/generate_junit_test.py
index d1c2ace..dcdcb14 100644
--- a/recipes/recipe_modules/chromium_tests/tests/steps/generate_junit_test.py
+++ b/recipes/recipe_modules/chromium_tests/tests/steps/generate_junit_test.py
@@ -32,18 +32,15 @@
update_step = api.bot_update.ensure_checkout()
single_spec = api.properties.get('single_spec')
- source_side_spec = {
+ test_spec = {
'test_buildername': {
'junit_tests': [single_spec] if single_spec else [],
}
}
- for test_spec in generators.generate_junit_tests(api, api.chromium_tests,
- 'test_group',
- 'test_buildername',
- source_side_spec,
- update_step):
- test = test_spec.get_test()
+ for test in generators.generate_junit_tests(api, api.chromium_tests,
+ 'test_group', 'test_buildername',
+ test_spec, update_step):
test.run(api, '')
diff --git a/recipes/recipe_modules/chromium_tests/tests/steps/generate_script.py b/recipes/recipe_modules/chromium_tests/tests/steps/generate_script.py
index 8dfc157..3fd4f76 100644
--- a/recipes/recipe_modules/chromium_tests/tests/steps/generate_script.py
+++ b/recipes/recipe_modules/chromium_tests/tests/steps/generate_script.py
@@ -33,7 +33,7 @@
}
}
- for test_spec in generators.generate_script_tests(
+ for test in generators.generate_script_tests(
api,
api.chromium_tests,
'test_group',
@@ -41,7 +41,6 @@
test_spec,
update_step,
scripts_compile_targets_fn=lambda: {'gtest_test.py': ['$name']}):
- test = test_spec.get_test()
try:
test.pre_run(api, '')
test.run(api, '')
