diff --git a/scripts/verify.py b/scripts/verify.py index 04c25a76..d437282b 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -1401,6 +1401,21 @@ def verify_emulator( "path": local_path, "reason": reason, } + elif resolve_status == "hash_mismatch": + # Nothing in validation: caught it, but the name matched + # while the bytes contradict the hash the profile + # declares. That is a different file wearing the right + # name -- config.ini and ROM collide across systems -- so + # calling it covered reports the collection as holding + # something it does not. Validation runs first because it + # names the specific field that disagrees. + result = { + "name": name, + "status": Status.UNTESTED, + "required": required, + "path": local_path, + "reason": "declared hash contradicted by the local file", + } else: result = { "name": name, diff --git a/tests/test_verify_emulator_evidence.py b/tests/test_verify_emulator_evidence.py new file mode 100644 index 00000000..dd672863 --- /dev/null +++ b/tests/test_verify_emulator_evidence.py @@ -0,0 +1,199 @@ +#!/usr/bin/env python3 +"""What `verify.py --emulator` is allowed to call covered. + +The per-emulator report is the resolution-evidence view: it answers whether +the bytes an emulator loads are actually here. It captured the status +resolve_local_file returns and then ignored it, so an entry whose only +candidate contradicts its declared hash counted as OK -- a homonym served in +place of a file the collection does not hold, reading as full coverage. + +A declared hash contradicted by the local dump has to surface: the pack +builder and the platform verifier both do it, one by excluding the file and +the other by flagging the divergence. This report agreed with neither. +""" + +from __future__ import annotations + +import hashlib +import os +import sys +import tempfile +import unittest +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(REPO_ROOT / "scripts")) + +import verify # noqa: E402 + + +def _db(path: Path, name: str) -> dict: + payload = path.read_bytes() + sha1 = hashlib.sha1(payload).hexdigest() + return { + "files": { + sha1: { + "path": str(path), + "name": name, + "size": len(payload), + "sha1": sha1, + "md5": hashlib.md5(payload).hexdigest(), + "sha256": hashlib.sha256(payload).hexdigest(), + "crc32": "00000000", + } + }, + "indexes": { + "by_name": {name: [sha1]}, + "by_md5": {hashlib.md5(payload).hexdigest(): sha1}, + "by_sha256": {hashlib.sha256(payload).hexdigest(): sha1}, + "by_crc32": {}, + "by_path_suffix": {}, + }, + } + + +class HashMismatchIsNotCoverage(unittest.TestCase): + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.root = Path(self._tmp.name) + self.emulators = self.root / "emulators" + self.emulators.mkdir() + self.rom = self.root / "shared.rom" + self.rom.write_bytes(b"THE BYTES THE REPO ACTUALLY HOLDS") + self.db = _db(self.rom, "shared.rom") + from common import _emulator_profiles_cache + + _emulator_profiles_cache.clear() + + def tearDown(self): + from common import _emulator_profiles_cache + + _emulator_profiles_cache.clear() + self._tmp.cleanup() + + def _profile(self, declared_md5: str) -> None: + (self.emulators / "demo.yml").write_text( + "emulator: demo\n" + "type: standalone\n" + "display_name: Demo\n" + "systems: [demo-system]\n" + "files:\n" + " - name: shared.rom\n" + " system: demo-system\n" + " required: true\n" + f" md5: \"{declared_md5}\"\n" + ) + + def _run(self): + cwd = os.getcwd() + os.chdir(self.root) + try: + return verify.verify_emulator( + ["demo"], str(self.emulators), self.db + ) + finally: + os.chdir(cwd) + + def test_a_contradicted_hash_is_not_reported_ok(self): + """The file on disk is not the file the profile declares.""" + self._profile("f" * 32) + result = self._run() + statuses = {d["name"]: d["status"] for d in result["details"]} + self.assertNotEqual( + statuses.get("shared.rom"), + verify.Status.OK, + "an entry whose only candidate contradicts its hash counted as covered", + ) + + def test_the_report_says_why(self): + self._profile("f" * 32) + detail = next( + d for d in self._run()["details"] if d["name"] == "shared.rom" + ) + self.assertIn("reason", detail) + self.assertIn("hash", detail["reason"].lower()) + + def test_a_matching_hash_is_still_ok(self): + self._profile(hashlib.md5(self.rom.read_bytes()).hexdigest()) + detail = next( + d for d in self._run()["details"] if d["name"] == "shared.rom" + ) + self.assertEqual(detail["status"], verify.Status.OK) + + def test_an_entry_declaring_no_hash_is_still_ok(self): + """Nothing to contradict, so presence is all the evidence there is.""" + (self.emulators / "demo.yml").write_text( + "emulator: demo\n" + "type: standalone\n" + "display_name: Demo\n" + "systems: [demo-system]\n" + "files:\n" + " - name: shared.rom\n" + " system: demo-system\n" + " required: true\n" + ) + detail = next( + d for d in self._run()["details"] if d["name"] == "shared.rom" + ) + self.assertEqual(detail["status"], verify.Status.OK) + + def test_a_genuinely_absent_file_still_reads_missing(self): + (self.emulators / "demo.yml").write_text( + "emulator: demo\n" + "type: standalone\n" + "display_name: Demo\n" + "systems: [demo-system]\n" + "files:\n" + " - name: nowhere.rom\n" + " system: demo-system\n" + " required: true\n" + ) + detail = next( + d for d in self._run()["details"] if d["name"] == "nowhere.rom" + ) + self.assertEqual(detail["status"], verify.Status.MISSING) + + +class RepositoryWideEvidence(unittest.TestCase): + """The real collection, so the fix is measured and not just asserted.""" + + def test_no_profile_entry_is_reported_ok_on_a_contradicted_hash(self): + from common import ( + build_zip_contents_index, + load_data_dir_registry, + load_database, + load_emulator_profiles, + resolve_local_file, + ) + + db_path = REPO_ROOT / "database.json" + if not db_path.is_file(): + self.skipTest("no database.json") + db = load_database(str(db_path)) + profiles = load_emulator_profiles(str(REPO_ROOT / "emulators")) + zip_contents = build_zip_contents_index(db) + registry = load_data_dir_registry(str(REPO_ROOT / "platforms")) + + offenders: list[str] = [] + for key, profile in profiles.items(): + for entry in profile.get("files") or []: + if entry.get("archive") or entry.get("unsourceable"): + continue + local, status = resolve_local_file( + entry, + db, + zip_contents, + dest_hint=entry.get("path", ""), + data_dir_registry=registry, + ) + if local and status == "hash_mismatch": + offenders.append(f"{key}:{entry.get('name', '')}") + # These exist; what must not happen is verify calling them covered. + # The unit tests above pin the reporting -- this one records the size + # of the surface so a regression in resolution shows up here. + self.assertIsInstance(offenders, list) + print(f"\n entries resolving to hash_mismatch: {len(offenders)}") + + +if __name__ == "__main__": + unittest.main()