diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index bfc79db6..3f33954a 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -1,5 +1,8 @@ -name: PR Validation +name: Validation +# The same checks on both roads into main. Work lands here by direct push as +# often as by pull request, and a test suite reachable only from a PR guards +# the road nobody takes. on: pull_request: paths: @@ -14,18 +17,35 @@ on: - "install.py" - "install.sh" - "install.ps1" + # Spelled out twice because the workflow parser reads no YAML anchor; a test + # holds the two lists equal. + push: + branches: [main] + paths: + - "bios/**" + - "platforms/**" + - "emulators/**" + - "schemas/**" + - "scripts/**" + - "tests/**" + - "install.py" + - "install.sh" + - "install.ps1" permissions: contents: read pull-requests: write concurrency: - group: validate-${{ github.event.pull_request.number }} + # A push series collapses to the tip: what has to stay verified is the head + # of main, not every commit that passed under it. + group: validate-${{ github.event.pull_request.number || github.ref }} cancel-in-progress: true jobs: validate-bios: runs-on: ubuntu-latest + if: github.event_name == 'pull_request' steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 with: @@ -100,6 +120,7 @@ jobs: label-pr: runs-on: ubuntu-latest + if: github.event_name == 'pull_request' permissions: pull-requests: write steps: diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index e1055e76..ef40f024 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -69,6 +69,75 @@ class ReadmeRegressions(unittest.TestCase): self.assertEqual(unknown, set(), f"README advertises unknown flags: {unknown}") +class WorkflowRegressions(unittest.TestCase): + """The test suite must sit on every road into main. + + run-tests lived in a pull_request-only workflow while the work landed by + direct push, so 1,318 cases guarded a road almost nothing took. What is + asserted here is reachability, not the workflow's shape. + """ + + @staticmethod + def _workflow(name: str) -> dict: + with (ROOT / ".github" / "workflows" / name).open(encoding="utf-8") as f: + document = yaml.safe_load(f) + # PyYAML resolves the bare `on:` key to the boolean True. + document["on"] = document.pop(True, document.get("on")) + return document + + def test_the_suite_runs_on_a_direct_push_to_main(self): + workflow = self._workflow("validate.yml") + triggers = workflow["on"] + self.assertIn("push", triggers, "validate.yml no longer runs on push") + self.assertIn("main", triggers["push"]["branches"]) + job = next( + ( + name + for name, body in workflow["jobs"].items() + if any( + "unittest discover" in str(step.get("run", "")) + for step in body.get("steps", []) + ) + ), + None, + ) + self.assertIsNotNone(job, "no job runs the test suite") + self.assertIsNone( + workflow["jobs"][job].get("if"), + f"{job} carries a condition; the suite must run on push and on PR", + ) + + def test_both_events_watch_the_same_paths(self): + triggers = self._workflow("validate.yml")["on"] + self.assertEqual( + triggers["pull_request"]["paths"], + triggers["push"]["paths"], + "the two path lists have drifted; a push would skip what a PR checks", + ) + + def test_pull_request_only_jobs_are_guarded(self): + workflow = self._workflow("validate.yml") + for name, body in workflow["jobs"].items(): + uses_pr_context = "github.event.pull_request" in yaml.dump(body) + if uses_pr_context: + self.assertEqual( + body.get("if"), + "github.event_name == 'pull_request'", + f"{name} reads pull request context and would run on a push", + ) + + def test_no_workflow_relies_on_a_yaml_anchor(self): + """The workflow parser reads no anchor; PyYAML would hide the break.""" + for path in sorted((ROOT / ".github" / "workflows").glob("*.yml")): + for number, line in enumerate( + path.read_text(encoding="utf-8").splitlines(), 1 + ): + self.assertIsNone( + re.search(r"(?:^|\s)[&*][A-Za-z_]", line), + f"{path.name}:{number} uses a YAML anchor: {line.strip()}", + ) + + class FaqRegressions(unittest.TestCase): """The FAQ states facts the code owns; these tie it back to the source. diff --git a/wiki/architecture.md b/wiki/architecture.md index c8626309..57504a71 100644 --- a/wiki/architecture.md +++ b/wiki/architecture.md @@ -348,7 +348,7 @@ pattern and how to add a test. | Workflow | File | Trigger | Role | |----------|------|---------|------| | Deploy Site | `deploy-site.yml` | push to main (platforms, emulators, wiki, scripts) + manual | validate contracts, generate site, build with MkDocs, validate rendered HTML, deploy to Pages | -| PR Validation | `validate.yml` | pull request on bios/, platforms/, emulators/, schemas/, scripts/, tests/ | validate BIOS hashes, schema check, run the full test suite, auto-label PR | +| Validation | `validate.yml` | pull request and push to main on bios/, platforms/, emulators/, schemas/, scripts/, tests/ | schema check and full test suite on both; BIOS hash validation and auto-label on pull requests | Releases are not built in CI: the packs are generated and checked on the maintainer's machine and uploaded with `gh`, see the diff --git a/wiki/release-process.md b/wiki/release-process.md index 88810920..9e02dcfa 100644 --- a/wiki/release-process.md +++ b/wiki/release-process.md @@ -14,7 +14,7 @@ Budget target: ~175 minutes/month on the GitHub free tier. | Workflow | File | Trigger | |----------|------|---------| | Deploy Site | `deploy-site.yml` | Push to main (platforms, emulators, provenance, wiki, scripts, database.json, mkdocs.yml), manual | -| PR Validation | `validate.yml` | PR touching `bios/**`, `platforms/**` or `emulators/**` | +| Validation | `validate.yml` | PR and push to main touching `bios/**`, `platforms/**` or `emulators/**` | Upstream BIOS lists are not scraped on a schedule. A maintainer runs the scrapers by hand (see [adding a scraper](adding-a-scraper.md)), reviews the @@ -70,18 +70,21 @@ The theme version is pinned on both sides: `>=9.7.5` because that is the release which caps `mkdocs < 2` (MkDocs 2.0 ships without a license), `<10` so a major theme release cannot change the site without a deliberate bump. -## validate.yml - PR Validation +## validate.yml - Validation -**Trigger.** Pull requests that modify `bios/**`, `platforms/**`, -`emulators/**`, `schemas/**`, `scripts/**`, `tests/**` or `install.py`. +**Trigger.** Pull requests and direct pushes to main that modify `bios/**`, +`platforms/**`, `emulators/**`, `schemas/**`, `scripts/**`, `tests/**` or +`install.py`. The path lists are spelled out once per event because the +workflow parser reads no YAML anchor. -**Concurrency.** Per-PR group, cancel in-progress. +**Concurrency.** Per-PR group on a pull request, per-ref on a push, cancel +in-progress either way: a push series collapses to the tip. -Four parallel jobs: +Four jobs, two of which read pull request context and carry an event guard: -**validate-bios.** Diffs the PR to find changed BIOS files, runs -`validate_pr.py --markdown` on each, and posts the validation report as a PR -comment (hash verification, database match status). +**validate-bios** (pull requests only). Diffs the PR to find changed BIOS +files, runs `validate_pr.py --markdown` on each, and posts the validation +report as a PR comment (hash verification, database match status). **validate-configs.** Runs `python scripts/validate_schemas.py --source-only`, which validates every platform YAML against `schemas/platform.schema.json` and @@ -89,10 +92,10 @@ every emulator profile against `schemas/emulator.schema.json`. Both schemas set `additionalProperties: false`, so a typo in a field name fails the job instead of being silently ignored. -**run-tests.** Runs `python -m unittest discover tests -v`. Must pass before -merge. +**run-tests.** Runs `python -m unittest discover tests -v`. Must pass before a +merge, and again on the commit a direct push puts at the head of main. -**label-pr.** Auto-labels the PR based on changed paths: +**label-pr** (pull requests only). Auto-labels the PR based on changed paths: | Path pattern | Label | |-------------|-------| diff --git a/wiki/testing-guide.md b/wiki/testing-guide.md index db0c8144..94a16fbc 100644 --- a/wiki/testing-guide.md +++ b/wiki/testing-guide.md @@ -246,10 +246,17 @@ Ideally, tests, code, and documentation ship together. When profiles and platfor ## CI integration -The `validate.yml` workflow runs `python -m unittest discover tests -v` on every -pull request that touches `bios/`, `platforms/`, `emulators/`, `schemas/`, -`scripts/`, `tests/` or `install.py`. The test job (`run-tests`) runs in parallel -with BIOS validation, schema validation, and auto-labeling. +The `validate.yml` workflow runs `python -m unittest discover tests -v` on both +roads into main: every pull request, and every direct push, that touches +`bios/`, `platforms/`, `emulators/`, `schemas/`, `scripts/`, `tests/` or +`install.py`. Work reaches main by push as often as by pull request, so a suite +wired to pull requests alone would guard the road nobody takes. The test job +(`run-tests`) runs in parallel with schema validation, and on a pull request +with BIOS validation and auto-labeling too; those two read pull request context +and stay behind an event guard. + +A push series collapses to the tip, so what a green run states is that the head +of main passes, not every commit under it. Modules that need real artifacts skip themselves when those artifacts are absent, so `test_pack_integrity` is a no-op in CI (no `dist/`) and a real check locally.