Skip to content

Commit dd12341

Browse files
mi-acCommit Bot
mi-ac
authored and
Commit Bot
committed
[infra] Add presubmit checks for test specifications
This adds presubmit checks and documentation for the features added on infra side in https://crrev.com/c/1019080. NOTRY=true Bug: chromium:830557 Change-Id: If3808f5931f7b1043a8a80c03a293df4602ce758 Reviewed-on: https://chromium-review.googlesource.com/1021140 Reviewed-by: Sergiy Byelozyorov <sergiyb@chromium.org> Commit-Queue: Michael Achenbach <machenbach@chromium.org> Cr-Commit-Position: refs/heads/master@{#52833}
1 parent fa0e55e commit dd12341

2 files changed

Lines changed: 212 additions & 4 deletions

File tree

‎infra/testing/PRESUBMIT.py‎

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
# Copyright 2018 the V8 project authors. All rights reserved.
2+
# Use of this source code is governed by a BSD-style license that can be
3+
# found in the LICENSE file.
4+
5+
"""
6+
Presubmit checks for the validity of V8-side test specifications in pyl files.
7+
8+
For simplicity, we check all pyl files on any changes in this folder.
9+
"""
10+
11+
import ast
12+
import os
13+
14+
15+
SUPPORTED_BUILDER_SPEC_KEYS = [
16+
'swarming_dimensions',
17+
'swarming_task_attrs',
18+
'tests',
19+
]
20+
21+
# This is not an exhaustive list. It only reflects what we currently use. If
22+
# there's need to specify a different dimension, just add it here.
23+
SUPPORTED_SWARMING_DIMENSIONS = [
24+
'cores'
25+
'cpu',
26+
'os',
27+
]
28+
29+
# This is not an exhaustive list. It only reflects what we currently use. If
30+
# there's need to specify a different property, just add it here.
31+
SUPPORTED_SWARMING_TASK_ATTRS = [
32+
'expiration',
33+
'hard_timeout',
34+
'priority',
35+
]
36+
37+
SUPPORTED_TEST_KEYS = [
38+
'name',
39+
'shards',
40+
'suffix',
41+
'swarming_dimensions',
42+
'swarming_task_attrs',
43+
'test_args',
44+
'variant',
45+
]
46+
47+
def check_keys(error_msg, src_dict, supported_keys):
48+
errors = []
49+
for key in src_dict.keys():
50+
if key not in supported_keys:
51+
errors += error_msg('Key "%s" must be one of %s' % (key, supported_keys))
52+
return errors
53+
54+
55+
def _check_properties(error_msg, src_dict, prop_name, supported_keys):
56+
properties = src_dict.get(prop_name, {})
57+
if not isinstance(properties, dict):
58+
return error_msg('Value for %s must be a dict' % prop_name)
59+
return check_keys(error_msg, properties, supported_keys)
60+
61+
62+
def _check_int_range(error_msg, src_dict, prop_name, lower_bound=None,
63+
upper_bound=None):
64+
if prop_name not in src_dict:
65+
# All properties are optional.
66+
return []
67+
try:
68+
value = int(src_dict[prop_name])
69+
except ValueError:
70+
return error_msg('If specified, %s must be an int' % prop_name)
71+
if lower_bound is not None and value < lower_bound:
72+
return error_msg('If specified, %s must be >=%d' % (prop_name, lower_bound))
73+
if upper_bound is not None and value > upper_bound:
74+
return error_msg('If specified, %s must be <=%d' % (prop_name, upper_bound))
75+
return []
76+
77+
78+
def _check_swarming_task_attrs(error_msg, src_dict):
79+
errors = []
80+
task_attrs = src_dict.get('swarming_task_attrs', {})
81+
errors += _check_int_range(
82+
error_msg, task_attrs, 'priority', lower_bound=25, upper_bound=100)
83+
errors += _check_int_range(
84+
error_msg, task_attrs, 'expiration', lower_bound=1)
85+
errors += _check_int_range(
86+
error_msg, task_attrs, 'hard_timeout', lower_bound=1)
87+
return errors
88+
89+
90+
def _check_swarming_config(error_msg, src_dict):
91+
errors = []
92+
errors += _check_properties(
93+
error_msg, src_dict, 'swarming_dimensions',
94+
SUPPORTED_SWARMING_DIMENSIONS)
95+
errors += _check_properties(
96+
error_msg, src_dict, 'swarming_task_attrs',
97+
SUPPORTED_SWARMING_TASK_ATTRS)
98+
errors += _check_swarming_task_attrs(error_msg, src_dict)
99+
return errors
100+
101+
102+
def _check_test(error_msg, test):
103+
if not isinstance(test, dict):
104+
return error_msg('Each test must be specified with a dict')
105+
errors = check_keys(error_msg, test, SUPPORTED_TEST_KEYS)
106+
if not test.get('name'):
107+
errors += error_msg('A test requires a name')
108+
errors += _check_swarming_config(error_msg, test)
109+
110+
test_args = test.get('test_args', [])
111+
if not isinstance(test_args, list):
112+
errors += error_msg('If specified, test_args must be a list of arguments')
113+
if not all(isinstance(x, basestring) for x in test_args):
114+
errors += error_msg('If specified, all test_args must be strings')
115+
116+
# Limit shards to 10 to avoid erroneous resource exhaustion.
117+
errors += _check_int_range(
118+
error_msg, test, 'shards', lower_bound=1, upper_bound=10)
119+
120+
variant = test.get('variant', 'default')
121+
if not variant or not isinstance(variant, basestring):
122+
errors += error_msg('If specified, variant must be a non-empty string')
123+
124+
return errors
125+
126+
127+
def _check_test_spec(file_path, raw_pyl):
128+
def error_msg(msg):
129+
return ['Error in %s:\n%s' % (file_path, msg)]
130+
131+
try:
132+
# Eval python literal file.
133+
full_test_spec = ast.literal_eval(raw_pyl)
134+
except SyntaxError as e:
135+
return error_msg('Pyl parsing failed with:\n%s' % e)
136+
137+
if not isinstance(full_test_spec, dict):
138+
return error_msg('Test spec must be a dict')
139+
140+
errors = []
141+
for buildername, builder_spec in full_test_spec.iteritems():
142+
def error_msg(msg):
143+
return ['Error in %s for builder %s:\n%s' % (file_path, buildername, msg)]
144+
145+
if not isinstance(buildername, basestring) or not buildername:
146+
errors += error_msg('Buildername must be a non-empty string')
147+
148+
if not isinstance(builder_spec, dict) or not builder_spec:
149+
errors += error_msg('Value must be a non-empty dict')
150+
continue
151+
152+
errors += check_keys(error_msg, builder_spec, SUPPORTED_BUILDER_SPEC_KEYS)
153+
errors += _check_swarming_config(error_msg, builder_spec)
154+
155+
for test in builder_spec.get('tests', []):
156+
errors += _check_test(error_msg, test)
157+
158+
return errors
159+
160+
161+
162+
def CheckChangeOnCommit(input_api, output_api):
163+
def file_filter(regexp):
164+
return lambda f: input_api.FilterSourceFile(f, white_list=(regexp,))
165+
166+
# Calculate which files are affected.
167+
if input_api.AffectedFiles(False, file_filter(r'.*PRESUBMIT\.py')):
168+
# If PRESUBMIT.py itself was changed, check all configs.
169+
affected_files = [
170+
f for f in os.listdir(input_api.PresubmitLocalPath())
171+
if f.endswith('.pyl')
172+
]
173+
else:
174+
# Otherwise, check only changed configs.
175+
affected_files = [
176+
f.AbsoluteLocalPath()
177+
for f in input_api.AffectedFiles(False, file_filter(r'.+\.pyl'))
178+
]
179+
180+
errors = []
181+
for file_path in affected_files:
182+
with open(file_path) as f:
183+
errors += _check_test_spec(file_path, f.read())
184+
return [output_api.PresubmitError(r) for r in errors]

‎infra/testing/README.md‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,30 +18,54 @@ The structure of each file is:
1818
'tests': [
1919
{
2020
'name': <test-spec name>,
21+
'suffix': <step suffix>,
2122
'variant': <variant name>,
2223
'shards': <number of shards>,
24+
'test_args': <list of flags>,
25+
'swarming_task_attrs': {...},
26+
'swarming_dimensions': {...},
2327
},
2428
...
2529
],
30+
'swarming_task_attrs': {...},
31+
'swarming_dimensions': {...},
2632
},
2733
...
2834
}
2935
```
3036
The `<buildername>` is a string name of the builder to execute the tests.
3137
`<test-spec name>` is a label defining a test specification matching the
3238
[infra-side](https://chromium.googlesource.com/chromium/tools/build/+/master/scripts/slave/recipe_modules/v8/testing.py#58).
33-
The `<variant name>` is a testing variant specified
39+
The optional `suffix` will be appended to test-step names for disambiguation.
40+
The optional `variant` is a testing variant specified
3441
[here](https://chromium.googlesource.com/v8/v8/+/master/tools/testrunner/local/variants.py).
35-
`<number of shards>` is optional (default 1), but can be provided to increase
36-
the swarming shards for long-running tests.
42+
The optional `shards` (default 1) can be provided to increase the swarming
43+
shards for long-running tests.
44+
The optional `test_args` is a list of string flags that will be passed to the
45+
V8 test driver.
46+
The optional `swarming_task_attrs` is a dict allowing to override the defaults
47+
for `priority`, `expiration` and `hard_timeout`.
48+
The optional `swarming_dimensions` is a dict allowing to override the defaults
49+
for `cpu`, `cores` and `os`.
50+
Both `swarming_task_attrs` and `swarming_dimensions` can be defined per builder
51+
and per test, whereas the latter takes precedence.
3752

3853
Example:
3954
```
4055
{
4156
'v8_linux64_rel_ng_triggered': {
4257
'tests': [
43-
{'name': 'v8testing', 'variant': 'nooptimization', 'shards': 2},
58+
{
59+
'name': 'v8testing',
60+
'suffix': 'stress',
61+
'variant': 'nooptimization',
62+
'shards': 2,
63+
'test_args': ['--gc-stress'],
64+
'swarming_dimensions': {'os': 'Ubuntu-14.4'},
65+
},
4466
],
67+
'swarming_properties': {'priority': 35},
68+
'swarming_dimensions': {'os': 'Ubuntu'},
4569
},
4670
}
4771
```

0 commit comments

Comments
 (0)