mirror of
https://github.com/Abdess/retroarch_system.git
synced 2026-10-15 07:53:23 -05:00
refactor: extract rename matching from the truth diff
_diff_system reached complexity 42, and its hash-based rename fallback is the part that stands alone: a platform is free to call a file whatever it likes, so a name matching nothing is not yet a gap, and counting one file as both missing and extra invents a discrepancy. That step is now its own function at complexity 31, with the tests it never had: pairing on any of the three digests, case folding, non-string values, and the case where two files simply have no hashes and so are not evidence of anything. diff_truth output is unchanged.
This commit is contained in:
1 parent
7a9e227e95
commit
00f10b0379
2 files changed
+144
-26
No files matched your search
+46
-26
@@ -422,6 +422,49 @@ def generate_platform_truth(
|
|||||||
# Platform truth diffing
|
# Platform truth diffing
|
||||||
|
|
||||||
|
|
||||||
|
def _match_renames(
|
||||||
|
unmatched_truth: list[dict], unmatched_scraped: dict
|
||||||
|
) -> tuple[set[str], set[str]]:
|
||||||
|
"""Pair files a platform renamed with the truth entry they came from.
|
||||||
|
|
||||||
|
A platform is free to call a file whatever it likes -- Batocera ships
|
||||||
|
the same ROM as ROM1 -- so a name that matches nothing is not yet a
|
||||||
|
gap. When an unmatched file on either side carries the same hash, it
|
||||||
|
is one file under two names, and counting it as both missing and extra
|
||||||
|
would report a gap that does not exist.
|
||||||
|
|
||||||
|
Returns the truth names and scraped keys that pair up.
|
||||||
|
"""
|
||||||
|
# Hash-based fallback: detect platform renames (e.g. Batocera ROM → ROM1)
|
||||||
|
# If an unmatched scraped file shares a hash with an unmatched truth file,
|
||||||
|
# it's the same file under a different name — a platform rename, not a gap.
|
||||||
|
rename_matched_truth: set[str] = set()
|
||||||
|
rename_matched_scraped: set[str] = set()
|
||||||
|
|
||||||
|
if unmatched_truth and unmatched_scraped:
|
||||||
|
# Build hash → truth file index for unmatched truth files
|
||||||
|
truth_hash_index: dict[str, dict] = {}
|
||||||
|
for fe in unmatched_truth:
|
||||||
|
for h in ("sha1", "md5", "crc32"):
|
||||||
|
val = fe.get(h)
|
||||||
|
if val and isinstance(val, str):
|
||||||
|
truth_hash_index[val.lower()] = fe
|
||||||
|
|
||||||
|
for s_key, s_entry in unmatched_scraped.items():
|
||||||
|
for h in ("sha1", "md5", "crc32"):
|
||||||
|
s_val = s_entry.get(h)
|
||||||
|
if not s_val or not isinstance(s_val, str):
|
||||||
|
continue
|
||||||
|
t_entry = truth_hash_index.get(s_val.lower())
|
||||||
|
if t_entry is not None:
|
||||||
|
# Rename detected — count as matched
|
||||||
|
rename_matched_truth.add(t_entry["name"].lower())
|
||||||
|
rename_matched_scraped.add(s_key)
|
||||||
|
break
|
||||||
|
|
||||||
|
return rename_matched_truth, rename_matched_scraped
|
||||||
|
|
||||||
|
|
||||||
def _diff_system(truth_sys: dict, scraped_sys: dict) -> dict:
|
def _diff_system(truth_sys: dict, scraped_sys: dict) -> dict:
|
||||||
"""Compare files between truth and scraped for a single system."""
|
"""Compare files between truth and scraped for a single system."""
|
||||||
# Build truth index: name.lower() -> entry, alias.lower() -> entry
|
# Build truth index: name.lower() -> entry, alias.lower() -> entry
|
||||||
@@ -498,32 +541,9 @@ def _diff_system(truth_sys: dict, scraped_sys: dict) -> dict:
|
|||||||
if s_key not in truth_index
|
if s_key not in truth_index
|
||||||
}
|
}
|
||||||
|
|
||||||
# Hash-based fallback: detect platform renames (e.g. Batocera ROM → ROM1)
|
rename_matched_truth, rename_matched_scraped = _match_renames(
|
||||||
# If an unmatched scraped file shares a hash with an unmatched truth file,
|
unmatched_truth, unmatched_scraped
|
||||||
# it's the same file under a different name — a platform rename, not a gap.
|
)
|
||||||
rename_matched_truth: set[str] = set()
|
|
||||||
rename_matched_scraped: set[str] = set()
|
|
||||||
|
|
||||||
if unmatched_truth and unmatched_scraped:
|
|
||||||
# Build hash → truth file index for unmatched truth files
|
|
||||||
truth_hash_index: dict[str, dict] = {}
|
|
||||||
for fe in unmatched_truth:
|
|
||||||
for h in ("sha1", "md5", "crc32"):
|
|
||||||
val = fe.get(h)
|
|
||||||
if val and isinstance(val, str):
|
|
||||||
truth_hash_index[val.lower()] = fe
|
|
||||||
|
|
||||||
for s_key, s_entry in unmatched_scraped.items():
|
|
||||||
for h in ("sha1", "md5", "crc32"):
|
|
||||||
s_val = s_entry.get(h)
|
|
||||||
if not s_val or not isinstance(s_val, str):
|
|
||||||
continue
|
|
||||||
t_entry = truth_hash_index.get(s_val.lower())
|
|
||||||
if t_entry is not None:
|
|
||||||
# Rename detected — count as matched
|
|
||||||
rename_matched_truth.add(t_entry["name"].lower())
|
|
||||||
rename_matched_scraped.add(s_key)
|
|
||||||
break
|
|
||||||
|
|
||||||
# Truth files not matched (by name, alias, or hash) -> missing
|
# Truth files not matched (by name, alias, or hash) -> missing
|
||||||
for fe in unmatched_truth:
|
for fe in unmatched_truth:
|
||||||
|
|||||||
@@ -0,0 +1,98 @@
|
|||||||
|
#!/usr/bin/env python3
|
||||||
|
"""Rename detection in the truth-vs-platform diff.
|
||||||
|
|
||||||
|
A platform may call a file whatever it likes: Batocera ships the same Sega CD
|
||||||
|
ROM as ROM1. A name that matches nothing in the profile is therefore not yet a
|
||||||
|
gap, and reporting it as both a missing file and an extra one would invent a
|
||||||
|
discrepancy that does not exist. Content decides, as everywhere else here.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import sys
|
||||||
|
import unittest
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||||
|
sys.path.insert(0, str(REPO_ROOT / "scripts"))
|
||||||
|
|
||||||
|
from truth import _match_renames # noqa: E402
|
||||||
|
|
||||||
|
|
||||||
|
def _entry(name: str, **hashes) -> dict:
|
||||||
|
return {"name": name, **hashes}
|
||||||
|
|
||||||
|
|
||||||
|
class RenameMatching(unittest.TestCase):
|
||||||
|
def test_a_shared_sha1_pairs_the_two_names(self):
|
||||||
|
truth = [_entry("bios_CD_U.bin", sha1="a" * 40)]
|
||||||
|
scraped = {"rom1.bin": _entry("ROM1.bin", sha1="a" * 40)}
|
||||||
|
matched_truth, matched_scraped = _match_renames(truth, scraped)
|
||||||
|
self.assertEqual(matched_truth, {"bios_cd_u.bin"})
|
||||||
|
self.assertEqual(matched_scraped, {"rom1.bin"})
|
||||||
|
|
||||||
|
def test_md5_and_crc32_pair_them_too(self):
|
||||||
|
for algorithm in ("md5", "crc32"):
|
||||||
|
with self.subTest(algorithm=algorithm):
|
||||||
|
value = "b" * 32
|
||||||
|
truth = [_entry("a.bin", **{algorithm: value})]
|
||||||
|
scraped = {"b.bin": _entry("b.bin", **{algorithm: value})}
|
||||||
|
matched_truth, _ = _match_renames(truth, scraped)
|
||||||
|
self.assertEqual(matched_truth, {"a.bin"})
|
||||||
|
|
||||||
|
def test_hash_comparison_ignores_case(self):
|
||||||
|
truth = [_entry("a.bin", sha1="AbCdEf" + "0" * 34)]
|
||||||
|
scraped = {"b.bin": _entry("b.bin", sha1="abcdef" + "0" * 34)}
|
||||||
|
self.assertEqual(_match_renames(truth, scraped)[0], {"a.bin"})
|
||||||
|
|
||||||
|
def test_different_content_is_not_a_rename(self):
|
||||||
|
truth = [_entry("a.bin", sha1="a" * 40)]
|
||||||
|
scraped = {"b.bin": _entry("b.bin", sha1="b" * 40)}
|
||||||
|
self.assertEqual(_match_renames(truth, scraped), (set(), set()))
|
||||||
|
|
||||||
|
def test_files_with_no_hashes_never_pair(self):
|
||||||
|
"""Two unknowns are not evidence of the same file."""
|
||||||
|
truth = [_entry("a.bin")]
|
||||||
|
scraped = {"b.bin": _entry("b.bin")}
|
||||||
|
self.assertEqual(_match_renames(truth, scraped), (set(), set()))
|
||||||
|
|
||||||
|
def test_a_non_string_hash_is_ignored_rather_than_crashing(self):
|
||||||
|
truth = [_entry("a.bin", crc32=12345678)]
|
||||||
|
scraped = {"b.bin": _entry("b.bin", crc32=12345678)}
|
||||||
|
self.assertEqual(_match_renames(truth, scraped), (set(), set()))
|
||||||
|
|
||||||
|
def test_empty_input_on_either_side_pairs_nothing(self):
|
||||||
|
self.assertEqual(_match_renames([], {}), (set(), set()))
|
||||||
|
self.assertEqual(_match_renames([_entry("a", sha1="a" * 40)], {}), (set(), set()))
|
||||||
|
self.assertEqual(
|
||||||
|
_match_renames([], {"b": _entry("b", sha1="a" * 40)}), (set(), set())
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_each_scraped_file_pairs_once(self):
|
||||||
|
"""A file matching on several digests must not be counted twice."""
|
||||||
|
truth = [_entry("a.bin", sha1="a" * 40, md5="b" * 32)]
|
||||||
|
scraped = {"b.bin": _entry("b.bin", sha1="a" * 40, md5="b" * 32)}
|
||||||
|
matched_truth, matched_scraped = _match_renames(truth, scraped)
|
||||||
|
self.assertEqual(len(matched_truth), 1)
|
||||||
|
self.assertEqual(len(matched_scraped), 1)
|
||||||
|
|
||||||
|
def test_several_renames_are_all_reported(self):
|
||||||
|
truth = [_entry("a.bin", sha1="a" * 40), _entry("c.bin", sha1="c" * 40)]
|
||||||
|
scraped = {
|
||||||
|
"b.bin": _entry("b.bin", sha1="a" * 40),
|
||||||
|
"d.bin": _entry("d.bin", sha1="c" * 40),
|
||||||
|
}
|
||||||
|
matched_truth, matched_scraped = _match_renames(truth, scraped)
|
||||||
|
self.assertEqual(matched_truth, {"a.bin", "c.bin"})
|
||||||
|
self.assertEqual(matched_scraped, {"b.bin", "d.bin"})
|
||||||
|
|
||||||
|
def test_an_unrelated_file_beside_a_rename_stays_unmatched(self):
|
||||||
|
truth = [_entry("a.bin", sha1="a" * 40), _entry("lonely.bin", sha1="f" * 40)]
|
||||||
|
scraped = {"b.bin": _entry("b.bin", sha1="a" * 40)}
|
||||||
|
matched_truth, _ = _match_renames(truth, scraped)
|
||||||
|
self.assertEqual(matched_truth, {"a.bin"})
|
||||||
|
self.assertNotIn("lonely.bin", matched_truth)
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
Reference in new issue
Block a user