Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -137,3 +137,43 @@ jobs:
with:
files: dist/*
generate_release_notes: true

# ── Required check ────────────────────────────────────────────────────────
# One job that succeeds only if every other job in this workflow did, so
# branch protection has a single stable name to require. #87 asks for a
# required check on master; this is the name to point it at.
#
# Requiring the jobs themselves does not work here. `test` is a 3x3 matrix,
# so its checks arrive as nine generated names that change whenever the
# matrix does, and a required name that no longer appears blocks every PR
# until someone edits the branch protection. `release` is skipped on every
# pull request, and a required check that is skipped never reports.
#
# `if: always()` matters: without it the gate is skipped whenever an earlier
# job fails, and a skipped required check leaves the PR waiting for a status
# that will never arrive -- the failure would present as a hang, not a red X.
#
# A non-success result of any kind fails the gate, `skipped` included. A job
# that did not run did not verify anything, so treating it as a pass is the
# same fail-open shape this repository has been removing elsewhere.
ci-gate:
name: CI Gate
runs-on: ubuntu-22.04
needs: [test, build]
if: always()
steps:
- name: Every job in this workflow must have succeeded
env:
RESULTS: ${{ toJSON(needs) }}
run: |
printf '%s\n' "$RESULTS"
bad=$(printf '%s' "$RESULTS" | jq -r '
to_entries[]
| select(.value.result != "success")
| " \(.key): \(.value.result)"')
if [ -n "$bad" ]; then
echo "::error::CI Gate failed. These jobs did not succeed:"
printf '%s\n' "$bad"
exit 1
fi
echo "All jobs succeeded."
201 changes: 201 additions & 0 deletions tests/unit/test_ci_gate.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
"""The CI workflow must expose one job that summarises all the others.

#87 asks for a required status check on `master`. Branch protection can only
require a check by name, and the names this workflow produces are not usable
for that directly: `test` is a matrix, so it arrives as nine generated names
that change with the matrix, and `release` is skipped on every pull request.

So `ci-gate` exists to be the one name to require. These tests keep it honest
-- specifically, they fail if someone adds a job to the workflow and does not
wire it into the gate, which would otherwise silently create a job that the
required check does not cover.
"""

import yaml
import pytest
from pathlib import Path


WORKFLOWS_DIR = Path(__file__).resolve().parents[2] / ".github" / "workflows"
WORKFLOW = WORKFLOWS_DIR / "ci.yml"

# The name branch protection is pointed at. Changing it silently un-requires
# the check, so it is pinned here rather than merely read.
GATE_ID = "ci-gate"
GATE_NAME = "CI Gate"

#: Every workflow that produces a pull-request status, and the display name a
#: maintainer must require for it. `CI Gate` covers ci.yml and nothing else --
#: cross-workflow `needs` is not something GitHub offers -- so the full
#: required set is this mapping, not one name.
#:
#: A workflow may sit in NO_GATE only with a reason about the workflow itself.
#: "It has no gate yet" is not one: that is the gap this table exists to make
#: visible.
REQUIRED_CHECKS = {
"ci.yml": GATE_NAME,
}

NO_GATE = {
"auto-assign.yml":
"assigns a reviewer; it verifies nothing, so requiring it would block "
"merges on a housekeeping step",
"claude-code-review.yml":
"posts advisory review comments and never fails on content",
"codeql.yml":
"already reports a single stable name, `CodeQL`, which should be "
"required directly rather than wrapped in a gate",
"book-build.yml":
"builds documentation; a docs failure should not block a code merge, "
"and this is a deliberate policy choice rather than an oversight",
"simulation-test.yml":
"has a gate job whose body still fails open on part of its `needs` -- "
"tracked separately; requiring it today would assert more than it "
"checks",
"vendor-drift.yml":
"reports third-party drift for triage and is expected to fail while a "
"vendored dependency is behind",
}


def _load(path):
return yaml.safe_load(path.read_text(encoding="utf-8"))


def _runs_on_pull_request(doc):
# PyYAML parses a bare `on:` key as the boolean True.
triggers = doc.get("on", doc.get(True, {}))
if isinstance(triggers, dict):
return "pull_request" in triggers
if isinstance(triggers, list):
return "pull_request" in triggers
return triggers == "pull_request"


def _pr_workflows():
found = {}
for path in sorted(WORKFLOWS_DIR.glob("*.yml")):
doc = _load(path)
if isinstance(doc, dict) and _runs_on_pull_request(doc):
found[path.name] = doc
assert found, f"no pull-request workflows found under {WORKFLOWS_DIR}"
return found


@pytest.fixture(scope="module")
def workflow():
assert WORKFLOW.is_file(), f"{WORKFLOW} does not exist"
return yaml.safe_load(WORKFLOW.read_text(encoding="utf-8"))


@pytest.fixture(scope="module")
def jobs(workflow):
return workflow["jobs"]


def _only_runs_on_tags(job):
"""Is this job gated to tag builds, and therefore skipped on every PR?"""
condition = str(job.get("if", ""))
return "refs/tags" in condition


def test_gate_job_exists(jobs):
assert GATE_ID in jobs, (
f"no {GATE_ID!r} job; branch protection has no single name to require"
)


def test_gate_display_name_is_pinned(jobs):
assert jobs[GATE_ID]["name"] == GATE_NAME, (
"the gate's display name is what branch protection matches on; "
"renaming it un-requires the check without failing anything"
)


def test_gate_runs_even_when_an_earlier_job_fails(jobs):
condition = str(jobs[GATE_ID].get("if", "")).strip()
assert condition == "always()", (
"the gate needs `if: always()`. Without it the gate is skipped when an "
"earlier job fails, and a skipped required check never reports -- the "
"pull request waits for a status that never arrives instead of showing "
"a failure"
)


def test_gate_covers_every_job_that_runs_on_a_pull_request(jobs):
expected = {
name for name, job in jobs.items()
if name != GATE_ID and not _only_runs_on_tags(job)
}
declared = set(jobs[GATE_ID].get("needs", []))

missing = expected - declared
assert not missing, (
f"these jobs run on pull requests but the gate does not wait for them: "
f"{sorted(missing)}. A job outside the gate is a job the required "
f"check does not cover."
)

unknown = declared - set(jobs)
assert not unknown, f"the gate needs jobs that do not exist: {sorted(unknown)}"


def test_jobs_left_out_of_the_gate_are_genuinely_tag_only(jobs):
"""Excluding a job from the gate must be justified, not just convenient."""
declared = set(jobs[GATE_ID].get("needs", []))
for name, job in jobs.items():
if name == GATE_ID or name in declared:
continue
assert _only_runs_on_tags(job), (
f"job {name!r} is not in the gate and is not tag-only; either add "
f"it to `needs` or give it an `if:` that explains why it cannot run "
f"on a pull request"
)


def test_every_pull_request_workflow_is_accounted_for():
"""A new workflow must be gated or explicitly excused, not silently added.

`CI Gate` summarises ci.yml only. Requiring that one name -- which is what
this PR's own description asked a maintainer to do -- leaves every other
workflow's checks unrequired, and the rot-guard above cannot see them
either. This is the test that notices.
"""
unaccounted = [
name for name in _pr_workflows()
if name not in REQUIRED_CHECKS and name not in NO_GATE
]
assert not unaccounted, (
f"these workflows produce pull-request checks but are neither gated "
f"nor excused: {sorted(unaccounted)}. Add a gate job and list it in "
f"REQUIRED_CHECKS, or add it to NO_GATE with the reason."
)


def test_gated_workflows_really_have_their_gate():
"""Each entry in REQUIRED_CHECKS names a job that exists and is a gate."""
workflows = _pr_workflows()
for filename, display in REQUIRED_CHECKS.items():
assert filename in workflows, (
f"{filename} is in REQUIRED_CHECKS but produces no pull-request "
f"checks; the required set names a check that never reports"
)
jobs = workflows[filename]["jobs"]
matching = [j for j in jobs.values() if j.get("name") == display]
assert matching, (
f"{filename} has no job displaying as {display!r}, so branch "
f"protection would wait forever for a status that never arrives"
)


def test_gate_fails_on_any_non_success_result(jobs):
"""A skipped or cancelled job must fail the gate, not pass it."""
steps = jobs[GATE_ID]["steps"]
script = "\n".join(str(s.get("run", "")) for s in steps)

assert '!= "success"' in script, (
"the gate must require success specifically. Checking only for "
"'failure' lets a skipped or cancelled job through, which is the "
"fail-open shape this repository has been removing elsewhere"
)
assert "exit 1" in script, "the gate must actually fail the job"
Loading