diff --git a/scripts/check_freshness.py b/scripts/check_freshness.py index ab765ab8..00b68eaf 100644 --- a/scripts/check_freshness.py +++ b/scripts/check_freshness.py @@ -374,6 +374,48 @@ def native_cache_state( return "" +def _export_tree(root: Path, platform: str) -> dict[str, bytes]: + """A platform's published export: its directory and its report.""" + tree = { + path.relative_to(root).as_posix(): path.read_bytes() + for path in (root / platform).rglob("*") if path.is_file() + } + report = root / f"{platform}.json" + if report.is_file(): + tree[report.name] = report.read_bytes() + return tree + + +def published_export_state( + platform: str, + exporter_class: type, + truth_dir: Path, + platforms_dir: Path, + cache_dir: Path, + published: Path, +) -> str: + """Why the committed export is not what the export writes now, '' when it is. + + exports/ is committed and the site publishes it, but CI cannot rebuild + it: the originals it patches live in the local cache. A profile change + that moves a hash leaves the published file behind until someone runs + the export again, and this is what says so. + """ + scratch_root = Path("tmp") + scratch_root.mkdir(exist_ok=True) + with tempfile.TemporaryDirectory(dir=scratch_root) as scratch: + with contextlib.redirect_stdout(io.StringIO()): + ok, messages = export_native.export_platform( + platform, exporter_class, truth_dir, Path(scratch), + str(platforms_dir), cache_dir, False, + ) + if not ok: + return f"the export fails: {messages[0] if messages else 'no message'}" + if _export_tree(Path(scratch), platform) != _export_tree(published, platform): + return f"{published.as_posix()}/{platform} is not what the export writes now" + return "" + + def _transcribed_at(platforms_dir: Path, platform: str) -> float: """Last rewrite of the platform file or of any file it inherits.""" newest = 0.0 @@ -390,7 +432,10 @@ def _transcribed_at(platforms_dir: Path, platform: str) -> float: return newest -def check_native(platforms_dir: Path, cache_dir: Path, truth_dir: Path) -> list[Finding]: +def check_native( + platforms_dir: Path, cache_dir: Path, truth_dir: Path, + published: Path = Path("exports"), +) -> list[Finding]: exporters = discover_exporters() index = cache_dir / export_native.SOURCES_INDEX recorded = export_native.load_sources(cache_dir) @@ -409,13 +454,21 @@ def check_native(platforms_dir: Path, cache_dir: Path, truth_dir: Path) -> list[ reason = native_cache_state( wanted, cache_dir / name, recorded, index, _transcribed_at(platforms_dir, name) ) + if reason: + reason += ", export_native.py --refresh-cache fetches it" + else: + reason = published_export_state( + name, exporter_class, truth_dir, platforms_dir, cache_dir, published + ) + if reason: + reason += f", export_native.py --platform {name} writes it again" findings.append( Finding( "native", name, STALE if reason else OK, local=f"{len(wanted)} file(s)", - detail=f"{reason}, export_native.py --refresh-cache fetches it" if reason else "", + detail=reason, ) ) return sorted(findings, key=lambda f: f.subject) diff --git a/scripts/refresh_stale.py b/scripts/refresh_stale.py index bb90263b..8712b101 100755 --- a/scripts/refresh_stale.py +++ b/scripts/refresh_stale.py @@ -14,7 +14,8 @@ Mapping (stale -> command): the export patches must be the one just scraped native/ python scripts/generate_truth.py --platform , then python scripts/export_native.py --platform - --refresh-cache + --refresh-cache, then the export itself, which + rewrites exports/ targets/ python -m scripts.scraper.targets. -o platforms/targets/.yml data/ python scripts/refresh_data_dirs.py --key @@ -172,9 +173,13 @@ def plan_jobs( if area == "platforms": # The truth reads the platform file: regenerated first, or the # native refresh refuses it as older than the profiles. - then = (tuple(_truth_refresh(subject)), tuple(_native_refresh(subject))) + then = ( + tuple(_truth_refresh(subject)), + tuple(_native_refresh(subject)), + tuple(_export_refresh(subject)), + ) elif area == "native": - then = (tuple(_native_refresh(subject)),) + then = (tuple(_native_refresh(subject)), tuple(_export_refresh(subject))) jobs.append( Job(area=area, subject=subject, command=command, log_path=log, then=then) ) @@ -196,6 +201,11 @@ def _native_refresh(platform: str) -> list[str]: ] +def _export_refresh(platform: str) -> list[str]: + """Rewrite the published export from the refreshed original.""" + return [sys.executable, "scripts/export_native.py", "--platform", platform] + + def _command_for( area: str, subject: str, registry: dict[str, dict] ) -> list[str] | None: diff --git a/tests/test_check_freshness.py b/tests/test_check_freshness.py index 08291331..30ecf390 100644 --- a/tests/test_check_freshness.py +++ b/tests/test_check_freshness.py @@ -12,6 +12,7 @@ import os import sys import tempfile import unittest +from unittest import mock from pathlib import Path REPO_ROOT = Path(__file__).resolve().parents[1] @@ -87,6 +88,39 @@ class NativeCacheTests(unittest.TestCase): self.assertIn("batocera", {r.subject for r in rows}) +class PublishedExportTests(unittest.TestCase): + """exports/ is committed and published, and CI cannot rebuild it.""" + + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.addCleanup(self._tmp.cleanup) + self.published = Path(self._tmp.name) + (self.published / "plat").mkdir() + (self.published / "plat" / "list.txt").write_text("a", encoding="utf-8") + (self.published / "plat.json").write_text("{}", encoding="utf-8") + + def _state(self, written: str, ok: bool = True) -> str: + def fake(platform, _cls, _truth, output, *_rest): + (output / platform).mkdir(parents=True) + (output / platform / "list.txt").write_text(written, encoding="utf-8") + (output / f"{platform}.json").write_text("{}", encoding="utf-8") + return ok, [] if ok else ["the platform's own file is required"] + + with mock.patch.object(cf.export_native, "export_platform", fake): + return cf.published_export_state( + "plat", object, Path("t"), Path("p"), Path("c"), self.published + ) + + def test_the_same_bytes_are_fresh(self): + self.assertEqual(self._state("a"), "") + + def test_a_moved_hash_is_named(self): + self.assertIn("is not what the export writes now", self._state("b")) + + def test_an_export_that_fails_is_named(self): + self.assertIn("the export fails", self._state("a", ok=False)) + + class PlatformDiffTests(unittest.TestCase): def _platform(self, version="1", files=None, cores=None): return { diff --git a/tests/test_refresh_stale.py b/tests/test_refresh_stale.py index 42ecb139..691f9f9a 100644 --- a/tests/test_refresh_stale.py +++ b/tests/test_refresh_stale.py @@ -46,7 +46,8 @@ class Planning(unittest.TestCase): self.assertEqual( [list(c[1:]) for c in jobs[0].then], [["scripts/generate_truth.py", "--platform", "retrobat"], - ["scripts/export_native.py", "--platform", "retrobat", "--refresh-cache"]], + ["scripts/export_native.py", "--platform", "retrobat", "--refresh-cache"], + ["scripts/export_native.py", "--platform", "retrobat"]], ) def test_a_native_row_of_a_rescraped_platform_is_not_a_second_job(self): @@ -64,7 +65,10 @@ class Planning(unittest.TestCase): ) self.assertEqual( [list(c[1:]) for c in jobs[0].then], - [["scripts/export_native.py", "--platform", "lakka", "--refresh-cache"]], + # exports/ is published: the corrected file is written again + # from the refreshed original. + [["scripts/export_native.py", "--platform", "lakka", "--refresh-cache"], + ["scripts/export_native.py", "--platform", "lakka"]], ) def test_a_platform_without_a_scraper_is_surfaced(self):