From 8c7c705169779f513fa7355aa69c269726bd862e Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 10:06:11 +0200 Subject: [PATCH] fix: ask the release only for registered names asset_candidates fell back to the bare name when no .gitignore line carried it: every online build asked large-files for aes.zip and five others and printed an HTTP 404 per name, pointing at an upload that never existed. The tests that fetch now register their names. --- scripts/largefiles.py | 8 +++++-- tests/test_large_file_cache.py | 38 ++++++++++++++++++++++++++++++++-- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/scripts/largefiles.py b/scripts/largefiles.py index 2c54fd43..13b09b1a 100644 --- a/scripts/largefiles.py +++ b/scripts/largefiles.py @@ -98,7 +98,8 @@ def asset_candidates(name: str, registered: Iterable[str]) -> list[str]: """Assets that may hold *name*, a registered path or a bare file name. A bare name shared by several registered paths has one asset per path, - and the caller's hash tells them apart. + and the caller's hash tells them apart. A bare name no registered path + carries has none. """ registered = list(registered) if "/" in name: @@ -114,7 +115,10 @@ def asset_candidates(name: str, registered: Iterable[str]) -> list[str]: names[p] for p in sorted(names) if "/.variants/" in p and os.path.basename(p).startswith(name + ".") ] - return matches or [name] + # A bare name no registered path carries is no asset: .gitignore is the + # registry. Asking the release anyway cost a 404 per name on every + # online build and named an upload that never existed. + return matches def fetch_large_file( diff --git a/tests/test_large_file_cache.py b/tests/test_large_file_cache.py index 54a322cf..ed6b9d16 100644 --- a/tests/test_large_file_cache.py +++ b/tests/test_large_file_cache.py @@ -37,6 +37,22 @@ PAYLOAD_B = b"B" * (256 * 1024) # network, so it is cleared here. _ONLINE = mock.patch.dict(os.environ, {"RETROBIOS_OFFLINE": ""}) +# The names these tests fetch, registered as .gitignore registers an asset: +# a bare name no registered path carries is never asked of the release. +TEST_REGISTERED = [ + "bios/Test/asset.bin", + "bios/Test/big.bin", + "bios/Sony/PlayStation 3/PS3UPDAT.PUP", +] + + +def _test_registry(case: unittest.TestCase) -> None: + patcher = mock.patch.object( + largefiles, "load_registered_paths", return_value=TEST_REGISTERED + ) + patcher.start() + case.addCleanup(patcher.stop) + def setUpModule(): _ONLINE.start() @@ -71,6 +87,7 @@ class _SlowResponse(io.BytesIO): class LargeFileCacheTest(unittest.TestCase): def setUp(self): + _test_registry(self) self.dir = tempfile.mkdtemp() self._urlopen = largefiles.urllib.request.urlopen @@ -237,6 +254,9 @@ class LargeFileCacheTest(unittest.TestCase): class TheOfflineSwitchHoldsEverywhere(unittest.TestCase): """RETROBIOS_OFFLINE stops a download whatever the caller asked.""" + def setUp(self): + _test_registry(self) + def test_no_request_is_made(self): with tempfile.TemporaryDirectory() as tmp, mock.patch.dict( os.environ, {"RETROBIOS_OFFLINE": "1"} @@ -600,7 +620,7 @@ class DownloadFailuresAndRevisions(unittest.TestCase): largefiles.urllib.request.urlopen = lambda *_a, **_k: _StalledResponse() result = largefiles.fetch_large_file( "big.bin", self.dir, expected_md5=hashlib.md5(PAYLOAD_A).hexdigest(), - registered=[], + registered=TEST_REGISTERED, ) self.assertIsNone(result) self.assertEqual(os.listdir(self.dir), []) @@ -613,7 +633,7 @@ class DownloadFailuresAndRevisions(unittest.TestCase): ) result = largefiles.fetch_large_file( "PS3UPDAT.PUP", self.dir, expected_md5=hashlib.md5(PAYLOAD_B).hexdigest(), - registered=[], + registered=TEST_REGISTERED, ) self.assertIsNone(result) self.assertTrue(cached.exists()) @@ -629,6 +649,20 @@ class DownloadFailuresAndRevisions(unittest.TestCase): ) +class AnUnregisteredNameIsNoAsset(unittest.TestCase): + """aes.zip and five others are in no .gitignore line: each online build + asked the release for them and printed an HTTP 404 per name.""" + + def test_no_candidate_and_no_request(self): + self.assertEqual(largefiles.asset_candidates("aes.zip", TEST_REGISTERED), []) + with tempfile.TemporaryDirectory() as tmp, \ + mock.patch.object(largefiles.urllib.request, "urlopen") as urlopen: + self.assertIsNone(largefiles.fetch_large_file( + "aes.zip", dest_dir=tmp, offline=False, registered=TEST_REGISTERED, + )) + urlopen.assert_not_called() + + class GameDataHasOneDefinition(unittest.TestCase): """The release notes and the composition table sorted game data apart.