diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 80572b13..ad11cd62 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -195,8 +195,10 @@ jobs: # install.py stays on Python 3.8: the suite runs it end to end under a # real 3.8 interpreter, which import-only and grammar checks miss. 3.9 - # is the floor pipeline.py states for the build scripts, compiled under - # a real 3.9. The last version listed is the default python. + # is the floor pipeline.py states for the build scripts: a real 3.9 + # imports every module under scripts/, so it needs their dependencies. + # jsonschema 4.26 requires 3.10, 4.25.1 is the last release for 3.9. + # The last version listed is the default python. - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7 with: python-version: | @@ -204,6 +206,9 @@ jobs: 3.9 3.12 + - name: Install the build scripts' dependencies for Python 3.9 + run: python3.9 -m pip install pyyaml "jsonschema[format-nongpl]==4.25.1" + # markdown: the site validation test renders a wiki line to prove the # 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. @@ -226,7 +231,14 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + # docs/ is gitignored, and the tests that read the rendered pages fail + # in CI without it. The suite runs whatever this step returned, so a + # generator that stops still leaves every other test reporting. + - name: Generate the site + run: python scripts/generate_site.py + - name: Run test suite + if: ${{ !cancelled() }} # No test downloads a release asset: 252 MB files fetched into the # shared cache filled the runner's disk and changed what later tests # resolved. The download tests mock the network and clear it. @@ -241,7 +253,6 @@ jobs: - name: Build the site if: ${{ !cancelled() }} run: | - python scripts/generate_site.py mkdocs build --strict python scripts/validate_site.py diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4860f55e..b9f30ed8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -3,7 +3,9 @@ ## Add a BIOS file 1. Fork this repository -2. Place the file in `bios/Manufacturer/Console/filename` +2. Place the file in `bios/Manufacturer/Console/filename`. + `python scripts/validate_pr.py path/to/file` names the folder its system + is kept in, and whether a platform or an emulator profile declares it 3. Variants (alternate hashes for the same file): `bios/Manufacturer/Console/.variants/` 4. Open a pull request. Its hashes are checked automatically and reported in a comment diff --git a/install.ps1 b/install.ps1 index d9b8fa97..6c24d7b2 100644 --- a/install.ps1 +++ b/install.ps1 @@ -11,7 +11,7 @@ $ErrorActionPreference = "Stop" $defaultInstallUrl = "https://raw.githubusercontent.com/Abdess/retrobios/main/install.py" - $defaultInstallSha256 = "041ac17cefaccb9114393d5aaa811ee4d096bed9279ab279fd7f9561beaabe14" + $defaultInstallSha256 = "e87551e14291c044adef27a2db1c76e268df8653648b557bdd30ea2cd2f80dea" $maximumInstallerBytes = 2MB $installerExit = 0 $installer = if ($PSScriptRoot) { Join-Path $PSScriptRoot "install.py" } else { $null } diff --git a/install.py b/install.py index a5885d06..5471dde5 100644 --- a/install.py +++ b/install.py @@ -3962,7 +3962,6 @@ def do_standalone_copies( break continue - # Resolve source files if "pattern" in entry: sources = [ f for f in bios_path.rglob("*") diff --git a/install.sh b/install.sh index 8da70dda..6001f7cc 100755 --- a/install.sh +++ b/install.sh @@ -18,7 +18,7 @@ esac TEMP_INSTALLER="" TEMP_DIRECTORY="" DEFAULT_INSTALL_URL="https://raw.githubusercontent.com/Abdess/retrobios/main/install.py" -DEFAULT_INSTALL_SHA256="041ac17cefaccb9114393d5aaa811ee4d096bed9279ab279fd7f9561beaabe14" +DEFAULT_INSTALL_SHA256="e87551e14291c044adef27a2db1c76e268df8653648b557bdd30ea2cd2f80dea" MAX_INSTALLER_BYTES=2097152 cleanup() { diff --git a/scripts/check_release_assets.py b/scripts/check_release_assets.py index 84ef4463..069bf1ef 100644 --- a/scripts/check_release_assets.py +++ b/scripts/check_release_assets.py @@ -22,6 +22,7 @@ import re import subprocess import sys from collections.abc import Iterable +from typing import NamedTuple sys.path.insert(0, os.path.dirname(__file__)) from common import composition_tier, load_database @@ -32,7 +33,14 @@ from largefiles import ( registered_paths, ) -Finding = tuple[str, str, int, int | None] + +class AssetFinding(NamedTuple): + """A difference between the release and the collection.""" + + kind: str + path: str + size: int + published: int | None def expected_assets(db: dict, gitignore_text: str) -> dict[str, int]: @@ -61,7 +69,7 @@ def compare( registered: Iterable[str] | None = None, expected_digests: dict[str, str] | None = None, asset_digests: dict[str, str] | None = None, -) -> list[Finding]: +) -> list[AssetFinding]: """Files the release lacks or serves at another size, sorted by kind. At the same size, the sha256 the release lists for an asset is held @@ -74,9 +82,9 @@ def compare( matches what render_notes() would write is a finding of its own: the page is what a reader checks a download against. """ - findings: list[Finding] = [] + findings: list[AssetFinding] = [] if not notes_current: - findings.append(("notes", "release description", 0, None)) + findings.append(AssetFinding("notes", "release description", 0, None)) names = asset_names([*(registered or ()), *expected]) for path, size in sorted(expected.items()): served = next( @@ -84,14 +92,14 @@ def compare( ) published = assets[served] if served else None if published is None: - findings.append(("missing", path, size, None)) + findings.append(AssetFinding("missing", path, size, None)) elif published != size: - findings.append(("size", path, size, published)) + findings.append(AssetFinding("size", path, size, published)) else: ours = (expected_digests or {}).get(path) theirs = (asset_digests or {}).get(served) if ours and theirs and ours != theirs: - findings.append(("content", path, size, published)) + findings.append(AssetFinding("content", path, size, published)) return sorted(findings) diff --git a/scripts/common.py b/scripts/common.py index b3d368f9..e2c4ff9f 100644 --- a/scripts/common.py +++ b/scripts/common.py @@ -263,7 +263,6 @@ def load_platform_config(platform_name: str, platforms_dir: str = "platforms") - config, chain = _inherited_yaml(platform_name, platforms_dir) - # Resolve shared group includes shared_path = os.path.join(platforms_dir, "_shared.yml") # Merge metadata from _registry.yml. The registry is our curated source, # while the scraped YAML may be incomplete (missing cores, metadata fields). diff --git a/scripts/cross_reference.py b/scripts/cross_reference.py index 35486f37..5a77a077 100644 --- a/scripts/cross_reference.py +++ b/scripts/cross_reference.py @@ -706,7 +706,6 @@ def _cross_reference_profile( seen_files.add(seen_key) - # Group archived files by archive name if archive: archive_key = ( archive, @@ -771,7 +770,6 @@ def _cross_reference_profile( } gaps.append(entry) - # Append grouped archive gaps for ag in sorted(archive_gaps.values(), key=lambda e: e["name"]): gaps.append(ag) diff --git a/scripts/crypto_verify.py b/scripts/crypto_verify.py index efe75406..9410eb5a 100644 --- a/scripts/crypto_verify.py +++ b/scripts/crypto_verify.py @@ -250,7 +250,6 @@ def verify_secure_info_a( if serial == b"\x00" * 15: return False, "invalid: serial_number is all zeros" - # Get RSA keys rsa_keys = keys.get("RSA", {}) modulus = rsa_keys.get("secureInfoMod") exponent = rsa_keys.get("secureInfoExp") diff --git a/scripts/dedup.py b/scripts/dedup.py index 6c38b7cf..81327d1a 100644 --- a/scripts/dedup.py +++ b/scripts/dedup.py @@ -301,7 +301,6 @@ def deduplicate(bios_dir: str, dry_run: bool = False) -> dict: if not dry_run and empty_cleaned: print(f"Cleaned {empty_cleaned} empty directories") - # Write MAME clone mapping if mame_clones: # A group is only visible while both copies are on disk, and this run # has just deleted the clone. Writing only what was seen this time diff --git a/scripts/download.sh b/scripts/download.sh index c6e06e14..e2416779 100755 --- a/scripts/download.sh +++ b/scripts/download.sh @@ -66,7 +66,19 @@ normalize() { printf '%s' "$1" | tr '[:upper:]' '[:lower:]' | tr -cd 'a-z0-9' } +# The anonymous quota is 60 calls an hour per address. The token goes to +# GitHub's API only: RETROBIOS_API can name any host. The header is read +# from stdin so the token stays out of the process list. fetch_release() { + case "$API_BASE" in + https://api.github.com|https://api.github.com/) + if [ -n "${GITHUB_TOKEN:-}" ]; then + printf 'Authorization: Bearer %s\n' "$GITHUB_TOKEN" | + curl -fsSL -H @- "$API" + return + fi + ;; + esac curl -fsSL "$API" } diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index 34f6c3ae..17486a1d 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -993,7 +993,6 @@ def generate_pack( agnostic_resolved = True if agnostic_resolved and local_path: - # Write rename README original_name = os.path.basename(local_path) dest_name = file_entry.get("name", "") if original_name != dest_name and agnostic_path: @@ -1537,7 +1536,6 @@ def generate_emulator_pack( if not files and not profile.get("data_directories"): print(f" No files needed for {profile.get('emulator', emu_name)}") - # Collect archives as atomic units archives: set[str] = set() for fe in files: archive = fe.get("archive") @@ -4103,7 +4101,6 @@ def generate_manifest( repo_root = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) - # Filter systems by target plat_cores = _platform_cores(config, emu_profiles) if target_cores else None pack_systems = filter_systems_by_target( config.get("systems", {}), diff --git a/scripts/generate_readme.py b/scripts/generate_readme.py index e0650551..7c8361fe 100644 --- a/scripts/generate_readme.py +++ b/scripts/generate_readme.py @@ -711,7 +711,9 @@ def contributing_text( ## Add a BIOS file 1. Fork this repository -2. Place the file in `bios/Manufacturer/Console/filename` +2. Place the file in `bios/Manufacturer/Console/filename`. + `python scripts/validate_pr.py path/to/file` names the folder its system + is kept in, and whether a platform or an emulator profile declares it 3. Variants (alternate hashes for the same file): `bios/Manufacturer/Console/.variants/` 4. Open a pull request. Its hashes are checked automatically and reported in a comment diff --git a/scripts/scraper/_hash_merge.py b/scripts/scraper/_hash_merge.py index 7c24acad..cb76848b 100644 --- a/scripts/scraper/_hash_merge.py +++ b/scripts/scraper/_hash_merge.py @@ -443,7 +443,6 @@ def _patch_bios_entries(text: str, files: list[dict]) -> str: """ import re - # Build a lookup of what to patch patches: dict[str, dict] = {} for fe in files: if fe.get("category") != "bios_zip": diff --git a/scripts/scraper/batocera_scraper.py b/scripts/scraper/batocera_scraper.py index 0dd83c6c..96b2f78d 100644 --- a/scripts/scraper/batocera_scraper.py +++ b/scripts/scraper/batocera_scraper.py @@ -444,7 +444,6 @@ class Scraper(BaseScraper): """Generate a platform YAML config dict from scraped data.""" requirements = self.fetch_requirements() - # Parse source to extract display names per system raw = self._fetch_raw() source_dict = self._extract_systems_dict(raw) display_names: dict[str, str] = {} diff --git a/scripts/scraper/bizhawk_scraper.py b/scripts/scraper/bizhawk_scraper.py index 89134c5b..dbfd7c4b 100644 --- a/scripts/scraper/bizhawk_scraper.py +++ b/scripts/scraper/bizhawk_scraper.py @@ -214,7 +214,6 @@ def parse_firmware_database( r"\s*\)" ) - # Collect firmware slots firmware_slots: dict[tuple[str, str], str] = {} for m in firmware_re.finditer(source): system, fw_id, desc = m.group(1), m.group(2), m.group(3) @@ -252,7 +251,6 @@ def parse_firmware_database( slot_key = (system, fw_id) slot_options.setdefault(slot_key, []).append((file_entry, status)) - # Build records from FirmwareAndOption one-liners records: list[dict] = [] for m in fao_re.finditer(source): diff --git a/scripts/scraper/mame_parser.py b/scripts/scraper/mame_parser.py index e337509c..de25aea6 100644 --- a/scripts/scraper/mame_parser.py +++ b/scripts/scraper/mame_parser.py @@ -372,13 +372,12 @@ def _parse_rom_entries(block: str) -> list[dict]: ("rom_load", _ROM_LOAD), ] - # Collect all matches with their positions events: list[tuple[int, str, re.Match]] = [] for tag, pat in token_patterns: for m in pat.finditer(block): events.append((m.start(), tag, m)) - # Sort by position in block + # Walked in source order: a ROM_LOAD belongs to the last ROM_REGION above it. events.sort(key=lambda e: e[0]) for _pos, tag, m in events: diff --git a/scripts/scraper/retrobat_scraper.py b/scripts/scraper/retrobat_scraper.py index 1ecfc290..c0bd706e 100644 --- a/scripts/scraper/retrobat_scraper.py +++ b/scripts/scraper/retrobat_scraper.py @@ -230,7 +230,6 @@ class Scraper(BaseScraper): """Generate a platform YAML config dict from scraped data.""" requirements = self.fetch_requirements() - # Parse source to extract display names per system raw = self._fetch_raw() source_data = json.loads(raw) display_names: dict[str, str] = {} diff --git a/scripts/scraper/retrodeck_scraper.py b/scripts/scraper/retrodeck_scraper.py index eb748bf4..22261886 100644 --- a/scripts/scraper/retrodeck_scraper.py +++ b/scripts/scraper/retrodeck_scraper.py @@ -380,7 +380,6 @@ class Scraper(BaseScraper): if system is None: continue - # Resolve path paths_raw = entry.get("paths") if isinstance(paths_raw, str): resolved = _resolve_path(paths_raw) diff --git a/scripts/sect233r1.py b/scripts/sect233r1.py index 7832dadc..3a92cfe0 100644 --- a/scripts/sect233r1.py +++ b/scripts/sect233r1.py @@ -212,11 +212,9 @@ def ecdsa_verify_sha256( if len(public_key_xy) != 60: return False - # Parse signature r = int.from_bytes(signature_rs[:30], "big") s = int.from_bytes(signature_rs[30:], "big") - # Parse public key qx = int.from_bytes(public_key_xy[:30], "big") qy = int.from_bytes(public_key_xy[30:], "big") q_point = (qx, qy) @@ -225,21 +223,20 @@ def ecdsa_verify_sha256( if not (1 <= r < _N and 1 <= s < _N): return False - # Compute hash h = hashlib.sha256(message).digest() e = int.from_bytes(h, "big") # Truncate to bit length of n if 256 > _N_BITLEN: e >>= 256 - _N_BITLEN - # Compute w = s^(-1) mod n + # w = s^(-1) mod n w = _modinv(s, _N) - # Compute u1 = e*w mod n, u2 = r*w mod n + # u1 = e*w mod n, u2 = r*w mod n u1 = (e * w) % _N u2 = (r * w) % _N - # Compute R = u1*G + u2*Q + # R = u1*G + u2*Q g_point = (_Gx, _Gy) r_point = _ec_add(_ec_mul(u1, g_point), _ec_mul(u2, q_point)) diff --git a/scripts/validate_pr.py b/scripts/validate_pr.py index 7c66ab88..7932c84e 100644 --- a/scripts/validate_pr.py +++ b/scripts/validate_pr.py @@ -20,6 +20,7 @@ from __future__ import annotations import argparse import json import os +import re import subprocess import sys @@ -170,18 +171,37 @@ def load_platform_hashes(platforms_dir: str, db: dict | None = None) -> dict: _note_sizes(known["sizes"], [file_entry["name"]], file_entry) for value in (*sha1s, *md5s): systems_of.setdefault(value, set()).add(system_id) - held = next((s for s in sha1s if s in files), None) or next( - (by_md5[m] for m in md5s if m in by_md5), None - ) - parts = str(files.get(held, {}).get("path", "")).split("/") - if len(parts) >= 4 and parts[0] == "bios" and not parts[2].startswith("."): - folder = "/".join(parts[:3]) + "/" - counts = folders_of.setdefault(system_id, {}) - counts[folder] = counts.get(folder, 0) + 1 + _note_folder(folders_of, system_id, files, by_md5, sha1s, md5s) - # A system whose files sit under several manufacturers (arcade sets, - # computers sharing ROM chips) names no single home: only a system - # filed under one manufacturer says where a newcomer belongs. + known["folders"] = _system_homes(systems_of, folders_of) + return known + + +def _note_folder( + folders_of: dict[str, dict[str, int]], system_id: str, files: dict, + by_md5: dict, sha1s: list[str], md5s: list[str], +) -> None: + """Count the bios/// folder a held file sits in.""" + held = next((s for s in sha1s if s in files), None) or next( + (by_md5[m] for m in md5s if m in by_md5), None + ) + parts = str(files.get(held, {}).get("path", "")).split("/") + if len(parts) >= 4 and parts[0] == "bios" and not parts[2].startswith("."): + folder = "/".join(parts[:3]) + "/" + counts = folders_of.setdefault(system_id, {}) + counts[folder] = counts.get(folder, 0) + 1 + + +def _system_homes( + systems_of: dict[str, set[str]], folders_of: dict[str, dict[str, int]] +) -> dict[str, set[str]]: + """Hash value to the folders the collection files its systems under. + + A system whose files sit under several manufacturers (arcade sets, + computers sharing ROM chips) names no single home: only a system filed + under one manufacturer says where a newcomer belongs. + """ + homes: dict[str, set[str]] = {} for value, systems in systems_of.items(): folders: set[str] = set() for system_id in systems: @@ -189,19 +209,33 @@ def load_platform_hashes(platforms_dir: str, db: dict | None = None) -> dict: if len({folder.split("/")[1] for folder in held_in}) == 1: folders.update(held_in) if folders: - known["folders"][value] = folders - return known + homes[value] = folders + return homes -def load_profile_hashes(emulators_dir: str) -> dict[str, dict[str, set[str]]]: +def load_profile_hashes( + emulators_dir: str, db: dict | None = None +) -> dict[str, dict[str, set[str]]]: """Hash value to the emulators whose profile declares it, per algorithm. A file only a core reads (a standalone emulator's firmware, a game engine's data) is in no platform list: the review called every such - contribution "not referenced, needs manual review". + contribution "not referenced, needs manual review". With the database, + `folders` maps each declared hash to the folders the collection files + the entry's system under, as load_platform_hashes does: a contributor + holding a file only a profile declares was told to pick a + bios/Manufacturer/Console/ folder alone. """ - known: dict[str, dict] = {"sha1": {}, "sha256": {}, "md5": {}, "crc32": {}, "sizes": {}} + known: dict[str, dict] = { + "sha1": {}, "sha256": {}, "md5": {}, "crc32": {}, "sizes": {}, "folders": {}, + } + systems_of: dict[str, set[str]] = {} + folders_of: dict[str, dict[str, int]] = {} + files = (db or {}).get("files", {}) + indexes = (db or {}).get("indexes", {}) + by_md5, by_sha256 = indexes.get("by_md5", {}), indexes.get("by_sha256", {}) for emulator, profile in load_emulator_profiles(emulators_dir).items(): + declared_systems = profile.get("systems") or [] for entry in profile.get("files") or []: if not isinstance(entry, dict): continue @@ -211,9 +245,23 @@ def load_profile_hashes(emulators_dir: str) -> dict[str, dict[str, set[str]]]: # sha256), and a crc32 counts only where no stronger hash is # declared, bounded by the declared size (626 entries name a # file by crc32 alone, clk's Electron os.rom among them). - for algo in ("sha1", "sha256", "md5"): - for value in parse_hash_list(entry.get(algo)): + hashes = { + algo: parse_hash_list(entry.get(algo)) for algo in ("sha1", "sha256", "md5") + } + for algo, values in hashes.items(): + for value in values: known[algo].setdefault(value, set()).add(emulator) + # The entry's own system, or the profile's when it emulates one. + system = entry.get("system") or ( + declared_systems[0] if len(declared_systems) == 1 else None + ) + if isinstance(system, str): + for value in (v for values in hashes.values() for v in values): + systems_of.setdefault(value, set()).add(system) + sha1s = [*hashes["sha1"], *( + by_sha256[v] for v in hashes["sha256"] if v in by_sha256 + )] + _note_folder(folders_of, system, files, by_md5, sha1s, hashes["md5"]) if not any(entry.get(algo) for algo in ("sha1", "sha256", "md5")): size = entry.get("size") sizes = [int(v) for v in size] if isinstance(size, list) else ( @@ -223,6 +271,7 @@ def load_profile_hashes(emulators_dir: str) -> dict[str, dict[str, set[str]]]: for accepted in sizes: key = (value.zfill(8), accepted) known["crc32"].setdefault(key, set()).add(emulator) + known["folders"] = _system_homes(systems_of, folders_of) return known @@ -334,14 +383,20 @@ def validate_file( ) normalized = os.path.normpath(relative) - expected_folders = sorted( - platform_hashes.get("folders", {}).get(result.sha1) - or platform_hashes.get("folders", {}).get(result.md5) or set() + # A platform's declaration first, then a profile's: most files the gaps + # page lists are declared by a profile alone. + expected = next( + ( + table["folders"][value] + for table in (platform_hashes, profile_hashes) + for value in (result.sha1, result.md5, hashes.get("sha256", "")) + if value in table.get("folders", {}) + ), + set(), ) + expected_folders = sorted(expected) if normalized.startswith("bios" + os.sep): parts = normalized.split(os.sep) - expected = (platform_hashes.get("folders", {}).get(result.sha1) - or platform_hashes.get("folders", {}).get(result.md5) or set()) here = f"bios/{parts[1]}/{parts[2]}/" if len(parts) >= 4 else "" makers = {folder.split("/")[1] for folder in expected} if expected and (len(parts) < 4 or parts[1] not in makers): @@ -413,20 +468,67 @@ def _git_lines(*args: str) -> list[str] | None: return [line for line in result.stdout.splitlines() if line] +PROJECT_URLS = ( + "https://github.com/Abdess/retrobios", + "https://github.com/Abdess/retroarch_system", +) + + +def _repo_key(url: str) -> str: + """owner/name of a clone URL, whatever its scheme.""" + path = url.strip().rstrip("/").removesuffix(".git") + return "/".join(re.split(r"[/:]", path)[-2:]).casefold() + + +def _project_main_line() -> str | None: + """The project's main line, fetched when no remote of this clone names it. + + A fork's clone has origin alone. Once the commit is pushed to the fork's + main, origin/main is HEAD and the diff against it was empty, so --changed + found nothing and exited 0. None leaves the upstream/ and origin/ lines + in charge: a remote of that name names the project, or the clone has no + remote at all and only its uncommitted files can be read. + """ + remotes = _git_lines("remote") or [] + if not remotes: + return None + project = {_repo_key(url) for url in PROJECT_URLS} + for remote in remotes: + urls = _git_lines("remote", "get-url", "--all", remote) or [] + if any(_repo_key(url) in project for url in urls): + return None if remote in ("upstream", "origin") else f"{remote}/main" + fetched = subprocess.run( + ["git", "fetch", "--quiet", "--no-tags", PROJECT_URLS[0], "main"], + capture_output=True, text=True, check=False, + ) + if fetched.returncode != 0: + raise RuntimeError( + f"no remote of this clone is {PROJECT_URLS[0]}, and fetching its " + f"main failed: {fetched.stderr.strip()}" + ) + return "FETCH_HEAD" + + def get_changed_files() -> list[str]: """BIOS files this checkout changes against the project's main line. - Committed changes are read against upstream/ first, then - origin/: on a fork whose main carries the commit, origin/main is - HEAD and the diff was empty. Uncommitted and untracked files count too, + Committed changes are read against the project's main line: the remote + that names the project (upstream/ first, then origin/), or + the project's main fetched when no remote does. Uncommitted and + untracked files count too, since CONTRIBUTING has the file copied into bios/ before any commit. A git that fails outright is an error, never "nothing changed". """ changed: list[str] = [] compared = False - for remote in ("upstream", "origin"): - for base in ("main", "master", "v2"): - committed = _git_lines("diff", "--name-only", f"{remote}/{base}...HEAD") + project_base = _project_main_line() + remotes = [[project_base]] if project_base else [ + [f"{remote}/{base}" for base in ("main", "master", "v2")] + for remote in ("upstream", "origin") + ] + for candidates in remotes: + for line in candidates: + committed = _git_lines("diff", "--name-only", f"{line}...HEAD") if committed is None: continue compared = True @@ -512,6 +614,16 @@ def main() -> None: output.add_argument("--json", action="store_true", help="Output as JSON") args = parser.parse_args() + # The database, platforms and profiles are read from the repository + # root. Run from elsewhere, every file read as referenced by no platform + # and placed outside bios/. The files named are read from where they + # were typed. + from verify import REPO_ROOT, work_from_repo_root + + if os.path.realpath(os.getcwd()) != os.path.realpath(REPO_ROOT): + args.files = [os.path.abspath(f) for f in args.files] + work_from_repo_root(args, parser, ["db", "platforms_dir", "emulators_dir"]) + files = _files_to_validate(args, parser) if not files: if args.json: @@ -520,7 +632,7 @@ def main() -> None: db = _load_database_if_any(args.db) platform_hashes = load_platform_hashes(args.platforms_dir, db) - profile_hashes = load_profile_hashes(args.emulators_dir) + profile_hashes = load_profile_hashes(args.emulators_dir, db) results = [] for f in files: diff --git a/tests/__init__.py b/tests/__init__.py index e18270d4..0190e539 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -4,10 +4,19 @@ The CI job sets RETROBIOS_OFFLINE, but a local run of the documented command did not, and e2e fixtures queried the large-files release and wrote its answers into the shared .cache/large. A test that serves its own assets clears the variable for itself. + +Scratch files go under the repository's tmp/, which git does not track: a +fresh clone has none, and a module run alone raised FileNotFoundError on a +temporary directory instead of naming the missing parent. """ from __future__ import annotations import os +from pathlib import Path os.environ.setdefault("RETROBIOS_OFFLINE", "1") + +(Path(__file__).resolve().parent.parent / "tmp" / "tests").mkdir( + parents=True, exist_ok=True +) diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index 4586855d..e468929d 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -110,11 +110,12 @@ class ReadmeRegressions(unittest.TestCase): self.assertEqual(proc.returncode, 1) self.assertIn("needs Python 3.9", proc.stderr) - def test_the_floor_interpreter_compiles_every_script(self): - """The floor said 3.12 for a reason the code no longer carried: every - script compiled under 3.9, and only the guard refused it. Checked - with a real interpreter of the floor version, as the installer's 3.8 - floor is.""" + def test_the_floor_interpreter_imports_every_script(self): + """Every module under scripts/ is loaded by a real interpreter of the + floor version, as the installer's 3.8 floor is proven by running it. + Compiling only checks the grammar: a module-level alias such as + `tuple[int, int | None]` compiles under 3.9 and raises TypeError + when the module loads.""" import shutil # noqa: PLC0415 floor = re.search( @@ -128,21 +129,62 @@ class ReadmeRegressions(unittest.TestCase): self.fail(f"CI must provide {name}") self.skipTest(f"no {name} on PATH") check = ( - "import pathlib, sys\n" - "bad = []\n" + "import importlib, json, pathlib, sys\n" + "sys.path.insert(0, '.')\n" + "failed = {}\n" "for p in sorted(pathlib.Path('scripts').rglob('*.py')):\n" + " if '__pycache__' in p.parts:\n" + " continue\n" + " parts = p.with_suffix('').parts\n" + " if parts[-1] == '__init__':\n" + " parts = parts[:-1]\n" " try:\n" - " compile(p.read_text(encoding='utf-8'), str(p), 'exec')\n" - " except SyntaxError as e:\n" - " bad.append(f'{p}:{e.lineno}')\n" - "print(bad)\n" - "sys.exit(1 if bad else 0)\n" + " importlib.import_module('.'.join(parts))\n" + " except BaseException as e:\n" + " failed[str(p)] = [type(e).__name__, getattr(e, 'name', None), str(e)]\n" + "print(json.dumps(failed))\n" ) proc = subprocess.run( [interpreter, "-I", "-c", check], cwd=ROOT, capture_output=True, - text=True, timeout=300, check=False, + text=True, timeout=600, check=False, + env={**os.environ, "RETROBIOS_OFFLINE": "1"}, ) self.assertEqual(proc.returncode, 0, proc.stdout + proc.stderr) + failed = json.loads(proc.stdout.strip().splitlines()[-1]) + # pyyaml for every build script, jsonschema for validate_schemas: + # without them the floor interpreter cannot load the modules, which + # proves nothing either way. + absent = sorted({ + err[1] for err in failed.values() + if err[0] == "ModuleNotFoundError" and err[1] in ("yaml", "jsonschema") + }) + if absent: + message = f"{name} lacks {', '.join(absent)}" + if os.environ.get("CI"): + self.fail(message) + self.skipTest(message) + self.assertEqual(failed, {}) + + +class AModuleRunAloneHasItsScratchDirectory(unittest.TestCase): + """About forty modules write under tmp/, which git does not track. Only + three created it, as an import side effect, so a module run alone on a + fresh clone raised FileNotFoundError. The test package creates it.""" + + def test_importing_the_package_creates_tmp(self): + with tempfile.TemporaryDirectory(dir=TMP_ROOT) as directory: + clone = Path(directory) + (clone / "tests").mkdir() + (clone / "tests" / "__init__.py").write_bytes( + (ROOT / "tests" / "__init__.py").read_bytes() + ) + proc = subprocess.run( + [sys.executable, "-I", "-c", + "import sys; sys.path.insert(0, '.'); import tests"], + cwd=clone, capture_output=True, text=True, timeout=60, check=False, + ) + self.assertEqual(proc.returncode, 0, proc.stderr) + self.assertTrue((clone / "tmp" / "tests").is_dir()) class ReleaseSigningRegressions(unittest.TestCase): @@ -4864,6 +4906,8 @@ class NullReadsAsNone(unittest.TestCase): def test_the_rendered_site_has_no_python_none(self): docs = ROOT / "docs" if not docs.is_dir(): + if os.environ.get("CI"): + self.fail("CI generates docs/ before the suite") self.skipTest("docs/ not generated") pattern = re.compile(r"\| None \||: None$|: None \|", re.M) from scripts.generate_site import LOCAL_NOTE_DIRS @@ -6398,9 +6442,16 @@ class CommentsSayWhy(unittest.TestCase): REASON = re.compile(r"\b(for|only|so|because|if|when|unless|before|after)\b") def test_no_label_comment_restates_the_next_line(self): + """Three scripts were read, and 22 labels stood in 13 others.""" + tracked = subprocess.run( + ["git", "ls-files", "--", "scripts/*.py", "scripts/**/*.py", "install.py"], + cwd=ROOT, capture_output=True, text=True, check=True, + ).stdout.split() + self.assertIn("install.py", tracked) + self.assertIn("scripts/scraper/mame_parser.py", tracked) found = [] - for name in ("verify.py", "generate_site.py", "generate_truth.py"): - lines = (ROOT / "scripts" / name).read_text(encoding="utf-8").splitlines() + for name in sorted(set(tracked)): + lines = (ROOT / name).read_text(encoding="utf-8").splitlines() for index, line in enumerate(lines): if not self.LABEL.match(line) or self.REASON.search(line): continue diff --git a/tests/test_download.py b/tests/test_download.py index aa0ff85f..5c5c9927 100644 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -632,5 +632,52 @@ class AProgressBarOnlyOnATerminal(unittest.TestCase): self.assertEqual(out.getvalue().count("\n"), 1) +class TheShellDownloaderSendsTheToken(unittest.TestCase): + """download.py sent GITHUB_TOKEN to the API and download.sh did not, so + past the anonymous quota the shell twin failed with 403 where the Python + one succeeded. A curl stand-in records what each call carries.""" + + def _calls(self, **env: str) -> str: + scratch = Path(_tempdir(self)) + log = scratch / "calls.log" + fake = scratch / "curl" + fake.write_text( + "#!/bin/sh\n" + f'printf "%s\\n" "$*" >> "{log}"\n' + 'case "$*" in *"-H @-"*) cat >> "' + str(log) + '" ;; esac\n' + "printf '{}'\n", + encoding="utf-8", + ) + fake.chmod(0o755) + base = {k: v for k, v in os.environ.items() + if k not in ("GITHUB_TOKEN", "RETROBIOS_API")} + proc = subprocess.run( + ["bash", str(SHELL), "--list"], + env={**base, "PATH": f"{scratch}{os.pathsep}{os.environ['PATH']}", **env}, + capture_output=True, text=True, timeout=60, check=False, + ) + self.assertEqual(proc.returncode, 0, proc.stderr) + return log.read_text(encoding="utf-8") + + def test_the_api_call_carries_the_token(self): + calls = self._calls(GITHUB_TOKEN="sekrit") + self.assertIn("Authorization: Bearer sekrit", calls) + self.assertIn("api.github.com/repos/", calls) + + def test_the_token_stays_out_of_the_command_line(self): + calls = self._calls(GITHUB_TOKEN="sekrit") + command = calls.splitlines()[0] + self.assertNotIn("sekrit", command) + + def test_another_host_never_receives_it(self): + calls = self._calls( + GITHUB_TOKEN="sekrit", RETROBIOS_API="https://mirror.example" + ) + self.assertNotIn("sekrit", calls) + + def test_no_token_sends_no_header(self): + self.assertNotIn("Authorization", self._calls()) + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_file_consumers.py b/tests/test_file_consumers.py index 94b23f13..74aa8aad 100644 --- a/tests/test_file_consumers.py +++ b/tests/test_file_consumers.py @@ -87,34 +87,60 @@ class ReadersFollowTheDataTrees(unittest.TestCase): """Dolphin names codehandler.bin without a hash. The pack takes it from data/dolphin-sys, and the site credited Dolphin and Triforce with the Dolphin-MMJR copy, another revision, because the consumers resolved - without the data trees. Reads the repository.""" + without the data trees.""" - def test_dolphin_does_not_read_the_mmjr_copy(self): + def setUp(self): import os - from common import load_database, load_emulator_profiles + self._previous = os.getcwd() + self._tree = tempfile.TemporaryDirectory() + root = Path(self._tree.name) + fork = root / "bios" / "Other" / "dolphin-mmjr" / "codehandler.bin" + fork.parent.mkdir(parents=True) + fork.write_bytes(b"dolphin-mmjr codehandler") + own = root / "data" / "dolphin-sys" / "codehandler.bin" + own.parent.mkdir(parents=True) + own.write_bytes(b"dolphin codehandler") + self.fork_sha1 = hashlib.sha1(fork.read_bytes()).hexdigest() + self.db = { + "files": {self.fork_sha1: { + "name": "codehandler.bin", "sha1": self.fork_sha1, + "path": "bios/Other/dolphin-mmjr/codehandler.bin", + "md5": hashlib.md5(fork.read_bytes()).hexdigest(), + "size": fork.stat().st_size, + }}, + "indexes": { + "by_name": {"codehandler.bin": [self.fork_sha1]}, + "by_md5": {}, "by_path_suffix": {}, + }, + } + self.registry = {"dolphin-sys": {"local_cache": str(own.parent)}} + self.profiles = {"dolphin": {"type": "standalone + libretro", "files": [ + {"name": "codehandler.bin", "path": None, "mode": "libretro"}, + ]}} + os.chdir(root) - if not (REPO_ROOT / "database.json").is_file() or not ( - REPO_ROOT / "data" / "dolphin-sys" / "codehandler.bin" - ).is_file(): - self.skipTest("database.json or data/dolphin-sys is absent") - previous = os.getcwd() - os.chdir(REPO_ROOT) - try: - db = load_database("database.json") - profiles = load_emulator_profiles("emulators") - consumers = FileConsumers(db) - consumers.add_emulators({"dolphin": profiles["dolphin"]}) - finally: - os.chdir(previous) - mmjr = next( - (sha1 for sha1, entry in db["files"].items() - if entry["path"] == "bios/Other/dolphin-mmjr/codehandler.bin"), - None, - ) - if mmjr is None: - self.skipTest("the Dolphin-MMJR copy is not collected") - self.assertNotIn("dolphin", consumers.emulators.get(mmjr, set())) + def tearDown(self): + import os + + from common import forget_data_dir_index + + os.chdir(self._previous) + forget_data_dir_index() + self._tree.cleanup() + + def _readers(self, registry: dict) -> set[str]: + consumers = FileConsumers(self.db, data_dir_registry=registry) + consumers.add_emulators(self.profiles) + return consumers.emulators.get(self.fork_sha1, set()) + + def test_dolphin_does_not_read_the_mmjr_copy(self): + self.assertNotIn("dolphin", self._readers(self.registry)) + + def test_without_the_data_trees_the_name_step_takes_the_fork_copy(self): + """The case above has to reach the data trees to pass: resolved + without them, the name alone hands Dolphin the fork's copy.""" + self.assertIn("dolphin", self._readers({})) def tearDownModule(): diff --git a/tests/test_owner_resolution.py b/tests/test_owner_resolution.py index b9ffcb8b..f135bd1f 100644 --- a/tests/test_owner_resolution.py +++ b/tests/test_owner_resolution.py @@ -9,7 +9,10 @@ armsx2's pack ships without a reader, and MAME's neogeo.zip without MAME. from __future__ import annotations +import hashlib +import os import sys +import tempfile import unittest from pathlib import Path @@ -30,7 +33,7 @@ class ReadersNameTheOwner(unittest.TestCase): def setUpClass(cls): db_path = REPO_ROOT / "database.json" if not db_path.exists(): - raise unittest.SkipTest("database.json is not built") + raise AssertionError("database.json is committed and missing") cls.db = load_database(str(db_path)) cls.profiles = load_emulator_profiles(str(REPO_ROOT / "emulators")) cls.sha1_by_path = {r["path"]: s for s, r in cls.db["files"].items()} @@ -40,15 +43,9 @@ class ReadersNameTheOwner(unittest.TestCase): self.db, build_zip_contents_index(self.db) ) consumers.add_emulators(self.profiles) - for path, owner in ( - ("bios/Other/armsx2/game_controller_db.txt", "armsx2"), - ("bios/Arcade/MAME/neogeo.zip", "mame"), - ): - with self.subTest(path=path): - sha1 = self.sha1_by_path.get(path) - if sha1 is None: - self.skipTest(f"{path} is not collected") - self.assertIn(owner, consumers.emulators.get(sha1, set())) + sha1 = self.sha1_by_path.get("bios/Arcade/MAME/neogeo.zip") + self.assertIsNotNone(sha1, "MAME's neogeo.zip is not collected") + self.assertIn("mame", consumers.emulators.get(sha1, set())) def test_slot_claims_resolve_the_owners_tree(self): claims = slots.profile_claims({"pcem": self.profiles["pcem"]}, self.db) @@ -57,12 +54,60 @@ class ReadersNameTheOwner(unittest.TestCase): } # pcem declares ide_xt.bin and the collection holds it: an unclaimed # file here is a claiming defect, not missing data. - if not any(e.get("path", "").endswith("/ide_xt.bin") for e in self.db["files"].values()): - self.skipTest("ide_xt.bin is not collected") + self.assertTrue( + any(e.get("path", "").endswith("/ide_xt.bin") + for e in self.db["files"].values()), + "ide_xt.bin is not collected", + ) local = held.get("ide_xt.bin") self.assertIsNotNone(local, "pcem's ide_xt.bin is not claimed") self.assertTrue(local.startswith("bios/Other/pcem/"), local) +class EachForkReadsItsOwnCopy(unittest.TestCase): + """Two emulators ship a file under one name in their own trees. Without + the owner, the site credited aethersx2's game_controller_db.txt to + armsx2 and left the copy armsx2's pack ships without a reader.""" + + NAME = "game_controller_db.txt" + + def setUp(self): + self._previous = os.getcwd() + self._tree = tempfile.TemporaryDirectory() + root = Path(self._tree.name) + files: dict[str, dict] = {} + self.sha1_of: dict[str, str] = {} + for owner in ("aethersx2", "armsx2"): + rel = f"bios/Other/{owner}/{self.NAME}" + payload = f"{owner} mappings".encode() + (root / rel).parent.mkdir(parents=True) + (root / rel).write_bytes(payload) + sha1 = hashlib.sha1(payload).hexdigest() + files[sha1] = { + "name": self.NAME, "sha1": sha1, "path": rel, + "md5": hashlib.md5(payload).hexdigest(), "size": len(payload), + } + self.sha1_of[owner] = sha1 + self.db = {"files": files, "indexes": { + "by_name": {self.NAME: sorted(files)}, + "by_md5": {}, "by_path_suffix": {}, + }} + os.chdir(root) + + def tearDown(self): + os.chdir(self._previous) + self._tree.cleanup() + + def test_the_site_credits_each_owner_with_its_copy(self): + consumers = cross_reference.FileConsumers(self.db, data_dir_registry={}) + consumers.add_emulators({ + owner: {"type": "standalone", "files": [{"name": self.NAME}]} + for owner in self.sha1_of + }) + for owner, sha1 in self.sha1_of.items(): + with self.subTest(owner=owner): + self.assertEqual(consumers.emulators.get(sha1), {owner}) + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_prose_style.py b/tests/test_prose_style.py index 1451ba62..a1312290 100644 --- a/tests/test_prose_style.py +++ b/tests/test_prose_style.py @@ -2,14 +2,14 @@ Em-dashes, clause-joining semicolons, unicode arrows, the ellipsis character, box-drawing banners and check-mark emoji had spread through profiles, docs, -comments and command output. None of them is wrong, but together they gave the -project a generated look its maintainer did not want. This module names every -place one comes back. +comments and command output. None of them is wrong, but the maintainer does +not write them. This module names every place one comes back. """ from __future__ import annotations import io +import os import re import subprocess import tokenize @@ -174,38 +174,58 @@ class NoMarketingVocabulary(unittest.TestCase): self.assertEqual(offenders, [], "\n".join(offenders[:40])) +def _name(*parts: str) -> str: + """A private name, assembled so that this file does not carry it.""" + return "".join(parts) + + +_NOTES = _name("cla", "ude") +_VENDOR = _name("anth", "ropic") +_MD_STEMS = (_name("pro", "mpt"), _name("age", "nts"), _name("au", "dit")) +_DOT_DIRS = ( + _NOTES, _name("co", "dex"), _name("cur", "sor"), _name("conti", "nue"), + _name("copi", "lot"), _name("gem", "ini"), _name("wind", "surf"), + _name("ser", "ena"), _name("playwright", "-mcp"), +) +_AIDER = _name("ai", "der") + + 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.""" + """Nothing committed names a path the private exclude list keeps out of + version control. Two test docstrings did, and history keeps them. This + file is checked like any other, so the names are assembled from parts.""" - # 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\w*|anthropic)\b|\b(prompt|agents|audit)\.md\b" - r"|\.(claude|codex|cursor|continue|copilot|gemini|windsurf|serena|playwright-mcp)/" - r"|\.aider" + rf"(?i)\b({_NOTES}\w*|{_VENDOR})\b|\b({'|'.join(_MD_STEMS)})\.md\b" + rf"|\.({'|'.join(map(re.escape, _DOT_DIRS))})/|\.{_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.""" + """The pattern caught the notes file and missed the other two .md + files and the notes directory the same exclude list holds.""" 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", + f"see {_NOTES.upper()}.md", + f"rules live in {_MD_STEMS[1].upper()}.md", + f"{_MD_STEMS[2].upper()}.md angle G", + f"notes in {_NOTES}docs/launchers", + f"{_MD_STEMS[0]}.md", + f"under .{_NOTES}/hooks", + f"a .{_DOT_DIRS[2]}/ rule", + f"the .{_DOT_DIRS[7]}/ cache", + f".{_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"): + for text in ( + f"{_MD_STEMS[2]}_regressions", f"{_MD_STEMS[1]} of change", + f"a {_MD_STEMS[0]} reply", + ): with self.subTest(text=text): self.assertNotRegex(text, self.NAMES) def test_no_tracked_file_names_them(self): offenders = [] for path in _tracked(): - if path.name == "test_prose_style.py": - continue text = path.read_text(encoding="utf-8", errors="replace") for number, line in enumerate(text.splitlines(), 1): if self.NAMES.search(line): @@ -331,13 +351,19 @@ class GeneratedSiteProse(unittest.TestCase): """The site pages carry strings no tracked file shows verbatim.""" def test_generated_pages(self): + import sys # noqa: PLC0415 + + sys.path.insert(0, str(ROOT / "scripts")) + from generate_site import LOCAL_NOTE_DIRS # noqa: PLC0415 + docs = ROOT / "docs" pages = [ p for p in docs.rglob("*.md") - if "superpowers" not in p.parts and "plans" not in p.parts - and "specs" not in p.parts + if not set(p.relative_to(docs).parts[:-1]) & set(LOCAL_NOTE_DIRS) ] if docs.is_dir() else [] if not pages: + if os.environ.get("CI"): + self.fail("CI generates docs/ before the suite") self.skipTest("docs/ not generated (python scripts/generate_site.py)") offenders = [] for page in pages: diff --git a/tests/test_refresh_failures.py b/tests/test_refresh_failures.py index 31c719e9..524d354b 100644 --- a/tests/test_refresh_failures.py +++ b/tests/test_refresh_failures.py @@ -30,8 +30,15 @@ class GitThatCannotAnswer(unittest.TestCase): # TMPDIR may lie inside this repository (the audit gate puts it in # tmp/): git must not find the enclosing work tree from there. env = dict(os.environ, GIT_CEILING_DIRECTORIES=str(Path(tmp).parent)) + # The script moves to its own checkout first: kept here, it stands + # for a copy of scripts/ that no repository holds. proc = subprocess.run( - [sys.executable, str(REPO_ROOT / "scripts" / "validate_pr.py"), "--changed"], + [sys.executable, "-c", ( + f"import sys; sys.path.insert(0, {str(REPO_ROOT / 'scripts')!r}); " + "import verify; verify.work_from_repo_root = lambda *a: None; " + "import validate_pr; sys.argv = ['validate_pr.py', '--changed']; " + "validate_pr.main()" + )], cwd=tmp, env=env, capture_output=True, text=True, timeout=60, check=False, ) self.assertEqual(proc.returncode, 2, proc.stdout + proc.stderr) @@ -80,6 +87,51 @@ class ChangesNotYetCommittedAreRead(unittest.TestCase): self.assertIn("bios/new.bin", proc.stdout) +class AForkPushedToItsOwnMain(unittest.TestCase): + """A fork's clone has origin alone. Once the commit was pushed to the + fork's main, origin/main was HEAD, the diff was empty, and --changed + said "No changed BIOS files detected" with rc 0.""" + + GIT = ["git", "-c", "user.email=a@b", "-c", "user.name=a", + "-c", "init.defaultBranch=main"] + + def _git(self, *args: str, cwd: str | None = None) -> None: + subprocess.run([*self.GIT, *args], cwd=cwd, check=True, capture_output=True) + + def _changed(self, clone: str, project: str) -> subprocess.CompletedProcess: + return subprocess.run( + [sys.executable, "-c", ( + f"import sys; sys.path.insert(0, {str(REPO_ROOT / 'scripts')!r}); " + "import validate_pr; " + f"validate_pr.PROJECT_URLS = ({project!r},); " + "print(validate_pr.get_changed_files())" + )], + cwd=clone, capture_output=True, text=True, timeout=60, check=False, + ) + + def test_the_project_main_is_fetched_and_compared(self): + with tempfile.TemporaryDirectory() as tmp: + project, fork, clone = (str(Path(tmp, n)) for n in ("project", "fork", "clone")) + self._git("init", "-q", project) + Path(project, "README").write_text("x") + self._git("add", "README", cwd=project) + self._git("commit", "-qm", "init", cwd=project) + self._git("clone", "-q", "--bare", project, fork) + self._git("clone", "-q", fork, clone) + Path(clone, "bios").mkdir() + Path(clone, "bios", "new.bin").write_bytes(b"x") + self._git("add", "bios/new.bin", cwd=clone) + self._git("commit", "-qm", "add", cwd=clone) + self._git("push", "-q", "origin", "main", cwd=clone) + proc = self._changed(clone, project) + self.assertEqual(proc.returncode, 0, proc.stderr) + self.assertIn("bios/new.bin", proc.stdout) + # A project that cannot be reached is an error, never "nothing". + missing = self._changed(clone, str(Path(tmp, "absent"))) + self.assertNotEqual(missing.returncode, 0) + self.assertIn("fetching its main failed", missing.stderr) + + class RefreshFailuresAreFailures(unittest.TestCase): def test_a_failed_refresh_is_returned_and_exits_nonzero(self): report = {"entries": [{"status": "UPDATED", "key": "dolphin-sys"}, diff --git a/tests/test_scraper_contract.py b/tests/test_scraper_contract.py index 6df589b5..19e37d69 100644 --- a/tests/test_scraper_contract.py +++ b/tests/test_scraper_contract.py @@ -657,10 +657,11 @@ class EveryGitHubApiCallerSendsTheToken(unittest.TestCase): refresh_stale had put a token in their environment.""" def test_each_module_naming_the_api_reads_the_token(self): + # download.sh is download.py's twin and called the API bare. scripts = Path(__file__).resolve().parent.parent / "scripts" silent = sorted( str(path.relative_to(scripts)) - for path in scripts.rglob("*.py") + for path in [*scripts.rglob("*.py"), *scripts.rglob("*.sh")] if "api.github.com" in (source := path.read_text(encoding="utf-8")) and "GITHUB_TOKEN" not in source and "github_headers(" not in source diff --git a/tests/test_site_validation.py b/tests/test_site_validation.py index 75472aad..7ebe6ca5 100644 --- a/tests/test_site_validation.py +++ b/tests/test_site_validation.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import os import re import sys import tempfile @@ -216,8 +217,10 @@ class EveryTestDependencyIsInstalledInCi(unittest.TestCase): } workflow = (root / ".github" / "workflows" / "validate.yml").read_text() job = workflow[workflow.index("run-tests:"):] + # The suite runs on the default python: a line that installs into + # another interpreter (python3.9 -m pip) does not count. install_line = next( - line for line in job.splitlines() if "pip install" in line + line for line in job.splitlines() if "run: pip install" in line ) missing = sorted( m for m in third_party if self.PACKAGES.get(m, m) not in install_line @@ -289,6 +292,8 @@ class PlatformPagesPrintTheDeclaredCrc32(unittest.TestCase): def test_the_rendered_page(self): page = ROOT / "docs" / "platforms" / "retroarch.md" if not page.is_file(): + if os.environ.get("CI"): + self.fail("CI generates docs/ before the suite") self.skipTest("site not generated") self.assertIn("CRC32: `91e93107`", page.read_text(encoding="utf-8")) diff --git a/tests/test_slot.py b/tests/test_slot.py index b61b0748..46dfd09b 100644 --- a/tests/test_slot.py +++ b/tests/test_slot.py @@ -153,8 +153,7 @@ class TestRepoProfiles(unittest.TestCase): os.path.join(os.path.dirname(__file__), "..", "emulators", "*.yml") ) ) - if not self.paths: - self.skipTest("emulators/ not present") + self.assertTrue(self.paths, "emulators/ holds no profile") self.profiles = {} for path in self.paths: with open(path, encoding="utf-8") as fh: @@ -176,19 +175,19 @@ class TestRepoProfiles(unittest.TestCase): number, so on its own scale the US slot resolves to it. The merged index is a different story: pcsx1 disagrees, and the conflict test below asserts that neither order wins there.""" - index = slot.build_slot_index( - {"duckstation": self.profiles["duckstation"]} - ) - ds = self.profiles.get("duckstation") - if not ds: - self.skipTest("duckstation profile not present") + ds = self.profiles["duckstation"] + index = slot.build_slot_index({"duckstation": ds}) + # Membership, not equality: an image DuckStation selects for several + # territories lists north-america beside the others. members = [ (f["name"], f["name"]) for f in ds["files"] - if f.get("region") == ["north-america"] and f.get("priority") is not None + if "north-america" in (f.get("region") or []) + and f.get("priority") is not None ] - if len(members) < 2: - self.skipTest("no US candidate set to decide") + self.assertGreaterEqual( + len(members), 2, "DuckStation ranks no US candidates any more" + ) drops, undecidable = slot.resolve_slot_drops( {"sony-playstation": members}, index ) @@ -201,9 +200,7 @@ class TestRepoProfiles(unittest.TestCase): serves both cores, so neither order may drop the other's first pick.""" index = slot.build_slot_index(self.profiles) for name in ("scph1001.bin", "scph5501.bin"): - entry = index.get(name) - if entry is None: - self.skipTest(f"{name} not in any profile") + self.assertIn(name, index, f"{name} is in no profile") # pcsx1 and DuckStation both rank these and disagree: losing the mark # is the failure this test exists for, not a reason to skip it. self.assertTrue(index["scph1001.bin"].get("conflict")) @@ -234,8 +231,7 @@ class TestRepoProfiles(unittest.TestCase): """numero is alone on ti-83 and ranks all three ROMs, so this is the one slot the repo can currently decide end to end.""" index = slot.build_slot_index(self.profiles) - if "ti83se.rom" not in index: - self.skipTest("numero profile not present") + self.assertIn("ti83se.rom", index, "numero no longer ranks ti83se.rom") drops, undecidable = slot.resolve_slot_drops( {"ti-83": _pairs("ti83se.rom", "ti83plus.rom", "ti83.rom")}, index ) @@ -245,14 +241,13 @@ class TestRepoProfiles(unittest.TestCase): def test_ties_leave_the_group_untouched(self): """Japanese PlayStation ties at 5, so nothing may be dropped there.""" index = slot.build_slot_index(self.profiles) - ds = self.profiles.get("duckstation") - if not ds: - self.skipTest("duckstation profile not present") + ds = self.profiles["duckstation"] members = [ (f["name"], f["name"]) for f in ds["files"] - if f.get("region") == ["japan"] and f.get("priority") is not None + if "japan" in (f.get("region") or []) and f.get("priority") is not None ] + self.assertGreaterEqual(len(members), 2) drops, undecidable = slot.resolve_slot_drops( {"sony-playstation": members}, index ) diff --git a/tests/test_validate_pr.py b/tests/test_validate_pr.py index 941ad32b..b5acf0c5 100644 --- a/tests/test_validate_pr.py +++ b/tests/test_validate_pr.py @@ -99,6 +99,35 @@ class AFileOnlyACoreReadsIsKnown(_Fixture): result.checks) self.assertFalse(any("not referenced" in message for _s, message in result.checks)) + def test_a_missing_file_is_told_the_folder_of_its_system(self): + """The folder came from platform lists only: a file only a profile + declares, absent from the collection, got the generic advice.""" + import hashlib # noqa: PLC0415 + + held, missing = b"held rom", b"missing rom" + emulators = self.tmp / "emulators" + emulators.mkdir() + (emulators / "engine.yml").write_text( + "emulator: Engine\ntype: standalone\nsystems: [x, y]\nfiles:\n" + f" - name: \"a.bin\"\n system: x\n sha1: \"{hashlib.sha1(held).hexdigest()}\"\n" + f" - name: \"b.bin\"\n system: x\n md5: \"{hashlib.md5(missing).hexdigest()}\"\n" + ) + db = {"files": {hashlib.sha1(held).hexdigest(): {"path": "bios/Maker/Console/a.bin"}}, + "indexes": {"by_md5": {}, "by_sha256": {}}} + known = validate_pr.load_profile_hashes(str(emulators), db) + result = validate_pr.validate_file( + self._bios_file("Downloads/b.bin", missing), db, EMPTY_HASHES, known, + ) + self.assertIn( + ("WARN", "File is not under bios/ yet. Its system's files are in " + "`bios/Maker/Console/`"), + result.checks, + ) + placed = validate_pr.validate_file( + self._bios_file("bios/Other/Place/b.bin", missing), db, EMPTY_HASHES, known, + ) + self.assertTrue(any("Placed under bios/Other/Place/" in m for _s, m in placed.checks)) + class ASizeEveryDeclarationRulesOutIsNotAVariant(_Fixture): """1024 random bytes named scph5501.bin, a name every declaration sizes diff --git a/tests/test_validate_pr_flow.py b/tests/test_validate_pr_flow.py index c6fb86c1..e5930f57 100644 --- a/tests/test_validate_pr_flow.py +++ b/tests/test_validate_pr_flow.py @@ -7,7 +7,9 @@ cannot hold has to be an action the contributor can take. from __future__ import annotations +import json import os +import subprocess import sys import tempfile import unittest @@ -119,5 +121,30 @@ class OversizedFileAdvice(unittest.TestCase): ) +class RunFromAnyDirectory(unittest.TestCase): + """database.json, platforms/ and emulators/ were read from the current + directory. Run from elsewhere, scph5501.bin came back referenced by no + platform and not under bios/, with rc 0.""" + + def test_the_report_is_the_one_the_root_gives(self): + path = REPO_ROOT / "bios" / "Sony" / "PlayStation" / "scph5501.bin" + self.assertTrue(path.is_file(), path) + reports = {} + with tempfile.TemporaryDirectory() as elsewhere: + for cwd, arg in ((REPO_ROOT, "bios/Sony/PlayStation/scph5501.bin"), + (Path(elsewhere), str(path))): + proc = subprocess.run( + [sys.executable, str(REPO_ROOT / "scripts" / "validate_pr.py"), + "--json", arg], + cwd=cwd, capture_output=True, text=True, timeout=120, check=False, + ) + self.assertEqual(proc.returncode, 0, proc.stderr) + reports[cwd == REPO_ROOT] = json.loads(proc.stdout)[0]["checks"] + self.assertEqual(reports[False], reports[True]) + messages = [c["message"] for c in reports[False]] + self.assertIn("SHA1 matches known platform requirement", messages) + self.assertIn("Already in the database at this path", messages) + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_workflow_paths.py b/tests/test_workflow_paths.py index a2aba810..88636e12 100644 --- a/tests/test_workflow_paths.py +++ b/tests/test_workflow_paths.py @@ -133,6 +133,18 @@ class ThePullRequestBuildsTheSite(unittest.TestCase): with self.subTest(package=package): self.assertIn(package, runs) + def test_the_pages_exist_before_the_suite_reads_them(self): + """docs/ is gitignored. Built after the suite, the tests that read + the rendered pages found none and skipped on every CI run.""" + steps = self._workflow("validate.yml")["jobs"]["run-tests"]["steps"] + runs = [str(step.get("run", "")) for step in steps] + generate = next( + i for i, run in enumerate(runs) if "scripts/generate_site.py" in run + ) + suite = next(i for i, run in enumerate(runs) if "unittest discover" in run) + self.assertLess(generate, suite) + self.assertEqual(steps[suite].get("if"), "${{ !cancelled() }}") + def test_every_site_input_triggers_validation(self): deploy = self._workflow("deploy-site.yml") validate = self._workflow("validate.yml") diff --git a/wiki/testing-guide.md b/wiki/testing-guide.md index 8376d164..f7fe1eb7 100644 --- a/wiki/testing-guide.md +++ b/wiki/testing-guide.md @@ -22,7 +22,9 @@ python -m unittest tests.test_install tests.test_install_cores -v The table below says which module covers what. -Running the build needs Python 3.9 or newer and `pyyaml` alone. CI runs the suite on 3.12. The +Running the build needs Python 3.9 or newer and `pyyaml` alone. CI runs the suite on 3.12, +and one test imports every module under `scripts/` with a real `python3.9`, which +needs `pyyaml` and `jsonschema` installed for that interpreter. The schema contract tests also need `jsonschema[format-nongpl]`, and one site test renders a wiki line with `markdown`. The CI installs both: without them those tests are reported as skipped, so install them before a pull request