From 00f10b0379ca9e4ac33a28fa1bb1d270dc2a25c8 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Tue, 11 Aug 2026 19:54:45 +0200 Subject: [PATCH] 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. --- scripts/truth.py | 72 ++++++++++++++++++----------- tests/test_truth_diff.py | 98 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+), 26 deletions(-) create mode 100644 tests/test_truth_diff.py diff --git a/scripts/truth.py b/scripts/truth.py index f61fc14f..4a234fec 100644 --- a/scripts/truth.py +++ b/scripts/truth.py @@ -422,6 +422,49 @@ def generate_platform_truth( # 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: """Compare files between truth and scraped for a single system.""" # 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 } - # 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 + rename_matched_truth, rename_matched_scraped = _match_renames( + unmatched_truth, unmatched_scraped + ) # Truth files not matched (by name, alias, or hash) -> missing for fe in unmatched_truth: diff --git a/tests/test_truth_diff.py b/tests/test_truth_diff.py new file mode 100644 index 00000000..19d0fb30 --- /dev/null +++ b/tests/test_truth_diff.py @@ -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()