From 5587c25675126ef6d5b1709f0fbfab2c354a27d3 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Fri, 4 Sep 2026 13:40:11 +0200 Subject: [PATCH] fix: run the suite on both roads into main validate.yml triggered on pull_request alone, and it holds the only unittest invocation in the repository: deploy-site.yml stops at validate_schemas, generation and the freshness diff. Work lands on main by direct push far more often than by pull request, so 1,318 cases were guarding the road almost nothing takes. The suite and the schema check now run on both events. validate-bios and label-pr read pull request context and carry an event guard. The concurrency group falls back to the ref, so a push series collapses to the tip: what stays verified is the head of main. The path lists are spelled out per event because the workflow parser reads no YAML anchor, which PyYAML would have accepted in silence. Four tests hold the wiring: the suite reachable from a push, the two path lists equal, every job reading pull request context guarded, and no anchor in any workflow. --- .github/workflows/validate.yml | 25 +++++++++++- tests/test_audit_regressions.py | 69 +++++++++++++++++++++++++++++++++ wiki/architecture.md | 2 +- wiki/release-process.md | 27 +++++++------ wiki/testing-guide.md | 15 +++++-- 5 files changed, 119 insertions(+), 19 deletions(-) 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.