From f4b45c641d7c7ab19354d80fbfcd4acee91d2df8 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Fri, 6 Nov 2026 14:55:03 +0100 Subject: [PATCH] feat: run the PR checks with one command --- .github/workflows/deploy-site.yml | 1 + CONTRIBUTING.md | 28 +++++---- scripts/generate_readme.py | 22 +++---- scripts/pr_checks.py | 99 +++++++++++++++++++++++++++++++ tests/test_contributing_text.py | 67 ++++++++++++++++++++- wiki/tools.md | 1 + 6 files changed, 196 insertions(+), 22 deletions(-) create mode 100644 scripts/pr_checks.py diff --git a/.github/workflows/deploy-site.yml b/.github/workflows/deploy-site.yml index 4ea290a9b..04b329d26 100644 --- a/.github/workflows/deploy-site.yml +++ b/.github/workflows/deploy-site.yml @@ -38,6 +38,7 @@ on: - "scripts/packreadme.py" - "scripts/packresolve.py" - "scripts/packverify.py" + - "scripts/pr_checks.py" - "scripts/provenance_report.py" - "scripts/refresh_data_dirs.py" - "scripts/region.py" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f7b2d05f5..ff2a2baec 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -73,17 +73,25 @@ the checks CI runs on a pull request (Python 3.12 or newer): ```bash pip install pyyaml "jsonschema[format-nongpl]==4.26.0" "markdown==3.10.2" "pyte==0.8.2" "hypothesis==6.170.8" "vermin==1.9.2" "setuptools==80.9.0" -python scripts/validate_pr.py --changed # the BIOS files the branch adds -python scripts/validate_schemas.py --source-only # profiles and platform files -python scripts/profile_sync.py --added --strict # each profile the branch adds +python scripts/pr_checks.py +``` + +`pr_checks.py` runs these, in CI's order, carries on past a failure as CI +does, and names at the end each check that failed or could not run: + +```bash +python scripts/validate_pr.py --changed # the BIOS files the branch adds +python scripts/validate_schemas.py --source-only # profiles and platform files +python scripts/profile_sync.py --added --strict # each profile the branch adds +vermin --no-tips -t=3.8- --violations install.py # install.py stays on Python 3.8 python scripts/export_native.py --all --fetch --output-dir tmp/native # files the exporter tests compare with -python scripts/refresh_data_dirs.py # data directories the pack-building tests read -python scripts/generate_site.py # the pages the site tests read -npm ci --prefix website && npm run build --prefix website # the built site, Node 24 -python -m unittest discover -s tests -t . -python scripts/validate_site.py --site-dir website/dist # links and page budgets -vermin --no-tips -t=3.8- --violations install.py # install.py stays on Python 3.8 -python3.8 -m unittest tests.test_install_tui # the full screen on the installer's floor +python scripts/refresh_data_dirs.py # data directories the pack-building tests read +python scripts/generate_site.py # the pages the site tests read +npm ci --prefix website # the site's packages, Node 24 +npm run build --prefix website # the built site +python -m unittest discover -s tests -t . # the suite +python3.8 -m unittest tests.test_install_tui # the full screen on the installer's floor +python scripts/validate_site.py --site-dir website/dist # links and page budgets ``` The versions are the ones CI pins. Three tools come from the system, not from diff --git a/scripts/generate_readme.py b/scripts/generate_readme.py index 51588abb4..572f3d1eb 100644 --- a/scripts/generate_readme.py +++ b/scripts/generate_readme.py @@ -42,6 +42,7 @@ import release_record from siteguides import install_command from validate_pr import LARGE_FILE_ISSUE_URL from largefiles import load_registered_paths +import pr_checks from verify import verify_platform @@ -939,6 +940,8 @@ def contributing_text( .md for the site); *site_words* names the site from where the text is read. """ pip_line, floor_line = ci_install_lines() + width = max(len(step.command) for step in pr_checks.STEPS) + steps = "\n".join(f"{step.command:<{width}} # {step.why}" for step in pr_checks.STEPS) return f"""{title} ## Add a BIOS file @@ -1014,17 +1017,14 @@ the checks CI runs on a pull request (Python 3.12 or newer): ```bash {pip_line} -python scripts/validate_pr.py --changed # the BIOS files the branch adds -python scripts/validate_schemas.py --source-only # profiles and platform files -python scripts/profile_sync.py --added --strict # each profile the branch adds -python scripts/export_native.py --all --fetch --output-dir tmp/native # files the exporter tests compare with -python scripts/refresh_data_dirs.py # data directories the pack-building tests read -python scripts/generate_site.py # the pages the site tests read -npm ci --prefix website && npm run build --prefix website # the built site, Node 24 -python -m unittest discover -s tests -t . -python scripts/validate_site.py --site-dir website/dist # links and page budgets -vermin --no-tips -t=3.8- --violations install.py # install.py stays on Python 3.8 -python3.8 -m unittest tests.test_install_tui # the full screen on the installer's floor +python scripts/pr_checks.py +``` + +`pr_checks.py` runs these, in CI's order, carries on past a failure as CI +does, and names at the end each check that failed or could not run: + +```bash +{steps} ``` The versions are the ones CI pins. Three tools come from the system, not from diff --git a/scripts/pr_checks.py b/scripts/pr_checks.py new file mode 100644 index 000000000..cd7741295 --- /dev/null +++ b/scripts/pr_checks.py @@ -0,0 +1,99 @@ +#!/usr/bin/env python3 +"""Run the checks validate.yml runs on a pull request, in its order. + +Usage: + python scripts/pr_checks.py + python scripts/pr_checks.py --list + +Each step runs even when an earlier one failed, as CI runs them, and the +summary names every step that failed or could not run. A step whose tool is +absent (npm, python3.8, vermin) is reported as not run, with what provides it. +""" + +from __future__ import annotations + +import argparse +import os +import shutil +import subprocess +import sys +from dataclasses import dataclass, field +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent + + +@dataclass(frozen=True) +class Step: + command: str + why: str + needs: str = "" + provider: str = "" + env: dict[str, str] = field(default_factory=dict) + + +# The order of validate.yml: data fetched, site generated and built, then the +# suite, its 3.8 run and the built-site check. +STEPS: tuple[Step, ...] = ( + Step("python scripts/validate_pr.py --changed", "the BIOS files the branch adds"), + Step("python scripts/validate_schemas.py --source-only", "profiles and platform files"), + Step("python scripts/profile_sync.py --added --strict", "each profile the branch adds"), + Step("vermin --no-tips -t=3.8- --violations install.py", "install.py stays on Python 3.8", + needs="vermin", provider="the pip line of CONTRIBUTING.md"), + Step("python scripts/export_native.py --all --fetch --output-dir tmp/native", + "files the exporter tests compare with"), + Step("python scripts/refresh_data_dirs.py", "data directories the pack-building tests read"), + Step("python scripts/generate_site.py", "the pages the site tests read"), + Step("npm ci --prefix website", "the site's packages, Node 24", + needs="npm", provider="Node 24"), + Step("npm run build --prefix website", "the built site", + needs="npm", provider="Node 24"), + Step("python -m unittest discover -s tests -t .", "the suite", + env={"RETROBIOS_OFFLINE": "1"}), + Step("python3.8 -m unittest tests.test_install_tui", "the full screen on the installer's floor", + needs="python3.8", provider="a Python 3.8 with pyte", + env={"RETROBIOS_OFFLINE": "1"}), + Step("python scripts/validate_site.py --site-dir website/dist", "links and page budgets"), +) + + +def argv(step: Step) -> list[str]: + """The command, with `python` run by the interpreter running this script.""" + words = step.command.split() + return [sys.executable, *words[1:]] if words[0] == "python" else words + + +def run(steps: tuple[Step, ...] = STEPS) -> int: + failed: list[str] = [] + not_run: list[str] = [] + for step in steps: + print(f"\n== {step.command}", flush=True) + if step.needs and shutil.which(step.needs) is None: + print(f"not run: {step.needs} is not installed ({step.provider})") + not_run.append(f"{step.command} ({step.needs}: {step.provider})") + continue + code = subprocess.run(argv(step), cwd=ROOT, env={**os.environ, **step.env}).returncode + if code: + failed.append(f"{step.command} (exit {code})") + print() + for line in failed: + print(f"FAILED {line}") + for line in not_run: + print(f"NOT RUN {line}") + print(f"{len(steps) - len(failed) - len(not_run)}/{len(steps)} checks passed") + return 1 if failed else 0 + + +def main() -> None: + parser = argparse.ArgumentParser(allow_abbrev=False, description=__doc__.splitlines()[0]) + parser.add_argument("--list", action="store_true", help="print the steps and exit") + args = parser.parse_args() + if args.list: + for step in STEPS: + print(step.command) + return + sys.exit(run()) + + +if __name__ == "__main__": + main() diff --git a/tests/test_contributing_text.py b/tests/test_contributing_text.py index 579f8a617..10f9a3d9b 100644 --- a/tests/test_contributing_text.py +++ b/tests/test_contributing_text.py @@ -65,7 +65,7 @@ class BeforeOpeningAPullRequest(unittest.TestCase): text = (REPO_ROOT / "CONTRIBUTING.md").read_text(encoding="utf-8") section = text.split("## Before opening a PR", 1)[1].split("\n## ", 1)[0] - block = _re.search(r"```bash\n(.*?)```", section, _re.S).group(1) + block = "".join(_re.findall(r"```bash\n(.*?)```", section, _re.S)) for command in ("validate_pr.py --changed", "validate_schemas.py --source-only", "unittest discover"): self.assertIn(command, block) @@ -217,3 +217,68 @@ class TheGuideRebuildsWhenItsPinsMove(unittest.TestCase): (REPO_ROOT / ".github" / "workflows" / "deploy-site.yml").read_text(encoding="utf-8")) paths = deploy[True]["push"]["paths"] self.assertIn(".github/workflows/validate.yml", paths) + + +class OneCommandRunsTheChecks(unittest.TestCase): + """A contributor copied ten commands by hand before a pull request, and + skipping one (refresh_data_dirs) failed the pack tests with no hint.""" + + CHECK = re.compile(r"(?:xargs -0 |run: |^\s*)((?:python3?\.?8? -m unittest|python scripts/\w+\.py" + r"|vermin|npm (?:ci|run build)))") + + def test_every_check_ci_runs_is_a_step(self): + import pr_checks + + workflow = (REPO_ROOT / ".github" / "workflows" / "validate.yml").read_text(encoding="utf-8") + ran = {m.group(1) for line in workflow.splitlines() + if not line.strip().startswith("#") for m in [self.CHECK.search(line)] if m} + self.assertGreaterEqual(len(ran), 10) + steps = [step.command for step in pr_checks.STEPS] + for check in sorted(ran): + with self.subTest(check=check): + self.assertTrue(any(command.startswith(check) for command in steps)) + + def test_steps_keep_ci_order(self): + import pr_checks + + workflow = (REPO_ROOT / ".github" / "workflows" / "validate.yml").read_text(encoding="utf-8") + order = [workflow.index(key) for key in ( + "refresh_data_dirs.py", "run: python scripts/generate_site.py", + "npm run build --prefix website", "python -m unittest discover", + "validate_site.py")] + self.assertEqual(order, sorted(order)) + commands = [step.command for step in pr_checks.STEPS] + positions = [next(i for i, c in enumerate(commands) if key in c) for key in ( + "refresh_data_dirs.py", "generate_site.py", "npm run build", + "unittest discover", "validate_site.py")] + self.assertEqual(positions, sorted(positions)) + + def test_the_guide_gives_the_runner_and_what_it_runs(self): + import pr_checks + + text = generate_readme.generate_contributing() + section = text.split("## Before opening a PR", 1)[1].split("\n## ", 1)[0] + self.assertIn("python scripts/pr_checks.py\n", section) + for step in pr_checks.STEPS: + with self.subTest(step=step.command): + self.assertIn(step.command, section) + + def test_a_missing_tool_is_reported_and_a_failure_does_not_stop_the_run(self): + import contextlib + import io + + import pr_checks + + steps = ( + pr_checks.Step("python -c raise(SystemExit(3))", "fails"), + pr_checks.Step("no-such-tool-here --x", "absent", needs="no-such-tool-here", + provider="nothing"), + pr_checks.Step("python -c pass", "passes"), + ) + out = io.StringIO() + with contextlib.redirect_stdout(out): + code = pr_checks.run(steps) + self.assertEqual(code, 1) + self.assertIn("FAILED python -c raise(SystemExit(3)) (exit 3)", out.getvalue()) + self.assertIn("NOT RUN no-such-tool-here --x", out.getvalue()) + self.assertIn("1/3 checks passed", out.getvalue()) diff --git a/wiki/tools.md b/wiki/tools.md index 608f60318..a8ad76c26 100644 --- a/wiki/tools.md +++ b/wiki/tools.md @@ -1353,6 +1353,7 @@ before a release (wiki/release-process.md). Its lines stay in | `slots.py` | Destinations the platform and a core both claim, and how the pack settles each (`--platform`, `--target`, `--json`, `--strict`, and pipeline step 3b passes the run's own `--target`) | | `region_audit.py` | Confront every profile `region:` with the regions No-Intro names for the same dump (`--json`, `--db`, `--emulators-dir`). An opinion, never an authority: the code decides, and a disagreement is a line to read in the emulator's source | | `validate_pr.py` | Validate BIOS files in pull requests, post markdown report | +| `pr_checks.py` | Run the checks `validate.yml` runs on a pull request, in its order, past a failure as CI does, and name each check that failed or whose tool is absent (`--list` prints them) | | `validate_schemas.py` | Validate the data contracts: schemas and semantic invariants. `--source-only` checks `emulators/` and `platforms/` alone, which is what PR validation runs | | `generate_event_schema.py` | Write `schemas/install-events.schema.json` from `EVENT_KEYS` in `install.py` (`--check` exits 1 when the file is stale, which `validate_schemas.py` also reports) | | `generate_installer_doc.py` | Rewrite the generated blocks of `wiki/installer.md` from `install.py`: the tables (options, events, exit codes, keys per route, card markers), the line menu, and the run samples `install.py` prints in a sandbox against the committed `install/*.json`, so a new list rewrites the page (`--check` exits 1 when a block is stale, `--page FILE` writes another copy of the page). Pipeline step 7 |