From b0d38bf5895744f0a966a9a577c561483280667c Mon Sep 17 00:00:00 2001 From: LeSingh1 Date: Sat, 8 Aug 2026 18:28:58 -0700 Subject: [PATCH] fix(ci): fail cleanly on a malformed cuda.build.version in versions.yml check_pixi_cuda_version.py returns a diagnostic exit code for every problem it anticipates -- missing versions.yml, missing cuda.build.version, missing pixi.toml, missing feature key -- and then unpacks the version with no checking at all: major, minor, *_ = build_version.split(".") YAML makes that easy to break. `version: "13.3.0"` is quoted today, but drop the quotes and a two-component value loads as a float and a bare number as an int, neither of which has `.split`. Against the real pixi.toml files: version: 13.3 -> AttributeError: 'float' object has no attribute 'split' version: 13 -> AttributeError: 'int' object has no attribute 'split' version: -> AttributeError: 'NoneType' object has no attribute 'split' version: [13, 3] -> AttributeError: 'list' object has no attribute 'split' version: "13" -> ValueError: not enough values to unpack (expected at least 2, got 1) All five escape as an uncaught traceback from a pre-commit hook, pointing at this script rather than at the line the contributor edited. Add `parse_build_version`, which returns `(major, minor)` only for a `.[.]` string of digits, and have `main` report the bad value with the same `return 2` shape as its neighbours. The message names the YAML quoting trap, since that is how the value goes wrong in practice. Adds the first tests for this script. The parse tests cover the accepted shapes and every rejected one; the end-to-end tests drive `main()` against a temporary repo layout and assert exit 2 plus the diagnostic. --- ci/tools/check_pixi_cuda_version.py | 29 ++++- .../tests/test_check_pixi_cuda_version.py | 112 ++++++++++++++++++ 2 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 ci/tools/tests/test_check_pixi_cuda_version.py diff --git a/ci/tools/check_pixi_cuda_version.py b/ci/tools/check_pixi_cuda_version.py index 1ca931b9b48..c3e9a68c530 100644 --- a/ci/tools/check_pixi_cuda_version.py +++ b/ci/tools/check_pixi_cuda_version.py @@ -16,6 +16,24 @@ PIXI_FILES = [ROOT / d / "pixi.toml" for d in ("cuda_bindings", "cuda_core")] +def parse_build_version(build_version: object) -> tuple[str, str] | None: + """Split ``cuda.build.version`` into ``(major, minor)``, or ``None``. + + Returns ``None`` for anything that is not a ``.[.…]`` string + of digits. YAML makes this easy to get wrong: an unquoted ``13.3`` loads as + the float ``13.3`` and an unquoted ``13`` as the int ``13``, neither of + which has ``.split``. Without this check those -- and a quoted but + single-component ``"13"`` -- escaped as a raw traceback from a pre-commit + hook whose every other failure path returns a diagnostic exit code. + """ + if not isinstance(build_version, str): + return None + parts = build_version.split(".") + if len(parts) < 2 or not all(part.isdigit() for part in parts[:2]): + return None + return parts[0], parts[1] + + def main() -> int: """Verify cuda_bindings/cuda_core pixi pins match ci/versions.yml.""" if not VERSIONS_FILE_PATH.is_file(): @@ -27,7 +45,16 @@ def main() -> int: print(f"error: cuda.build.version not found in {VERSIONS_FILE_PATH}", file=sys.stderr) return 2 - major, minor, *_ = build_version.split(".") + parsed = parse_build_version(build_version) + if parsed is None: + print( + f"error: cuda.build.version={build_version!r} in {VERSIONS_FILE_PATH} is not a " + f"'.[.]' version string. Quote the value in YAML so it is " + f"not loaded as a number (13.3 becomes a float, 13 becomes an int).", + file=sys.stderr, + ) + return 2 + major, minor = parsed expected = f"{major}.{minor}.*" cuda_feature = f"cu{major}" diff --git a/ci/tools/tests/test_check_pixi_cuda_version.py b/ci/tools/tests/test_check_pixi_cuda_version.py new file mode 100644 index 00000000000..84130c30f27 --- /dev/null +++ b/ci/tools/tests/test_check_pixi_cuda_version.py @@ -0,0 +1,112 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +from __future__ import annotations + +import os +import sys +import textwrap + +import pytest + +# check_pixi_cuda_version imports PyYAML at module scope (the pre-commit hook +# declares it via additional_dependencies), so skip rather than fail collection +# when this module is exercised outside that environment. +pytest.importorskip("yaml") + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) +import check_pixi_cuda_version as mod +from check_pixi_cuda_version import parse_build_version + +PIXI_TOML = textwrap.dedent("""\ + [workspace.build-variants] + cuda-version = ["12.*", "13.3.*"] + + [feature.cu13.dependencies] + cuda-version = "13.3.*" + """) + + +@pytest.mark.agent_authored(model="claude-opus-5") +@pytest.mark.parametrize( + ("raw", "expected"), + [ + pytest.param("13.3.0", ("13", "3"), id="three-part"), + pytest.param("12.9.1", ("12", "9"), id="three-part-other"), + pytest.param("13.3", ("13", "3"), id="two-part"), + pytest.param("13.3.0.1", ("13", "3"), id="four-part"), + ], +) +def test_parse_build_version_accepts_version_strings(raw, expected): + assert parse_build_version(raw) == expected + + +@pytest.mark.agent_authored(model="claude-opus-5") +@pytest.mark.parametrize( + "raw", + [ + # YAML turns an unquoted `version: 13.3` into a float and an unquoted + # `version: 13` into an int. Neither has .split(), so the tool used to + # die with an AttributeError traceback. + pytest.param(13.3, id="float-from-unquoted-yaml"), + pytest.param(13, id="int-from-unquoted-yaml"), + pytest.param(None, id="none-from-empty-yaml-value"), + pytest.param(["13", "3"], id="list"), + # Quoted, but not a . version: the tuple unpacking used + # to die with "not enough values to unpack". + pytest.param("13", id="single-component"), + pytest.param("", id="empty-string"), + pytest.param("13.", id="trailing-dot"), + pytest.param(".3", id="leading-dot"), + pytest.param("cuda.13", id="non-numeric-major"), + ], +) +def test_parse_build_version_rejects_everything_else(raw): + assert parse_build_version(raw) is None + + +@pytest.mark.agent_authored(model="claude-opus-5") +@pytest.mark.parametrize( + ("yaml_value", "note"), + [ + pytest.param("13.3", "unquoted two-part version loads as a float", id="unquoted-float"), + pytest.param("13", "unquoted single number loads as an int", id="unquoted-int"), + pytest.param('"13"', "quoted but missing a minor component", id="quoted-single-component"), + ], +) +def test_main_reports_a_malformed_build_version(tmp_path, monkeypatch, capsys, yaml_value, note): + """A malformed ci/versions.yml must produce this tool's own diagnostic and + exit 2, not an uncaught traceback out of a pre-commit hook.""" + (tmp_path / "ci").mkdir() + (tmp_path / "ci" / "versions.yml").write_text(f"cuda:\n build:\n version: {yaml_value}\n", encoding="utf-8") + pixi_files = [] + for package in ("cuda_bindings", "cuda_core"): + (tmp_path / package).mkdir() + path = tmp_path / package / "pixi.toml" + path.write_text(PIXI_TOML, encoding="utf-8") + pixi_files.append(path) + + monkeypatch.setattr(mod, "ROOT", tmp_path) + monkeypatch.setattr(mod, "VERSIONS_FILE_PATH", tmp_path / "ci" / "versions.yml") + monkeypatch.setattr(mod, "PIXI_FILES", pixi_files) + + assert mod.main() == 2, note + assert "is not a '.[.]' version string" in capsys.readouterr().err + + +@pytest.mark.agent_authored(model="claude-opus-5") +def test_main_accepts_a_well_formed_build_version(tmp_path, monkeypatch): + (tmp_path / "ci").mkdir() + (tmp_path / "ci" / "versions.yml").write_text('cuda:\n build:\n version: "13.3.0"\n', encoding="utf-8") + pixi_files = [] + for package in ("cuda_bindings", "cuda_core"): + (tmp_path / package).mkdir() + path = tmp_path / package / "pixi.toml" + path.write_text(PIXI_TOML, encoding="utf-8") + pixi_files.append(path) + + monkeypatch.setattr(mod, "ROOT", tmp_path) + monkeypatch.setattr(mod, "VERSIONS_FILE_PATH", tmp_path / "ci" / "versions.yml") + monkeypatch.setattr(mod, "PIXI_FILES", pixi_files) + + assert mod.main() == 0