diff --git a/Cargo.lock b/Cargo.lock index 00aeaa21..c96fed54 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -679,9 +679,9 @@ checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" [[package]] name = "openjd-expr" -version = "0.7.0" +version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b6e97fab933bcfb13e42a45d1f39311149508c454b21bf931274e288583d4e08" +checksum = "f5396e716811e785c9a81ad276f0b74b37db297ec2fb830ff5a002715787578c" dependencies = [ "regex", "regex-syntax", @@ -696,9 +696,9 @@ dependencies = [ [[package]] name = "openjd-model" -version = "0.7.1" +version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ca2537f12cbe572cb0f0a36bca44c858e8e943a30603c583c959aa8453a2c62d" +checksum = "10cc7ef51861ddc5170c9a03eaa261161a5819c007cdbd9b0ed6c8da23fbc1bb" dependencies = [ "indexmap", "openjd-expr", @@ -728,9 +728,9 @@ dependencies = [ [[package]] name = "openjd-sessions" -version = "0.5.8" +version = "0.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9dc782dfcc53e6850422c30526e94fb80657650c7123152f05166b9873e20e49" +checksum = "c52db94b553d39934d6f6c13d16a31fe692215f4917fad4b4d695460ef3c8b22" dependencies = [ "bitflags", "futures-util", diff --git a/THIRD-PARTY-LICENSES.txt b/THIRD-PARTY-LICENSES.txt index d2851ffa..b9e27ea0 100644 --- a/THIRD-PARTY-LICENSES.txt +++ b/THIRD-PARTY-LICENSES.txt @@ -2534,9 +2534,9 @@ limitations under the License. ** itoa; version 1.0.18 -- https://crates.io/crates/itoa ** libc; version 0.2.189 -- https://crates.io/crates/libc ** manyhow-macros; version 0.11.4 -- https://crates.io/crates/manyhow-macros -** openjd-expr; version 0.7.0 -- https://crates.io/crates/openjd-expr -** openjd-model; version 0.7.1 -- https://crates.io/crates/openjd-model -** openjd-sessions; version 0.5.8 -- https://crates.io/crates/openjd-sessions +** openjd-expr; version 0.8.0 -- https://crates.io/crates/openjd-expr +** openjd-model; version 0.8.0 -- https://crates.io/crates/openjd-model +** openjd-sessions; version 0.6.0 -- https://crates.io/crates/openjd-sessions ** pin-project-lite; version 0.2.17 -- https://crates.io/crates/pin-project-lite ** portable-atomic; version 1.15.0 -- https://crates.io/crates/portable-atomic ** proc-macro2; version 1.0.107 -- https://crates.io/crates/proc-macro2 diff --git a/rust-bindings/Cargo.toml b/rust-bindings/Cargo.toml index ab2b966b..c27c12f5 100644 --- a/rust-bindings/Cargo.toml +++ b/rust-bindings/Cargo.toml @@ -12,9 +12,9 @@ name = "_openjd_rs" crate-type = ["cdylib", "rlib"] [dependencies] -openjd-expr = "0.7.0" -openjd-model = "0.7.1" -openjd-sessions = "0.5.8" +openjd-expr = "0.8.0" +openjd-model = "0.8.0" +openjd-sessions = "0.6.0" tokio = { version = "1", features = ["rt-multi-thread"] } uuid = { version = "1", features = ["v4"] } serde_json = "1" diff --git a/rust-bindings/src/expr/format_string.rs b/rust-bindings/src/expr/format_string.rs index 7c6b1eba..c53ee193 100644 --- a/rust-bindings/src/expr/format_string.rs +++ b/rust-bindings/src/expr/format_string.rs @@ -80,6 +80,28 @@ impl PyFormatString { self.inner.is_literal() } + /// Number of parsed segments: literal runs and ``{{...}}`` expressions. + /// + /// A format string with more than one segment always concatenates to + /// a single string on resolution; only a whole-field single-expression + /// format string can resolve to ``None`` or to a list. ``is_literal`` + /// distinguishes the two single-segment cases. + fn segment_count(&self) -> usize { + self.inner.segment_count() + } + + /// The literal (non-expression) text runs, in order. + /// + /// Literal runs appear verbatim in every possible resolution, so a + /// property that holds for one holds for every string this format + /// string can resolve to. Expression source text is not included. + fn literal_segments(&self) -> Vec { + self.inner + .literal_segments() + .map(|s| s.to_string()) + .collect() + } + /// Copy symbol table entries referenced by this format string's expressions /// from `source` into `dest`. Only copies the actual values referenced, /// stopping at property/method access (e.g. for `Param.Name.upper()`, diff --git a/specs/python-expr-interface.md b/specs/python-expr-interface.md index 7ad6b38b..d47d685f 100644 --- a/specs/python-expr-interface.md +++ b/specs/python-expr-interface.md @@ -763,6 +763,13 @@ fs.is_literal() # False fs.expression_names() # ["Param.Frame"] fs.has_complex_expressions() # False (simple name reference) +# Static shape: literal runs appear verbatim in every resolution, and a +# format string with more than one segment always concatenates to a +# single string (only a whole-field single expression can resolve to +# None or to a list). +fs.segment_count() # 2 +fs.literal_segments() # ["render --frame "] + # Resolve st = SymbolTable({"Param.Frame": 42}) fs.resolve_string(st) # "render --frame 42" diff --git a/src/openjd/_openjd_rs.pyi b/src/openjd/_openjd_rs.pyi index 9547295d..8586f25f 100644 --- a/src/openjd/_openjd_rs.pyi +++ b/src/openjd/_openjd_rs.pyi @@ -893,6 +893,25 @@ class FormatString: def has_complex_expressions(self) -> builtins.bool: ... def expression_names(self) -> builtins.list[builtins.str]: ... def is_literal(self) -> builtins.bool: ... + def segment_count(self) -> builtins.int: + r""" + Number of parsed segments: literal runs and ``{{...}}`` expressions. + + A format string with more than one segment always concatenates to + a single string on resolution; only a whole-field single-expression + format string can resolve to ``None`` or to a list. ``is_literal`` + distinguishes the two single-segment cases. + """ + + def literal_segments(self) -> builtins.list[builtins.str]: + r""" + The literal (non-expression) text runs, in order. + + Literal runs appear verbatim in every possible resolution, so a + property that holds for one holds for every string this format + string can resolve to. Expression source text is not included. + """ + def copy_used_symtab_values(self, source: SymbolTable, dest: typing.Any) -> None: r""" Copy symbol table entries referenced by this format string's expressions diff --git a/test/openjd/expr/test_lists.py b/test/openjd/expr/test_lists.py index 93f23073..15c8b632 100644 --- a/test/openjd/expr/test_lists.py +++ b/test/openjd/expr/test_lists.py @@ -510,6 +510,99 @@ def test_path_not_in_list(self) -> None: ) +class TestListMembershipElementTypeCheck: + """``in`` / ``not in`` type-check the item against the list's element type + (Expression Language §2.1.3: ``__contains__(list: list[T], item: T)``). + + openjd-expr 0.8.0 (openjd-rs#396). Before it, ``'a' in [1, 2]`` evaluated to + ``False`` where the spec gives it no signature at all. Cases mirror the + upstream ``test_comparison.rs``; the int/float and path/string coercions the + equality rule already allowed are kept. + """ + + @pytest.mark.parametrize( + "expr, detail", + [ + ( + "'1' in [1, 2, 3]", + "item of type string is not compatible with the element type int of list[int]", + ), + ( + "1 in ['a', 'b']", + "item of type int is not compatible with the element type string of list[string]", + ), + ( + "true in [1, 2]", + "item of type bool is not compatible with the element type int of list[int]", + ), + ( + "null in [1, 2]", + "item of type nulltype is not compatible with the element type int of list[int]", + ), + ( + "['a'] in [[1], [2]]", + "item of type list[string] is not compatible with the element type list[int] of list[list[int]]", + ), + ( + "'a' in [x for x in [1, 2]]", + "item of type string is not compatible with the element type int of list[int]", + ), + ( + "[1, 2] in [1, 2]", + "item of type list[int] is not compatible with the element type int of list[int]", + ), + ], + ) + def test_incompatible_item_type_is_refused(self, expr: str, detail: str) -> None: + with pytest.raises(ExpressionError) as excinfo: + evaluate_expression(expr) + assert f"Cannot use 'in' operator: {detail}" in str(excinfo.value) + + def test_not_in_is_refused_the_same_way(self) -> None: + with pytest.raises(ExpressionError) as excinfo: + evaluate_expression("'a' not in [1, 2]") + assert ( + "Cannot use 'not in' operator: item of type string is not compatible " + "with the element type int of list[int]" + ) in str(excinfo.value) + + @pytest.mark.parametrize( + "expr, expected", + [ + ("1 in [1.0, 2.0]", True), + ("3 in [1.0, 2.0]", False), + ("1.0 in [1, 2]", True), + ("1.5 in [1, 2]", False), + ("[1] in [[1.0], [2.0]]", True), + ("[1.5] in [[1], [2]]", False), + ], + ) + def test_int_float_coercion_is_kept(self, expr: str, expected: bool) -> None: + assert evaluate_expression(expr).item() is expected + + @pytest.mark.parametrize( + "expr, expected", + [ + ("path(['/a']) in ['/a', '/b']", True), + ("'/a' in [path(['/a']), path(['/b'])]", True), + ("'/c' in [path(['/a']), path(['/b'])]", False), + ], + ) + def test_path_string_coercion_is_kept(self, expr: str, expected: bool) -> None: + assert evaluate_expression(expr, path_format=PathFormat.POSIX).item() is expected + + @pytest.mark.parametrize( + "expr", + ["1 in []", "'a' in []", "path(['/a']) in []", "[1] in []", "[] in []"], + ) + def test_empty_list_accepts_any_item_type(self, expr: str) -> None: + assert evaluate_expression(expr, path_format=PathFormat.POSIX).item() is False + + def test_empty_list_item_against_nested_lists(self) -> None: + assert evaluate_expression("[] in [[1]]").item() is False + assert evaluate_expression("[] in [[]]").item() is True + + class TestSortedReversed: """Tests for sorted() and reversed() functions.""" diff --git a/test/openjd/expr/test_parse_expression.py b/test/openjd/expr/test_parse_expression.py index 0edb0502..208c3182 100644 --- a/test/openjd/expr/test_parse_expression.py +++ b/test/openjd/expr/test_parse_expression.py @@ -218,6 +218,39 @@ def test_has_complex_expressions(self): assert not FormatString("{{Param.Name}}").has_complex_expressions() assert FormatString("{{Param.A + Param.B}}").has_complex_expressions() + @pytest.mark.parametrize( + "raw, count", + [ + ("hello", 1), + ("{{Param.X}}", 1), + ("v{{Param.X}}", 2), + ("{{Param.X}}-{{Param.Y}}", 3), + ("", 0), + ], + ) + def test_segment_count(self, raw: str, count: int): + # Cases mirror openjd-expr's segment_count_by_shape. A single literal + # run and a whole-field expression are both one segment. + assert FormatString(raw).segment_count() == count + + def test_segment_count_single_segment_disambiguated_by_is_literal(self): + assert FormatString("hello").is_literal() + assert not FormatString("{{Param.X}}").is_literal() + assert FormatString("").is_literal() + + @pytest.mark.parametrize( + "raw, literals", + [ + ("a-{{ Param.X }}-b{{ 'y' }}", ["a-", "-b"]), + ("{{ Param.X }}", []), + ("plain", ["plain"]), + ("", []), + ], + ) + def test_literal_segments(self, raw: str, literals: list[str]): + # Expression source text is excluded, so "'y'" never appears. + assert FormatString(raw).literal_segments() == literals + def test_literal_resolve(self): fs = FormatString("no interpolation") st = SymbolTable() diff --git a/test/openjd/model_v1/test_create_job.py b/test/openjd/model_v1/test_create_job.py index 56a6d015..83c567e0 100644 --- a/test/openjd/model_v1/test_create_job.py +++ b/test/openjd/model_v1/test_create_job.py @@ -1581,3 +1581,138 @@ def test_not_in_v1_top_level(self) -> None: def test_import_raises_import_error(self) -> None: with pytest.raises(ImportError): from openjd.model._v1 import TokenError # type: ignore[attr-defined] # noqa: F401 + + +class TestPreprocessAcceptsItsOwnEmptyListPath: + """``preprocess_job_parameters`` must accept every value it emits, because + callers feed it its own output (preprocess, then ``create_job``, which + re-checks constraints). openjd-model 0.8.0 (openjd-rs#384) fixes the one + value that broke this: a ``LIST[PATH]`` parameter defaulting to ``[]``, which + 0.7.1 refused on the second pass with ``Cannot coerce list to LIST[PATH]``. + """ + + @staticmethod + def _template() -> Any: + return decode_job_template( + template={ + "specificationVersion": "jobtemplate-2023-09", + "name": "T", + "extensions": ["EXPR"], + "parameterDefinitions": [{"name": "Empty", "type": "LIST[PATH]", "default": []}], + "steps": [{"name": "S", "script": {"actions": {"onRun": {"command": "echo"}}}}], + }, + supported_extensions=["EXPR"], + ) + + def test_second_pass_accepts_the_first_pass_output(self) -> None: + template = self._template() + first = preprocess_job_parameters( + job_template=template, + job_parameter_values={}, + job_template_dir=Path.cwd(), + current_working_dir=Path.cwd(), + ) + second = preprocess_job_parameters( + job_template=template, + job_parameter_values=first, + job_template_dir=Path.cwd(), + current_working_dir=Path.cwd(), + ) + assert first == second + assert first["Empty"].type == JobParameterType.LIST_PATH + assert first["Empty"].value == "[]" + + def test_create_job_accepts_the_preprocessed_value(self) -> None: + template = self._template() + values = preprocess_job_parameters( + job_template=template, + job_parameter_values={}, + job_template_dir=Path.cwd(), + current_working_dir=Path.cwd(), + ) + job = create_job(job_template=template, job_parameter_values=values) + assert job.parameters["Empty"].value.item() == [] + + +class TestResolvedJobNameControlCharacters: + """Template Schemas §1.1.1 forbids Cc characters in the job name. When the name + is interpolated, the resolved value is only known at ``create_job``, which + since openjd-model 0.8.0 (openjd-rs#397) rejects a control character there. + 0.7.1 re-checked only emptiness and length, so ``Suffix = "a\\nb"`` produced a + job named ``render-a\\nb``. + """ + + @staticmethod + def _template() -> Any: + return decode_job_template( + template={ + "specificationVersion": "jobtemplate-2023-09", + "name": "render-{{ Param.Suffix }}", + "extensions": ["EXPR"], + "parameterDefinitions": [{"name": "Suffix", "type": "STRING"}], + "steps": [{"name": "S", "script": {"actions": {"onRun": {"command": "echo"}}}}], + }, + supported_extensions=["EXPR"], + ) + + @pytest.mark.parametrize("suffix", ["a\nb", "a\tb", "\x7f"], ids=["newline", "tab", "DEL"]) + def test_control_character_in_the_resolved_name_is_rejected(self, suffix: str) -> None: + with pytest.raises(DecodeValidationError) as excinfo: + create_job(job_template=self._template(), job_parameter_values={"Suffix": suffix}) + assert "Job name must not contain control characters" in str(excinfo.value) + + def test_clean_resolved_name_is_accepted(self) -> None: + job = create_job(job_template=self._template(), job_parameter_values={"Suffix": "ok"}) + assert job.name == "render-ok" + + +class TestDeferredSingleValuedAllOfIsRecheckedAtJobCreation: + """Companion to ``test_parse.py::TestResolvedValueConstraintsAtTemplateValidation``. + A whole-field expression element of a single-valued ``allOf`` passes template + validation because it may resolve to ``null`` and skip itself. openjd-model + 0.8.0 (openjd-rs#397) then re-checks the resolved element count at job creation. + """ + + @staticmethod + def _template() -> Any: + return decode_job_template( + template={ + "specificationVersion": "jobtemplate-2023-09", + "name": "T", + "extensions": ["EXPR"], + "parameterDefinitions": [{"name": "X", "type": "STRING"}], + "steps": [ + { + "name": "S", + "script": {"actions": {"onRun": {"command": "echo"}}}, + "hostRequirements": { + "attributes": [ + { + "name": "attr.worker.os.family", + "allOf": [ + "linux", + "{{ Param.X if Param.X != 'skip' else null }}", + ], + } + ] + }, + } + ], + }, + supported_extensions=["EXPR"], + ) + + def test_a_second_resolved_value_is_rejected(self) -> None: + with pytest.raises(DecodeValidationError) as excinfo: + create_job(job_template=self._template(), job_parameter_values={"X": "windows"}) + assert ( + "steps[0] -> hostRequirements -> attributes[0] -> allOf: " + "single-valued attribute cannot have more than 1 element after resolution" + ) in str(excinfo.value) + + def test_a_null_skipped_element_leaves_one_value(self) -> None: + job = create_job(job_template=self._template(), job_parameter_values={"X": "skip"}) + host_requirements = job.steps[0].host_requirements + assert host_requirements is not None + assert host_requirements.attributes is not None + assert host_requirements.attributes[0].all_of == ["linux"] diff --git a/test/openjd/model_v1/test_errors.py b/test/openjd/model_v1/test_errors.py index 26d83278..67764b0c 100644 --- a/test/openjd/model_v1/test_errors.py +++ b/test/openjd/model_v1/test_errors.py @@ -86,7 +86,13 @@ def test_chunk_default_task_count_invalid_int(self) -> None: """A CHUNK[INT] ``defaultTaskCount`` format string that resolves to a non-integer value at ``create_job`` time raises ``ExpressionError`` (not the generic - ``ModelValidationError``).""" + ``ModelValidationError``). + + The message is the evaluator's int coercion failure: since + openjd-model 0.8.0 the whole-field expression resolves with + target type ``int`` (Expression Language §1.2.3), so the + STRING value is refused inside the expression rather than by a + parse of the concatenated text.""" t = decode_job_template( template={ "specificationVersion": "jobtemplate-2023-09", @@ -128,5 +134,8 @@ def test_chunk_default_task_count_invalid_int(self) -> None: }, supported_extensions=["TASK_CHUNKING"], ) - with pytest.raises(ExpressionError, match="not a valid integer"): + with pytest.raises( + ExpressionError, + match=r"chunks\.defaultTaskCount: Cannot convert 'not-an-integer' to int", + ): create_job(job_template=t, job_parameter_values={}) diff --git a/test/openjd/model_v1/test_parse.py b/test/openjd/model_v1/test_parse.py index feefc1c2..dbca11f8 100644 --- a/test/openjd/model_v1/test_parse.py +++ b/test/openjd/model_v1/test_parse.py @@ -504,3 +504,162 @@ def test_max_env_count_counts_repeated_names_separately(self) -> None: caller_limits=CallerLimits(max_env_count=4), ) assert "total environments (5) exceeds caller limit of 4" in str(excinfo.value) + + +def _one_step_template(**overrides: Any) -> dict[str, Any]: + """Minimal 2023-09 job template with one echo step; ``overrides`` replace + top-level fields.""" + template: dict[str, Any] = { + "specificationVersion": "jobtemplate-2023-09", + "name": "T", + "steps": [{"name": "S", "script": {"actions": {"onRun": {"command": "echo"}}}}], + } + template.update(overrides) + return template + + +class TestEmbeddedFileFilenameIsASinglePathComponent(object): + """openjd-sessions joins an embedded file's ``filename`` to the session + directory, so openjd-model requires it to be a plain single path component. + Template Schemas §6.1.1 only says "characters allowed in filenames on the host + operating system", so this is the implementation's rule, not a conformance one. + openjd-model 0.8.0 (openjd-rs#359) rejects ``.``, ``..`` and a null byte; 0.7.1 + only rejected ``/`` and ``\\``. The v0 reference accepts all three, so this pins + a v1 behaviour v0 does not share. + """ + + @staticmethod + def _template(filename: str) -> dict[str, Any]: + return _one_step_template( + steps=[ + { + "name": "S", + "script": { + "actions": {"onRun": {"command": "echo"}}, + "embeddedFiles": [ + {"name": "F", "type": "TEXT", "filename": filename, "data": "x"} + ], + }, + } + ] + ) + + @pytest.mark.parametrize( + "filename, detail", + [ + pytest.param(".", "must not be '.'.", id="dot"), + pytest.param("..", "must not be '..'.", id="dot-dot"), + pytest.param("a\x00b", "must not contain null characters.", id="null byte"), + ], + ) + def test_unsafe_filename_is_rejected(self, filename: str, detail: str) -> None: + with pytest.raises(ModelValidationError) as excinfo: + decode_job_template(template=self._template(filename)) + message = str(excinfo.value) + assert "steps[0] -> script -> embeddedFiles[0] -> filename" in message + assert detail in message + + def test_plain_filename_is_accepted(self) -> None: + """Control: a dotted basename is still a single component.""" + assert decode_job_template(template=self._template("scene.v2.ma")) + + +class TestResolvedValueConstraintsAtTemplateValidation(object): + """Constraints the spec places on what a format string resolves to are + checked at template validation when the value is statically knowable. + openjd-model 0.8.0 (openjd-rs#383, follow-ups in #397). Each test says what + 0.7.1 did with the same template; two cases below are controls 0.7.1 already + handled, kept so the checks they exercise cannot regress together. + """ + + def test_job_name_that_resolves_over_128_characters_is_rejected(self) -> None: + """Template Schemas §1.1.1: the job name resolves to at most 128 characters. + The expression has no free symbols, so the length is known at validation. + 0.7.1 accepted this template.""" + template = _one_step_template(extensions=["EXPR"], name="{{ 'x' * 129 }}") + with pytest.raises(ModelValidationError) as excinfo: + decode_job_template(template=template, supported_extensions=["EXPR"]) + message = str(excinfo.value) + assert "name:" in message + assert "resolves to at least 129 characters, exceeding the maximum of 128." in message + + def test_job_name_that_resolves_to_128_characters_is_accepted(self) -> None: + """Boundary control.""" + template = _one_step_template(extensions=["EXPR"], name="{{ 'x' * 128 }}") + assert decode_job_template(template=template, supported_extensions=["EXPR"]) + + def test_control_character_in_a_literal_run_of_an_interpolated_name_is_rejected( + self, + ) -> None: + """A literal run appears verbatim in every resolution, so a tab there is a + certain §1.1.1 violation even though ``Param.S`` is unknown until job + creation. 0.8.0 checks ``FormatString.literal_segments``; 0.7.1 also + rejected this, by scanning the raw string, so this is a control.""" + template = _one_step_template( + extensions=["EXPR"], + name="a\t-{{ Param.S }}", + parameterDefinitions=[{"name": "S", "type": "STRING"}], + ) + with pytest.raises(ModelValidationError) as excinfo: + decode_job_template(template=template, supported_extensions=["EXPR"]) + message = str(excinfo.value) + assert "name:" in message + assert "contains control characters." in message + + def test_interpolated_name_with_clean_literal_runs_is_accepted(self) -> None: + """Control: what the expression resolves to is not this check's business.""" + template = _one_step_template( + extensions=["EXPR"], + name="a-{{ Param.S }}", + parameterDefinitions=[{"name": "S", "type": "STRING"}], + ) + assert decode_job_template(template=template, supported_extensions=["EXPR"]) + + @staticmethod + def _os_family_all_of(values: list[str]) -> dict[str, Any]: + return _one_step_template( + extensions=["EXPR"], + parameterDefinitions=[{"name": "X", "type": "STRING"}], + steps=[ + { + "name": "S", + "script": {"actions": {"onRun": {"command": "echo"}}}, + "hostRequirements": { + "attributes": [{"name": "attr.worker.os.family", "allOf": values}] + }, + } + ], + ) + + @pytest.mark.parametrize( + "values", + [ + pytest.param(["linux", "windows"], id="two literals"), + pytest.param(["linux", "{{ Param.X }}-y"], id="literal and multi-segment"), + pytest.param(["{{ Param.X }}-a", "{{ Param.X }}-b"], id="two multi-segment"), + ], + ) + def test_two_certain_allof_values_on_a_single_valued_attribute_are_rejected( + self, values: list[str] + ) -> None: + """A host has one ``attr.worker.os.family`` (Template Schemas §3.3.2), so an + ``allOf`` with two elements is unsatisfiable. An element is certain to be + present when it is a literal or has more than one segment: a multi-segment + format string always concatenates to one string (Expression Language §1.3.2). + 0.7.1 rejected all three shapes too; this is the control for the deferral + test below.""" + with pytest.raises(ModelValidationError) as excinfo: + decode_job_template( + template=self._os_family_all_of(values), supported_extensions=["EXPR"] + ) + message = str(excinfo.value) + assert "steps[0] -> hostRequirements -> attributes[0] -> allOf" in message + assert "single-valued attribute cannot have more than 1 element." in message + + def test_a_whole_field_expression_allof_element_is_deferred_to_job_creation(self) -> None: + """Only a whole-field single-expression element can resolve to ``null`` and skip + itself, so its contribution is unknowable at validation. 0.8.0 (openjd-rs#397) + accepts the template here; 0.7.1 rejected it. Job creation re-checks the + resolved count, so the constraint is deferred, not dropped.""" + template = self._os_family_all_of(["linux", "{{ Param.X }}"]) + assert decode_job_template(template=template, supported_extensions=["EXPR"]) diff --git a/test/openjd/model_v1/test_step_param_space_def.py b/test/openjd/model_v1/test_step_param_space_def.py index 4b790915..8999f9e7 100644 --- a/test/openjd/model_v1/test_step_param_space_def.py +++ b/test/openjd/model_v1/test_step_param_space_def.py @@ -24,8 +24,11 @@ See report finding #11 (`StepTemplate.parameter_space` exposure). """ +import pytest + from openjd.expr import FormatString from openjd.model._v1 import decode_job_template +from openjd.model._v1.errors import ModelValidationError from openjd.model._v1.template import ( ChunkIntTaskParameterDefinition, ChunksDefinition, @@ -37,7 +40,7 @@ ) -def _decode_step(parameter_space=None, *, extensions=None): +def _decode_step(parameter_space=None, *, extensions=None, parameter_definitions=None): """Build a one-step template with the supplied ``parameterSpace`` block and run it through ``decode_job_template``. @@ -55,6 +58,8 @@ def _decode_step(parameter_space=None, *, extensions=None): } if parameter_space is not None: template["steps"][0]["parameterSpace"] = parameter_space + if parameter_definitions is not None: + template["parameterDefinitions"] = parameter_definitions if extensions: template["extensions"] = list(extensions) t = decode_job_template( @@ -297,6 +302,7 @@ def test_chunks_with_format_string_default_task_count(self) -> None: an ``int``.""" step = _decode_step( extensions=["TASK_CHUNKING", "EXPR"], + parameter_definitions=[{"name": "ChunkSize", "type": "INT"}], parameter_space={ "taskParameterDefinitions": [ { @@ -316,6 +322,32 @@ def test_chunks_with_format_string_default_task_count(self) -> None: assert isinstance(chunks.default_task_count, FormatString) assert chunks.default_task_count.raw() == "{{Param.ChunkSize}}" + def test_chunks_default_task_count_undeclared_symbol_rejected(self) -> None: + """A ``defaultTaskCount`` expression naming a job parameter the + template does not declare fails at decode, as it does on the v0 + reference. openjd-model 0.8.0 validates chunk fields at template + validation (openjd-rs#383); 0.7.1 accepted this template at decode.""" + with pytest.raises(ModelValidationError) as exc_info: + _decode_step( + extensions=["TASK_CHUNKING", "EXPR"], + parameter_space={ + "taskParameterDefinitions": [ + { + "name": "F", + "type": "CHUNK[INT]", + "range": "1-100", + "chunks": { + "defaultTaskCount": "{{Param.ChunkSize}}", + "rangeConstraint": "CONTIGUOUS", + }, + } + ] + }, + ) + msg = str(exc_info.value) + assert "taskParameterDefinitions[0] -> chunks -> defaultTaskCount" in msg + assert "Undefined variable: 'Param.ChunkSize'" in msg + # ── Mixed dispatch ── diff --git a/test/openjd/model_v1/test_step_param_space_iter.py b/test/openjd/model_v1/test_step_param_space_iter.py index 9101322b..957409fe 100644 --- a/test/openjd/model_v1/test_step_param_space_iter.py +++ b/test/openjd/model_v1/test_step_param_space_iter.py @@ -1008,3 +1008,65 @@ def test_yielded_values_round_trip_through_contains(self) -> None: fresh = StepParameterSpaceIterator(step=step, chunks_task_count_override=1) for params in list(it): assert params in fresh + + +class TestNoncontiguousChunkRendering: + """A NONCONTIGUOUS chunk renders its values the way the v0 reference's + ``IntRangeExpr.from_list`` does: commit to the first gap as the step, and + consume a consecutive pair atomically. openjd-model 0.8.0 (openjd-rs#398). + 0.7.1 used a compressor that only collapsed runs of three or more agreeing + gaps, so ``[1, 2, 4, 6]`` rendered as ``1,2-6:2``. Every spelling parses to + the same integers, which is why nothing caught the divergence; the rendered + text is what a task sees in ``{{Task.Param.Frame}}``. + + Expected values are the v0 reference's output for the same template, checked + by running both paths side by side when this test was written. + """ + + @staticmethod + def _frames(range_values: list[str], default_task_count: int) -> list[str]: + t = decode_job_template( + template={ + "specificationVersion": "jobtemplate-2023-09", + "name": "T", + "extensions": ["TASK_CHUNKING"], + "steps": [ + { + "name": "S", + "parameterSpace": { + "taskParameterDefinitions": [ + { + "name": "Frame", + "type": "CHUNK[INT]", + "range": range_values, + "chunks": { + "defaultTaskCount": default_task_count, + "rangeConstraint": "NONCONTIGUOUS", + }, + } + ] + }, + "script": { + "actions": { + "onRun": {"command": "echo", "args": ["{{Task.Param.Frame}}"]} + } + }, + } + ], + }, + supported_extensions=["TASK_CHUNKING"], + ) + step = create_job(job_template=t, job_parameter_values={}).steps[0] + return [params["Frame"].value for params in StepParameterSpaceIterator(step=step)] + + @pytest.mark.parametrize( + "range_values, expected", + [ + pytest.param(["1", "2", "4", "6"], "1,2,4,6", id="pair then even progression"), + pytest.param(["1", "3", "4", "5"], "1,3,4,5", id="gap then consecutive run"), + pytest.param(["7", "8"], "7,8", id="lone pair"), + pytest.param(["1", "3", "5", "7"], "1-7:2", id="control: uniform step compresses"), + ], + ) + def test_chunk_renders_like_the_reference(self, range_values: list[str], expected: str) -> None: + assert self._frames(range_values, default_task_count=4) == [expected]