From 1ded91c933982c29bce052c64baadf1ce62a0b11 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 19:07:01 +0000 Subject: [PATCH 1/4] test: red tests for preflight's gate matrix over the real workflows `just preflight` dies with a bare FileNotFoundError: the command still defaults `--conventions` to `.github/workflows/conventions.yml`, which #861 deleted when the #834 epic split it into six per-domain workflows. Pointing the default at one survivor would trade the crash for a green run covering one of eight lanes, so the matrix has to come from every caller in `.github/workflows/`. The colocated unit tests feed `parse_gate_matrix` inline YAML, so none of them can see that the default target stopped existing. These read the repo's own workflows instead: * integration: the command with its real defaults must cover every job that calls the reusable workflow, spanning every file that holds one, and must name a missing `--conventions` target rather than raise. * e2e: the packaged console script from the repo root, same two properties. Nothing ran internals/checks' integration tier in CI, which is why the tier that reads real workflow files could not have caught the rename; internals-checks-ci now runs it. Refs #973 --- .github/workflows/internals-checks-ci.yml | 19 ++++ internals/checks/tests/e2e/preflight_test.py | 51 +++++++++++ .../tests/integration/preflight_test.py | 86 +++++++++++++++++++ 3 files changed, 156 insertions(+) create mode 100644 internals/checks/tests/e2e/preflight_test.py create mode 100644 internals/checks/tests/integration/preflight_test.py diff --git a/.github/workflows/internals-checks-ci.yml b/.github/workflows/internals-checks-ci.yml index 4223db0e8..d3ed8bc0e 100644 --- a/.github/workflows/internals-checks-ci.yml +++ b/.github/workflows/internals-checks-ci.yml @@ -32,3 +32,22 @@ jobs: config: internals/checks/testing-conventions.toml base: ${{ github.event.pull_request.base.sha || 'origin/main' }} gates: '["colocated-test", "unit-lint", "integration-lint", "unit-coverage", "mutation", "e2e-verify"]' + + internals-checks-integration: + # The conventions job runs this package's colocated unit tier and checks its + # e2e attestation; neither runs tests/integration/, the tier that reads the + # repo's real workflow files. Without it a workflow rename can delete the + # ground truth a check reads and no job notices (#973). + name: internals/checks integration tests + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + - name: Install uv + uses: astral-sh/setup-uv@v7 + with: + python-version: "3.11" + + - name: Run integration tests + run: uv run python -m pytest tests/integration/ -x -q --tb=short + working-directory: internals/checks diff --git a/internals/checks/tests/e2e/preflight_test.py b/internals/checks/tests/e2e/preflight_test.py new file mode 100644 index 000000000..0b6fa13ad --- /dev/null +++ b/internals/checks/tests/e2e/preflight_test.py @@ -0,0 +1,51 @@ +"""E2E test for `dirsql-checks preflight` through the real CLI. + +No mocking of any kind: spawns the packaged `dirsql-checks` console script from +the repo root, where its defaults have to resolve the real `.github/workflows/` +on their own. `--dry-run` prints the derived matrix without running a gate. +""" + +from __future__ import annotations + +import shutil +import subprocess +from pathlib import Path + +REPO = Path(__file__).resolve().parents[4] + + +def _cli() -> str: + dirsql_checks = shutil.which("dirsql-checks") + assert dirsql_checks is not None, ( + "`dirsql-checks` console script not on PATH -- run " + "`uv run --project internals/checks pytest tests/e2e` " + "or `uv sync --project internals/checks`" + ) + return dirsql_checks + + +def preflight(*args) -> subprocess.CompletedProcess: + return subprocess.run( + [_cli(), "preflight", "--dry-run", *args], + cwd=REPO, + capture_output=True, + text=True, + timeout=120, + ) + + +def describe_dirsql_checks_preflight(): + def it_prints_the_matrix_from_the_repo_root_with_no_arguments(): + proc = preflight() + + assert proc.returncode == 0, f"stdout={proc.stdout!r} stderr={proc.stderr!r}" + assert "==> python-sdk [python] unit-lint" in proc.stdout + assert "==> internals-checks [python] unit-lint" in proc.stdout + assert "==> rust-core [rust] unit-lint" in proc.stdout + + def it_exits_with_an_actionable_message_when_a_named_workflow_is_missing(): + proc = preflight("--conventions", ".github/workflows/conventions.yml") + + assert proc.returncode == 1 + assert "Traceback" not in proc.stderr + assert ".github/workflows/conventions.yml" in proc.stdout + proc.stderr diff --git a/internals/checks/tests/integration/preflight_test.py b/internals/checks/tests/integration/preflight_test.py new file mode 100644 index 000000000..fd096f99e --- /dev/null +++ b/internals/checks/tests/integration/preflight_test.py @@ -0,0 +1,86 @@ +"""Integration tests for `dirsql-checks preflight` against the repo's real workflows. + +The colocated unit tests feed `parse_gate_matrix` inline YAML fixtures, so nothing +noticed when #834 split `conventions.yml` into six per-domain workflows and the +command's default target stopped existing (#973). These invoke the command with +its real defaults over the real `.github/workflows/`, the only place that drift +shows: a caller the matrix misses is a lane `just preflight` silently skips. +""" + +from __future__ import annotations + +import os +from pathlib import Path + +import pytest +import yaml +from click.testing import CliRunner + +from checks.preflight.cli import cli +from checks.preflight.matrix import REUSABLE + +REPO = Path(__file__).resolve().parents[4] +WORKFLOWS = REPO / ".github" / "workflows" + + +def caller_jobs() -> dict[str, str]: + """Every reusable-workflow caller in the real workflows, as job -> workflow file. + + Walked from parsed YAML rather than from preflight's own discovery, so the two + agree only when discovery is right. + """ + jobs = {} + for path in sorted(WORKFLOWS.glob("*.yml")): + document = yaml.safe_load(path.read_text(encoding="utf-8")) or {} + for job, spec in (document.get("jobs") or {}).items(): + if REUSABLE in (spec.get("uses") or ""): + jobs[job] = path.name + return jobs + + +@pytest.fixture +def repo_root(): + """The command resolves `.github/workflows` relative to the cwd, as a user does.""" + previous = os.getcwd() + os.chdir(REPO) + yield REPO + os.chdir(previous) + + +def dry_run(*args): + return CliRunner().invoke(cli, ["--dry-run", *args]) + + +def describe_preflight_over_the_real_workflows(): + def it_derives_a_matrix_with_no_arguments(repo_root): + result = dry_run() + + assert result.exit_code == 0, f"{result.output}{result.exception!r}" + assert result.output.count("==> ") > 1 + + def it_covers_every_reusable_workflow_caller_the_repo_declares(repo_root): + jobs = caller_jobs() + assert jobs, f"no caller job found under {WORKFLOWS}" + + output = dry_run().output + + assert {job for job in jobs if f"==> {job} [" in output} == set(jobs) + + def it_spans_every_workflow_holding_a_caller_not_just_one(repo_root): + # The #834 shape: the callers live in six files, so a matrix read from a + # single workflow is a green run that covers one lane. + jobs = caller_jobs() + assert len(set(jobs.values())) > 1 + + output = dry_run().output + + assert {file for job, file in jobs.items() if f"==> {job} [" in output} == set(jobs.values()) + + def it_names_a_missing_workflow_and_the_fix_instead_of_raising(repo_root): + result = dry_run("--conventions", ".github/workflows/conventions.yml") + + assert result.exit_code == 1 + assert not isinstance(result.exception, FileNotFoundError) + message = result.output + result.stderr + assert ".github/workflows/conventions.yml" in message + assert "--conventions" in message From d750069e196ab221125b83b9ead99da80b47c6f3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 19:18:18 +0000 Subject: [PATCH 2/4] fix(checks): discover preflight's gate matrix instead of naming one workflow `--conventions` becomes repeatable and defaults to nothing: with no workflow named, every file in `.github/workflows/` that calls the reusable workflow is discovered, and the matrix is the concatenation of their callers -- 40 pairs across 8 scan roots in 6 workflows today, where the old default named a file #861 deleted and resolved to 0. Discovery rather than a list of six names for the same reason #781 derived the pairs from the workflow in the first place: a name is a restatement that the next rename invalidates, and the failure it produces is silent (a green run covering one lane) or fatal (a bare FileNotFoundError from `just preflight`). Both unresolvable cases now name the fix, per the actionable-error rule: a named workflow that cannot be read, or that declares no caller, and a workflows directory that is missing or holds no caller at all. Fixes #973 --- internals/checks/README.md | 10 +- internals/checks/src/checks/preflight/cli.py | 24 ++- .../checks/src/checks/preflight/cli_test.py | 85 +++++++--- internals/checks/src/checks/preflight/gate.py | 12 +- .../checks/src/checks/preflight/gate_test.py | 33 +++- .../checks/src/checks/preflight/matrix.py | 98 ++++++++++- .../src/checks/preflight/matrix_test.py | 152 +++++++++++++++++- internals/checks/tests/e2e/preflight_test.py | 3 +- justfile | 14 +- 9 files changed, 377 insertions(+), 54 deletions(-) diff --git a/internals/checks/README.md b/internals/checks/README.md index 09880a217..55b7613e6 100644 --- a/internals/checks/README.md +++ b/internals/checks/README.md @@ -23,15 +23,17 @@ Each check follows the same shape: raising `SystemExit` with its return code. Every module (except empty `__init__.py`s) carries a colocated `*_test.py`, gated by -`conventions.yml`'s `internals-checks` job: `colocated-test`, `unit-lint`, `unit-coverage`, +`internals-checks-ci.yml`'s `internals-checks` job: `colocated-test`, `unit-lint`, `unit-coverage`, `mutation`, and `e2e-verify`. ## Test tiers - `tests/integration/` -- exercises each check's `gate.run()` against real collaborators (real - `git`, real pytest subprocess) rather than the packaged CLI. Gated by the `integration-lint` gate - in `conventions.yml`'s `internals-checks` job, which derives `tests/integration/` from the package - root (#417). + `git`, real pytest subprocess, the repo's own workflow files) rather than the packaged CLI. Gated + by the `integration-lint` gate in `internals-checks-ci.yml`'s `internals-checks` job, which + derives `tests/integration/` from the package root (#417), and *run* by that workflow's + `internals-checks-integration` job -- the tier reads real repo state, so a rename that a colocated + fixture cannot see fails here instead (#973). - `tests/e2e/` -- spawns the real `dirsql-checks` CLI as a subprocess with nothing mocked. Not run in CI; gated only via `e2e-attestation.json` freshness (see AGENTS.md, "E2E Attestation"). diff --git a/internals/checks/src/checks/preflight/cli.py b/internals/checks/src/checks/preflight/cli.py index 347da8e32..4d0a8a330 100644 --- a/internals/checks/src/checks/preflight/cli.py +++ b/internals/checks/src/checks/preflight/cli.py @@ -1,7 +1,8 @@ """The preflight check -- repo-only (#781). Backs `dirsql-checks preflight`: derive the testing-conventions gate matrix from -`.github/workflows/conventions.yml` and run every pair locally. +the workflows in `.github/workflows/` that call the reusable workflow, and run +every pair locally. """ from __future__ import annotations @@ -11,13 +12,18 @@ import click from .gate import default_runner, read_e2e, run +from .matrix import NoGateMatrix, WORKFLOWS, sources @click.command() @click.option( "--conventions", - default=".github/workflows/conventions.yml", - help="Workflow whose reusable-workflow callers define the gate matrix.", + "conventions", + multiple=True, + help=( + "Workflow whose reusable-workflow callers define the gate matrix (repeatable). " + f"Default: every caller in {WORKFLOWS}." + ), ) @click.option("--base", default="origin/main", help="Base ref the diff-scoped gates measure against.") @click.option( @@ -27,12 +33,16 @@ help="Run only these gates (repeatable). Default: every gate each root declares.", ) @click.option("--dry-run", is_flag=True, help="Print the derived matrix without running it.") -def cli(conventions: str, base: str, gates: tuple[str, ...], dry_run: bool) -> None: - with open(conventions, encoding="utf-8") as handle: - text = handle.read() +def cli(conventions: tuple[str, ...], base: str, gates: tuple[str, ...], dry_run: bool) -> None: + try: + workflows = sources(conventions) + except NoGateMatrix as err: + click.echo(f"preflight: {err}", err=True) + raise SystemExit(1) from err + click.echo(f"preflight: gate matrix from {', '.join(path for path, _text in workflows)}") raise SystemExit( run( - text, + [text for _path, text in workflows], base, runner=default_runner, exists=os.path.exists, diff --git a/internals/checks/src/checks/preflight/cli_test.py b/internals/checks/src/checks/preflight/cli_test.py index e6fdaf9a7..4103282bc 100644 --- a/internals/checks/src/checks/preflight/cli_test.py +++ b/internals/checks/src/checks/preflight/cli_test.py @@ -1,6 +1,6 @@ """Colocated unit tests for the preflight command (isolation -- no `CliRunner`). -Driven through `.callback`; `run` and `open` are mocked at their import site. +Driven through `.callback`; `sources` and `run` are mocked at their import site. """ from unittest import mock @@ -9,35 +9,82 @@ from checks.preflight.cli import cli +WORKFLOWS = [(".github/workflows/a-ci.yml", "jobs: {}"), (".github/workflows/b-ci.yml", "jobs: {}")] -def invoke(gates=(), dry_run=False, **kwargs): - with mock.patch("checks.preflight.cli.open", mock.mock_open(read_data="jobs: {}")) as opened: - with mock.patch("checks.preflight.cli.run", **kwargs) as run: - with pytest.raises(SystemExit) as exc_info: - cli.callback(conventions="wf.yml", base="origin/x", gates=gates, dry_run=dry_run) - return opened, run, exc_info.value.code +class NoGateMatrix(Exception): + """Stand-in for `matrix.NoGateMatrix` -- faked rather than imported. -def test_reads_the_workflow_and_exits_with_runs_return_code(): - opened, run, code = invoke(return_value=0) - opened.assert_called_once_with("wf.yml", encoding="utf-8") - assert run.call_args.args == ("jobs: {}", "origin/x") + Patched over the name the `except` clause resolves at raise time, so the + command's error path is exercised without importing the collaborator. + """ + + +def invoke(conventions=(), gates=(), dry_run=False, resolve=None, **kwargs): + with ( + mock.patch("checks.preflight.cli.NoGateMatrix", NoGateMatrix), + mock.patch( + "checks.preflight.cli.sources", resolve or mock.Mock(return_value=WORKFLOWS) + ) as sources, + mock.patch("checks.preflight.cli.run", **kwargs) as run, + mock.patch("checks.preflight.cli.click.echo") as echo, + ): + with pytest.raises(SystemExit) as exc_info: + cli.callback( + conventions=conventions, + base="origin/x", + gates=gates, + dry_run=dry_run, + ) + return sources, run, echo, exc_info.value.code + + +def test_runs_every_resolved_workflow_and_exits_with_runs_return_code(): + _sources, run, _echo, code = invoke(return_value=0) + assert run.call_args.args == (["jobs: {}", "jobs: {}"], "origin/x") assert code == 0 +def test_passes_the_named_workflows_through_to_the_resolver(): + sources, _run, _echo, _code = invoke(conventions=("wf.yml",), return_value=0) + assert sources.call_args.args == (("wf.yml",),) + + +def test_announces_which_workflows_the_matrix_came_from(): + _sources, _run, echo, _code = invoke(return_value=0) + assert echo.call_args_list[0] == mock.call( + "preflight: gate matrix from .github/workflows/a-ci.yml, .github/workflows/b-ci.yml" + ) + + def test_propagates_a_failing_matrix_as_exit_one(): - assert invoke(return_value=1)[2] == 1 + assert invoke(return_value=1)[3] == 1 + + +def test_reports_an_unresolvable_matrix_without_running_anything(): + # #973: a workflow that no longer exists has to say so, not raise. + resolve = mock.Mock(side_effect=NoGateMatrix("--conventions gone.yml: no such workflow.")) + _sources, run, echo, code = invoke(resolve=resolve, return_value=0) + assert (run.called, code) == (False, 1) + assert echo.call_args == mock.call( + "preflight: --conventions gone.yml: no such workflow.", err=True + ) def test_forwards_the_gate_filter_and_dry_run_flag(): - _opened, run, _code = invoke(gates=("unit-lint",), dry_run=True, return_value=0) + _sources, run, _echo, _code = invoke(gates=("unit-lint",), dry_run=True, return_value=0) assert run.call_args.kwargs["only"] == ("unit-lint",) assert run.call_args.kwargs["dry_run"] is True -def test_defaults_to_the_ci_workflow_and_main(): - conventions, base, gates, dry_run = cli.params - assert conventions.default == ".github/workflows/conventions.yml" - assert base.default == "origin/main" - assert gates.multiple is True - assert dry_run.is_flag is True +def test_defaults_to_discovering_the_workflows_and_main(): + # Parsed rather than read off the params: the effective default for + # `--conventions` is what #973 turned into a crash, and an empty one is what + # sends the command to discovery. + with cli.make_context("preflight", []) as ctx: + assert ctx.params == { + "conventions": (), + "base": "origin/main", + "gates": (), + "dry_run": False, + } diff --git a/internals/checks/src/checks/preflight/gate.py b/internals/checks/src/checks/preflight/gate.py index d478864c5..e822316a2 100644 --- a/internals/checks/src/checks/preflight/gate.py +++ b/internals/checks/src/checks/preflight/gate.py @@ -1,7 +1,7 @@ """Run every (root, language, gate) pair CI declares, and report real exit codes (#781). Two failure modes this exists to remove: a hand-maintained pair list that drifts -from `conventions.yml`, and hand-transcribed shell one-liners whose non-zero exit +from what CI declares, and hand-transcribed shell one-liners whose non-zero exit gets masked (`... ; echo ok`). Here the argv is derived and the exit code is the return value. @@ -153,7 +153,7 @@ def prepare(roots: list[Root], exists: Callable[[str], bool]) -> list[tuple[str, def run( - conventions: str, + workflows: Sequence[str], base: str, *, runner: Callable[[Sequence[str], str], int], @@ -163,8 +163,12 @@ def run( only: Sequence[str] = (), dry_run: bool = False, ) -> int: - """Run the whole matrix; return 0 only when every pair passed.""" - roots = parse_gate_matrix(conventions) + """Run the whole matrix; return 0 only when every pair passed. + + `workflows` is the text of each workflow holding callers -- six of them post + #834, so the roots are the concatenation rather than one file's (#973). + """ + roots = [root for text in workflows for root in parse_gate_matrix(text)] failures = [] skipped = [] diff --git a/internals/checks/src/checks/preflight/gate_test.py b/internals/checks/src/checks/preflight/gate_test.py index 9d9b82919..22f702fb4 100644 --- a/internals/checks/src/checks/preflight/gate_test.py +++ b/internals/checks/src/checks/preflight/gate_test.py @@ -212,14 +212,14 @@ def it_keeps_going_past_a_non_python_root_to_the_ones_after_it(): ] -def drive(conventions=None, **kwargs): +def drive(workflows=None, **kwargs): defaults = { "runner": lambda _argv, _cwd: 0, "exists": has_manifest, "e2e_config": lambda _config: {}, "echo": lambda _line: None, } - return run(conventions or CONVENTIONS, "origin/main", **{**defaults, **kwargs}) + return run(workflows or [CONVENTIONS], "origin/main", **{**defaults, **kwargs}) def describe_run(): @@ -246,7 +246,7 @@ def it_counts_any_non_zero_exit_as_a_failure_including_a_negative_one(): def it_skips_an_artifact_gate_without_failing_and_says_so(): lines = [] code = drive( - conventions=ARTIFACT_FIRST, + workflows=[ARTIFACT_FIRST], runner=lambda _argv, _cwd: 0, echo=lines.append, ) @@ -260,7 +260,7 @@ def it_skips_an_artifact_gate_without_failing_and_says_so(): def it_keeps_going_past_a_skipped_gate_to_the_pairs_after_it(): lines = [] drive( - conventions=ARTIFACT_FIRST, + workflows=[ARTIFACT_FIRST], only=["packaging", "unit-lint"], echo=lines.append, ) @@ -305,11 +305,30 @@ def it_prints_every_pair_without_running_any_when_dry_run(): assert (calls, code) == ([], 0) assert len([line for line in lines if line.startswith("==>")]) == 4 - def it_takes_conventions_and_base_by_keyword(): + def it_runs_the_pairs_of_every_workflow_it_is_given(): + # Post-#834 the callers live in six workflows, so a matrix built from the + # first one alone would be a green run covering one lane (#973). + lines = [] + drive( + workflows=[CONVENTIONS, CONVENTIONS.replace("python-sdk", "internals-checks")], + only=["unit-lint"], + echo=lines.append, + ) + assert [line.split(": ")[0] for line in lines if line.startswith("==>")] == [ + "==> python-sdk [python] unit-lint", + "==> internals-checks [python] unit-lint", + ] + + def it_derives_no_pair_from_a_workflow_with_no_callers(): + lines = [] + assert drive(workflows=["jobs: {}"], echo=lines.append) == 0 + assert lines == ["preflight: 0 failing pair(s), 0 skipped"] + + def it_takes_the_workflows_and_base_by_keyword(): # `*` (not `/`) before the injected seams: the two leading parameters must - # stay nameable, since every caller passes the workflow text by name. + # stay nameable, since every caller passes the workflow texts by name. assert run( - conventions=CONVENTIONS, + workflows=[CONVENTIONS], base="origin/main", runner=lambda _argv, _cwd: 0, exists=has_manifest, diff --git a/internals/checks/src/checks/preflight/matrix.py b/internals/checks/src/checks/preflight/matrix.py index 258151b07..a26aaa656 100644 --- a/internals/checks/src/checks/preflight/matrix.py +++ b/internals/checks/src/checks/preflight/matrix.py @@ -1,19 +1,28 @@ -"""Derive the testing-conventions gate matrix from `.github/workflows/conventions.yml` (#781). +"""Derive the testing-conventions gate matrix from the workflows that call it (#781). The justfile's `test-conventions` recipe restated the (source, gates) pairs by hand and covered 3 of the 8 scan roots CI declares, so a locally-green run said -nothing about 34 of the ~40 pairs. Reading the workflow makes that drift -impossible: a caller added to `conventions.yml` is a pair `preflight` runs. +nothing about most of the pairs. Reading the workflows makes that drift +impossible: a caller added to `.github/workflows/` is a pair `preflight` runs. + +Which workflows hold those callers is itself discovered rather than named. The +matrix used to come from `conventions.yml` alone; #834 split it into six +per-domain workflows and deleted it, so the named default resolved to nothing +and `just preflight` died on a bare `FileNotFoundError` (#973). A list of six +names would only move the same failure one rename away. """ from __future__ import annotations import json +import os +from collections.abc import Callable, Sequence from dataclasses import dataclass import yaml REUSABLE = ".github/workflows/testing-conventions.yml" +WORKFLOWS = ".github/workflows" # The reusable workflow's `gates` input defaults to '' -- "every applicable # gate". Names are the allowlist from its own input description; a root that @@ -28,6 +37,11 @@ "e2e-verify", ] + +class NoGateMatrix(Exception): + """No workflow to derive the matrix from -- carries the contributor-facing fix.""" + + @dataclass class Gate: """A gate's CLI subcommand and which of the shared options it accepts. @@ -74,9 +88,16 @@ class Root: def parse_gate_matrix(text: str) -> list[Root]: - """Every `conventions.yml` job that calls the reusable workflow, as a Root.""" + """Every job in `text` that calls the reusable workflow, as a Root. + + Tolerant of a document that declares no jobs: discovery hands this every + file in `.github/workflows`, and most of them call nothing. + """ + document = yaml.safe_load(text) + if not isinstance(document, dict): + return [] roots = [] - for job, spec in yaml.safe_load(text)["jobs"].items(): + for job, spec in (document.get("jobs") or {}).items(): if REUSABLE not in spec.get("uses", ""): continue inputs = spec.get("with", {}) @@ -92,6 +113,73 @@ def parse_gate_matrix(text: str) -> list[Root]: return roots +def read_text(path: str) -> str: + with open(path, encoding="utf-8") as handle: + return handle.read() + + +def named(path: str, read: Callable[[str], str]) -> str: + """The text of an explicitly named workflow, or a NoGateMatrix carrying the fix.""" + try: + text = read(path) + except OSError as err: + raise NoGateMatrix( + f"--conventions {path}: no such workflow. Name one that calls {REUSABLE}, " + f"or drop the flag to derive the matrix from every caller in {WORKFLOWS}." + ) from err + if not parse_gate_matrix(text): + raise NoGateMatrix( + f"--conventions {path}: no job in it calls {REUSABLE}, so it declares no " + f"gates. Name a workflow that does, or drop the flag to derive the matrix " + f"from every caller in {WORKFLOWS}." + ) + return text + + +def discovered( + directory: str, + listdir: Callable[[str], list[str]], + read: Callable[[str], str], +) -> list[tuple[str, str]]: + """(path, text) for every workflow in `directory` holding a caller.""" + try: + names = sorted(listdir(directory)) + except OSError as err: + raise NoGateMatrix( + f"no {directory} directory here. Run preflight from the repo root, or " + "name a workflow with --conventions." + ) from err + found = [] + for name in names: + if not name.endswith((".yml", ".yaml")): + continue + path = f"{directory}/{name}" + text = read(path) + if parse_gate_matrix(text): + found.append((path, text)) + if not found: + raise NoGateMatrix( + f"no workflow in {directory} calls {REUSABLE}, so there is no gate matrix " + "to run. If the reusable workflow moved, update REUSABLE in " + "internals/checks/src/checks/preflight/matrix.py; otherwise name the " + "workflow with --conventions." + ) + return found + + +def sources( + conventions: Sequence[str], + *, + directory: str = WORKFLOWS, + listdir: Callable[[str], list[str]] = os.listdir, + read: Callable[[str], str] = read_text, +) -> list[tuple[str, str]]: + """(path, text) for the workflows the gate matrix is derived from.""" + if conventions: + return [(path, named(path, read)) for path in conventions] + return discovered(directory, listdir, read) + + def pairs(roots: list[Root]) -> list[tuple[Root, str, str]]: """Flatten the matrix into the (root, language, gate) triples to run.""" return [(root, lang, gate) for root in roots for lang in root.languages for gate in root.gates] diff --git a/internals/checks/src/checks/preflight/matrix_test.py b/internals/checks/src/checks/preflight/matrix_test.py index b60a4ada6..f5d66d99b 100644 --- a/internals/checks/src/checks/preflight/matrix_test.py +++ b/internals/checks/src/checks/preflight/matrix_test.py @@ -1,6 +1,22 @@ """Colocated unit tests for the CI gate-matrix parser (#781).""" -from checks.preflight.matrix import GATES, ROOT_GATES, parse_gate_matrix, pairs +from unittest import mock + +import pytest + +from checks.preflight.matrix import ( + GATES, + NoGateMatrix, + REUSABLE, + ROOT_GATES, + WORKFLOWS, + discovered, + named, + pairs, + parse_gate_matrix, + read_text, + sources, +) CONVENTIONS = """ jobs: @@ -49,6 +65,17 @@ def it_ignores_jobs_that_do_not_call_the_reusable_workflow(): # not stop the walk at the first non-caller. assert [e.job for e in parse_gate_matrix(CONVENTIONS)] == ["python-sdk", "rust"] + def it_reads_nothing_from_a_document_that_is_not_a_mapping(): + # Discovery hands it every file in the workflows directory, so a stray + # list or scalar has to come back empty rather than raise. + assert parse_gate_matrix("- one\n- two\n") == [] + + def it_reads_nothing_from_a_workflow_that_declares_no_jobs(): + assert parse_gate_matrix("name: Docs\non: push\n") == [] + + def it_reads_nothing_from_an_empty_jobs_key(): + assert parse_gate_matrix("jobs:\n") == [] + def describe_GATES(): def it_names_a_cli_subcommand_for_every_default_gate(): @@ -95,3 +122,126 @@ def it_crosses_every_root_language_and_gate(): def it_drops_a_root_that_declares_no_languages(): text = CONVENTIONS.replace("'[\"rust\"]'", "'[]'") assert all(lang != "rust" for _root, lang, _gate in pairs(parse_gate_matrix(text))) + + +NOT_A_CALLER = "jobs:\n build:\n runs-on: ubuntu-latest\n" + + +def reader(files): + """A `read` that only knows the files it was given -- anything else raises.""" + return lambda path: files[path] + + +def describe_read_text(): + def it_reads_a_workflow_as_utf8(): + with mock.patch("checks.preflight.matrix.open", mock.mock_open(read_data=CONVENTIONS)) as opened: + assert read_text("wf.yml") == CONVENTIONS + opened.assert_called_once_with("wf.yml", encoding="utf-8") + + +def describe_named(): + def it_returns_the_text_of_a_workflow_that_calls_the_reusable_workflow(): + assert named("wf.yml", reader({"wf.yml": CONVENTIONS})) == CONVENTIONS + + def it_names_the_file_and_the_fix_when_it_cannot_be_read(): + # The #973 regression exactly: the default target stopped existing and the + # command died on a bare FileNotFoundError instead of saying what to do. + def read(path): + raise FileNotFoundError(2, "No such file or directory", path) + + with pytest.raises(NoGateMatrix) as caught: + named(".github/workflows/conventions.yml", read) + + message = str(caught.value) + assert "--conventions .github/workflows/conventions.yml: no such workflow" in message + assert REUSABLE in message + assert WORKFLOWS in message + + def it_rejects_a_workflow_that_declares_no_caller(): + # Accepting it would leave a green run whose matrix is empty. + with pytest.raises(NoGateMatrix) as caught: + named("docs.yml", reader({"docs.yml": NOT_A_CALLER})) + + assert "--conventions docs.yml: no job in it calls" in str(caught.value) + + +def describe_discovered(): + FILES = { + ".github/workflows/b-ci.yml": CONVENTIONS, + ".github/workflows/a-ci.yaml": CONVENTIONS, + ".github/workflows/docs.yml": NOT_A_CALLER, + } + + def it_returns_every_caller_it_finds_in_sorted_order(): + found = discovered( + ".github/workflows", + lambda _directory: ["b-ci.yml", "docs.yml", "a-ci.yaml"], + reader(FILES), + ) + + assert found == [ + (".github/workflows/a-ci.yaml", CONVENTIONS), + (".github/workflows/b-ci.yml", CONVENTIONS), + ] + + def it_never_reads_a_file_that_is_not_a_workflow(): + # `reader` raises for anything it was not given, so a README that got + # opened fails here rather than passing quietly. + found = discovered( + ".github/workflows", + lambda _directory: ["README.md", "b-ci.yml"], + reader(FILES), + ) + + assert [path for path, _text in found] == [".github/workflows/b-ci.yml"] + + def it_names_the_directory_and_the_fix_when_it_does_not_exist(): + def listdir(directory): + raise FileNotFoundError(2, "No such file or directory", directory) + + with pytest.raises(NoGateMatrix) as caught: + discovered(".github/workflows", listdir, reader(FILES)) + + assert "no .github/workflows directory here" in str(caught.value) + assert "--conventions" in str(caught.value) + + def it_fails_when_no_workflow_in_the_directory_calls_the_reusable_workflow(): + with pytest.raises(NoGateMatrix) as caught: + discovered(".github/workflows", lambda _directory: ["docs.yml"], reader(FILES)) + + message = str(caught.value) + assert f"no workflow in .github/workflows calls {REUSABLE}" in message + assert "matrix.py" in message + + +def describe_sources(): + def it_reads_each_named_workflow_in_the_order_given(): + files = {"b.yml": CONVENTIONS, "a.yml": CONVENTIONS} + + assert sources(["b.yml", "a.yml"], read=reader(files)) == [ + ("b.yml", CONVENTIONS), + ("a.yml", CONVENTIONS), + ] + + def it_discovers_the_callers_when_no_workflow_is_named(): + found = sources( + (), + directory="wf", + listdir=lambda _directory: ["ci.yml"], + read=reader({"wf/ci.yml": CONVENTIONS}), + ) + + assert found == [("wf/ci.yml", CONVENTIONS)] + + def it_discovers_from_the_workflows_directory_by_default(): + # The default is the whole point: #834 deleted the workflow the command + # used to name, and nothing pinned where it looks instead. + looked = [] + + sources( + (), + listdir=lambda directory: looked.append(directory) or ["ci.yml"], + read=reader({".github/workflows/ci.yml": CONVENTIONS}), + ) + + assert looked == [WORKFLOWS] == [".github/workflows"] diff --git a/internals/checks/tests/e2e/preflight_test.py b/internals/checks/tests/e2e/preflight_test.py index 0b6fa13ad..4e8ee2720 100644 --- a/internals/checks/tests/e2e/preflight_test.py +++ b/internals/checks/tests/e2e/preflight_test.py @@ -40,8 +40,9 @@ def it_prints_the_matrix_from_the_repo_root_with_no_arguments(): assert proc.returncode == 0, f"stdout={proc.stdout!r} stderr={proc.stderr!r}" assert "==> python-sdk [python] unit-lint" in proc.stdout + assert "==> typescript-sdk [typescript] unit-lint" in proc.stdout + assert "==> rust [rust] unit-lint" in proc.stdout assert "==> internals-checks [python] unit-lint" in proc.stdout - assert "==> rust-core [rust] unit-lint" in proc.stdout def it_exits_with_an_actionable_message_when_a_named_workflow_is_missing(): proc = preflight("--conventions", ".github/workflows/conventions.yml") diff --git a/justfile b/justfile index 625113998..f7731ad05 100644 --- a/justfile +++ b/justfile @@ -73,12 +73,14 @@ e2e-attest-ts: cd packages/ts && testing-conventions e2e attest 'pnpm test:e2e' # Every testing-conventions gate CI declares, derived from the (source, gates) -# pairs in .github/workflows/conventions.yml -- 39 pairs across 8 scan roots -# (#781). Supersedes the hand-restated `test-conventions` / `unit-coverage` / -# `mutation` / `e2e-verify` recipes, which named 6 pairs across 3 roots and -# drifted from the workflow. `--gate` narrows it (repeatable); `--dry-run` -# prints the matrix without running it. Needs the native build first for the -# suite-executing gates (maturin / napi); run `just build` if missing. +# pairs of every workflow in .github/workflows that calls the reusable workflow +# -- 40 pairs across 8 scan roots in 6 workflows today (#781, #973). Supersedes +# the hand-restated `test-conventions` / `unit-coverage` / `mutation` / +# `e2e-verify` recipes, which named 6 pairs across 3 roots and drifted from the +# workflow. `--conventions` narrows it to named workflows (repeatable), `--gate` +# to named gates; `--dry-run` prints the matrix without running it. Needs the +# native build first for the suite-executing gates (maturin / napi); run +# `just build` if missing. # # `packaging` is reported SKIP, not run: it inspects a BUILT artifact, which CI # builds from each manifest. Use `just test-packaging` for that locally. From 4ffd060d6a90862e0a96b71b28240c31aacc3c5a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 19:24:22 +0000 Subject: [PATCH 3/4] test: pin the gate-matrix seams as keyword-only `sources` takes its `directory` / `listdir` / `read` seams after a `*`, and nothing failed when the mutation gate turned that into `/`: a caller could then pass a directory as the second positional argument and have it land on `listdir`. The sibling `run` already carries the same assertion. --- internals/checks/src/checks/preflight/matrix_test.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internals/checks/src/checks/preflight/matrix_test.py b/internals/checks/src/checks/preflight/matrix_test.py index f5d66d99b..2c2250c8b 100644 --- a/internals/checks/src/checks/preflight/matrix_test.py +++ b/internals/checks/src/checks/preflight/matrix_test.py @@ -233,6 +233,12 @@ def it_discovers_the_callers_when_no_workflow_is_named(): assert found == [("wf/ci.yml", CONVENTIONS)] + def it_takes_its_seams_by_keyword_only(): + # `*` (not `/`) before the injected seams: a third positional argument + # would otherwise land silently on `listdir`. + with pytest.raises(TypeError): + sources((), "wf", lambda _directory: ["ci.yml"], reader({"wf/ci.yml": CONVENTIONS})) + def it_discovers_from_the_workflows_directory_by_default(): # The default is the whole point: #834 deleted the workflow the command # used to name, and nothing pinned where it looks instead. From a1b35b6c1a1d4c9c37ff38bd3f7f5a78e01e62c9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 19:24:42 +0000 Subject: [PATCH 4/4] e2e attestation for claude/tackle-973-4e1epu --- .../checks/e2e-attestations/claude-tackle-973-4e1epu.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 internals/checks/e2e-attestations/claude-tackle-973-4e1epu.json diff --git a/internals/checks/e2e-attestations/claude-tackle-973-4e1epu.json b/internals/checks/e2e-attestations/claude-tackle-973-4e1epu.json new file mode 100644 index 000000000..3100f6108 --- /dev/null +++ b/internals/checks/e2e-attestations/claude-tackle-973-4e1epu.json @@ -0,0 +1,7 @@ +{ + "command": "uv run python -m pytest tests/e2e -x -q", + "ran_at": 1787253882, + "exit_code": 0, + "commit": "4ffd060d6a90862e0a96b71b28240c31aacc3c5a", + "branch": "claude/tackle-973-4e1epu" +}