From 6494aa38bfad25d18519ac4e8a9826fd77bb19eb Mon Sep 17 00:00:00 2001 From: leynos Date: Tue, 28 Jul 2026 12:23:26 +0200 Subject: [PATCH 1/6] Inline maturin pin checks; consolidate wheel-snapshot helper tests/helpers/maturin.py had grown to 394 lines / 15 defs serving one consumer (cuprum/unittests/test_maturin_build.py). Per #97 and #99: - Inline all pin-reading logic (read_expected_maturin_version, read_maturin_pins, read_manylinux_aarch64_container_ref, workflow_uses_manylinux_aarch64_container_ref, _require_pin_match) and its regexes directly into the sole consumer as private helpers. - Consolidate wheel-snapshot parsing from six functions (_header_value, _parse_metadata, _normalise_wheel_entry, _locate_dist_info_wheel, _parse_wheel_header, wheel_build_snapshot) to three by folding the single-purpose extractors into wheel_build_snapshot and _parse_metadata. - Fold _maturin_module_available into toolchain_available. - Document the defer-until-second-consumer policy in the module docstring. The wheel entry-normalisation is retained unchanged: every branch (dist-info suffixes, /sboms/, the .cpython-.so extension) is exercised by the test_maturin_wheel_build_snapshot syrupy snapshot across the build matrix, so none is speculative. Snapshot output is byte- identical. Helper drops to 218 lines / 6 defs. Closes #97 Closes #99 Co-Authored-By: Claude Opus 4.8 (1M context) --- cuprum/unittests/test_maturin_build.py | 92 ++++++++- tests/helpers/maturin.py | 268 +++++-------------------- 2 files changed, 127 insertions(+), 233 deletions(-) diff --git a/cuprum/unittests/test_maturin_build.py b/cuprum/unittests/test_maturin_build.py index 22f5e012..13661cdc 100644 --- a/cuprum/unittests/test_maturin_build.py +++ b/cuprum/unittests/test_maturin_build.py @@ -12,16 +12,10 @@ from tests.helpers.docs import repo_root from tests.helpers.maturin import ( - _AARCH64_CONTAINER_PIN_RE, - _AARCH64_CONTAINER_USAGE_RE, MaturinBuildError, build_native_wheel_artifact, - read_expected_maturin_version, - read_manylinux_aarch64_container_ref, - read_maturin_pins, toolchain_available, wheel_build_snapshot, - workflow_uses_manylinux_aarch64_container_ref, ) if typ.TYPE_CHECKING: @@ -31,14 +25,90 @@ from syrupy.assertion import SnapshotAssertion +_MATURIN_PIN_RE = re.compile(r"maturin==(\d+\.\d+\.\d+)") +_WORKFLOW_PIN_RE = re.compile(r'MATURIN_VERSION:\s*"(\d+\.\d+\.\d+)"') +_ACTION_PIN_RE = re.compile(r'default:\s*"(\d+\.\d+\.\d+)"') +_AARCH64_CONTAINER_PIN_RE = re.compile( + r"^\s*MANYLINUX_AARCH64_CONTAINER:\s*([^\s#]+)\s+#\s*\S.*$", + re.MULTILINE, +) +_AARCH64_CONTAINER_USAGE_RE = re.compile( + r"^\s*container:\s*\$\{\{\s*env\.MANYLINUX_AARCH64_CONTAINER\s*\}\}\s*$", + re.MULTILINE, +) _MANYLINUX_CONTAINER_SHA256_RE = re.compile( r"^ghcr\.io/rust-cross/manylinux_2_28-cross@sha256:[0-9a-f]{64}$" ) +def _require_pin_match( + match: re.Match[str] | None, + location: str, + *, + subject: str = "maturin version pin", +) -> str: + """Return the captured version from a pin match or raise with the location.""" + if match is None: + msg = f"Could not locate {subject} in {location}" + raise AssertionError(msg) + return match.group(1) + + +def _read_text(root: pth.Path, relative: str) -> str: + """Read a repository-relative UTF-8 text file.""" + return (root / relative).read_text(encoding="utf-8") + + +def _read_expected_maturin_version(root: pth.Path) -> str: + """Read the maturin version pinned as the dev dependency in pyproject.toml.""" + return _require_pin_match( + _MATURIN_PIN_RE.search(_read_text(root, "pyproject.toml")), + "pyproject.toml", + ) + + +def _read_maturin_pins(root: pth.Path) -> dict[str, str]: + """Read the maturin version pins from every synchronised location.""" + return { + "pyproject.toml": _require_pin_match( + _MATURIN_PIN_RE.search(_read_text(root, "pyproject.toml")), + "pyproject.toml", + ), + "build-wheels.yml": _require_pin_match( + _WORKFLOW_PIN_RE.search( + _read_text(root, ".github/workflows/build-wheels.yml"), + ), + ".github/workflows/build-wheels.yml", + ), + "build-wheels/action.yml": _require_pin_match( + _ACTION_PIN_RE.search( + _read_text(root, ".github/actions/build-wheels/action.yml"), + ), + ".github/actions/build-wheels/action.yml", + ), + } + + +def _read_manylinux_aarch64_container_ref(root: pth.Path) -> str: + """Read the pinned manylinux aarch64 container reference from the workflow.""" + return _require_pin_match( + _AARCH64_CONTAINER_PIN_RE.search( + _read_text(root, ".github/workflows/build-wheels.yml"), + ), + ".github/workflows/build-wheels.yml", + subject="MANYLINUX_AARCH64_CONTAINER pin", + ) + + +def _workflow_uses_manylinux_aarch64_container_ref(root: pth.Path) -> bool: + """Report whether the workflow references the pinned manylinux container.""" + workflow = _read_text(root, ".github/workflows/build-wheels.yml") + return _AARCH64_CONTAINER_USAGE_RE.search(workflow) is not None + + def test_maturin_pins_are_synchronized() -> None: """Maturin version pins stay aligned across CI and dev dependencies.""" - pins = read_maturin_pins(repo_root()) + pins = _read_maturin_pins(repo_root()) assert len(set(pins.values())) == 1, f"Expected one maturin pin, found {pins!r}" @@ -46,7 +116,7 @@ def test_installed_maturin_matches_expected_pin() -> None: """The active maturin CLI matches the pinned development dependency.""" if shutil.which("maturin") is None: pytest.skip("maturin is not installed.") - expected = read_expected_maturin_version(repo_root()) + expected = _read_expected_maturin_version(repo_root()) installed = im.version("maturin") assert installed == expected, ( f"Expected maturin {expected}, but {installed} is installed" @@ -55,7 +125,7 @@ def test_installed_maturin_matches_expected_pin() -> None: def test_manylinux_aarch64_container_is_pinned_to_sha256() -> None: """Aarch64 manylinux container pin must be immutable.""" - container_ref = read_manylinux_aarch64_container_ref(repo_root()) + container_ref = _read_manylinux_aarch64_container_ref(repo_root()) assert _MANYLINUX_CONTAINER_SHA256_RE.fullmatch(container_ref), ( f"Expected SHA-256 pinned container ref, found {container_ref!r}" ) @@ -63,7 +133,7 @@ def test_manylinux_aarch64_container_is_pinned_to_sha256() -> None: def test_manylinux_aarch64_container_is_referenced_by_build_step() -> None: """The build job should consume the pinned aarch64 container variable.""" - assert workflow_uses_manylinux_aarch64_container_ref(repo_root()), ( + assert _workflow_uses_manylinux_aarch64_container_ref(repo_root()), ( "Expected build-wheels.yml to reference env.MANYLINUX_AARCH64_CONTAINER" ) @@ -195,7 +265,7 @@ def test_maturin_wheel_build_snapshot( ) -> None: """Native wheel metadata and layout match the expected maturin output.""" root = repo_root() - expected = read_expected_maturin_version(root) + expected = _read_expected_maturin_version(root) if not toolchain_available(): pytest.skip("Rust toolchain unavailable.") diff --git a/tests/helpers/maturin.py b/tests/helpers/maturin.py index f2caa9f0..23b6654e 100644 --- a/tests/helpers/maturin.py +++ b/tests/helpers/maturin.py @@ -1,4 +1,12 @@ -"""Shared helpers for maturin build and pin contract tests.""" +"""Helpers for building and introspecting the native maturin wheel. + +Maturin pin-synchronisation checks are inlined in +``cuprum/unittests/test_maturin_build.py``, their sole consumer. This module +retains only the wheel build and snapshot machinery, which wraps ``subprocess`` +and ``zipfile`` introspection that does not inline cleanly. Do not re-externalise +further helpers here until a second concrete consumer exists and the shared +interface can be designed against real requirements. +""" from __future__ import annotations @@ -13,17 +21,6 @@ if typ.TYPE_CHECKING: from pathlib import Path -_MATURIN_PIN_RE = re.compile(r"maturin==(\d+\.\d+\.\d+)") -_WORKFLOW_PIN_RE = re.compile(r'MATURIN_VERSION:\s*"(\d+\.\d+\.\d+)"') -_ACTION_PIN_RE = re.compile(r'default:\s*"(\d+\.\d+\.\d+)"') -_AARCH64_CONTAINER_PIN_RE = re.compile( - r"^\s*MANYLINUX_AARCH64_CONTAINER:\s*([^\s#]+)\s+#\s*\S.*$", - re.MULTILINE, -) -_AARCH64_CONTAINER_USAGE_RE = re.compile( - r"^\s*container:\s*\$\{\{\s*env\.MANYLINUX_AARCH64_CONTAINER\s*\}\}\s*$", - re.MULTILINE, -) _GENERATOR_RE = re.compile(r"^Generator:\s*maturin\s*\(([^)]+)\)\s*$", re.MULTILINE) _EXTENSION_MODULE_RE = re.compile( r"^cuprum/_rust_backend_native\.cpython-[^/]+\.so$", @@ -71,150 +68,16 @@ def __str__(self) -> str: ) -def read_expected_maturin_version(root: Path) -> str: - """Read the maturin version pinned in ``pyproject.toml``. - - Raises - ------ - AssertionError - If the maturin dependency pin is missing. - FileNotFoundError - If ``pyproject.toml`` is absent. - OSError - If ``pyproject.toml`` cannot be read. - UnicodeDecodeError - If ``pyproject.toml`` is not valid UTF-8. - """ - pyproject = (root / "pyproject.toml").read_text(encoding="utf-8") - match = _MATURIN_PIN_RE.search(pyproject) - if match is None: - msg = "Could not locate maturin dev dependency pin in pyproject.toml" - raise AssertionError(msg) - return match.group(1) - - -def _require_pin_match( - match: re.Match[str] | None, - location: str, - *, - subject: str = "maturin version pin", -) -> str: - """Extract a version from a regex match or raise AssertionError with location.""" - if match is None: - msg = f"Could not locate {subject} in {location}" - raise AssertionError(msg) - return match.group(1) - - -def read_maturin_pins(root: Path) -> dict[str, str]: - """Read maturin version pins from the synchronized locations. - - Raises - ------ - AssertionError - If any maturin version pin is missing. - FileNotFoundError - If any pin source file is absent. - OSError - If any pin source file cannot be read. - UnicodeDecodeError - If any pin source file is not valid UTF-8. - """ - pyproject = (root / "pyproject.toml").read_text(encoding="utf-8") - workflow = (root / ".github/workflows/build-wheels.yml").read_text(encoding="utf-8") - action = (root / ".github/actions/build-wheels/action.yml").read_text( - encoding="utf-8" - ) - - return { - "pyproject.toml": _require_pin_match( - _MATURIN_PIN_RE.search(pyproject), - "pyproject.toml", - ), - "build-wheels.yml": _require_pin_match( - _WORKFLOW_PIN_RE.search(workflow), - ".github/workflows/build-wheels.yml", - ), - "build-wheels/action.yml": _require_pin_match( - _ACTION_PIN_RE.search(action), - ".github/actions/build-wheels/action.yml", - ), - } - - -def read_manylinux_aarch64_container_ref(root: Path) -> str: - """Read the pinned manylinux aarch64 container reference. - - Parameters - ---------- - root - Repository root containing ``.github/workflows/build-wheels.yml``. - - Returns - ------- - str - The pinned ``MANYLINUX_AARCH64_CONTAINER`` image reference. - - Raises - ------ - AssertionError - If the ``MANYLINUX_AARCH64_CONTAINER`` pin is missing. - FileNotFoundError - If ``.github/workflows/build-wheels.yml`` is absent. - OSError - If ``.github/workflows/build-wheels.yml`` cannot be read. - UnicodeDecodeError - If ``.github/workflows/build-wheels.yml`` is not valid UTF-8. - """ - workflow = (root / ".github/workflows/build-wheels.yml").read_text(encoding="utf-8") - return _require_pin_match( - _AARCH64_CONTAINER_PIN_RE.search(workflow), - ".github/workflows/build-wheels.yml", - subject="MANYLINUX_AARCH64_CONTAINER pin", - ) - - -def workflow_uses_manylinux_aarch64_container_ref(root: Path) -> bool: - """Report whether the workflow references the pinned manylinux container. - - Parameters - ---------- - root - Repository root containing ``.github/workflows/build-wheels.yml``. - - Returns - ------- - bool - ``True`` when the Linux aarch64 build step uses - ``env.MANYLINUX_AARCH64_CONTAINER``; otherwise ``False``. - - Raises - ------ - FileNotFoundError - If ``.github/workflows/build-wheels.yml`` is absent. - OSError - If ``.github/workflows/build-wheels.yml`` cannot be read. - UnicodeDecodeError - If ``.github/workflows/build-wheels.yml`` is not valid UTF-8. - """ - workflow = (root / ".github/workflows/build-wheels.yml").read_text(encoding="utf-8") - return _AARCH64_CONTAINER_USAGE_RE.search(workflow) is not None - - -def _maturin_module_available() -> bool: - """Return whether the maturin module can be resolved.""" +def toolchain_available() -> bool: + """Return whether the Rust toolchain and the maturin module are available.""" try: - return importlib.util.find_spec("maturin") is not None + maturin_available = importlib.util.find_spec("maturin") is not None except ImportError: - return False - - -def toolchain_available() -> bool: - """Return whether the Rust toolchain and maturin are available.""" + maturin_available = False return ( shutil.which("cargo") is not None and shutil.which("rustc") is not None - and _maturin_module_available() + and maturin_available ) @@ -260,16 +123,8 @@ def build_native_wheel_artifact(root: Path, out_dir: Path) -> Path: return wheels[0] -def _header_value(headers: dict[str, list[str]], key: str) -> str | None: - """Return the first header value for the given key, or None if absent.""" - values = headers.get(key) - if not values: - return None - return values[0] - - def _parse_metadata(raw_metadata: str) -> dict[str, typ.Any]: - """Parse RFC 2822-style metadata headers into a normalised dict.""" + """Parse RFC 2822-style ``METADATA`` headers into a normalised dict.""" headers: dict[str, list[str]] = {} current_key: str | None = None for line in raw_metadata.splitlines(): @@ -282,17 +137,20 @@ def _parse_metadata(raw_metadata: str) -> dict[str, typ.Any]: current_key = key.strip() headers.setdefault(current_key, []).append(value.strip()) + name = headers.get("Name") + version = headers.get("Version") + requires_python = headers.get("Requires-Python") return { - "name": _header_value(headers, "Name"), - "version": _header_value(headers, "Version"), - "requires_python": _header_value(headers, "Requires-Python"), + "name": name[0] if name else None, + "version": version[0] if version else None, + "requires_python": requires_python[0] if requires_python else None, "requires_dist": sorted(headers.get("Requires-Dist", [])), "classifiers": sorted(headers.get("Classifier", [])), } def _normalise_wheel_entry(name: str) -> str: - """Normalize platform/version wheel entry names to stable placeholders.""" + """Normalise platform- and version-specific wheel entry names to placeholders.""" if _EXTENSION_MODULE_RE.match(name): return "cuprum/_rust_backend_native.cpython-.so" if "/sboms/" in name: @@ -303,49 +161,36 @@ def _normalise_wheel_entry(name: str) -> str: return name -def _locate_dist_info_wheel(entry_names: list[str]) -> str: - """Return the .dist-info/WHEEL entry name from a wheel archive's namelist. +def wheel_build_snapshot(whl_path: Path) -> dict[str, typ.Any]: + """Return a normalised snapshot of wheel metadata and layout. - Parameters - ---------- - entry_names: - All entry names returned by ``zipfile.ZipFile.namelist()``. + Reads the archive's ``.dist-info/WHEEL`` and ``.dist-info/METADATA`` members, + extracts the maturin generator and ``Root-Is-Purelib`` flag, and normalises + platform-specific entry names so the snapshot is stable across the build + matrix. Raises ------ AssertionError - If no ``.dist-info/WHEEL`` entry is present. + If the wheel metadata is missing expected maturin fields. + OSError + If the wheel file cannot be opened or read. + zipfile.BadZipFile + If the wheel file is not a valid zip archive. """ - wheel_name = next( - (name for name in entry_names if name.endswith(".dist-info/WHEEL")), - None, - ) - if wheel_name is None: - msg = "wheel is missing .dist-info/WHEEL metadata" - raise AssertionError(msg) - return wheel_name - - -def _parse_wheel_header(wheel_payload: str, whl_path: Path) -> tuple[str, str]: - """Extract the maturin generator string and Root-Is-Purelib value. - - Parameters - ---------- - wheel_payload: - Decoded text content of the ``.dist-info/WHEEL`` file. - whl_path: - Path to the wheel archive; used only in error messages. - - Returns - ------- - tuple[str, str] - ``(generator, root_is_purelib)`` extracted from the WHEEL headers. + with zipfile.ZipFile(whl_path) as archive: + entry_names = archive.namelist() + wheel_name = next( + (name for name in entry_names if name.endswith(".dist-info/WHEEL")), + None, + ) + if wheel_name is None: + msg = "wheel is missing .dist-info/WHEEL metadata" + raise AssertionError(msg) + metadata_name = wheel_name.replace("/WHEEL", "/METADATA") + wheel_payload = archive.read(wheel_name).decode("utf-8") + metadata_payload = archive.read(metadata_name).decode("utf-8") - Raises - ------ - AssertionError - If either field cannot be parsed. - """ generator_match = _GENERATOR_RE.search(wheel_payload) if generator_match is None: msg = f"Could not parse maturin generator from WHEEL metadata: {whl_path}" @@ -361,30 +206,9 @@ def _parse_wheel_header(wheel_payload: str, whl_path: Path) -> tuple[str, str]: if root_is_purelib is None: msg = "wheel is missing Root-Is-Purelib metadata" raise AssertionError(msg) - return generator_match.group(1), root_is_purelib - -def wheel_build_snapshot(whl_path: Path) -> dict[str, typ.Any]: - """Return a normalised snapshot of wheel metadata and layout. - - Raises - ------ - AssertionError - If the wheel metadata is missing expected maturin fields. - OSError - If the wheel file cannot be opened or read. - zipfile.BadZipFile - If the wheel file is not a valid zip archive. - """ - with zipfile.ZipFile(whl_path) as archive: - entry_names = archive.namelist() - wheel_name = _locate_dist_info_wheel(entry_names) - metadata_name = wheel_name.replace("/WHEEL", "/METADATA") - wheel_payload = archive.read(wheel_name).decode("utf-8") - metadata_payload = archive.read(metadata_name).decode("utf-8") - generator, root_is_purelib = _parse_wheel_header(wheel_payload, whl_path) return { - "generator": generator, + "generator": generator_match.group(1), "metadata": _parse_metadata(metadata_payload), "wheel": { "root_is_purelib": root_is_purelib, From 724c455850083af0422818b497519eacb34dd318 Mon Sep 17 00:00:00 2001 From: leynos Date: Tue, 28 Jul 2026 13:37:42 +0200 Subject: [PATCH 2/6] Split wheel_build_snapshot to satisfy CodeScene complexity gate Consolidating the wheel parsing pushed wheel_build_snapshot to a cyclomatic complexity of 10 (threshold 9) and the module mean to 4.43 (threshold 4). Extract _read_wheel_members (archive read + member lookup) and _parse_wheel_header (generator + Root-Is-Purelib), leaving wheel_build_snapshot as a thin orchestrator. Behaviour and snapshot output are unchanged; CodeScene health returns to 10.0. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/helpers/maturin.py | 46 ++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/tests/helpers/maturin.py b/tests/helpers/maturin.py index 23b6654e..15136dca 100644 --- a/tests/helpers/maturin.py +++ b/tests/helpers/maturin.py @@ -161,23 +161,8 @@ def _normalise_wheel_entry(name: str) -> str: return name -def wheel_build_snapshot(whl_path: Path) -> dict[str, typ.Any]: - """Return a normalised snapshot of wheel metadata and layout. - - Reads the archive's ``.dist-info/WHEEL`` and ``.dist-info/METADATA`` members, - extracts the maturin generator and ``Root-Is-Purelib`` flag, and normalises - platform-specific entry names so the snapshot is stable across the build - matrix. - - Raises - ------ - AssertionError - If the wheel metadata is missing expected maturin fields. - OSError - If the wheel file cannot be opened or read. - zipfile.BadZipFile - If the wheel file is not a valid zip archive. - """ +def _read_wheel_members(whl_path: Path) -> tuple[list[str], str, str]: + """Return the entry names and the decoded ``WHEEL``/``METADATA`` payloads.""" with zipfile.ZipFile(whl_path) as archive: entry_names = archive.namelist() wheel_name = next( @@ -190,7 +175,11 @@ def wheel_build_snapshot(whl_path: Path) -> dict[str, typ.Any]: metadata_name = wheel_name.replace("/WHEEL", "/METADATA") wheel_payload = archive.read(wheel_name).decode("utf-8") metadata_payload = archive.read(metadata_name).decode("utf-8") + return entry_names, wheel_payload, metadata_payload + +def _parse_wheel_header(wheel_payload: str, whl_path: Path) -> tuple[str, str]: + """Extract the maturin generator and ``Root-Is-Purelib`` from a WHEEL file.""" generator_match = _GENERATOR_RE.search(wheel_payload) if generator_match is None: msg = f"Could not parse maturin generator from WHEEL metadata: {whl_path}" @@ -206,9 +195,30 @@ def wheel_build_snapshot(whl_path: Path) -> dict[str, typ.Any]: if root_is_purelib is None: msg = "wheel is missing Root-Is-Purelib metadata" raise AssertionError(msg) + return generator_match.group(1), root_is_purelib + +def wheel_build_snapshot(whl_path: Path) -> dict[str, typ.Any]: + """Return a normalised snapshot of wheel metadata and layout. + + Reads the archive's ``.dist-info/WHEEL`` and ``.dist-info/METADATA`` members, + extracts the maturin generator and ``Root-Is-Purelib`` flag, and normalises + platform-specific entry names so the snapshot is stable across the build + matrix. + + Raises + ------ + AssertionError + If the wheel metadata is missing expected maturin fields. + OSError + If the wheel file cannot be opened or read. + zipfile.BadZipFile + If the wheel file is not a valid zip archive. + """ + entry_names, wheel_payload, metadata_payload = _read_wheel_members(whl_path) + generator, root_is_purelib = _parse_wheel_header(wheel_payload, whl_path) return { - "generator": generator_match.group(1), + "generator": generator, "metadata": _parse_metadata(metadata_payload), "wheel": { "root_is_purelib": root_is_purelib, From 8b3df4abec36eaefc5c222c6389e715042dca642 Mon Sep 17 00:00:00 2001 From: leynos Date: Tue, 28 Jul 2026 22:54:14 +0200 Subject: [PATCH 3/6] Document the maturin helper boundary and reuse policy The developers guide still described tests/helpers/maturin.py as providing shared helpers for the pin-contract tests, which is the opposite of the boundary this branch implements: the pin checks are now private helpers in cuprum/unittests/test_maturin_build.py and only the wheel build/snapshot machinery remains shared. Rewrite the section intro to state both halves of the split, name the retained module's narrow scope, and record the defer-until-second-consumer reuse policy the abstraction policy in AGENTS.md requires. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/developers-guide.md | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index ab54405e..8667074c 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -1400,8 +1400,22 @@ change alters the architecture of the lint gate, update ## Maturin pin synchronization and native wheel tests -The `tests/helpers/maturin.py` module provides shared helpers for tests that -validate the maturin version pin contract and native wheel build output. +These checks live in `cuprum/unittests/test_maturin_build.py`, split across two +boundaries. The **pin-synchronization** checks are inlined there as private +helpers (`_read_maturin_pins`, `_read_expected_maturin_version`, +`_read_manylinux_aarch64_container_ref`, and their regexes): they read +repository files, have a single consumer, and gain nothing from indirection. +The **native wheel build and snapshot** machinery stays in +`tests/helpers/maturin.py`, because it wraps `subprocess` and `zipfile` +introspection that does not inline cleanly. + +That module's scope is deliberately narrow: build a wheel with the pinned +maturin (`build_native_wheel_artifact`), report toolchain availability +(`toolchain_available`), and normalize a built wheel into a stable snapshot +(`wheel_build_snapshot`). Its re-use policy is to stay that way — do not +re-externalize further helpers into it until a second concrete consumer exists +and the shared interface can be designed against real requirements rather than +anticipated ones. **Pin synchronization** (`test_maturin_pins_are_synchronized`) Asserts that the maturin version declared in `pyproject.toml`, From cc49f1fc4fdde7dd6d55bbed4e6ed1638144f8ce Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 29 Jul 2026 00:51:33 +0200 Subject: [PATCH 4/6] Correct maturin helper docs and guard missing wheel METADATA Review round on the helper simplification: - _read_wheel_members derived the METADATA member name and read it without checking membership, so a wheel missing that member raised KeyError from ZipFile.read rather than the AssertionError wheel_build_snapshot documents. Assert membership first, matching the existing WHEEL guard, and cover both missing-member cases with a parametrised test. Verified non-vacuous: removing the guard reproduces the KeyError the finding predicted. - build_native_wheel_artifact runs `python -m maturin` under the current interpreter, so it uses whichever maturin that environment provides; it does not select a pinned version. Correct that claim in both the docstring and the developers guide, and name the two mechanisms that actually enforce alignment: the installed-CLI pin check and the snapshot test's Generator assertion. - Add NumPy-style Returns sections to toolchain_available and wheel_build_snapshot, matching the Parameters/Raises sections already used in this module. - Move docstring prose to Oxford -ize spelling per AGENTS.md. Identifiers keep the -ise spelling that prevails across the repository, since the rule governs comments rather than names. Co-Authored-By: Claude Opus 4.8 (1M context) --- cuprum/unittests/test_maturin_build.py | 38 +++++++++++++++++- docs/developers-guide.md | 12 ++++-- tests/helpers/maturin.py | 55 +++++++++++++++++++++----- 3 files changed, 92 insertions(+), 13 deletions(-) diff --git a/cuprum/unittests/test_maturin_build.py b/cuprum/unittests/test_maturin_build.py index 13661cdc..ccbab32b 100644 --- a/cuprum/unittests/test_maturin_build.py +++ b/cuprum/unittests/test_maturin_build.py @@ -7,6 +7,7 @@ import shutil import subprocess # noqa: S404 - tests assert trusted maturin command handling. import typing as typ +import zipfile import pytest @@ -68,7 +69,7 @@ def _read_expected_maturin_version(root: pth.Path) -> str: def _read_maturin_pins(root: pth.Path) -> dict[str, str]: - """Read the maturin version pins from every synchronised location.""" + """Read the maturin version pins from every synchronized location.""" return { "pyproject.toml": _require_pin_match( _MATURIN_PIN_RE.search(_read_text(root, "pyproject.toml")), @@ -277,3 +278,38 @@ def test_maturin_wheel_build_snapshot( assert snapshot_payload == snapshot, ( "Built wheel metadata, file list, and build settings changed." ) + + +@pytest.mark.parametrize( + ("members", "expected_message"), + [ + pytest.param( + {"cuprum-0.1.0.dist-info/METADATA": "Name: cuprum\n"}, + "wheel is missing .dist-info/WHEEL metadata", + id="missing_wheel", + ), + pytest.param( + {"cuprum-0.1.0.dist-info/WHEEL": "Root-Is-Purelib: false\n"}, + "wheel is missing .dist-info/METADATA metadata", + id="missing_metadata", + ), + ], +) +def test_wheel_build_snapshot_reports_missing_dist_info( + tmp_path: pth.Path, + members: dict[str, str], + expected_message: str, +) -> None: + """A wheel missing either dist-info member fails with AssertionError. + + ``wheel_build_snapshot`` documents ``AssertionError`` for absent metadata, + so a missing ``METADATA`` must not surface as the ``KeyError`` that + ``ZipFile.read`` would otherwise raise. + """ + whl_path = tmp_path / "cuprum-0.1.0-py3-none-any.whl" + with zipfile.ZipFile(whl_path, "w") as archive: + for name, payload in members.items(): + archive.writestr(name, payload) + + with pytest.raises(AssertionError, match=re.escape(expected_message)): + wheel_build_snapshot(whl_path) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 8667074c..40fa72f1 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -1409,10 +1409,16 @@ The **native wheel build and snapshot** machinery stays in `tests/helpers/maturin.py`, because it wraps `subprocess` and `zipfile` introspection that does not inline cleanly. -That module's scope is deliberately narrow: build a wheel with the pinned -maturin (`build_native_wheel_artifact`), report toolchain availability +That module's scope is deliberately narrow: build a wheel +(`build_native_wheel_artifact`), report toolchain availability (`toolchain_available`), and normalize a built wheel into a stable snapshot -(`wheel_build_snapshot`). Its re-use policy is to stay that way — do not +(`wheel_build_snapshot`). The build runs `python -m maturin` under the current +interpreter, so it uses whichever maturin that environment provides rather than +selecting a version itself. Two separate mechanisms enforce alignment with the +declared pin: `test_installed_maturin_matches_expected_pin` compares the +installed CLI against the `pyproject.toml` pin, and the snapshot test asserts +the built wheel's `Generator` matches that same pin, so a wheel built by an +unexpected maturin fails the suite. Its re-use policy is to stay that way — do not re-externalize further helpers into it until a second concrete consumer exists and the shared interface can be designed against real requirements rather than anticipated ones. diff --git a/tests/helpers/maturin.py b/tests/helpers/maturin.py index 15136dca..ca27a4d1 100644 --- a/tests/helpers/maturin.py +++ b/tests/helpers/maturin.py @@ -1,9 +1,9 @@ """Helpers for building and introspecting the native maturin wheel. -Maturin pin-synchronisation checks are inlined in +Maturin pin-synchronization checks are inlined in ``cuprum/unittests/test_maturin_build.py``, their sole consumer. This module retains only the wheel build and snapshot machinery, which wraps ``subprocess`` -and ``zipfile`` introspection that does not inline cleanly. Do not re-externalise +and ``zipfile`` introspection that does not inline cleanly. Do not re-externalize further helpers here until a second concrete consumer exists and the shared interface can be designed against real requirements. """ @@ -69,7 +69,15 @@ def __str__(self) -> str: def toolchain_available() -> bool: - """Return whether the Rust toolchain and the maturin module are available.""" + """Report whether the Rust toolchain and the maturin module are available. + + Returns + ------- + bool + ``True`` only when ``cargo`` and ``rustc`` are both on ``PATH`` and the + ``maturin`` module is importable by the current interpreter; ``False`` + if any of the three is missing. + """ try: maturin_available = importlib.util.find_spec("maturin") is not None except ImportError: @@ -82,7 +90,13 @@ def toolchain_available() -> bool: def build_native_wheel_artifact(root: Path, out_dir: Path) -> Path: - """Build a native wheel with the pinned maturin version. + """Build a native wheel using the current interpreter's maturin. + + Invokes ``python -m maturin`` with the running interpreter, so the maturin + that builds the wheel is whichever version is installed in that + environment. Alignment with the declared pin is asserted separately by + ``test_installed_maturin_matches_expected_pin`` and by the snapshot test's + generator check, not selected here. Raises ------ @@ -124,7 +138,7 @@ def build_native_wheel_artifact(root: Path, out_dir: Path) -> Path: def _parse_metadata(raw_metadata: str) -> dict[str, typ.Any]: - """Parse RFC 2822-style ``METADATA`` headers into a normalised dict.""" + """Parse RFC 2822-style ``METADATA`` headers into a normalized dict.""" headers: dict[str, list[str]] = {} current_key: str | None = None for line in raw_metadata.splitlines(): @@ -150,7 +164,7 @@ def _parse_metadata(raw_metadata: str) -> dict[str, typ.Any]: def _normalise_wheel_entry(name: str) -> str: - """Normalise platform- and version-specific wheel entry names to placeholders.""" + """Normalize platform- and version-specific wheel entry names to placeholders.""" if _EXTENSION_MODULE_RE.match(name): return "cuprum/_rust_backend_native.cpython-.so" if "/sboms/" in name: @@ -162,7 +176,13 @@ def _normalise_wheel_entry(name: str) -> str: def _read_wheel_members(whl_path: Path) -> tuple[list[str], str, str]: - """Return the entry names and the decoded ``WHEEL``/``METADATA`` payloads.""" + """Return the entry names and the decoded ``WHEEL``/``METADATA`` payloads. + + Raises + ------ + AssertionError + If the archive is missing either ``.dist-info`` member. + """ with zipfile.ZipFile(whl_path) as archive: entry_names = archive.namelist() wheel_name = next( @@ -173,6 +193,12 @@ def _read_wheel_members(whl_path: Path) -> tuple[list[str], str, str]: msg = "wheel is missing .dist-info/WHEEL metadata" raise AssertionError(msg) metadata_name = wheel_name.replace("/WHEEL", "/METADATA") + # Check membership rather than letting `read` raise: a missing METADATA + # is the same "wheel lacks expected metadata" failure as a missing + # WHEEL, and callers document AssertionError, not KeyError. + if metadata_name not in entry_names: + msg = "wheel is missing .dist-info/METADATA metadata" + raise AssertionError(msg) wheel_payload = archive.read(wheel_name).decode("utf-8") metadata_payload = archive.read(metadata_name).decode("utf-8") return entry_names, wheel_payload, metadata_payload @@ -199,13 +225,24 @@ def _parse_wheel_header(wheel_payload: str, whl_path: Path) -> tuple[str, str]: def wheel_build_snapshot(whl_path: Path) -> dict[str, typ.Any]: - """Return a normalised snapshot of wheel metadata and layout. + """Return a normalized snapshot of wheel metadata and layout. Reads the archive's ``.dist-info/WHEEL`` and ``.dist-info/METADATA`` members, - extracts the maturin generator and ``Root-Is-Purelib`` flag, and normalises + extracts the maturin generator and ``Root-Is-Purelib`` flag, and normalizes platform-specific entry names so the snapshot is stable across the build matrix. + Returns + ------- + dict[str, typ.Any] + A stable mapping for the wheel: ``generator`` (the maturin version that + built it), ``metadata`` (name, version, ``requires_python``, sorted + ``requires_dist`` and ``classifiers``), ``wheel`` + (``root_is_purelib`` plus a placeholder ``tag``), and ``entries`` — the + sorted archive entry names with platform- and version-specific parts + replaced by placeholders, so the value is identical across the build + matrix and can be snapshot-compared. + Raises ------ AssertionError From abf8142d16cf38cf91e66e069283bd4b637ef47b Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 29 Jul 2026 11:42:01 +0200 Subject: [PATCH 5/6] Dedupe the pyproject pin read in _read_maturin_pins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _read_maturin_pins duplicated _read_expected_maturin_version exactly for its pyproject.toml entry: same read, same regex, same location string. Delegate to that helper so how to read the pyproject pin lives in one place. The file-read count is unchanged. Raised in review alongside a suggestion to cache the pin file contents. The caching half is declined: all seven reads a full run performs total 0.124 ms against a 1.32 s module runtime (0.009%, and that runtime is with a warm wheel build), so a module-level cache would trade real complexity — shared state, invalidation, test isolation — for no measurable gain, in a PR whose purpose is removing speculative abstraction. Co-Authored-By: Claude Opus 4.8 (1M context) --- cuprum/unittests/test_maturin_build.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/cuprum/unittests/test_maturin_build.py b/cuprum/unittests/test_maturin_build.py index ccbab32b..058d6894 100644 --- a/cuprum/unittests/test_maturin_build.py +++ b/cuprum/unittests/test_maturin_build.py @@ -71,10 +71,9 @@ def _read_expected_maturin_version(root: pth.Path) -> str: def _read_maturin_pins(root: pth.Path) -> dict[str, str]: """Read the maturin version pins from every synchronized location.""" return { - "pyproject.toml": _require_pin_match( - _MATURIN_PIN_RE.search(_read_text(root, "pyproject.toml")), - "pyproject.toml", - ), + # Reuse the dev-dependency reader so "how to read the pyproject pin" + # lives in exactly one place. + "pyproject.toml": _read_expected_maturin_version(root), "build-wheels.yml": _require_pin_match( _WORKFLOW_PIN_RE.search( _read_text(root, ".github/workflows/build-wheels.yml"), From 8dc5353f91a735bb86c3efd87fa7a7b4f77da5a2 Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 29 Jul 2026 14:45:52 +0200 Subject: [PATCH 6/6] Import maturin to test availability; correct the pin-test wording MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three of four review findings applied. toolchain_available located the maturin module with find_spec, which reports success for a module that is findable but fails to import — a broken native dependency, for instance. The build runs `python -m maturin` and needs the module to actually import, so the check now imports it and treats ImportError as unavailable. Confirmed the two differ: a meta-path finder returning a spec whose loader raises makes find_spec report available while the import correctly reports unavailable. _read_wheel_members carried a Raises section on a private helper whose neighbours (_parse_metadata, _normalise_wheel_entry, _parse_wheel_header) are all one-line. Reduced to a single line that still states the non-obvious guarantee callers rely on — that a missing member asserts rather than raising KeyError — with the reasoning left in the body comment beside the check. The guide described test_installed_maturin_matches_expected_pin as comparing "the installed CLI" against the pin. It actually reads the installed distribution's version from the current interpreter's package metadata via importlib.metadata.version, and only gates on a maturin CLI being on PATH. The metadata is the right thing to read, since that same interpreter runs the build; the wording now says so, including the gate. The fourth finding asked that the pin regexes not be described as shared. Nothing described them that way — the only "shared" in the section is the helper module's reuse policy — so no correction was needed, but the sentence now names them explicitly as module-private constants local to the test file, since the wording evidently read ambiguously. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/developers-guide.md | 11 ++++++++--- tests/helpers/maturin.py | 22 +++++++++++----------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 40fa72f1..62890048 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -1403,8 +1403,9 @@ change alters the architecture of the lint gate, update These checks live in `cuprum/unittests/test_maturin_build.py`, split across two boundaries. The **pin-synchronization** checks are inlined there as private helpers (`_read_maturin_pins`, `_read_expected_maturin_version`, -`_read_manylinux_aarch64_container_ref`, and their regexes): they read -repository files, have a single consumer, and gain nothing from indirection. +`_read_manylinux_aarch64_container_ref`) alongside the module-private pin +regexes they use, all local to that test file: they read repository files, have +a single consumer, and gain nothing from indirection. The **native wheel build and snapshot** machinery stays in `tests/helpers/maturin.py`, because it wraps `subprocess` and `zipfile` introspection that does not inline cleanly. @@ -1416,7 +1417,11 @@ That module's scope is deliberately narrow: build a wheel interpreter, so it uses whichever maturin that environment provides rather than selecting a version itself. Two separate mechanisms enforce alignment with the declared pin: `test_installed_maturin_matches_expected_pin` compares the -installed CLI against the `pyproject.toml` pin, and the snapshot test asserts +`pyproject.toml` pin against the installed maturin distribution's version, read +from the current interpreter's package metadata with +`importlib.metadata.version("maturin")` — the same interpreter that runs the +build — while only gating on a `maturin` CLI being present on `PATH`; and the +snapshot test asserts the built wheel's `Generator` matches that same pin, so a wheel built by an unexpected maturin fails the suite. Its re-use policy is to stay that way — do not re-externalize further helpers into it until a second concrete consumer exists diff --git a/tests/helpers/maturin.py b/tests/helpers/maturin.py index ca27a4d1..2fa81dc8 100644 --- a/tests/helpers/maturin.py +++ b/tests/helpers/maturin.py @@ -10,7 +10,7 @@ from __future__ import annotations -import importlib.util +import importlib import re import shutil import subprocess # noqa: S404 - tests invoke pinned maturin build commands. @@ -75,13 +75,19 @@ def toolchain_available() -> bool: ------- bool ``True`` only when ``cargo`` and ``rustc`` are both on ``PATH`` and the - ``maturin`` module is importable by the current interpreter; ``False`` - if any of the three is missing. + ``maturin`` module imports successfully in the current interpreter; + ``False`` if any of the three is missing. + + The module is imported rather than located with ``find_spec``: the build + runs ``python -m maturin``, which needs the module to import, and a module + that is findable can still fail to import. """ try: - maturin_available = importlib.util.find_spec("maturin") is not None + importlib.import_module("maturin") except ImportError: maturin_available = False + else: + maturin_available = True return ( shutil.which("cargo") is not None and shutil.which("rustc") is not None @@ -176,13 +182,7 @@ def _normalise_wheel_entry(name: str) -> str: def _read_wheel_members(whl_path: Path) -> tuple[list[str], str, str]: - """Return the entry names and the decoded ``WHEEL``/``METADATA`` payloads. - - Raises - ------ - AssertionError - If the archive is missing either ``.dist-info`` member. - """ + """Return entry names and decoded ``WHEEL``/``METADATA``, asserting both exist.""" with zipfile.ZipFile(whl_path) as archive: entry_names = archive.namelist() wheel_name = next(