Skip to content

Commit 7baf527

Browse files
ci(vault): lint scripts/ too, and refuse to plant a tree over a real checkout
Two gaps a verification pass found in the guards added an hour ago. The lint job named src/ and tests/ and stopped there, so scripts/ was linted by nothing. That is where check_release_consistency.py lives: the one script CI runs to protect a release was itself unchecked, and it was unformatted. Both ruff steps cover scripts/ now, and test_the_lint_job_covers_every_python_directory fails if a directory of Python falls outside the linter's named paths again. It ships with its own vacuity test, because the assertion would otherwise pass just as happily against a lint job that named no paths at all. _plant() writes two FIXED filenames, pyproject.toml and src/qp_vault/__init__.py, into a directory the caller names. Pointed at a real checkout it would have overwritten this package's own version declarations. That is the arbitrary-file-write shape a directory check plus a fixed name always has. It refuses an existing directory now, so it can only land where nothing lives. Six functions gained the docstrings the house standard requires.
1 parent 8556aa3 commit 7baf527

4 files changed

Lines changed: 114 additions & 6 deletions

File tree

.github/workflows/python-ci.yaml

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,13 @@ jobs:
4141
with:
4242
python-version: ${{ env.LINT_PYTHON }}
4343
- run: pip install "ruff==${{ env.RUFF_VERSION }}"
44-
- run: ruff check src/ tests/
44+
# `scripts/` is in scope, and it was not. The release guard that gates publishing lives
45+
# there, so leaving it out meant the one script CI executes to protect a release was itself
46+
# linted by nothing. Found by running ruff over the paths the workflow does NOT name.
47+
- run: ruff check src/ tests/ scripts/
4548
# Formatting is checked, not merely available. 95 files were unformatted when this line was
4649
# added, which is what happens when a formatter is installed and never asserted on.
47-
- run: ruff format --check src/ tests/
50+
- run: ruff format --check src/ tests/ scripts/
4851

4952
typecheck:
5053
runs-on: ubuntu-latest

CHANGELOG.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,12 @@ the code released here.
2424
and upgrading either is a deliberate line in a diff.
2525
- **`publish` is gated on `typecheck`.** It was gated only on `lint` and `test`, which is how 1.5.2
2626
reached the index from a run whose overall conclusion reads `failure`.
27-
- **`ruff format --check` runs in CI**, and `src/` and `tests/` were formatted to match. 95 files
28-
were unformatted, which is what happens when a formatter is installed and never asserted on.
27+
- **`ruff check` and `ruff format --check` run in CI over `src/`, `tests/` AND `scripts/`**, and
28+
all three were formatted to match. 95 files were unformatted, which is what happens when a
29+
formatter is installed and never asserted on. `scripts/` was added after a verification pass
30+
found that the release guard protecting publishing was itself linted by nothing, and
31+
`test_the_lint_job_covers_every_python_directory` now fails if a directory of Python falls
32+
outside the linter's named paths again.
2933
- **The tag must match both declared versions.** `scripts/check_release_consistency.py` refuses a
3034
publish whose tag disagrees with `pyproject.toml` or `__version__`. `pyproject` read 1.7.0 while
3135
`main` sat fourteen commits past that tag, and nothing in the build objected.

scripts/check_release_consistency.py

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,25 @@ def check(root: pathlib.Path, tag: str) -> list[str]:
7676

7777

7878
def _plant(root: pathlib.Path, project_version: str, dunder: str) -> pathlib.Path:
79-
"""Write the smallest tree this guard can read, carrying the versions given."""
79+
"""Write the smallest tree this guard can read, carrying the versions given.
80+
81+
Args:
82+
root: A directory that must NOT already exist. See the refusal below.
83+
project_version: Version to write into ``pyproject.toml``.
84+
dunder: Version to write into ``__init__.py``.
85+
86+
Returns:
87+
The tree root.
88+
89+
Raises:
90+
FileExistsError: ``root`` already exists.
91+
"""
92+
# `exist_ok=False` is the guard, not a default. This function writes two FIXED filenames,
93+
# `pyproject.toml` and `src/qp_vault/__init__.py`, into a directory the caller names. Pointed
94+
# at a real checkout it would silently overwrite this package's own version declarations,
95+
# which is the arbitrary-file-write shape a dir check plus a fixed name always has. Refusing
96+
# an existing directory means it can only ever land somewhere nothing lives yet.
97+
root.mkdir(parents=True, exist_ok=False)
8098
(root / "src" / "qp_vault").mkdir(parents=True, exist_ok=True)
8199
(root / "pyproject.toml").write_text(
82100
f'[project]\nname = "qp-vault"\nversion = "{project_version}"\n'
@@ -100,7 +118,9 @@ def self_test() -> int:
100118
base = pathlib.Path(tmp)
101119

102120
agree = _plant(base / "agree", "9.9.9", "9.9.9")
103-
checks.append(("passes when tag, pyproject and __version__ agree", not check(agree, "v9.9.9")))
121+
checks.append(
122+
("passes when tag, pyproject and __version__ agree", not check(agree, "v9.9.9"))
123+
)
104124

105125
stale = _plant(base / "stale", "9.9.8", "9.9.8")
106126
checks.append(("fires when the tag is ahead of pyproject", bool(check(stale, "v9.9.9"))))
@@ -127,6 +147,17 @@ def self_test() -> int:
127147

128148

129149
def main(argv: list[str] | None = None) -> int:
150+
"""Run the guard, or its self-test, and report an exit code CI can act on.
151+
152+
Args:
153+
argv: Command-line arguments, or None to read ``sys.argv``.
154+
155+
Returns:
156+
0 when the versions agree (or the self-test passes), 1 when they disagree, and 2 when the
157+
tree could not be read at all. Two is deliberately distinct from one: "could not look" is
158+
not "nothing wrong", and a caller that collapses them turns an unreadable checkout into a
159+
pass.
160+
"""
130161
parser = argparse.ArgumentParser(description=__doc__)
131162
parser.add_argument("--tag", help="the git tag being published, e.g. v1.8.0")
132163
parser.add_argument("--root", default=".", help="repository checkout to read")

tests/test_release_guards.py

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@
3535

3636

3737
def _current_version() -> str:
38+
"""Read the version this checkout declares.
39+
40+
Returns:
41+
The ``project.version`` string from ``pyproject.toml``.
42+
"""
3843
import tomllib
3944

4045
with PYPROJECT.open("rb") as handle:
@@ -61,6 +66,15 @@ def _fixture_tree(root: pathlib.Path, project_version: str, dunder: str) -> path
6166

6267

6368
def _run_guard(root: pathlib.Path, tag: str) -> subprocess.CompletedProcess[str]:
69+
"""Invoke the guard as CI does, out of process.
70+
71+
Args:
72+
root: The tree the guard should read.
73+
tag: The tag to compare against, with or without its leading ``v``.
74+
75+
Returns:
76+
The completed process, so a test can assert on the exit code AND the message.
77+
"""
6478
return subprocess.run(
6579
[sys.executable, str(GUARD), "--tag", tag, "--root", str(root)],
6680
capture_output=True,
@@ -139,6 +153,14 @@ def test_the_release_guard_agrees_with_the_real_tree() -> None:
139153

140154

141155
def _ruff() -> str:
156+
"""Locate the ruff the workflow would run.
157+
158+
Returns:
159+
Absolute path to the ruff executable.
160+
161+
Raises:
162+
Skipped: Via ``pytest.skip`` when ruff is not installed.
163+
"""
142164
ruff = shutil.which("ruff")
143165
if ruff is None:
144166
pytest.skip("ruff is not on PATH")
@@ -181,6 +203,15 @@ def test_ruff_format_check_passes_on_the_real_source_tree() -> None:
181203

182204

183205
def _workflow() -> dict:
206+
"""Parse the CI workflow so its gates can be asserted rather than eyeballed.
207+
208+
Returns:
209+
The parsed workflow document.
210+
211+
Raises:
212+
Skipped: Via ``importorskip`` when PyYAML is absent, which should not happen: it is
213+
declared in the dev extra precisely so these assertions run instead of skipping.
214+
"""
184215
yaml = pytest.importorskip(
185216
"yaml", reason="PyYAML is declared in the dev extra; a skip here means it went missing"
186217
)
@@ -232,6 +263,45 @@ def test_the_workflow_pins_its_linters_to_exact_versions() -> None:
232263
assert 'pip install "ruff==${{ env.RUFF_VERSION }}"' in text
233264

234265

266+
def test_the_lint_job_covers_every_python_directory() -> None:
267+
"""No directory of Python may sit outside the linter's named paths.
268+
269+
The lint job named ``src/`` and ``tests/`` and stopped there, so ``scripts/`` was linted by
270+
nothing. That is where ``check_release_consistency.py`` lives: the one script CI runs to
271+
protect a release was itself unchecked, and it was unformatted when this test was written.
272+
A guard outside the guarded set is the shape this whole file exists to catch.
273+
"""
274+
lint_steps = _workflow()["jobs"]["lint"]["steps"]
275+
commands = " ".join(step.get("run", "") for step in lint_steps)
276+
277+
covered = {part.rstrip("/") for part in commands.split() if part.endswith("/")}
278+
python_dirs = {
279+
child.name
280+
for child in REPO.iterdir()
281+
if child.is_dir()
282+
and not child.name.startswith((".", "_"))
283+
and child.name not in {"dist", "docs", "examples", "evals", "node_modules"}
284+
and any(child.rglob("*.py"))
285+
}
286+
287+
missing = python_dirs - covered
288+
assert not missing, f"lint does not cover: {sorted(missing)} (covers {sorted(covered)})"
289+
290+
291+
def test_the_coverage_assertion_can_fail() -> None:
292+
"""The vacuity test for the one above.
293+
294+
Without it, that assertion would pass just as happily against a lint job that named no paths
295+
at all, since an empty `python_dirs` minus anything is still empty.
296+
"""
297+
commands = "ruff check src/ tests/"
298+
299+
covered = {part.rstrip("/") for part in commands.split() if part.endswith("/")}
300+
missing = {"src", "tests", "scripts"} - covered
301+
302+
assert missing == {"scripts"}
303+
304+
235305
def test_the_workflow_runs_on_a_schedule() -> None:
236306
"""A silent red must become a dated red.
237307

0 commit comments

Comments
 (0)