From 4d514336c987e8d68adb9cc96c24ebc9b59d477a Mon Sep 17 00:00:00 2001
From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com>
Date: Wed, 21 Oct 2026 05:14:55 +0200
Subject: [PATCH] fix: close a data dir refresh on its counts

refresh_data_dirs printed one line per entry and no total, a dry run named
the cached tag only, and cutting a quoted ETag at 12 characters kept one
quote. The run now ends on refreshed / up to date / failed counts, a dry
run shows the remote tag beside the cached one, and tags lose their quotes
before they are cut.
---
 scripts/refresh_data_dirs.py    | 34 ++++++++++++++++++++++++++-------
 tests/test_refresh_data_dirs.py | 31 ++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 7 deletions(-)

diff --git a/scripts/refresh_data_dirs.py b/scripts/refresh_data_dirs.py
index 89eaa63d..4c06b995 100644
--- a/scripts/refresh_data_dirs.py
+++ b/scripts/refresh_data_dirs.py
@@ -383,6 +383,14 @@ def _get_remote_etag(source_url: str) -> str | None:
         return None
 
 
+def _short_tag(tag: str | None) -> str:
+    """A commit sha or ETag cut for a log line, quotes dropped first: a
+    quoted ETag cut at 12 characters kept its opening quote only."""
+    if not tag:
+        return "none"
+    return tag.strip().removeprefix("W/").strip('"')[:12] or "none"
+
+
 def refresh_entry(
     key: str,
     entry: dict,
@@ -431,16 +439,17 @@ def _refresh_entry(
         needs_refresh = remote_tag != cached_tag
 
     if not needs_refresh:
-        log.info("[%s] up to date (tag: %s)", key, (cached_tag or "?")[:12])
+        log.info("[%s] up to date (tag: %s)", key, _short_tag(cached_tag))
         return False
 
     if dry_run:
-        log.info(
-            "[%s] would refresh (type: %s, cached: %s)",
-            key,
-            source_type,
-            cached_tag or "none",
-        )
+        if remote_tag is not None:
+            reason = f"cached: {_short_tag(cached_tag)}, remote: {_short_tag(remote_tag)}"
+        elif force:
+            reason = "forced"
+        else:
+            reason = "not cached"
+        log.info("[%s] would refresh (type: %s, %s)", key, source_type, reason)
         return True
 
     if remote_tag is None:
@@ -516,6 +525,16 @@ def refresh_all(
     return results
 
 
+def summarize(outcomes: dict[str, bool | None], dry_run: bool) -> str:
+    """One closing line: how many entries changed, were current, failed."""
+    refreshed = sum(1 for o in outcomes.values() if o is True)
+    current = sum(1 for o in outcomes.values() if o is False)
+    failed = sum(1 for o in outcomes.values() if o is None)
+    verb = "would refresh" if dry_run else "refreshed"
+    line = f"{len(outcomes)} entries: {refreshed} {verb}, {current} up to date"
+    return f"{line}, {failed} failed" if failed else line
+
+
 def main() -> None:
     parser = argparse.ArgumentParser(
         allow_abbrev=False,
@@ -567,6 +586,7 @@ def main() -> None:
             registry, force=args.force, dry_run=args.dry_run, platform=args.platform
         )
 
+    log.info(summarize(outcomes, args.dry_run))
     failed = sorted(key for key, outcome in outcomes.items() if outcome is None)
     if failed:
         log.error("refresh failed: %s", ", ".join(failed))
diff --git a/tests/test_refresh_data_dirs.py b/tests/test_refresh_data_dirs.py
index 4727289a..d737c811 100644
--- a/tests/test_refresh_data_dirs.py
+++ b/tests/test_refresh_data_dirs.py
@@ -166,5 +166,36 @@ class EmulatorPackHoldsTheCache(unittest.TestCase):
             self.assertEqual(held, [True])
 
 
+class DryRunReport(unittest.TestCase):
+    """What a dry run says: why each entry would be fetched, and how many."""
+
+    def test_a_quoted_etag_is_cut_without_its_quotes(self):
+        self.assertEqual(rdd._short_tag('"0x8DF0B72EB52A1C0"'), "0x8DF0B72EB5")
+        self.assertEqual(rdd._short_tag('W/"2685251384"'), "2685251384")
+        self.assertEqual(rdd._short_tag(None), "none")
+
+    def test_the_remote_tag_is_shown_beside_the_cached_one(self):
+        with tempfile.TemporaryDirectory() as tmp:
+            versions = Path(tmp) / "versions.json"
+            versions.write_text(json.dumps({"k": {"etag": '"2725165595"'}}))
+            entry = {"source_type": "zip", "source_url": "https://x.invalid/a.zip",
+                     "local_cache": tmp}
+            with mock.patch.object(rdd, "_get_remote_etag", return_value='"2685251384"'), \
+                    self.assertLogs(rdd.log, "INFO") as logs:
+                self.assertTrue(rdd.refresh_entry("k", entry, dry_run=True,
+                                                  versions_path=str(versions)))
+        self.assertIn("cached: 2725165595, remote: 2685251384", logs.output[0])
+
+    def test_a_run_ends_on_its_counts(self):
+        self.assertEqual(
+            rdd.summarize({"a": True, "b": False, "c": False}, dry_run=True),
+            "3 entries: 1 would refresh, 2 up to date",
+        )
+        self.assertEqual(
+            rdd.summarize({"a": True, "b": None}, dry_run=False),
+            "2 entries: 1 refreshed, 0 up to date, 1 failed",
+        )
+
+
 if __name__ == "__main__":
     unittest.main()