From 509be4c6bcccc62b0c53a70d6bc7a18e79605fa1 Mon Sep 17 00:00:00 2001 From: golem Date: Mon, 5 Oct 2026 14:30:41 -0600 Subject: [PATCH] fix(ci): smoke guard catches tagless builds; DEBUG import must hit the install; pin more workflow invariants Review findings on PR #13: - release-branch-semver gives 0.1.0.devN without tags, so the 0.0 guard never fired; check_version() now requires >= 0.2 (unit-tested) - the APPSKELLINGTON_DEBUG child import could resolve the source tree from the cwd and false-pass; it now reports its location and must match - test_ci_workflow: no continue-on-error, matrix python drives container and UV_PYTHON, WIP gate compared whole (mutation-tested) Co-Authored-By: Claude Opus 5.5 (1M context) --- scripts/smoke_import.py | 47 +++++++++++++++++++++++++++++++------- tests/test_ci_workflow.py | 21 +++++++++++++++-- tests/test_smoke_import.py | 35 ++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 10 deletions(-) create mode 100644 tests/test_smoke_import.py diff --git a/scripts/smoke_import.py b/scripts/smoke_import.py index caa7ce4..c5ab3d9 100644 --- a/scripts/smoke_import.py +++ b/scripts/smoke_import.py @@ -1,21 +1,40 @@ """Smoke-test an *installed* app_skellington (wheel, sdist or index release). -Run it from outside the source tree, so the installed copy is the one imported: +Run it from outside the source tree: cd / && python /path/to/scripts/smoke_import.py [EXPECTED_VERSION] + +The script's own directory (scripts/) heads sys.path, so the main import +can't see the source tree. But the APPSKELLINGTON_DEBUG child process puts +its cwd first, which is why the cwd must be outside the repo. The child's +import location is checked too, so a wrong cwd fails instead of passing. """ import importlib.metadata import os import pathlib +import re import subprocess import sys +# Any real release is >= 0.2. With no tags reachable (a shallow clone), +# setuptools_scm's release-branch-semver scheme yields 0.1.0.devN, and other +# schemes yield 0.0.x. +MIN_RELEASE = (0, 2) + def fail(msg): sys.exit("smoke FAILED: " + msg) +def check_version(version, expected): + major_minor = tuple(int(n) for n in re.match(r"(\d+)\.(\d+)", version).groups()) + if major_minor < MIN_RELEASE: + fail("setuptools_scm found no tags (shallow clone?): %s" % version) + if expected is not None and version != expected: + fail("expected version %s, got %s" % (expected, version)) + + def main(argv): expected = argv[1] if len(argv) > 1 else None @@ -35,19 +54,31 @@ def main(argv): SubMenu, ) - version = importlib.metadata.version("app_skellington") - if version.startswith("0.0"): - fail("setuptools_scm found no tags (shallow clone?): %s" % version) - if expected is not None and version != expected: - fail("expected version %s, got %s" % (expected, version)) + check_version(importlib.metadata.version("app_skellington"), expected) # _bootstrap.py takes a different code path when APPSKELLINGTON_DEBUG is set. env = dict(os.environ, APPSKELLINGTON_DEBUG="1") - proc = subprocess.run([sys.executable, "-c", "import app_skellington"], env=env) + proc = subprocess.run( + [ + sys.executable, + "-c", + "import app_skellington, pathlib;" + "print(pathlib.Path(app_skellington.__file__).resolve())", + ], + env=env, + stdout=subprocess.PIPE, + universal_newlines=True, + ) if proc.returncode != 0: fail("import failed with APPSKELLINGTON_DEBUG=1") + child_location = proc.stdout.strip().splitlines()[-1] + if child_location != str(location): + fail( + "APPSKELLINGTON_DEBUG import resolved %s, not the installed %s" + " (run from outside the source tree)" % (child_location, location) + ) - print("smoke ok:", version, location) + print("smoke ok:", importlib.metadata.version("app_skellington"), location) if __name__ == "__main__": diff --git a/tests/test_ci_workflow.py b/tests/test_ci_workflow.py index 710b1dd..b086eec 100644 --- a/tests/test_ci_workflow.py +++ b/tests/test_ci_workflow.py @@ -60,10 +60,27 @@ def test_superseded_pr_runs_cancel_but_push_runs_never_do(): def test_every_job_is_wip_gated(): + # Compared whole, not by substring: `&&` turned into `||` must fail. + gate = "github.event_name != 'pull_request' || ( %s && %s )" % WIP_CLAUSES for name, job in load()["jobs"].items(): cond = flat(job.get("if", "")) - for clause in WIP_CLAUSES: - assert clause in cond, "%s lacks WIP gate clause %s" % (name, clause) + assert cond in (gate, "always() && ( %s )" % gate), name + + +def test_no_gate_can_be_softened_by_continue_on_error(): + # continue-on-error makes needs..result read success, so ci-ok + # would go green over a failure. + for name, job in load()["jobs"].items(): + assert "continue-on-error" not in job, name + for step in job.get("steps", []): + assert "continue-on-error" not in step, name + + +def test_each_lane_really_runs_its_matrix_python(): + test = load()["jobs"]["test"] + assert test["container"] == "python:${{ matrix.python }}" + assert test["env"]["UV_PYTHON"] == "${{ matrix.python }}" + assert test["env"]["UV_PYTHON_DOWNLOADS"] == "never" def test_matrix_matches_requires_python(): diff --git a/tests/test_smoke_import.py b/tests/test_smoke_import.py new file mode 100644 index 0000000..2535bc5 --- /dev/null +++ b/tests/test_smoke_import.py @@ -0,0 +1,35 @@ +"""Unit tests for scripts/smoke_import.py's version checks.""" + +import importlib.util +import pathlib + +import pytest + +SCRIPT = pathlib.Path(__file__).resolve().parent.parent / "scripts" / "smoke_import.py" +spec = importlib.util.spec_from_file_location("smoke_import", SCRIPT) +smoke_import = importlib.util.module_from_spec(spec) +spec.loader.exec_module(smoke_import) + + +@pytest.mark.parametrize( + "version", + [ + "0.0.0", + "0.0.1.dev3+gabc1234", + # release-branch-semver with no tags reachable (shallow clone): + "0.1.0.dev1+g6bd717f23", + ], +) +def test_tagless_versions_are_rejected(version): + with pytest.raises(SystemExit, match="no tags"): + smoke_import.check_version(version, expected=None) + + +@pytest.mark.parametrize("version", ["0.2.3", "0.3.0.dev23+gc361b286c", "1.0.0"]) +def test_tag_derived_versions_pass(version): + smoke_import.check_version(version, expected=None) + + +def test_expected_version_must_match_exactly(): + with pytest.raises(SystemExit, match="expected version 0.2.3"): + smoke_import.check_version("0.2.3.dev1", expected="0.2.3")