From 6445e5d7d61ecf070a29fe263947b475ca607cef Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Fri, 6 Nov 2026 03:21:53 +0100 Subject: [PATCH] fix: refuse a missing emulator profile directory --- scripts/common.py | 20 ++++--- scripts/fileless_audit.py | 3 +- scripts/mame_ref_audit.py | 2 + scripts/scraper/arcade_parent_importer.py | 4 +- scripts/scraper/romset_dat_importer.py | 3 +- tests/test_audit_regressions.py | 11 +--- tests/test_system_selection.py | 69 +++++++++++++++++++++++ 7 files changed, 91 insertions(+), 21 deletions(-) diff --git a/scripts/common.py b/scripts/common.py index 21ef30930..7e78ead96 100644 --- a/scripts/common.py +++ b/scripts/common.py @@ -43,7 +43,12 @@ def require_directory(path: str, flag: str = "--bios-dir") -> None: raise SystemExit(f"ERROR: {flag} {path} is not a directory") -class DatabaseUnreadable(FileNotFoundError): +class InputMissing(FileNotFoundError): + """An input a command was pointed at does not exist. run_cli prints the + message on one line instead of a stack trace.""" + + +class DatabaseUnreadable(InputMissing): """The database a command was pointed at does not exist or is not JSON.""" @@ -2181,12 +2186,11 @@ def load_emulator_profiles( return {} profiles = {} emu_path = Path(emulators_dir) - if not emu_path.exists(): - print( - f"warning: no emulator profile directory at {emulators_dir}", - file=sys.stderr, - ) - return profiles + if not emu_path.is_dir(): + # Read as "no emulator", a mistyped --emulators-dir built a pack + # without any core file marked OK, and sitesources wrote {} over + # source-paths.json, both exiting 0. + raise InputMissing(f"no emulator profile directory at {emulators_dir}") for f in source_yaml_paths(emu_path): with open(f) as fh: profile = yaml_load(fh) or {} @@ -3571,7 +3575,7 @@ def run_cli(main: Callable[[], int | None]) -> None: try: code = main() sys.stdout.flush() - except DatabaseUnreadable as exc: + except InputMissing as exc: print(f"ERROR: {exc}", file=sys.stderr) sys.exit(1) except BrokenPipeError: diff --git a/scripts/fileless_audit.py b/scripts/fileless_audit.py index deea6b65b..cbb5b1cb1 100644 --- a/scripts/fileless_audit.py +++ b/scripts/fileless_audit.py @@ -25,7 +25,7 @@ from pathlib import Path sys.path.insert(0, os.path.dirname(__file__)) import upstream -from common import suggest_names +from common import require_directory, suggest_names from profile_sync import collect_citations, ref_role, select_views, split_source_ref from safeparse import yaml_load @@ -122,6 +122,7 @@ def main() -> None: args = parser.parse_args() if args.all == bool(args.emulators): parser.error("name profiles or pass --all, not both") + require_directory(args.emulators_dir, "--emulators-dir") known = sorted(path.stem for path in Path(args.emulators_dir).glob("*.yml")) unknown = [name for name in args.emulators if name not in known] diff --git a/scripts/mame_ref_audit.py b/scripts/mame_ref_audit.py index c770bdbaf..387e42fd3 100644 --- a/scripts/mame_ref_audit.py +++ b/scripts/mame_ref_audit.py @@ -27,6 +27,7 @@ from dataclasses import dataclass sys.path.insert(0, os.path.dirname(__file__)) import upstream +from common import require_directory from safeparse import yaml_load from scraper.mame_parser import _MACHINE_MACROS, strip_comments @@ -145,6 +146,7 @@ def main() -> None: help="rewrite the refs the audit can settle", ) args = parser.parse_args() + require_directory(args.emulators_dir, "--emulators-dir") total = 0 for name in args.emulators: diff --git a/scripts/scraper/arcade_parent_importer.py b/scripts/scraper/arcade_parent_importer.py index de14eaf19..ee1eca55a 100644 --- a/scripts/scraper/arcade_parent_importer.py +++ b/scripts/scraper/arcade_parent_importer.py @@ -50,7 +50,7 @@ from dataclasses import dataclass, field from pathlib import Path from ..artifacts import write_text_atomic -from ..common import load_emulator_profiles +from ..common import load_emulator_profiles, run_cli from .romset_dat_importer import ( LISTXML_CHUNK, MAME_LISTXML_URL, @@ -1203,4 +1203,4 @@ def main() -> int: if __name__ == "__main__": - sys.exit(main()) + run_cli(main) diff --git a/scripts/scraper/romset_dat_importer.py b/scripts/scraper/romset_dat_importer.py index 55a081af8..57cb22bfe 100644 --- a/scripts/scraper/romset_dat_importer.py +++ b/scripts/scraper/romset_dat_importer.py @@ -45,6 +45,7 @@ from ..common import ( list_registered_platforms, load_emulator_profiles, load_platform_config, + run_cli, ) from ..dumpcatalog import build_snapshot, load_provenance_snapshots, write_snapshot from .logiqx_parser import LogiqxDat, parse_logiqx @@ -665,4 +666,4 @@ def main() -> int: if __name__ == "__main__": - sys.exit(main()) + run_cli(main) diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index d22888989..b1e24bdf8 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -2836,17 +2836,10 @@ class ACheckThatCannotAnswerDoesNotPass(unittest.TestCase): ) def test_a_missing_profile_directory_says_so(self): - import io as _io - from scripts import common as _common - stderr = _io.StringIO() - with contextlib.redirect_stderr(stderr): - profiles = _common.load_emulator_profiles( - str(ROOT / "no-such-emulator-dir") - ) - self.assertEqual(profiles, {}) - self.assertIn("no emulator profile directory", stderr.getvalue()) + with self.assertRaisesRegex(_common.InputMissing, "no emulator profile directory"): + _common.load_emulator_profiles(str(ROOT / "no-such-emulator-dir")) class TestEntryPointsRunEveryClass(unittest.TestCase): diff --git a/tests/test_system_selection.py b/tests/test_system_selection.py index 8ab0fe7cd..56c16cba1 100644 --- a/tests/test_system_selection.py +++ b/tests/test_system_selection.py @@ -262,6 +262,75 @@ class AMissingBiosDirIsRefused(unittest.TestCase): self.assertFalse(Path(out).exists()) +def _scripts_taking(flag: str) -> list[Path]: + pattern = re.compile(r'add_argument\(\s*"%s"' % re.escape(flag)) + return [ + path for path in sorted((REPO_ROOT / "scripts").glob("*.py")) + + sorted((REPO_ROOT / "scripts" / "scraper").glob("*.py")) + if pattern.search(path.read_text(encoding="utf-8")) + ] + + +def _command(path: Path) -> list[str]: + if path.parent.name == "scraper": + return [sys.executable, "-m", f"scripts.scraper.{path.stem}"] + return [sys.executable, f"scripts/{path.name}"] + + +class AMissingProfileDirIsRefused(unittest.TestCase): + """A mistyped --emulators-dir read as "no emulator": generate_pack built + a pack without its core files marked OK, verify --strict passed, and + sitesources wrote {} over source-paths.json, every one exiting 0.""" + + EXTRA = { + "arcade_parent_importer": ["--all", "--offline"], + "fileless_audit": ["--all"], + "generate_pack": ["--platform", "retroarch", "--system", "atari-lynx", + "--offline", "--output-dir", "{out}"], + "generate_site": ["--docs-dir", "{out}"], + "generate_truth": ["--all", "--output-dir", "{out}"], + "mame_ref_audit": ["mame"], + "profile_sync": ["--all", "--triage"], + "romset_dat_importer": ["--source", "mame", "--pack", "{dat}", "--output", "{out}/r.json"], + "sitesources": ["--cache-dir", "{cache}", "--output", "{out}/source-paths.json"], + "validate_pr": ["bios/Sony/PlayStation/scph5501.bin"], + "verify": ["--platform", "retroarch", "--strict"], + } + + def test_every_script_taking_an_emulators_dir_refuses_a_missing_one(self): + scratch = tempfile.TemporaryDirectory(dir=REPO_ROOT / "tmp") + self.addCleanup(scratch.cleanup) + root = Path(scratch.name) + (root / "cache").mkdir() + (root / "empty.dat").write_bytes(b"") + values = {"out": str(root / "out"), "cache": str(root / "cache"), + "dat": str(root / "empty.dat")} + taking = _scripts_taking("--emulators-dir") + self.assertGreater(len(taking), 15) + tracked = ["git", "diff", "--name-only"] + before = subprocess.run(tracked, cwd=REPO_ROOT, capture_output=True, text=True).stdout + for path in taking: + extra = [arg.format(**values) for arg in self.EXTRA.get(path.stem, [])] + with self.subTest(script=path.stem): + proc = subprocess.run( + [*_command(path), "--emulators-dir", "nosuch", *extra], + capture_output=True, text=True, cwd=str(REPO_ROOT), timeout=600, + check=False, env={**os.environ, "RETROBIOS_OFFLINE": "1"}, + ) + self.assertNotEqual(proc.returncode, 0, proc.stdout[-500:]) + self.assertNotIn("Traceback", proc.stderr, proc.stderr[-1500:]) + self.assertRegex(proc.stderr, r"ERROR: .*nosuch") + written = [ + str(p.relative_to(root)) for p in (root / "out").rglob("*") + if p.is_file() and p.name != ".lock" + ] + self.assertEqual(written, []) + # Read as empty, generate_site rewrote published-parts.json and the + # FAQ, and sitesources source-paths.json. + after = subprocess.run(tracked, cwd=REPO_ROOT, capture_output=True, text=True).stdout + self.assertEqual(after, before) + + class StrictVerifyFailsOnACriticalMiss(unittest.TestCase): """verify exited 0 with a required file missing on an md5 platform, so the pipeline printed "verify OK" over a pack the frontend rejects."""