From ae0b23a80101a90735669d001a3d86d92e044b92 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Fri, 6 Nov 2026 14:46:17 +0100 Subject: [PATCH] fix: show the check's own error in refresh_stale --- scripts/refresh_stale.py | 28 +++++++++++++++++++--------- tests/test_refresh_stale.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/scripts/refresh_stale.py b/scripts/refresh_stale.py index bf65bb6ac..b50e472d8 100755 --- a/scripts/refresh_stale.py +++ b/scripts/refresh_stale.py @@ -148,18 +148,28 @@ def _run_check_freshness(areas: tuple[str, ...], extra: list[str]) -> list[dict] if areas and set(areas) != set(AUTO_AREAS + MANUAL_AREAS): cmd += ["--only", ",".join(areas)] cmd += extra - proc = subprocess.run( - cmd, cwd=REPO_ROOT, capture_output=True, text=True, check=False, timeout=1800 - ) - if proc.returncode not in (0, 1): + try: + proc = subprocess.run( + cmd, cwd=REPO_ROOT, capture_output=True, text=True, check=False, timeout=1800 + ) + except subprocess.TimeoutExpired as exc: + raise RuntimeError( + f"check_freshness.py did not finish in {exc.timeout:.0f} s: run " + f"`{' '.join(cmd[1:])} > report.json` alone, then pass --findings report.json" + ) from exc + # Exit 1 is the STALE rows, and also a crash: only a JSON list on stdout + # tells them apart, and the crash's own message is on stderr. + try: + findings = json.loads(proc.stdout) if proc.returncode in (0, 1) else None + except json.JSONDecodeError: + findings = None + if not isinstance(findings, list): tail = (proc.stderr or proc.stdout).strip().splitlines()[-3:] raise RuntimeError( - "check_freshness.py failed:\n" + "\n".join(tail or ["(no output)"]) + f"check_freshness.py failed (exit {proc.returncode}):\n" + + "\n".join(tail or ["(no output)"]) ) - try: - return json.loads(proc.stdout) - except json.JSONDecodeError as exc: - raise RuntimeError(f"check_freshness.py returned unreadable JSON: {exc}") from exc + return findings def load_findings(path: Path, areas: tuple[str, ...]) -> list[dict]: diff --git a/tests/test_refresh_stale.py b/tests/test_refresh_stale.py index e92887a66..1830b2f55 100644 --- a/tests/test_refresh_stale.py +++ b/tests/test_refresh_stale.py @@ -8,10 +8,12 @@ by the refresh of the original its export patches. from __future__ import annotations +import subprocess import sys import tempfile import unittest from pathlib import Path +from unittest import mock REPO_ROOT = Path(__file__).resolve().parents[1] sys.path.insert(0, str(REPO_ROOT / "scripts")) @@ -279,5 +281,31 @@ class SavedFindings(unittest.TestCase): rs.load_findings(path, ("data",)) +class AFailedCheckIsNamed(unittest.TestCase): + """A crash of check_freshness exits 1 like a STALE row: its traceback, + on stderr, was dropped for "unreadable JSON", and a timeout ended on + a traceback of refresh_stale itself.""" + + def _run(self, raises: Exception | None = None, **result): + proc = subprocess.CompletedProcess([], **result) if result else None + with mock.patch.object(rs.subprocess, "run", side_effect=raises, return_value=proc): + return rs._run_check_freshness(("coreinfo",), []) # noqa: SLF001 + + def test_a_crash_shows_its_stderr(self): + with self.assertRaises(RuntimeError) as raised: + self._run(returncode=1, stdout="", stderr="Traceback\nRuntimeError: no listing\n") + self.assertIn("RuntimeError: no listing", str(raised.exception)) + self.assertNotIn("JSON", str(raised.exception)) + + def test_stale_rows_are_read(self): + self.assertEqual(self._run(returncode=1, stdout='[{"area": "coreinfo"}]', stderr=""), + [{"area": "coreinfo"}]) + + def test_a_timeout_says_what_to_run(self): + with self.assertRaises(RuntimeError) as raised: + self._run(raises=subprocess.TimeoutExpired(["x"], 1800)) + self.assertIn("--findings", str(raised.exception)) + + if __name__ == "__main__": unittest.main()