From 9c0a430f5c643eec17d48e97f299de668f83f65d Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Sun, 18 Oct 2026 04:07:57 +0200 Subject: [PATCH] fix: settle export sizes, never write None A truth that accepts several sizes left an export with no size even where the platform states one of them for the same dump, and RomM's fixture then received the string None, which RomM cannot parse. A size list now admits the platform's own size, RomM keeps its value when nothing settles one, and validation requires a number. --- scripts/exporter/baseline.py | 16 +++++++- scripts/exporter/romm_exporter.py | 10 ++++- tests/test_exporters.py | 66 +++++++++++++++++++++++++++++-- 3 files changed, 85 insertions(+), 7 deletions(-) diff --git a/scripts/exporter/baseline.py b/scripts/exporter/baseline.py index 0f5ac5c5..66295d03 100644 --- a/scripts/exporter/baseline.py +++ b/scripts/exporter/baseline.py @@ -188,9 +188,21 @@ class NativeFile: "platform": (self.platform, self.truth), "truth": (self.truth,), }.get(source, (self.truth, self.platform)) + # A list names every size the code accepts (geargrafx's syscard3.pce, + # headerless or headered). It states no single size, but it still + # admits the one the platform writes: System.dat's 262144 for the + # same md5 lost its size field when the list came first. + accepted: set[int] | None = None for entry in order: - if entry and entry.get("size"): - return _single_size(entry["size"]) + if not entry or not entry.get("size"): + continue + single = _single_size(entry["size"]) + if single is None: + if accepted is None: + accepted = {int(v) for v in entry["size"]} + continue + if accepted is None or single in accepted: + return single return None def native(self, key: str, default: object = "") -> object: diff --git a/scripts/exporter/romm_exporter.py b/scripts/exporter/romm_exporter.py index 89b87af2..fcdef4a2 100644 --- a/scripts/exporter/romm_exporter.py +++ b/scripts/exporter/romm_exporter.py @@ -127,7 +127,13 @@ class Exporter(BaseExporter): continue entry: OrderedDict[str, str] = OrderedDict() # The fixture states every value as a string, size included. - entry["size"] = str(fe.size()) + # Where the model settles no size, RomM's own value stands; + # str(None) wrote "None", and RomM's int() on it raised. + size = fe.size() + if size is None and fe.platform is not None: + size = fe.platform.get("size") + if size is not None: + entry["size"] = str(int(size)) crc = fe.record_hash("crc32") if crc: entry["crc"] = crc @@ -159,7 +165,7 @@ class Exporter(BaseExporter): issues.append(f"key without a platform slug: {key}") elif not self._known_platform(slug): issues.append(f"platform slug RomM does not know: {slug}") - if not entry.get("size"): + if not str(entry.get("size", "")).isdigit(): issues.append(f"entry without a size, never verifiable: {key}") if not any(entry.get(h) for h in ("md5", "sha1", "crc")): issues.append(f"entry without a hash, never verifiable: {key}") diff --git a/tests/test_exporters.py b/tests/test_exporters.py index 912f6644..7e99eb78 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -158,6 +158,23 @@ class Reconciliation(unittest.TestCase): ) self.assertEqual(agreeing.size(), 955352) + def test_a_size_list_keeps_the_platform_size_it_accepts(self): + """geargrafx accepts syscard3.pce headerless or headered; System.dat + states 262144 for the same dump. The list came first, stated no + single size, and the rom line went out with none.""" + both = NativeFile( + "syscard3.pce", "syscard3.pce", "pce", + platform={"md5": "c" * 32, "size": 262144}, + truth={"md5": "c" * 32, "size": [262144, 262656]}, + ) + self.assertEqual(both.size(), 262144) + refused = NativeFile( + "syscard3.pce", "syscard3.pce", "pce", + platform={"md5": "c" * 32, "size": 131072}, + truth={"md5": "c" * 32, "size": [262144, 262656]}, + ) + self.assertIsNone(refused.size()) + def test_a_file_the_truth_says_nothing_about_is_kept(self): systems, _ = model() names = {fe.name for fe in systems["psx"].files} @@ -736,13 +753,20 @@ class RommExport(unittest.TestCase): return exporter, systems, exporter.render(systems, report, {}, None) def test_every_entry_can_actually_verify(self): - """RomM matches on size and then on one hash; without both, never.""" - _, _, produced = self._render() + """RomM matches on size and then on one hash; without both, never. + An entry no source gives a size to carries none and validate() names + it: this test passed for one only because it held the string + "None".""" + exporter, systems, produced = self._render() data = json.loads(produced["known_bios_files.json"]) self.assertTrue(data) + issues = exporter.validate(systems, produced) for key, entry in data.items(): with self.subTest(key=key): - self.assertTrue(entry.get("size")) + if "size" in entry: + self.assertTrue(entry["size"].isdigit()) + else: + self.assertIn(f"entry without a size, never verifiable: {key}", issues) self.assertTrue( any(entry.get(h) for h in ("md5", "sha1", "crc")) ) @@ -758,6 +782,42 @@ class RommExport(unittest.TestCase): self.assertTrue(all(k.startswith("psx:") for k in data)) +class RommSizeIsNeverNone(unittest.TestCase): + """size() is None where the truth contradicts RomM's hash without a size + or names several sizes; str(None) went out as "None", validate() passed + it as truthy, and RomM's int("None") raised.""" + + def _export(self, platform_size): + entry = {"name": "scph5501.bin", "destination": "psx/scph5501.bin", + "native_system": "psx", "md5": "a" * 32} + if platform_size is not None: + entry["size"] = platform_size + scraped = {"systems": {"sony-playstation": {"native_id": "psx", "files": [entry]}}} + truth = {"systems": {"sony-playstation": {"files": [{ + "name": "scph5501.bin", "path": "scph5501.bin", "md5": "b" * 32, + "required": True, "_cores": ["beetle_psx"], + }]}}} + systems, report = build_native_model(truth, scraped) + exporter = discover_exporters()["romm"]() + produced = exporter.render(systems, report, {}, None) + return exporter, systems, produced, json.loads(produced["known_bios_files.json"]) + + def test_the_platform_size_stands_when_none_is_settled(self): + _, _, _, data = self._export(524288) + self.assertEqual(data["psx:scph5501.bin"]["size"], "524288") + + def test_no_size_is_written_rather_than_none(self): + exporter, systems, produced, data = self._export(None) + self.assertNotIn("size", data["psx:scph5501.bin"]) + self.assertTrue(any("without a size" in i for i in exporter.validate(systems, produced))) + + def test_validate_refuses_the_string_none(self): + exporter, systems, _, _ = self._export(524288) + broken = {"known_bios_files.json": json.dumps( + {"psx:scph5501.bin": {"size": "None", "md5": "b" * 32}})} + self.assertTrue(any("without a size" in i for i in exporter.validate(systems, broken))) + + class BizhawkExport(unittest.TestCase): SHA1 = "a" * 40