From de8fe6af4b79c1b242d5620992b9d04c557f1f8c Mon Sep 17 00:00:00 2001 From: zackees Date: Sun, 21 Jun 2026 23:57:38 -0700 Subject: [PATCH] fix: stale cli.py / _bin/ references break tests + action_surface (closes #9) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Followup cleanup to #7 (drop Python CLI shim, ship raw wheel script). The package-side refactor worked at the wheel-build level but left stale source-tree references in two artifact-walking tools — same failure shape as fbuild#748 (a packaging refactor that worked at the wheel level but broke a tool walking the source tree a different way). Stale references found by `git grep '_bin\|template_python_rust_cmd\.cli'`: 1. `tests/test_cli.py` — imported `template_python_rust_cmd.cli. packaged_binary_path` (module deleted in #7). Whole file failed to collect with ImportError; the suite was silently 1 file short. 2. `ci/gates/action_surface.py::_binary_path()` — listed `src/template_python_rust_cmd/_bin/template-cli[.exe]` as its FIRST lookup candidate. That directory was removed in #7. Fallback to target/release worked locally but the gate doesn't honor CARGO_TARGET_DIR (pinned to ~/.template-python-rust-cmd/cargo-target/ wheel-build in #5), so it silently misses the binary on a wheel-build-driven runner. 3. `tests/README.md` — described `test_cli.py` as covering "the Python CLI shim's binary-discovery logic" (module gone). Fix: - `tests/test_cli.py` rewritten as a runtime contract check: probe PATH for `template-cli`, skip if absent (dev venv without install), otherwise assert it's invokable and produces non-empty --version output. Matches the new shipping mechanism (raw wheel script) which doesn't leave a source-tree probe target. - `ci/gates/action_surface.py::_binary_path()` now searches, in order: 1. $CARGO_TARGET_DIR/{release,debug}/ if set 2. ROOT/target/{release,debug}/ for the local ./test flow 3. PATH via shutil.which for pip/uv-installed binaries Updated the "no binary found" warning to match. - `tests/README.md` describes what test_cli.py actually does now. Verified: - `uv run pytest tests/test_cli.py tests/test_version.py tests/test_bindings.py -v` → 2 pass, 2 skip (test_cli skips because dev venv has no installed template-cli — expected behavior). - `./ci.sh test` → 34 pass, 2 skip, [test] ok. - `./ci.sh action_surface` → finds the binary, gate passes. Closes #9. Co-Authored-By: Claude Opus 4.7 --- ci/gates/action_surface.py | 48 ++++++++++++++++++++------- tests/README.md | 2 +- tests/test_cli.py | 66 +++++++++++++++++++++++++++++++++++--- 3 files changed, 99 insertions(+), 17 deletions(-) diff --git a/ci/gates/action_surface.py b/ci/gates/action_surface.py index 95e32e9..574dbbc 100644 --- a/ci/gates/action_surface.py +++ b/ci/gates/action_surface.py @@ -22,6 +22,7 @@ from __future__ import annotations +import os import re import shutil import subprocess @@ -43,16 +44,41 @@ def _binary_path() -> Path | None: - # Prefer the staged location used by the wheel; fall back to debug - # for local dev (`cargo build`). - candidates = [ - ROOT / "src" / "template_python_rust_cmd" / "_bin" / BINARY_NAME, - ROOT / "target" / "release" / BINARY_NAME, - ROOT / "target" / "debug" / BINARY_NAME, - ] + """Locate `template-cli` for the runtime --help probe. + + Searches, in order: + 1. `CARGO_TARGET_DIR/{release,debug}/` if the env var is set — + `ci/build_wheel.py` pins this to + `~/.template-python-rust-cmd/cargo-target/wheel-build/` so the + wheel build's binary doesn't get rebuilt every time. + 2. The repo's `target/{release,debug}/` for the `./test` / + `cargo build` flow. + 3. PATH (`shutil.which`) for the case where the user has done + `pip install` / `uv tool install` of the built wheel and + wants the gate to validate against the installed binary. + + The earlier `src/template_python_rust_cmd/_bin/` candidate was + removed when #7 dropped the package-side staging in favor of + shipping the binary as a raw wheel script. See #9. + """ + candidates: list[Path] = [] + + cargo_target = os.environ.get("CARGO_TARGET_DIR") + if cargo_target: + candidates.append(Path(cargo_target) / "release" / BINARY_NAME) + candidates.append(Path(cargo_target) / "debug" / BINARY_NAME) + + candidates.append(ROOT / "target" / "release" / BINARY_NAME) + candidates.append(ROOT / "target" / "debug" / BINARY_NAME) + for path in candidates: if path.is_file(): return path + + on_path = shutil.which("template-cli") + if on_path: + return Path(on_path) + return None @@ -101,10 +127,10 @@ def run() -> int: # best-effort against whatever build is available locally. print( "action_surface: no template-cli binary found " - "(src/template_python_rust_cmd/_bin/, target/release/, " - "target/debug/). Skipping runtime surface check. Run " - "`cargo build -p template-cli` or `./test` to materialize " - "the binary and re-run." + "($CARGO_TARGET_DIR/{release,debug}/, target/release/, " + "target/debug/, PATH). Skipping runtime surface check. " + "Run `cargo build -p template-cli` or `./test` to " + "materialize the binary and re-run." ) return 0 if not shutil.which(str(binary)) and not binary.exists(): diff --git a/tests/README.md b/tests/README.md index ca5aec8..76f9d96 100644 --- a/tests/README.md +++ b/tests/README.md @@ -8,7 +8,7 @@ Python-side test suite. Picked up by `pytest` per the configuration in | File | What it tests | |-------------------|------------------------------------------------------------------------| | `test_bindings.py`| The PyO3 extension surface via `template_python_rust_cmd.bindings`. | -| `test_cli.py` | The Python CLI shim's binary-discovery logic. | +| `test_cli.py` | `template-cli` is on PATH after `pip install` (or `uv tool install`) and `template-cli --version` exits 0. Skips when the binary hasn't been installed yet — the wheel-built binary is shipped as a raw script in `-.data/scripts/` (#7), not as a Python launcher, so there's no source-tree path to probe. | | `test_version.py` | Package `__version__` is non-empty and matches the manifest. | | `test_gates.py` | Each gate registered in `ci.py::GATE_ORDER` is importable and exposes `def run() -> int`. The contract test for the gates infra itself. | diff --git a/tests/test_cli.py b/tests/test_cli.py index 1f9dba8..1125e3a 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,9 +1,65 @@ +"""Sanity check: `template-cli` is on PATH after `pip install`. + +The wheel ships `template-cli[.exe]` as a raw script at +`-.data/scripts/` (see `ci/build_wheel.py` and #7), which +pip drops straight into the venv's `Scripts/` (Windows) or `bin/` +(POSIX) directory. The previous shape — a Python `cli.py` shim that +subprocess-launched a staged `_bin/template-cli` — was removed in #7 +because on Windows the shim raced the next shell prompt ahead of the +child's stdout. + +This test is a *runtime contract* check: if a downstream user does +`pip install template-python-rust-cmd`, can they then run +`template-cli --version`? It probes PATH directly rather than the +package source tree, because the wheel-side delivery mechanism (raw +script) is fundamentally not visible from the source tree anymore — +there's no `_bin/` directory to look at. See #9. +""" +from __future__ import annotations + import os +import shutil +import subprocess + +import pytest + +BINARY_NAME = "template-cli.exe" if os.name == "nt" else "template-cli" + + +@pytest.fixture(scope="module") +def cli_on_path() -> str: + binary = shutil.which("template-cli") + if binary is None: + pytest.skip( + "template-cli not on PATH; this test runs only after " + "`pip install` (or `uv tool install`) of the built wheel. " + "Run `ci/build_wheel.py` and install the result to exercise " + "this gate locally." + ) + return binary + + +def test_cli_on_path_has_expected_name(cli_on_path: str) -> None: + assert os.path.basename(cli_on_path).lower() == BINARY_NAME.lower() -from template_python_rust_cmd.cli import packaged_binary_path +def test_cli_on_path_invokes(cli_on_path: str) -> None: + """The PATH-resolved `template-cli` runs and exits with code 0 on --version. -def test_packaged_binary_path_points_into_package() -> None: - expected = "template-cli.exe" if os.name == "nt" else "template-cli" - assert packaged_binary_path().name == expected - assert "_bin" in str(packaged_binary_path()) + Doubles as a Windows stdout-ordering smoke test: if the wheel + accidentally regressed back to a Python launcher (e.g. someone + added `[project.scripts]` again), `subprocess.run` would still + succeed but the test wouldn't catch the cmd.exe shell-prompt race + — that one needs an interactive console. The `file` / `unzip -l` + check in #2's acceptance criteria covers the static shape; this + test covers "the binary runs at all". + """ + proc = subprocess.run( + [cli_on_path, "--version"], + capture_output=True, + text=True, + timeout=10, + check=False, + ) + assert proc.returncode == 0, proc.stderr + assert proc.stdout.strip(), "template-cli --version produced empty stdout"