Skip to content

Commit 75941e7

Browse files
authored
[CI] Add ability to configure release tests with a matrix (#51721)
To ensure Ray Data works performantly in a wide range of scenarios, we want to run release tests with many variants. For example, for our aggregation release tests, we want to run release tests with and without autoscaling, with different cardinalities, and with different implementations. To decrease the maintenance cost and improve the readability of the release test config, this PR introduces a matrix syntax based on [Buildkite's](https://buildkite.com/docs/pipelines/configure/workflows/build-matrix). --------- Signed-off-by: Balaji Veeramani <bveeramani@berkeley.edu>
1 parent 706e6b9 commit 75941e7

File tree

2 files changed

+208
-25
lines changed

2 files changed

+208
-25
lines changed

release/ray_release/config.py

Lines changed: 98 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,22 @@
11
import copy
2+
import itertools
23
import json
34
import os
45
import re
5-
from typing import Dict, List, Optional, Tuple, Any
6+
from typing import Any, Dict, List, Optional, Tuple
67

78
import jsonschema
89
import yaml
9-
from ray_release.test import (
10-
Test,
11-
TestDefinition,
12-
)
1310
from ray_release.anyscale_util import find_cloud_by_name
1411
from ray_release.bazel import bazel_runfile
1512
from ray_release.exception import ReleaseTestCLIError, ReleaseTestConfigError
1613
from ray_release.logger import logger
14+
from ray_release.test import (
15+
Test,
16+
TestDefinition,
17+
)
1718
from ray_release.util import DeferredEnvVar, deep_update
1819

19-
2020
DEFAULT_WHEEL_WAIT_TIMEOUT = 7200 # Two hours
2121
DEFAULT_COMMAND_TIMEOUT = 1800
2222
DEFAULT_BUILD_TIMEOUT = 3600
@@ -84,6 +84,11 @@ def parse_test_definition(test_definitions: List[TestDefinition]) -> List[Test]:
8484
default_definition = {}
8585
tests = []
8686
for test_definition in test_definitions:
87+
if "matrix" in test_definition and "variations" in test_definition:
88+
raise ReleaseTestConfigError(
89+
"You can't specify both 'matrix' and 'variations' in a test definition"
90+
)
91+
8792
if test_definition["name"] == "DEFAULTS":
8893
default_definition = copy.deepcopy(test_definition)
8994
continue
@@ -93,29 +98,102 @@ def parse_test_definition(test_definitions: List[TestDefinition]) -> List[Test]:
9398
copy.deepcopy(default_definition), test_definition
9499
)
95100

96-
if "variations" not in test_definition:
101+
if "variations" in test_definition:
102+
tests.extend(_parse_test_definition_with_variations(test_definition))
103+
elif "matrix" in test_definition:
104+
tests.extend(_parse_test_definition_with_matrix(test_definition))
105+
else:
97106
tests.append(Test(test_definition))
98-
continue
99107

100-
variations = test_definition.pop("variations")
108+
return tests
109+
110+
111+
def _parse_test_definition_with_variations(
112+
test_definition: TestDefinition,
113+
) -> List[Test]:
114+
tests = []
115+
116+
variations = test_definition.pop("variations")
117+
_test_definition_invariant(
118+
test_definition,
119+
variations,
120+
"variations field cannot be empty in a test definition",
121+
)
122+
for variation in variations:
101123
_test_definition_invariant(
102124
test_definition,
103-
variations,
104-
"variations field cannot be empty in a test definition",
125+
"__suffix__" in variation,
126+
"missing __suffix__ field in a variation",
105127
)
106-
for variation in variations:
107-
_test_definition_invariant(
108-
test_definition,
109-
"__suffix__" in variation,
110-
"missing __suffix__ field in a variation",
128+
test = copy.deepcopy(test_definition)
129+
test["name"] = f'{test["name"]}.{variation.pop("__suffix__")}'
130+
test = deep_update(test, variation)
131+
tests.append(Test(test))
132+
133+
return tests
134+
135+
136+
def _parse_test_definition_with_matrix(test_definition: TestDefinition) -> List[Test]:
137+
tests = []
138+
139+
matrix = test_definition.pop("matrix")
140+
variables = tuple(matrix["setup"].keys())
141+
combinations = itertools.product(*matrix["setup"].values())
142+
for combination in combinations:
143+
test = test_definition
144+
for variable, value in zip(variables, combination):
145+
test = _substitute_variable(test, variable, str(value))
146+
tests.append(Test(test))
147+
148+
adjustments = matrix.pop("adjustments", [])
149+
for adjustment in adjustments:
150+
if not set(adjustment["with"]) == set(variables):
151+
raise ReleaseTestConfigError(
152+
"You need to specify all matrix variables in the adjustment."
111153
)
112-
test = copy.deepcopy(test_definition)
113-
test["name"] = f'{test["name"]}.{variation.pop("__suffix__")}'
114-
test = deep_update(test, variation)
115-
tests.append(Test(test))
154+
155+
test = test_definition
156+
for variable, value in adjustment["with"].items():
157+
test = _substitute_variable(test, variable, str(value))
158+
tests.append(Test(test))
159+
116160
return tests
117161

118162

163+
def _substitute_variable(data: Dict, variable: str, replacement: str) -> Dict:
164+
"""Substitute a variable in the provided dictionary with a replacement value.
165+
166+
This function traverses dict and list values, and substitutes the variable in
167+
string values. The syntax for variables is `{{variable}}`.
168+
169+
Args:
170+
data: The dictionary to substitute the variable in.
171+
variable: The variable to substitute.
172+
replacement: The replacement value.
173+
174+
Returns:
175+
A new dictionary with the variable substituted.
176+
177+
Examples:
178+
>>> test_definition = {"name": "test-{{arg}}"}
179+
>>> _substitute_variable(test_definition, "arg", "1")
180+
{'name': 'test-1'}
181+
"""
182+
# Create a copy to avoid mutating the original.
183+
data = copy.deepcopy(data)
184+
185+
pattern = r"\{\{\s*" + re.escape(variable) + r"\s*\}\}"
186+
for key, value in data.items():
187+
if isinstance(value, dict):
188+
data[key] = _substitute_variable(value, variable, replacement)
189+
elif isinstance(value, list):
190+
data[key] = [re.sub(pattern, replacement, string) for string in value]
191+
elif isinstance(value, str):
192+
data[key] = re.sub(pattern, replacement, value)
193+
194+
return data
195+
196+
119197
def load_schema_file(path: Optional[str] = None) -> Dict:
120198
path = path or RELEASE_TEST_SCHEMA_FILE
121199
with open(path, "rt") as fp:

release/ray_release/tests/test_config.py

Lines changed: 110 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
11
import sys
2-
import yaml
3-
import pytest
42

5-
from ray_release.test import Test
3+
import pytest
4+
import yaml
65
from ray_release.config import (
7-
read_and_validate_release_test_collection,
8-
validate_cluster_compute,
6+
_substitute_variable,
97
load_schema_file,
108
parse_test_definition,
9+
read_and_validate_release_test_collection,
10+
validate_cluster_compute,
1111
validate_test,
1212
)
1313
from ray_release.exception import ReleaseTestConfigError
14+
from ray_release.test import Test
1415

1516
_TEST_COLLECTION_FILES = [
1617
"release/release_tests.yaml",
@@ -129,6 +130,110 @@ def test_parse_test_definition_with_defaults():
129130
assert test_with_override["working_dir"] == "overridden_working_dir"
130131

131132

133+
def test_parse_test_definition_with_matrix_and_variations_raises():
134+
# Matrix and variations are mutually exclusive.
135+
test_definitions = yaml.safe_load(
136+
"""
137+
- name: test
138+
frequency: nightly
139+
team: team
140+
working_dir: sample_dir
141+
cluster:
142+
byod:
143+
type: gpu
144+
cluster_compute: "{{os}}.yaml"
145+
matrix:
146+
setup:
147+
os: [windows, linux]
148+
run:
149+
timeout: 100
150+
script: python script.py
151+
variations:
152+
- __suffix__: amd64
153+
- __suffix__: arm64
154+
"""
155+
)
156+
with pytest.raises(ReleaseTestConfigError):
157+
parse_test_definition(test_definitions)
158+
159+
160+
def test_parse_test_definition_with_matrix_and_adjustments():
161+
test_definitions = yaml.safe_load(
162+
"""
163+
- name: "test-{{compute}}-{{arg}}"
164+
matrix:
165+
setup:
166+
compute: [fixed, autoscaling]
167+
arg: [0, 1]
168+
adjustments:
169+
- with:
170+
# Only run arg 2 with fixed compute
171+
compute: fixed
172+
arg: 2
173+
frequency: nightly
174+
team: team
175+
working_dir: sample_dir
176+
cluster:
177+
byod:
178+
type: gpu
179+
runtime_env:
180+
- SCALING_MODE={{compute}}
181+
cluster_compute: "{{compute}}.yaml"
182+
run:
183+
timeout: 100
184+
script: python script.py --arg "{{arg}}"
185+
"""
186+
)
187+
tests = parse_test_definition(test_definitions)
188+
schema = load_schema_file()
189+
190+
assert len(tests) == 5 # 4 from matrix, 1 from adjustments
191+
assert not any(validate_test(test, schema) for test in tests)
192+
for i, (compute, arg) in enumerate(
193+
[
194+
("fixed", 0),
195+
("fixed", 1),
196+
("autoscaling", 0),
197+
("autoscaling", 1),
198+
("fixed", 2),
199+
]
200+
):
201+
assert tests[i]["name"] == f"test-{compute}-{arg}"
202+
assert tests[i]["cluster"]["cluster_compute"] == f"{compute}.yaml"
203+
assert tests[i]["cluster"]["byod"]["runtime_env"] == [f"SCALING_MODE={compute}"]
204+
205+
206+
class TestSubstituteVariable:
207+
def test_does_not_mutate_original(self):
208+
test_definition = {"name": "test-{{arg}}"}
209+
210+
substituted = _substitute_variable(test_definition, "arg", "1")
211+
212+
assert substituted is not test_definition
213+
assert test_definition == {"name": "test-{{arg}}"}
214+
215+
def test_substitute_variable_in_string(self):
216+
test_definition = {"name": "test-{{arg}}"}
217+
218+
substituted = _substitute_variable(test_definition, "arg", "1")
219+
220+
assert substituted == {"name": "test-1"}
221+
222+
def test_substitute_variable_in_list(self):
223+
test_definition = {"items": ["item-{{arg}}"]}
224+
225+
substituted = _substitute_variable(test_definition, "arg", "1")
226+
227+
assert substituted == {"items": ["item-1"]}
228+
229+
def test_substitute_variable_in_dict(self):
230+
test_definition = {"outer": {"inner": "item-{{arg}}"}}
231+
232+
substituted = _substitute_variable(test_definition, "arg", "1")
233+
234+
assert substituted == {"outer": {"inner": "item-1"}}
235+
236+
132237
def test_schema_validation():
133238
test = VALID_TEST.copy()
134239

0 commit comments

Comments
 (0)