From 7fe9d184c49448717cd51eb6d2575d111f59dc09 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 21:14:35 -0400 Subject: [PATCH 1/3] test: make plugin test results mean something Every one of these standalone scripts signals only through an exit code, and without a shared convention "not applicable here" was indistinguishable from "broken". That cost real time: the same seven baseball/football "failures" were re-baselined three separate times in one session to prove a change had not caused them. Categorising them properly, the seven were: 3 only a missing RGBMatrixEmulator in the runner's virtualenv -- they pass in the core venv 1 a deliberate skip (test_score_antialiasing already exits 2) 2 interactive/hardware scripts: one prompts on stdin, one drives a real or emulated matrix and loads the core config template 1 genuinely broken That last one is the reason this matters. test_test_mode_live_games raised a _StopAfterTestUpdate sentinel from its fake, but suppressed only AttributeError -- so the sentinel escaped and the test died before reaching a single assertion. It had been silently verifying nothing for as long as it had been "failing", hidden in noise everyone had learned to ignore. Fixed by suppressing both the sentinel and the buggy path's AttributeError; it now runs its invariants and passes. The convention: 0 pass, 2 skip (prerequisites absent), 1 fail. Scripts opt in by printing "SKIP: " and exiting 2. scripts/run_plugin_tests.py runs a plugin's scripts and reports pass/skip/fail, exiting non-zero only on real failures. The two interactive scripts now pre-flight their prerequisites and skip instead of failing. baseball + football: 21 passed, 3 skipped, 0 failed -- a clean signal for the first time. Fleet-wide `--all` reports 63 passed, 3 skipped, 7 failed. Those seven are left for a follow-up and are now legible rather than lumped together: two are "not applicable" (a font that ships with the core, a missing optional astral dep) and five look like genuine staleness -- a test importing a class name that no longer exists, a MockLogger without setLevel, a CacheManager called with a keyword the core no longer takes, a bare cache_manager import, and two lacrosse assertions. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 --- .../test_baseball_plugin.py | 14 +++ .../test_test_mode_live_games.py | 15 ++- .../test_football_plugin.py | 15 +++ scripts/run_plugin_tests.py | 115 ++++++++++++++++++ 4 files changed, 154 insertions(+), 5 deletions(-) create mode 100644 scripts/run_plugin_tests.py diff --git a/plugins/baseball-scoreboard/test_baseball_plugin.py b/plugins/baseball-scoreboard/test_baseball_plugin.py index 921f5fdc..fdb60181 100755 --- a/plugins/baseball-scoreboard/test_baseball_plugin.py +++ b/plugins/baseball-scoreboard/test_baseball_plugin.py @@ -384,6 +384,17 @@ def main(): if __name__ == "__main__": + # Pre-flight. This script drives a real or emulated matrix and loads the + # core's config template relative to the working directory, so outside a + # LEDMatrix checkout it cannot run at all. Skip (exit 2) rather than fail + # (exit 1): reporting "not applicable" as a regression is what trained + # everyone to ignore this plugin's results. See + # scripts/run_plugin_tests.py for the exit-code convention. + if not os.path.exists(os.path.join("config", "config.template.json")): + print("SKIP: needs the core config template — run from a LEDMatrix " + "checkout (cwd is %s)" % os.getcwd()) + sys.exit(2) + try: success = main() sys.exit(0 if success else 1) @@ -391,6 +402,9 @@ def main(): print("\n\n[STOP] Tests interrupted by user") sys.exit(1) except Exception as e: + if "fallback mode" in str(e).lower(): + print(f"SKIP: needs a real or emulated LED matrix ({e})") + sys.exit(2) print(f"\n[FAIL] Unexpected error: {e}") import traceback traceback.print_exc() diff --git a/plugins/baseball-scoreboard/test_test_mode_live_games.py b/plugins/baseball-scoreboard/test_test_mode_live_games.py index 1937869c..da445a8b 100644 --- a/plugins/baseball-scoreboard/test_test_mode_live_games.py +++ b/plugins/baseball-scoreboard/test_test_mode_live_games.py @@ -80,11 +80,16 @@ def fake_test_update(): live._fetch_data = fake_fetch live._test_mode_update = fake_test_update - # Guard update(): without the fix it enters the live-fetch path and trips over - # attributes a bare instance doesn't have. Suppress only that expected - # AttributeError so an unrelated failure still surfaces; the invariants below - # assert the real behavior regardless of how far the buggy path gets. - with contextlib.suppress(AttributeError): + # Guard update() on two expected exits, and only those, so an unrelated + # failure still surfaces: + # _StopAfterTestUpdate - the fixed path; fake_test_update raises it to stop + # update() right after the short-circuit branch is taken. + # AttributeError - the buggy path; it enters the live fetch and trips + # over attributes a bare instance doesn't have. + # The invariants below assert the real behaviour either way. Suppressing only + # AttributeError (as this did) meant the sentinel escaped uncaught and the + # test died before reaching a single assertion. + with contextlib.suppress(AttributeError, _StopAfterTestUpdate): live.update() assert called["test_update"], "expected _test_mode_update() to run in test mode" diff --git a/plugins/football-scoreboard/test_football_plugin.py b/plugins/football-scoreboard/test_football_plugin.py index bf874a29..9e87123f 100644 --- a/plugins/football-scoreboard/test_football_plugin.py +++ b/plugins/football-scoreboard/test_football_plugin.py @@ -423,5 +423,20 @@ def main(): return 1 +def _skip_if_not_interactive(reason): + """Exit 2 (skip) rather than 1 (fail) when this script's prerequisites are + absent. It drives a real/emulated LED matrix and prompts on stdin, so a + headless or non-tty run is "not applicable", not "broken" -- and reporting + it as a failure trained everyone to ignore this plugin's test results. + See scripts/run_plugin_tests.py for the exit-code convention. + """ + import sys as _sys + print(f"SKIP: {reason}") + _sys.exit(2) + + if __name__ == "__main__": + if not sys.stdin.isatty(): + _skip_if_not_interactive( + "needs an interactive terminal (this script prompts on stdin)") sys.exit(main()) diff --git a/scripts/run_plugin_tests.py b/scripts/run_plugin_tests.py new file mode 100644 index 00000000..e17ad403 --- /dev/null +++ b/scripts/run_plugin_tests.py @@ -0,0 +1,115 @@ +#!/usr/bin/env python3 +"""Run a plugin's `test_*.py` scripts and report pass / skip / fail honestly. + + python scripts/run_plugin_tests.py baseball-scoreboard + python scripts/run_plugin_tests.py --all + python scripts/run_plugin_tests.py baseball-scoreboard --core /path/to/LEDMatrix + +## Why this exists + +These are standalone scripts, not a pytest suite, and they signal only through +an exit code. Without a shared convention every non-zero exit looks the same, +so a script that is *not applicable* here — it wants a tty, or an LED matrix, +or a font that ships with the core — was indistinguishable from a real +regression. + +That cost real time: during one session the same seven "failures" were +re-baselined three separate times to prove a change hadn't caused them. Three +of the seven were only a missing `RGBMatrixEmulator` in the runner's +virtualenv, one was a deliberate skip, two were interactive scripts, and +exactly one was a genuinely broken test — which had been silently +non-functional, dying before its first assertion, for as long as it had been +"failing". + +Noise that everyone learns to ignore is worse than no signal at all, because a +real regression hides in it. + +## The convention + + 0 pass + 2 skip — prerequisites absent (no tty, no matrix, no font). Not a failure. + 1 fail — a genuine problem. Anything else is treated as a failure too. + +Scripts opt into skipping by printing `SKIP: ` and exiting 2. +""" + +from __future__ import annotations + +import argparse +import os +import subprocess +import sys +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[1] +PLUGINS = REPO_ROOT / "plugins" + +PASS, FAIL, SKIP = 0, 1, 2 + + +def run_one(script: Path, core: Path | None, timeout: int) -> tuple[int, str]: + env = dict(os.environ) + if core: + env["PYTHONPATH"] = f"{core}{os.pathsep}{env.get('PYTHONPATH', '')}" + try: + proc = subprocess.run( + [sys.executable, script.name], + cwd=script.parent, env=env, capture_output=True, + text=True, timeout=timeout, stdin=subprocess.DEVNULL, + ) + except subprocess.TimeoutExpired: + return FAIL, f"timed out after {timeout}s" + + if proc.returncode == SKIP: + for line in proc.stdout.splitlines(): + if line.startswith("SKIP:"): + return SKIP, line[len("SKIP:"):].strip() + return SKIP, "skipped" + if proc.returncode == PASS: + return PASS, "" + + tail = [ln for ln in (proc.stdout + proc.stderr).splitlines() if ln.strip()] + return FAIL, (tail[-1][:120] if tail else f"exit {proc.returncode}") + + +def main() -> int: + ap = argparse.ArgumentParser(description=__doc__) + ap.add_argument("plugin_ids", nargs="*") + ap.add_argument("--all", action="store_true", help="Every plugin") + ap.add_argument("--core", type=Path, default=None, + help="Path to a LEDMatrix checkout to put on PYTHONPATH") + ap.add_argument("--timeout", type=int, default=180) + args = ap.parse_args() + + ids = (sorted(p.name for p in PLUGINS.iterdir() if p.is_dir()) + if args.all else args.plugin_ids) + if not ids: + ap.error("give plugin ids or --all") + + totals = {PASS: 0, SKIP: 0, FAIL: 0} + failures: list[str] = [] + + for pid in ids: + scripts = sorted((PLUGINS / pid).glob("test_*.py")) + if not scripts: + continue + print(f"\n{pid}") + for script in scripts: + status, detail = run_one(script, args.core, args.timeout) + totals[status] += 1 + label = {PASS: "pass", SKIP: "SKIP", FAIL: "FAIL"}[status] + print(f" [{label}] {script.name}" + (f" -- {detail}" if detail else "")) + if status == FAIL: + failures.append(f"{pid}/{script.name}: {detail}") + + print(f"\n{totals[PASS]} passed, {totals[SKIP]} skipped, {totals[FAIL]} failed") + if failures: + print("\nFailures:", file=sys.stderr) + for f in failures: + print(f" - {f}", file=sys.stderr) + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) From bd9b6e5494ba5655252a3fb572f1d13e77562291 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 14:31:43 -0400 Subject: [PATCH 2/3] docs(scripts): say which interpreter run_plugin_tests needs Running it under a bare `python3` that lacks the plugins' dependencies reports every suite as failed -- 22 failures where the only thing wrong was the interpreter. Exactly the misclassification this script was written to end, so it should not be the thing that causes it. Scripts are spawned with sys.executable, and --core only sets PYTHONPATH. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 --- scripts/run_plugin_tests.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/scripts/run_plugin_tests.py b/scripts/run_plugin_tests.py index e17ad403..c98a6281 100644 --- a/scripts/run_plugin_tests.py +++ b/scripts/run_plugin_tests.py @@ -1,9 +1,15 @@ #!/usr/bin/env python3 """Run a plugin's `test_*.py` scripts and report pass / skip / fail honestly. - python scripts/run_plugin_tests.py baseball-scoreboard - python scripts/run_plugin_tests.py --all - python scripts/run_plugin_tests.py baseball-scoreboard --core /path/to/LEDMatrix + /bin/python scripts/run_plugin_tests.py baseball-scoreboard \ + --core /path/to/LEDMatrix + /bin/python scripts/run_plugin_tests.py --all --core /path/to/LEDMatrix + +Invoke it with the interpreter that has the plugins' dependencies -- the core +checkout's venv. Scripts are spawned with `sys.executable`, so running this +under a bare `python3` that lacks e.g. pytz reports every suite as failed, when +the only thing wrong is the interpreter. `--core` sets PYTHONPATH; it does not +change which Python runs. ## Why this exists From d92078a3072f133d0896d7812b9f66c5de3cb0f5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 15:41:32 -0400 Subject: [PATCH 3/3] test: give the runner an explicit core contract, and reject unknown ids Both findings on #246 reproduce. Unknown plugin ids looked like success. `run_plugin_tests.py a-typo` found no scripts, ran nothing, and returned 0. A typo or a removed plugin therefore reported clean. Unknown ids now fail. --core did not reach the scripts. It only prepended to PYTHONPATH, while children run with cwd set to the plugin directory -- so a script resolving a core asset relatively looked in the wrong place and skipped even though a core had been supplied. LEDMATRIX_CORE now carries the resolved absolute path, and baseball's pre-flight uses it, chdir'ing to the core because the core's own ConfigManager and DisplayManager resolve config/ and assets/ relatively too. That fix turns a SKIP into a FAIL, and the FAIL is correct: with the core finally reachable, test_baseball_plugin gets far enough to show it has drifted from the plugin API. It checks `plugin.initialized`, which no longer exists, and then `plugin.leagues`, which does not either. The script has been skipping for long enough that nobody noticed it had gone stale. I briefly patched around `initialized` and reverted it. Chasing the drift is a different job, and papering over it would be the same misclassification this whole change set exists to end -- a script that cannot run reported as fine. It belongs in the stale-test tranche this PR already catalogues, now with a concrete cause rather than "needs a LEDMatrix tree". Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 --- .../baseball-scoreboard/test_baseball_plugin.py | 17 ++++++++++++++--- scripts/run_plugin_tests.py | 13 +++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/plugins/baseball-scoreboard/test_baseball_plugin.py b/plugins/baseball-scoreboard/test_baseball_plugin.py index fdb60181..56963280 100755 --- a/plugins/baseball-scoreboard/test_baseball_plugin.py +++ b/plugins/baseball-scoreboard/test_baseball_plugin.py @@ -390,11 +390,22 @@ def main(): # (exit 1): reporting "not applicable" as a regression is what trained # everyone to ignore this plugin's results. See # scripts/run_plugin_tests.py for the exit-code convention. - if not os.path.exists(os.path.join("config", "config.template.json")): - print("SKIP: needs the core config template — run from a LEDMatrix " - "checkout (cwd is %s)" % os.getcwd()) + # The runner passes the core checkout in LEDMATRIX_CORE; fall back to the + # working directory so a manual run from a core tree still works. + _core_root = os.environ.get("LEDMATRIX_CORE", "") + if not os.path.exists(os.path.join(_core_root, "config", "config.template.json")): + print("SKIP: needs the core config template — set LEDMATRIX_CORE or " + "run from a LEDMatrix checkout (looked in %s)" + % (_core_root or os.getcwd())) sys.exit(2) + if _core_root: + # This script drives the core's DisplayManager and CacheManager, and + # they resolve config/ and assets/ relative to the working directory. + # The runner starts us in the plugin directory, so move to the core -- + # the plugin itself is already importable via sys.path above. + os.chdir(_core_root) + try: success = main() sys.exit(0 if success else 1) diff --git a/scripts/run_plugin_tests.py b/scripts/run_plugin_tests.py index c98a6281..884cccb3 100644 --- a/scripts/run_plugin_tests.py +++ b/scripts/run_plugin_tests.py @@ -56,7 +56,14 @@ def run_one(script: Path, core: Path | None, timeout: int) -> tuple[int, str]: env = dict(os.environ) if core: + core = core.resolve() env["PYTHONPATH"] = f"{core}{os.pathsep}{env.get('PYTHONPATH', '')}" + # Children run with cwd set to the plugin directory, so a script that + # resolves a core asset relatively -- config/config.template.json, a + # bundled font -- looks in the wrong place and skips even though a core + # was supplied. PYTHONPATH alone cannot tell it where the core is. + # LEDMATRIX_CORE is that contract, and it is absolute. + env["LEDMATRIX_CORE"] = str(core) try: proc = subprocess.run( [sys.executable, script.name], @@ -92,6 +99,12 @@ def main() -> int: if not ids: ap.error("give plugin ids or --all") + # A typo used to look like success: no scripts found, nothing run, exit 0. + unknown = [pid for pid in ids if not (PLUGINS / pid).is_dir()] + if unknown: + print(f"Unknown plugin id(s): {', '.join(unknown)}", file=sys.stderr) + return 1 + totals = {PASS: 0, SKIP: 0, FAIL: 0} failures: list[str] = []