diff --git a/src/cloudai/models/scenario.py b/src/cloudai/models/scenario.py index 4cfd76a7b..4d7f2b510 100644 --- a/src/cloudai/models/scenario.py +++ b/src/cloudai/models/scenario.py @@ -71,6 +71,14 @@ class TestRunModel(BaseModel): id: str = Field(min_length=1) test_name: Optional[str] = None + path: Optional[str] = Field( + default=None, + min_length=1, + description=( + "Path to a test TOML file, resolved relative to this scenario file's own directory. " + "Alternative to 'test_name': references a test by file location instead of by name." + ), + ) num_nodes: int | list[int] | None = None nodes: list[str] = Field(default_factory=list) exclude_nodes: list[str] = Field( @@ -126,14 +134,17 @@ def tdef_model_dump(self, by_alias: bool) -> dict: @model_validator(mode="after") def check_test_name_or_type_is_set(self): - has_base = self.test_name is not None + if self.test_name is not None and self.path is not None: + raise ValueError("'test_name' and 'path' must not both be set; use only one to reference a test.") + + has_base = self.test_name is not None or self.path is not None if not has_base and (self.test_template_name is None or self.name is None or self.description is None): raise ValueError( - "When 'test_name' is not set, the following fields must be set: " + "When neither 'test_name' nor 'path' is set, the following fields must be set: " "'test_template_name', 'name', 'description'." ) - if not self.test_name: + if not has_base: if not self.test_template_name: raise ValueError("'test_template_name' must be set if 'test_name' is not set.") @@ -145,7 +156,7 @@ def check_test_name_or_type_is_set(self): ) else: if self.test_template_name: - raise ValueError("'test_template_name' must not be set if 'test_name' is set.") + raise ValueError("'test_template_name' must not be set if 'test_name' or 'path' is set.") return self diff --git a/src/cloudai/test_scenario_parser.py b/src/cloudai/test_scenario_parser.py index 48e3acad5..f357b93cf 100644 --- a/src/cloudai/test_scenario_parser.py +++ b/src/cloudai/test_scenario_parser.py @@ -38,7 +38,7 @@ ) from .models.scenario import TestRunModel, TestScenarioModel from .models.workload import TestDefinition -from .test_parser import TestParser +from .test_parser import TestParser, load_test_toml_file from .toml_utils import format_toml_decode_error @@ -228,12 +228,25 @@ def _prepare_tdef(self, test_info: TestRunModel) -> TestDefinition: tc_defined = test_info.tdef_model_dump(by_alias=True) merged_data = deep_merge(test_defined, tc_defined) test = tp.load_test_definition(merged_data) + elif test_info.path: # test referenced by file path, relative to this scenario file's directory + resolved_path = (self.file_path.parent / test_info.path).resolve() + if not resolved_path.is_file(): + raise TestScenarioParsingError( + f"Test case '{test_info.id}' references path '{test_info.path}', " + f"which resolves to '{resolved_path}', but that file does not exist." + ) + tp.current_file = resolved_path + with resolved_path.open() as fh: + test_defined = load_test_toml_file(fh, resolved_path) + tc_defined = test_info.tdef_model_dump(by_alias=True) + merged_data = deep_merge(test_defined, tc_defined) + test = tp.load_test_definition(merged_data) elif test_info.test_template_name: # test fully defined in the scenario test = tp._parse_data(test_info.tdef_model_dump(by_alias=True)) else: # this should never happen, because we check for this in the modelvalidator raise ValueError( - f"Cannot configure test case '{test_info.id}' with both 'test_name' and 'test_template_name'." + f"Test case '{test_info.id}' has none of 'test_name', 'path', or 'test_template_name' set." ) return test diff --git a/tests/test_test_scenario.py b/tests/test_test_scenario.py index a02a48b24..ab9a46c49 100644 --- a/tests/test_test_scenario.py +++ b/tests/test_test_scenario.py @@ -33,6 +33,7 @@ TestRun, TestScenario, TestScenarioParser, + TestScenarioParsingError, ) from cloudai.models.scenario import TestRunModel, TestScenarioModel from cloudai.report_generator.training import TrainingReportGenerationStrategy @@ -322,7 +323,8 @@ def test_without_base(self, missing_arg: str): with pytest.raises(ValueError) as exc_info: TestRunModel.model_validate(spec) assert exc_info.match( - "When 'test_name' is not set, the following fields must be set: 'test_template_name', 'name', 'description'" + "When neither 'test_name' nor 'path' is set, the following fields must be set: " + "'test_template_name', 'name', 'description'" ) def test_name_is_not_in_mapping(self, test_scenario_parser: TestScenarioParser): @@ -349,7 +351,7 @@ def test_cant_override_template_name(self): spec = {"id": "1", "test_name": "nccl", "test_template_name": "NcclTest"} with pytest.raises(ValueError) as exc_info: TestRunModel.model_validate(spec) - assert exc_info.match("'test_template_name' must not be set if 'test_name' is set.") + assert exc_info.match("'test_template_name' must not be set if 'test_name' or 'path' is set.") def test_spec_with_unknown_test_type(self): with pytest.raises(ValueError) as exc_info: @@ -359,7 +361,7 @@ def test_spec_with_unknown_test_type(self): def test_type_is_not_allowed_when_name_is_set(self): with pytest.raises(ValueError) as exc_info: TestRunModel(id="1", test_name="nccl", test_template_name="NcclTest") - assert exc_info.match("'test_template_name' must not be set if 'test_name' is set.") + assert exc_info.match("'test_template_name' must not be set if 'test_name' or 'path' is set.") def test_spec_without_base(self, test_scenario_parser: TestScenarioParser): model = TestScenarioModel.model_validate( @@ -922,3 +924,109 @@ def test_nsys_disable_override(self, test_scenario_parser: TestScenarioParser, s assert tdef.nsys is not None assert tdef.nsys.enable is False assert tdef.nsys.output == "/base/output" + + +class TestPathReference: + def test_path_and_test_name_together_is_rejected(self): + with pytest.raises(ValueError) as exc_info: + TestRunModel(id="1", test_name="nccl", path="nccl.toml") + assert exc_info.match("'test_name' and 'path' must not both be set") + + def test_path_and_test_template_name_together_is_rejected(self): + with pytest.raises(ValueError) as exc_info: + TestRunModel(id="1", path="nccl.toml", test_template_name="NcclTest") + assert exc_info.match("'test_template_name' must not be set if 'test_name' or 'path' is set.") + + def test_path_alone_satisfies_the_base_requirement(self): + model = TestRunModel(id="1", path="nccl.toml") + assert model.path == "nccl.toml" + + def test_empty_path_is_rejected(self): + with pytest.raises(ValueError) as exc_info: + TestRunModel(id="1", path="") + assert exc_info.match("String should have at least 1 character") + + def test_path_is_resolved_relative_to_the_scenario_file(self, tmp_path: Path, slurm_system: SlurmSystem): + (tmp_path / "tests").mkdir() + (tmp_path / "tests" / "nccl.toml").write_text( + """ + name = "nccl" + description = "desc" + test_template_name = "NcclTest" + + [cmd_args] + docker_image_url = "fake://url/nccl" + """ + ) + scenario_path = tmp_path / "scenario.toml" + scenario_path.write_text("") # only its parent directory matters for resolution + parser = TestScenarioParser(scenario_path, slurm_system, {}, {}) + + test_info = TestRunModel(id="1", path="tests/nccl.toml") + tdef = parser._prepare_tdef(test_info) + + assert tdef.name == "nccl" + assert isinstance(tdef, NCCLTestDefinition) + assert tdef.cmd_args.docker_image_url == "fake://url/nccl" + + def test_scenario_level_overrides_are_merged_over_the_referenced_file( + self, tmp_path: Path, slurm_system: SlurmSystem + ): + (tmp_path / "nccl.toml").write_text( + """ + name = "nccl" + description = "desc" + test_template_name = "NcclTest" + + [cmd_args] + docker_image_url = "fake://url/nccl" + """ + ) + scenario_path = tmp_path / "scenario.toml" + scenario_path.write_text("") + parser = TestScenarioParser(scenario_path, slurm_system, {}, {}) + + test_info = TestRunModel(id="1", path="nccl.toml", cmd_args=CmdArgs.model_validate({"nthreads": 42})) + tdef = parser._prepare_tdef(test_info) + + assert tdef.cmd_args.nthreads == 42 + assert tdef.cmd_args.docker_image_url == "fake://url/nccl" + + def test_missing_referenced_file_raises_a_clear_error(self, tmp_path: Path, slurm_system: SlurmSystem): + scenario_path = tmp_path / "scenario.toml" + scenario_path.write_text("") + parser = TestScenarioParser(scenario_path, slurm_system, {}, {}) + + test_info = TestRunModel(id="1", path="does-not-exist.toml") + with pytest.raises(TestScenarioParsingError) as exc_info: + parser._prepare_tdef(test_info) + + assert exc_info.match("does not exist") + + def test_full_scenario_toml_with_path_reference(self, tmp_path: Path, slurm_system: SlurmSystem): + (tmp_path / "nccl.toml").write_text( + """ + name = "nccl" + description = "desc" + test_template_name = "NcclTest" + + [cmd_args] + docker_image_url = "fake://url/nccl" + """ + ) + scenario_path = tmp_path / "scenario.toml" + scenario_path.write_text( + """ + name = "test" + + [[Tests]] + id = "1" + path = "nccl.toml" + """ + ) + parser = TestScenarioParser(scenario_path, slurm_system, {}, {}) + model = TestScenarioModel.model_validate(toml.loads(scenario_path.read_text())) + + tdef = parser._prepare_tdef(model.tests[0]) + + assert tdef.name == "nccl"