diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 4e010a75..efacfa22 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -17,6 +17,17 @@ on: - "install.py" - "install.sh" - "install.ps1" + # Every input of the site build, so a page that would fail the deploy + # fails here first (tests/test_workflow_paths.py). + - "wiki/**" + - "docs_assets/**" + - "provenance/**" + - "exports/**" + - "install/**" + - "database.json" + - "release.json" + - "derivable.json" + - "mkdocs.yml" # Spelled out twice because the workflow parser reads no YAML anchor. A test # holds the two lists equal. push: @@ -31,6 +42,15 @@ on: - "install.py" - "install.sh" - "install.ps1" + - "wiki/**" + - "docs_assets/**" + - "provenance/**" + - "exports/**" + - "install/**" + - "database.json" + - "release.json" + - "derivable.json" + - "mkdocs.yml" permissions: contents: read @@ -190,9 +210,10 @@ jobs: 3.12 # markdown: the site validation test renders a wiki line to prove the - # heading guard's premise, and skipped on every run without it. + # heading guard's premise, and skipped on every run without it. The docs + # group builds the site as deploy-site.yml does, with the same bounds. - name: Install dependencies - run: pip install pyyaml "jsonschema[format-nongpl]==4.26.0" "markdown==3.10.2" + run: pip install pyyaml "jsonschema[format-nongpl]==4.26.0" "markdown==3.10.2" "mkdocs-material>=9.7.5,<10" "pymdown-extensions>=10.14" # The exporter fidelity cases compare what we hand back against the # file each platform publishes, so they need those files. They skip @@ -218,6 +239,17 @@ jobs: RETROBIOS_OFFLINE: "1" run: python -m unittest discover -s tests -t . -v + # deploy-site.yml runs this after the merge, and a dead link or a stray + # heading failed it there with nothing in front of it. Run whatever the + # suite returned, so one red step does not hide the other. The release + # assets are not restored: they change counts, not links or headings. + - name: Build the site + if: ${{ !cancelled() }} + run: | + python scripts/generate_site.py + mkdocs build --strict + python scripts/validate_site.py + label-pr: runs-on: ubuntu-latest if: github.event_name == 'pull_request' diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index 0e8b6f28..b6f309b5 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -1893,6 +1893,23 @@ class CheckoutCompletenessRegressions(unittest.TestCase): os.chdir(cwd) +def _git_paths(*args: str, stdin: list[str] | None = None) -> set[str] | None: + """The NUL-separated paths a git command prints, or None outside a + checkout. check-ignore exits 1 when it matches nothing, an answer.""" + try: + result = subprocess.run( + ["git", *args], + input="".join(f"{p}\0" for p in stdin or []), + capture_output=True, text=True, cwd=ROOT, check=False, + ) + except FileNotFoundError: + return None + answered = (0, 1) if args[0] == "check-ignore" else (0,) + if result.returncode not in answered: + return None + return {p for p in result.stdout.split("\0") if p} + + class EveryManifestEntryIsFetchable(unittest.TestCase): """An install manifest may not list a file the installer cannot get. @@ -1980,28 +1997,50 @@ class EveryManifestEntryIsFetchable(unittest.TestCase): the large-files release. Marking a committed file as a release asset sends the installer to an asset nobody uploaded (sdlpal/5.avi, 83 MB in git, was announced as release asset `5.avi`). Marking a gitignored - file as a repo file sends it to a raw URL that has no content. + file as a repo file sends it to a raw URL that has no content. A + file neither committed nor ignored is a commit nobody made, and is + named as such rather than as a release asset. """ - tracked = set( - subprocess.run( - ["git", "ls-files", "-z", "--", "bios"], - capture_output=True, text=True, cwd=ROOT, check=True, - ).stdout.split("\0") - ) - if not tracked: + tracked = _git_paths("ls-files", "-z", "--", "bios") + if tracked is None: self.skipTest("not a git checkout") + manifests: dict[str, list[dict]] = {} for path in self._manifests(): - with self.subTest(manifest=path.name): - data = json.loads(path.read_text(encoding="utf-8")) + data = json.loads(path.read_text(encoding="utf-8")) + manifests[path.name] = [ + entry + for entry in data.get("files", []) + data.get("case_variants", []) + if entry.get("repo_path") + ] + outside = sorted( + {e["repo_path"] for group in manifests.values() for e in group} - tracked + ) + ignored = _git_paths("check-ignore", "-z", "--no-index", "--stdin", stdin=outside) + if ignored is None: + self.skipTest("not a git checkout") + for name, group in manifests.items(): + with self.subTest(manifest=name): wrong = [] - for entry in data.get("files", []) + data.get("case_variants", []): - repo_path = entry.get("repo_path") - if not repo_path: - continue - committed = repo_path in tracked - if bool(entry.get("release_asset")) == committed: - wrong.append((entry["dest"], repo_path, "committed" if committed else "gitignored")) - self.assertEqual(wrong, [], f"{path.name} picks the wrong source: {wrong[:3]}") + for entry in group: + repo_path = entry["repo_path"] + state = ( + "committed" if repo_path in tracked + else "gitignored" if repo_path in ignored + else "untracked, never committed" + ) + if bool(entry.get("release_asset")) != (state == "gitignored"): + wrong.append((entry["dest"], repo_path, state)) + self.assertEqual(wrong, [], f"{name} picks the wrong source: {wrong[:3]}") + + def test_no_collected_file_is_left_uncommitted(self): + """157 files of the 17 September campaign sat in bios/ untracked: + the database indexed them, the manifests gave them a repo_path and + the installer got a 404. A file under bios/ is either committed or + listed in .gitignore as a release asset, nothing in between.""" + untracked = _git_paths("ls-files", "-z", "--others", "--exclude-standard", "--", "bios") + if untracked is None: + self.skipTest("not a git checkout") + self.assertEqual(sorted(untracked)[:20], [], f"{len(untracked)} untracked files under bios/") class ReleaseAssetCriterion(unittest.TestCase): diff --git a/tests/test_install.py b/tests/test_install.py index 3ae1f221..d266f555 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -1461,25 +1461,40 @@ class TargetFlagIsAppliedOrRefused(unittest.TestCase): overrides: Batocera x86_64 lacked citron and xenia, which the pack for that target ships, and carried the Amiga machine variants and the launchers the builder drops. + + Both ways: a target the builder knows and the installer lacks answers + "unknown target" on one side only, and a published null left behind + once the builder filters a target makes the installer serve every core. """ import sys as _sys _sys.path.insert(0, str(REPO_ROOT / "scripts")) - from common import load_target_config + from common import list_available_targets, load_target_config - manifests = sorted((REPO_ROOT / "install" / "targets").glob("*.json")) + platforms = str(REPO_ROOT / "platforms") + sources = sorted( + path.stem for path in (REPO_ROOT / "platforms" / "targets").glob("*.yml") + if not path.name.startswith("_") + ) + manifests = { + path.stem: path for path in (REPO_ROOT / "install" / "targets").glob("*.json") + } if not manifests: self.skipTest("no target manifest generated yet") - for path in manifests: - targets = json.loads(path.read_text()) + self.assertEqual(sorted(manifests), sources) + for platform in sources: + targets = json.loads(manifests[platform].read_text()) + known = { + name + for target in list_available_targets(platform, platforms) + for name in (target["name"], *target["aliases"]) + } + with self.subTest(platform=platform): + self.assertEqual(sorted(targets), sorted(known)) for target, cores in targets.items(): - if cores is None: - continue - with self.subTest(platform=path.stem, target=target): - resolved = load_target_config( - path.stem, target, str(REPO_ROOT / "platforms") - ) - self.assertEqual(cores, sorted(resolved)) + with self.subTest(platform=platform, target=target): + resolved = load_target_config(platform, target, platforms) + self.assertEqual(cores, None if resolved is None else sorted(resolved)) class TestBaseUrlScheme(unittest.TestCase): diff --git a/tests/test_prose_style.py b/tests/test_prose_style.py index e59c4d22..322b771c 100644 --- a/tests/test_prose_style.py +++ b/tests/test_prose_style.py @@ -178,10 +178,29 @@ class NoToolingNames(unittest.TestCase): """Nothing committed names the private notes or the agent tooling the maintainer works with. Two test docstrings did, and history keeps them.""" + # The names the repository keeps out of version control through its + # private exclude list, plus the assistant itself. `claude\w*` covers + # claudedocs/. NAMES = re.compile( - r"(?i)\b(claude|anthropic)\b|prompt\.md|\.claude/" + r"(?i)\b(claude\w*|anthropic)\b|\b(prompt|agents|audit)\.md\b" + r"|\.(claude|codex|cursor|continue|copilot|gemini|windsurf|serena|playwright-mcp)/" + r"|\.aider" ) + def test_the_pattern_covers_every_private_name(self): + """The pattern caught CLAUDE.md and missed AGENTS.md, AUDIT.md and + claudedocs/, which the same rule keeps out of the repository.""" + for text in ( + "see CLAUDE.md", "rules live in AGENTS.md", "AUDIT.md angle G", + "notes in claudedocs/launchers", "prompt.md", "under .claude/hooks", + "a .cursor/ rule", "the .serena/ cache", ".aider.conf.yml", + ): + with self.subTest(text=text): + self.assertRegex(text, self.NAMES) + for text in ("When Superpowers Collide", "audit_regressions", "an agent of change"): + with self.subTest(text=text): + self.assertNotRegex(text, self.NAMES) + def test_no_tracked_file_names_them(self): offenders = [] for path in _tracked(): diff --git a/tests/test_workflow_paths.py b/tests/test_workflow_paths.py index c831e372..a2aba810 100644 --- a/tests/test_workflow_paths.py +++ b/tests/test_workflow_paths.py @@ -108,6 +108,45 @@ class DeploySiteTriggers(unittest.TestCase): self.assertIn("scripts/sourceref.py", closure(["generate_site.py"])) +class ThePullRequestBuildsTheSite(unittest.TestCase): + """mkdocs was installed by deploy-site.yml alone, which runs after the + merge, and validate.yml did not watch wiki/: a dead link or a line read as + a second heading passed every check and failed the deploy on main.""" + + @staticmethod + def _workflow(name: str) -> dict: + return yaml.safe_load( + (REPO_ROOT / ".github/workflows" / name).read_text(encoding="utf-8") + ) + + def test_the_test_job_runs_the_deploy_build(self): + steps = self._workflow("validate.yml")["jobs"]["run-tests"]["steps"] + runs = "\n".join(str(step.get("run", "")) for step in steps) + for command in ( + "python scripts/generate_site.py", + "mkdocs build --strict", + "python scripts/validate_site.py", + ): + with self.subTest(command=command): + self.assertIn(command, runs) + for package in ("mkdocs-material", "pymdown-extensions"): + with self.subTest(package=package): + self.assertIn(package, runs) + + def test_every_site_input_triggers_validation(self): + deploy = self._workflow("deploy-site.yml") + validate = self._workflow("validate.yml") + watched = set(validate.get("on", validate.get(True))["push"]["paths"]) + inputs = { + path + for path in deploy.get("on", deploy.get(True))["push"]["paths"] + if not path.startswith(("scripts/", ".github/")) + } + self.assertIn("wiki/**", inputs) + self.assertEqual(sorted(inputs - watched), []) + self.assertIn("scripts/**", watched) + + class DocumentedSteps(unittest.TestCase): def test_the_profiling_reference_names_every_profile_field(self): """built_by, source_branch, bios_mode, standalone_path and diff --git a/wiki/release-process.md b/wiki/release-process.md index 7135962a..fe95fd93 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 | -| Validation | `validate.yml` | PR and push to main touching `bios/**`, `platforms/**`, `emulators/**`, `schemas/**`, `scripts/**`, `tests/**`, `install.py`, `install.sh` or `install.ps1` | +| Validation | `validate.yml` | PR and push to main touching `bios/**`, `platforms/**`, `emulators/**`, `schemas/**`, `scripts/**`, `tests/**`, `install.py`, `install.sh`, `install.ps1`, or any other input of the site build (`wiki/**`, `docs_assets/**`, `provenance/**`, `exports/**`, `install/**`, `database.json`, `release.json`, `derivable.json`, `mkdocs.yml`) | 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 @@ -110,8 +110,10 @@ A profile written from a clone can cite lines that do not hold the value it declares while schemas and tests pass it. This job confronts it with its upstream. An edited profile is not held to it. -**run-tests.** Runs `python -m unittest discover -s tests -t . -v`. Must pass before a -merge, and again on the commit a direct push puts at the head of main. +**run-tests.** Runs `python -m unittest discover -s tests -t . -v`, then builds the +site as Deploy Site does (`generate_site.py`, `mkdocs build --strict`, +`validate_site.py`) whatever the suite returned. Must pass before a merge, and +again on the commit a direct push puts at the head of main. **label-pr** (pull requests only). Auto-labels the PR based on changed paths: diff --git a/wiki/testing-guide.md b/wiki/testing-guide.md index 6f195833..ca9d9b56 100644 --- a/wiki/testing-guide.md +++ b/wiki/testing-guide.md @@ -299,8 +299,10 @@ files it points at gives the installer a 404. The `validate.yml` workflow runs `python -m unittest discover -s tests -t . -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 +`bios/`, `platforms/`, `emulators/`, `schemas/`, `scripts/`, `tests/`, +`install.py` or an input of the site build such as `wiki/`. The same job then +builds the site with `mkdocs build --strict`, so a page that would fail the +deploy fails the pull request. 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