chore: merge audit fixes (scrapers)

tests/test_artifact_lock.py

@@ -110,6 +110,36 @@ class PackLockCliTest(unittest.TestCase):
         self.assertEqual(proc.returncode, 1, proc.stdout + proc.stderr)
         self.assertIn("is in use by another run", proc.stdout)
 
+    def test_a_held_output_is_refused_before_the_data_refresh(self):
+        """The refresh waits for every run holding a data tree, and a
+        pipeline holds them all for its whole run: probed after the refresh,
+        a second build sat silent for hours instead of being refused."""
+        from refresh_data_dirs import cache_lock  # noqa: PLC0415
+
+        platforms = Path(self.dir) / "platforms"
+        platforms.mkdir()
+        for name in ("_registry.yml", "_shared.yml", "misterfpga.yml"):
+            shutil.copyfile(REPO_ROOT / "platforms" / name, platforms / name)
+        cache = Path(self.dir) / "cache" / "probe"
+        cache.mkdir(parents=True)
+        (platforms / "_data_dirs.yml").write_text(
+            "data_directories:\n  probe:\n    source_type: zip\n"
+            f"    source_url: https://example.invalid/probe.zip\n"
+            f"    local_cache: {cache}\n"
+        )
+        output = Path(self.dir) / "out"
+        with artifact_lock(str(output)), cache_lock(cache):
+            try:
+                proc = subprocess.run(
+                    [sys.executable, "scripts/generate_pack.py", "--platform", "misterfpga",
+                     "--platforms-dir", str(platforms), "--output-dir", str(output)],
+                    capture_output=True, text=True, cwd=str(REPO_ROOT), timeout=120,
+                )
+            except subprocess.TimeoutExpired:
+                self.fail("generate_pack waited on the data tree instead of refusing")
+        self.assertEqual(proc.returncode, 1, proc.stdout + proc.stderr)
+        self.assertIn("is in use by another run", proc.stdout)
+
     def test_every_writing_mode_refuses_a_locked_output(self):
         """Two of seven modes took the lock, and five wrote straight into it.
 

tests/test_check_freshness.py

@@ -87,6 +87,25 @@ class NativeCacheTests(unittest.TestCase):
         self.assertEqual({r.status for r in rows}, {cf.STALE})
         self.assertIn("batocera", {r.subject for r in rows})
 
+    def test_the_model_build_notes_stay_out_of_the_report(self):
+        """generate_truth reports how two cores' hashes were settled. Under
+        `--only native` they opened the report and named no step to take."""
+        import contextlib
+        import io
+
+        def noisy(truth, scraped):
+            print("WARNING: hash conflict for GameIndex.yaml (sha1: a vs b, core yaps2)",
+                  file=sys.stderr)
+            print("other diagnostic", file=sys.stderr)
+            return {}, None
+
+        err = io.StringIO()
+        with mock.patch.object(cf, "build_native_model", side_effect=noisy), \
+                contextlib.redirect_stderr(err):
+            cf.check_native(REPO_ROOT / "platforms", self.cache, self.cache / "truth")
+        self.assertNotIn("hash conflict", err.getvalue())
+        self.assertIn("other diagnostic", err.getvalue())
+
 
 class PublishedExportTests(unittest.TestCase):
     """exports/ is committed and published, and CI cannot rebuild it."""
@@ -165,6 +184,32 @@ class PlatformDiffTests(unittest.TestCase):
         self.assertEqual(diff.cores_removed, ["b"])
         self.assertIn("+cores 2", diff.summary())
 
+    def test_every_other_scraped_field_is_a_change(self):
+        """launchers feed verify, dual_mode_cores the grouping, a system's
+        data_directories the pack: a rescrape that moves only them is stale."""
+        old = self._platform()
+        old["launchers"] = [{"emulator": "e"}]
+        old["dual_mode_cores"] = ["flycast"]
+        old["systems"]["sys"].update(core="c1", native_id="psx",
+                                     data_directories=[{"ref": "x"}])
+        new = self._platform()
+        new["launchers"] = []
+        new["systems"]["sys"].update(core="c2", native_id="psx")
+        diff = cf.diff_platform(old, new)
+        self.assertTrue(diff.content_changed)
+        self.assertEqual(
+            diff.fields_changed,
+            ["dual_mode_cores", "launchers", "sys/core", "sys/data_directories"],
+        )
+        self.assertIn("~fields 4", diff.summary())
+
+    def test_the_revision_read_alone_is_not_a_change(self):
+        old, new = self._platform(), self._platform()
+        old["source"], new["source"] = "https://x/tar.gz/aaa", "https://x/tar.gz/bbb"
+        old["launchers"] = [{"emulator": "e", "source_ref": "es_features.cfg:11638-11646"}]
+        new["launchers"] = [{"emulator": "e", "source_ref": "es_features.cfg:11649-11657"}]
+        self.assertFalse(cf.diff_platform(old, new).changed)
+
     def test_target_stamp_is_not_a_change(self):
         old = {"scraped_at": "2026-01-01", "targets": {"t": {"cores": ["a"]}}}
         new = {"scraped_at": "2026-02-01", "targets": {"t": {"cores": ["a"]}}}
@@ -240,6 +285,36 @@ class CoreResolutionTests(unittest.TestCase):
         self.assertEqual(unprofiled, ["wqxemu"])
         self.assertEqual(standalone, [("eka2l1", "eka2l1")])
 
+    def test_a_changed_info_of_a_profiled_core_is_stale(self):
+        """A firmwareN_path added to a profiled core's .info changes its blob:
+        the names alone said OK."""
+        blobs = {"mednafen_psx": "new", "eka2l1": "same", "wqxemu": "other"}
+        recorded = {"mednafen_psx": "old", "eka2l1": "same"}
+        with tempfile.TemporaryDirectory() as tmp:
+            record = Path(tmp) / "_core_info.json"
+            record.write_text(cf.json.dumps({"blobs": recorded}))
+            with mock.patch.object(cf, "core_info_blobs", return_value=blobs):
+                rows = cf.check_coreinfo(self.PROFILES, "", True, record)
+        stale = {r.subject: r for r in rows if r.status == cf.STALE}
+        self.assertIn("mednafen_psx", stale)
+        self.assertIn("--record-coreinfo mednafen_psx", stale["mednafen_psx"].detail)
+        self.assertNotIn(".info changed", stale["eka2l1"].detail)
+        self.assertEqual(cf.coreinfo_drift(blobs, recorded, self.PROFILES), ["mednafen_psx"])
+
+    def test_recording_a_core_leaves_the_others_unread(self):
+        with tempfile.TemporaryDirectory() as tmp:
+            record = Path(tmp) / "_core_info.json"
+            record.write_text(cf.json.dumps({"blobs": {"a": "1", "b": "1"}}))
+            cf.record_core_info({"a": "2", "b": "2"}, ["a"], record)
+            self.assertEqual(cf.load_core_info_record(record), {"a": "2", "b": "1"})
+            with self.assertRaises(ValueError):
+                cf.record_core_info({"a": "2"}, ["ghost"], record)
+
+    def test_the_committed_record_holds_git_blobs(self):
+        recorded = cf.load_core_info_record()
+        self.assertIsNotNone(recorded)
+        self.assertTrue(all(len(sha) == 40 for sha in recorded.values()))
+
 
 class PinParsingTests(unittest.TestCase):
     WORKFLOW = """

tests/test_hash_merge.py

@@ -356,7 +356,8 @@ class TheScraperClonesTheReleaseItLabels(unittest.TestCase):
         with mock.patch.object(scraper, "_get_release_tag", return_value="mame0289"), \
                 mock.patch.object(scraper, "_run_git",
                                   side_effect=lambda args, cwd=None: calls.append(args)), \
-                mock.patch.object(scraper, "parse_mame_source_tree", return_value={}), \
+                mock.patch.object(scraper, "parse_mame_source_tree",
+                                  return_value={"neogeo": {"roms": []}}), \
                 mock.patch.object(scraper, "_get_commit", return_value="c" * 40), \
                 mock.patch.object(scraper, "_write_cache"), \
                 mock.patch.object(scraper, "_cleanup"), \

tests/test_large_file_cache.py

@@ -691,7 +691,10 @@ class ASameSizeReuploadIsSeen(unittest.TestCase):
     def replaced(self, digests: dict, size: int | None) -> bool:
         with mock.patch.object(largefiles, "_served_digests", lambda: digests), \
                 mock.patch.object(largefiles, "_served_size", lambda name: size):
-            return largefiles._replaced_on_release("dsi_nand.bin", str(self.cached), False)
+            return largefiles._replaced_on_release(
+                "dsi_nand.bin", hashlib.sha256(self.cached.read_bytes()).hexdigest(),
+                self.cached.stat().st_size, False,
+            )
 
     def test_the_digest_decides(self):
         same = hashlib.sha256(b"OLD NAND").hexdigest()
@@ -703,6 +706,30 @@ class ASameSizeReuploadIsSeen(unittest.TestCase):
         self.assertFalse(self.replaced({}, 8))
         self.assertTrue(self.replaced({}, 9))
 
+    def test_a_copy_promoted_during_the_check_is_the_one_judged(self):
+        """Another run promotes the served revision while this one hashes the
+        stale copy: the release check reread the path, found the served
+        bytes, and the caller was told the cache held no such file."""
+        new = b"NEW NAND"
+        real = largefiles.compute_hashes
+
+        def promote_while_hashing(path, *args, **kwargs):
+            result = real(path, *args, **kwargs)
+            if Path(path).read_bytes() != new:
+                promoted = self.dir / "promoted.tmp"
+                promoted.write_bytes(new)
+                os.replace(promoted, self.cached)
+            return result
+
+        digests = {"dsi_nand.bin": hashlib.sha256(new).hexdigest()}
+        with mock.patch.object(largefiles, "compute_hashes", promote_while_hashing), \
+                mock.patch.object(largefiles, "_served_digests", lambda: digests):
+            found = largefiles._fetch_asset(
+                "dsi_nand.bin", str(self.dir), hashlib.sha1(new).hexdigest(), "",
+                offline=False,
+            )
+        self.assertEqual(found, str(self.cached))
+
     def test_the_release_check_compares_digests_at_equal_size(self):
         expected = {"bios/Nintendo/DSi/dsi_nand.bin": 8}
         assets = {"dsi_nand.bin": 8}

tests/test_refresh_data_dirs.py

@@ -166,5 +166,63 @@ class EmulatorPackHoldsTheCache(unittest.TestCase):
             self.assertEqual(held, [True])
 
 
+class AWaitIsAnnounced(unittest.TestCase):
+    def test_a_held_tree_is_named_before_the_wait(self):
+        """A pipeline holds every tree for its run: the refresh beside it
+        waited with nothing on screen."""
+        with tempfile.TemporaryDirectory() as tmp:
+            cache = Path(tmp) / "probe"
+            taken = threading.Event()
+
+            def second_run() -> None:
+                with rdd.cache_lock(cache):
+                    taken.set()
+
+            with self.assertLogs(rdd.log, "WARNING") as logs:
+                with rdd.cache_lock(cache):
+                    worker = threading.Thread(target=second_run)
+                    worker.start()
+                    for _ in range(100):
+                        if logs.output:
+                            break
+                        threading.Event().wait(0.05)
+                    self.assertFalse(taken.is_set())
+                worker.join(10)
+            self.assertTrue(taken.is_set())
+            self.assertIn("waiting for", logs.output[0])
+            self.assertIn("probe", logs.output[0])
+
+
+class DryRunReport(unittest.TestCase):
+    """What a dry run says: why each entry would be fetched, and how many."""
+
+    def test_a_quoted_etag_is_cut_without_its_quotes(self):
+        self.assertEqual(rdd._short_tag('"0x8DF0B72EB52A1C0"'), "0x8DF0B72EB5")
+        self.assertEqual(rdd._short_tag('W/"2685251384"'), "2685251384")
+        self.assertEqual(rdd._short_tag(None), "none")
+
+    def test_the_remote_tag_is_shown_beside_the_cached_one(self):
+        with tempfile.TemporaryDirectory() as tmp:
+            versions = Path(tmp) / "versions.json"
+            versions.write_text(json.dumps({"k": {"etag": '"2725165595"'}}))
+            entry = {"source_type": "zip", "source_url": "https://x.invalid/a.zip",
+                     "local_cache": tmp}
+            with mock.patch.object(rdd, "_get_remote_etag", return_value='"2685251384"'), \
+                    self.assertLogs(rdd.log, "INFO") as logs:
+                self.assertTrue(rdd.refresh_entry("k", entry, dry_run=True,
+                                                  versions_path=str(versions)))
+        self.assertIn("cached: 2725165595, remote: 2685251384", logs.output[0])
+
+    def test_a_run_ends_on_its_counts(self):
+        self.assertEqual(
+            rdd.summarize({"a": True, "b": False, "c": False}, dry_run=True),
+            "3 entries: 1 would refresh, 2 up to date",
+        )
+        self.assertEqual(
+            rdd.summarize({"a": True, "b": None}, dry_run=False),
+            "2 entries: 1 refreshed, 0 up to date, 1 failed",
+        )
+
+
 if __name__ == "__main__":
     unittest.main()

tests/test_refresh_stale.py

@@ -101,6 +101,16 @@ class Planning(unittest.TestCase):
         )
         self.assertEqual([f["subject"] for f in surfaced], ["redump"])
 
+    def test_a_data_row_outside_the_registry_needs_a_human(self):
+        """refresh_data_dirs.py --key 'buildbot system' exits 1 on an unknown
+        key: the row asks for a registry entry, not a refresh."""
+        jobs, surfaced, _ = self._plan(
+            finding("data", "buildbot system"), finding("data", "_data_dirs.yml")
+        )
+        self.assertEqual(jobs, [])
+        self.assertEqual([f["subject"] for f in surfaced],
+                         ["buildbot system", "_data_dirs.yml"])
+
     def test_only_stale_rows_run_and_errors_are_shown(self):
         jobs, surfaced, ignored = self._plan(
             finding("data", "a", "OK"),

tests/test_romset_recipes.py

@@ -28,7 +28,9 @@ from romset_recipes import (  # noqa: E402
     write_reconstructions,
 )
 from scripts.scraper.romset_dat_importer import (  # noqa: E402
+    catalog_targets,
     compact_entries,
+    holds_target,
     listxml_entries,
     git_blob_sha,
     merge_snapshot,
@@ -386,6 +388,32 @@ class ListXmlRecipes(unittest.TestCase):
             [m["name"] for m in entries[0]["members"]], ["own.rom", "shared.rom"]
         )
 
+    def test_a_set_holding_a_missing_catalog_dump_is_kept(self):
+        """fm7740sx carries three No-Intro dumps the collection lacks. No
+        profile names it, so the missing report named no place to find them."""
+        targets = {(_crc(ROM_B), len(ROM_B)): {""}}
+        entries = listxml_entries(
+            self._opener(self._document()), "MAME 0.289", {"other.zip"}, targets
+        )
+        self.assertEqual([entry["name"] for entry in entries], ["childset.zip"])
+
+    def test_a_catalog_sha1_must_agree(self):
+        member = {"crc32": _crc(ROM_B), "size": len(ROM_B), "sha1": "a" * 40}
+        self.assertFalse(holds_target([member], {(_crc(ROM_B), len(ROM_B)): {"b" * 40}}))
+        self.assertTrue(holds_target([member], {(_crc(ROM_B), len(ROM_B)): {"a" * 40}}))
+
+    def test_catalog_targets_leave_out_what_the_collection_holds(self):
+        with tempfile.TemporaryDirectory(dir=TMP_ROOT) as directory:
+            root = Path(directory)
+            (root / "prov").mkdir()
+            (root / "prov" / "tosec.json").write_text(json.dumps({"source": "tosec", "entries": [
+                {"name": "held.rom", "crc32": "1", "size": 4, "sha1": "a" * 40},
+                {"name": "lacked.rom", "crc32": "2", "size": 4, "sha1": "b" * 40},
+            ]}))
+            (root / "db.json").write_text(json.dumps({"files": {"a" * 40: {}}}))
+            targets = catalog_targets(str(root / "prov"), str(root / "db.json"))
+        self.assertEqual(targets, {("00000002", 4): {"b" * 40}})
+
     def test_upstream_blobs_are_recorded_and_replaced(self):
         """The snapshot names the DAT blobs it was read from, so a later
         check can tell whether upstream moved without importing again."""

tests/test_scraper_contract.py

@@ -882,5 +882,127 @@ class OneNativeIdOneSystem(unittest.TestCase):
                 ids = {_norm_system_id(s) for s in data["systems"]}
                 self.assertEqual(wanted & ids & machines, wanted)
 
+
+class HashScraperRefusesAnEmptyRead(unittest.TestCase):
+    """A source tree that yields no BIOS set is never cached as a result.
+
+    Cached, it was served for a day: every profile entry came out "out of
+    scope", nothing was refreshed and pipeline steps 2a and 2a2 said OK.
+    """
+
+    def _fresh(self, sets: dict) -> dict:
+        from datetime import datetime, timezone  # noqa: PLC0415
+
+        return {"fetched_at": datetime.now(timezone.utc).isoformat(),
+                "commit": "abc", "bios_sets": sets}
+
+    def test_mame_raises_and_writes_no_cache(self):
+        from scraper import mame_hash_scraper as m  # noqa: PLC0415
+
+        with tempfile.TemporaryDirectory() as tmp:
+            cache = Path(tmp) / "mame-hashes.json"
+            with mock.patch.object(m, "_CACHE_PATH", cache), \
+                    mock.patch.object(m, "_get_release_tag", return_value="mame0289"), \
+                    mock.patch.object(m, "_sparse_clone"), \
+                    mock.patch.object(m, "_cleanup"), \
+                    mock.patch.object(m, "_get_commit", return_value="abc"), \
+                    mock.patch.object(m, "parse_mame_source_tree", return_value={}):
+                with self.assertRaises(RuntimeError):
+                    m._fetch_hashes(force=True)
+            self.assertFalse(cache.exists())
+
+    def test_mame_does_not_serve_an_empty_cache(self):
+        from scraper import mame_hash_scraper as m  # noqa: PLC0415
+
+        self.assertTrue(m._is_stale(self._fresh({})))
+        self.assertFalse(m._is_stale(self._fresh({"neogeo": {}})))
+
+    def test_fbneo_raises_and_writes_no_cache(self):
+        from scraper import fbneo_hash_scraper as f  # noqa: PLC0415
+
+        with tempfile.TemporaryDirectory() as tmp:
+            cache = Path(tmp) / "fbneo-hashes.json"
+            with mock.patch.object(f, "CACHE_PATH", cache), \
+                    mock.patch.object(f, "_sparse_clone"), \
+                    mock.patch.object(f, "_cleanup"), \
+                    mock.patch.object(f, "_extract_version", return_value=("v1.0.0.03", "abc")), \
+                    mock.patch.object(f, "parse_fbneo_source_tree", return_value={}):
+                with self.assertRaises(RuntimeError):
+                    f.fetch_and_cache(force=True)
+            self.assertFalse(cache.exists())
+
+    def test_fbneo_does_not_serve_an_empty_cache(self):
+        from scraper import fbneo_hash_scraper as f  # noqa: PLC0415
+
+        with tempfile.TemporaryDirectory() as tmp:
+            cache = Path(tmp) / "fbneo-hashes.json"
+            with mock.patch.object(f, "CACHE_PATH", cache):
+                cache.write_text(json.dumps(self._fresh({})))
+                self.assertFalse(f._is_cache_fresh())
+                cache.write_text(json.dumps(self._fresh({"neogeo": {}})))
+                self.assertTrue(f._is_cache_fresh())
+
+
+
+class EmptyScrapeWritesNothing(unittest.TestCase):
+    """An upstream that changed shape parses to nothing: the run fails and
+    the committed platform file stays as it was."""
+
+    def _run(self, scraper_class, output: Path) -> int:
+        from scraper.base_scraper import scraper_cli  # noqa: PLC0415
+
+        argv = ["scraper", "-o", str(output)]
+        with mock.patch.object(sys, "argv", argv), \
+                mock.patch("sys.stderr", new_callable=io.StringIO):
+            try:
+                scraper_cli(scraper_class)
+            except SystemExit as exc:
+                return int(exc.code or 0)
+        return 0
+
+    def test_no_requirement_exits_non_zero(self):
+        with tempfile.TemporaryDirectory() as tmp:
+            output = Path(tmp) / "platform.yml"
+            output.write_text("systems:\n  a:\n    files:\n    - name: x\n")
+            self.assertEqual(self._run(_StubScraper, output), 1)
+            self.assertIn("name: x", output.read_text())
+
+    def test_a_generated_file_without_files_exits_non_zero(self):
+        class NoSystems(_StubScraper):
+            def __init__(self):
+                super().__init__([BiosRequirement(name="a.bin", system="s")])
+
+            def generate_platform_yaml(self) -> dict:
+                return {"platform": "x", "systems": {}}
+
+        with tempfile.TemporaryDirectory() as tmp:
+            output = Path(tmp) / "platform.yml"
+            self.assertEqual(self._run(NoSystems, output), 1)
+            self.assertFalse(output.exists())
+
+    def test_a_platform_that_inherits_its_files_may_list_none(self):
+        class CoresOnly(_StubScraper):
+            lists_files = False
+
+            def generate_platform_yaml(self) -> dict:
+                return {"inherits": "retroarch", "cores": ["mgba"]}
+
+        with tempfile.TemporaryDirectory() as tmp:
+            output = Path(tmp) / "platform.yml"
+            with mock.patch("sys.stdout", new_callable=io.StringIO):
+                self.assertEqual(self._run(CoresOnly, output), 0)
+            self.assertIn("mgba", output.read_text())
+
+    def test_retropie_without_a_package_raises(self):
+        archive = RetroPiePackageList._archive(
+            {"scriptmodules/supplementary/autostart.sh": 'rp_module_id="autostart"\n'}
+        )
+        from scraper.retropie_scraper import Scraper  # noqa: PLC0415
+
+        scraper = Scraper(url="https://codeload.github.com/RetroPie/RetroPie-Setup/tar.gz/" + "0" * 40)
+        with self.assertRaises(ValueError):
+            scraper.module_ids(archive)
+
+
 if __name__ == "__main__":
     unittest.main()

tests/test_target_scrapers.py

@@ -307,7 +307,7 @@ class RetroPieModuleFlags(unittest.TestCase):
         }
 
         def fake_fetch(url, **_kwargs):
-            section = url.rstrip("/").rsplit("/", 1)[-1]
+            section = url.split("?", 1)[0].rstrip("/").rsplit("/", 1)[-1]
             if section in listings:
                 return json.dumps(listings[section])
             return modules[url.split("scriptmodules/", 1)[1]]
@@ -321,6 +321,40 @@ class RetroPieModuleFlags(unittest.TestCase):
 
 
 
+class TargetsReadThePlatformRevision(unittest.TestCase):
+    """A target list read on master filtered packages by flags the pinned
+    package list does not have: every request names the platform's pin."""
+
+    def _urls(self, module, scraper) -> list[str]:
+        seen: list[str] = []
+
+        def record(url, **_kwargs):
+            seen.append(url)
+            if "contents" in url:
+                return json.dumps([{"name": "lr-mgba.sh"}])
+            return 'rp_module_id="lr-mgba"'
+
+        with mock.patch.object(module, "_fetch", record):
+            scraper.fetch_targets()
+        return seen
+
+    def test_retropie_reads_its_codeload_pin(self):
+        pin = retropie_targets_scraper.Scraper.pinned_revision("retropie")
+        self.assertRegex(pin, r"^[0-9a-f]{40}$")
+        urls = self._urls(retropie_targets_scraper, retropie_targets_scraper.Scraper())
+        self.assertTrue(urls)
+        self.assertTrue(all(pin in url for url in urls), urls)
+
+    def test_emudeck_steamos_reads_its_pin(self):
+        pin = emudeck_targets_scraper.Scraper.pinned_revision("emudeck")
+        scraper = emudeck_targets_scraper.Scraper()
+        with mock.patch.object(scraper, "_load_retroarch_cores", return_value=[]):
+            urls = self._urls(emudeck_targets_scraper, scraper)
+        steamos = [u for u in urls if "dragoonDorise" in u]
+        self.assertTrue(steamos)
+        self.assertTrue(all(u.endswith(f"?ref={pin}") for u in steamos), steamos)
+
+
 class HashScrapersRefuseAnUnknownVersion(unittest.TestCase):
     """mame_hash_scraper wrote core_version "unknown" after a failed API call
     and cached it for a day, while the pipeline step still read OK."""