From bc027c921d8e6bbddbcc423f442c231d5c2af4a9 Mon Sep 17 00:00:00 2001 From: Ron Pierce Date: Mon, 8 Jun 2026 09:52:33 -0700 Subject: [PATCH] fix: check_plugin.py honors per-plugin test/harness.json (#365) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit check_one() always compares the render against committed golden images, but the CLI never loaded the plugin's test/harness.json — so the deterministic settings the goldens were generated with (config, mock data, frozen time, sizes) weren't applied. For any time/data-dependent plugin this means the CLI (and the plugins-repo CI workflow that calls it) renders live data and the golden drifts on every run, even with no real regression. The pytest matrix path already reads harness.json via load_harness_spec; the CLI now does too. - check_one loads load_harness_spec(plugin_dir) and layers it under explicit CLI flags: config = schema defaults < harness.json < --config; sizes = --sizes > LEDMATRIX_TEST_SIZES env > harness.json > default sample; mock_data/freeze_time/skip_update fall back to harness.json when not given on the CLI. - parse_sizes returns None (not DEFAULT_TEST_SIZES) when --sizes is omitted, so the env/harness.json/default fallback chain in resolve_test_sizes applies. - Regression tests: harness.json supplies render settings, and CLI flags override it. Use a temp fixture plugin so they run in core CI (no plugins). Co-authored-by: Claude Opus 4.8 --- scripts/check_plugin.py | 27 ++++++++++--- test/plugins/test_harness.py | 73 ++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 6 deletions(-) diff --git a/scripts/check_plugin.py b/scripts/check_plugin.py index e8afbe44..63fbb7e9 100644 --- a/scripts/check_plugin.py +++ b/scripts/check_plugin.py @@ -37,13 +37,13 @@ os.environ['EMULATOR'] = 'true' from src.logging_config import get_logger # noqa: E402 from src.plugin_system.testing.loading import ( # noqa: E402 - find_plugin_dir, load_config_defaults, + find_plugin_dir, load_config_defaults, load_harness_spec, ) from src.plugin_system.testing.harness import ( # noqa: E402 RenderResult, render_plugin_matrix, compare_to_goldens, write_goldens, ) from src.plugin_system.testing.sizes import ( # noqa: E402 - DEFAULT_TEST_SIZES, parse_size_token, safe_mode_filename, size_label, + parse_size_token, resolve_test_sizes, safe_mode_filename, size_label, ) logger = get_logger("[Check Plugin]") @@ -69,7 +69,7 @@ def discover_plugins(search_dirs: List[str]) -> List[str]: def parse_sizes(spec: Optional[str]): if not spec: - return DEFAULT_TEST_SIZES + return None sizes = [] for token in spec.split(','): if not token.strip(): @@ -90,15 +90,30 @@ def check_one(plugin_id: str, search_dirs: List[str], sizes, mock_data: Dict, logger.error("Plugin '%s' not found in: %s", plugin_id, search_dirs) return [RenderResult(plugin_id, 0, 0, "", error="plugin directory not found")] - # Start from config_schema defaults so plugins behave like a real install. + # Per-plugin test/harness.json holds the deterministic settings the committed + # goldens were generated with (config, mock data, frozen time, sizes). Load + # them so the CLI/CI render reproduces the golden the same way the pytest + # matrix path does; explicit CLI flags still override the file. + spec = load_harness_spec(plugin_dir) + + # config_schema defaults (real-install behavior), then harness.json config, + # then CLI --config — most specific wins. full_config = {"enabled": True} full_config.update(load_config_defaults(plugin_dir)) + full_config.update(spec.get("config", {})) full_config.update(config) + # Precedence: CLI flag > LEDMATRIX_TEST_SIZES env > harness.json > default. + effective_sizes = sizes if sizes else resolve_test_sizes(spec.get("sizes")) + # CLI value wins when provided, else fall back to the harness.json setting. + effective_mock_data = mock_data or spec.get("mock_data_contents", {}) + effective_freeze = freeze_time or spec.get("freeze_time") + effective_run_update = run_update and not spec.get("skip_update", False) + results = render_plugin_matrix( plugin_id=plugin_id, plugin_dir=plugin_dir, config=full_config, - mock_data=mock_data, sizes=sizes, run_update=run_update, - freeze_time=freeze_time, + mock_data=effective_mock_data, sizes=effective_sizes, + run_update=effective_run_update, freeze_time=effective_freeze, ) golden_dir = golden_dir_override or (plugin_dir / 'test' / 'golden') diff --git a/test/plugins/test_harness.py b/test/plugins/test_harness.py index 11dec7e7..e92f16ca 100644 --- a/test/plugins/test_harness.py +++ b/test/plugins/test_harness.py @@ -6,6 +6,10 @@ These don't load real plugins, so they run anywhere (including core CI where plugin-repos is empty). """ +import importlib.util +import json +from pathlib import Path + import pytest from PIL import Image @@ -180,3 +184,72 @@ class TestListModes: def test_falls_back_to_plugin_id(self): inst = type("P", (), {})() assert list_modes(inst, {}, "pid") == ["pid"] + + +def _load_check_plugin_cli(): + """Load scripts/check_plugin.py by path (it isn't an importable package).""" + root = Path(__file__).resolve().parents[2] + path = root / "scripts" / "check_plugin.py" + spec = importlib.util.spec_from_file_location("check_plugin_cli", path) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + + +def _make_fixture_plugin(tmp_path, harness): + """Create a minimal plugin dir with a test/harness.json; return its parent + (the search dir).""" + pdir = tmp_path / "plugins" / "demo-clock" + (pdir / "test").mkdir(parents=True) + (pdir / "manifest.json").write_text(json.dumps({ + "id": "demo-clock", "name": "Demo Clock", "version": "1.0.0", + "author": "test", "entry_point": "manager.py", "class_name": "DemoClock", + "display_modes": ["demo-clock"], "compatible_versions": ["*"], + })) + (pdir / "test" / "harness.json").write_text(json.dumps(harness)) + return pdir.parent + + +class TestCheckPluginHonorsHarnessJson: + """Regression: check_plugin.py (the CI tool) must apply test/harness.json so + its render reproduces the committed goldens — otherwise time/data-dependent + plugins drift on every CI run.""" + + def test_harness_json_supplies_render_settings(self, tmp_path, monkeypatch): + mod = _load_check_plugin_cli() + search = _make_fixture_plugin(tmp_path, { + "config": {"timezone": "UTC"}, + "freeze_time": "2025-08-01 15:25:00", + "sizes": [[128, 32]], + }) + captured = {} + monkeypatch.setattr(mod, "render_plugin_matrix", + lambda **kw: captured.update(kw) or []) + monkeypatch.setattr(mod, "compare_to_goldens", lambda *a, **k: []) + mod.check_one( + plugin_id="demo-clock", search_dirs=[str(search)], sizes=None, + mock_data={}, config={}, run_update=True, out_dir=None, + update_golden=False, golden_dir_override=None, freeze_time=None, + ) + assert captured["freeze_time"] == "2025-08-01 15:25:00" + assert captured["config"]["timezone"] == "UTC" + assert captured["sizes"] == [(128, 32)] + + def test_cli_flags_override_harness_json(self, tmp_path, monkeypatch): + mod = _load_check_plugin_cli() + search = _make_fixture_plugin(tmp_path, { + "config": {"timezone": "UTC"}, + "freeze_time": "2025-08-01 15:25:00", + }) + captured = {} + monkeypatch.setattr(mod, "render_plugin_matrix", + lambda **kw: captured.update(kw) or []) + monkeypatch.setattr(mod, "compare_to_goldens", lambda *a, **k: []) + mod.check_one( + plugin_id="demo-clock", search_dirs=[str(search)], sizes=None, + mock_data={}, config={"timezone": "America/New_York"}, + run_update=True, out_dir=None, update_golden=False, + golden_dir_override=None, freeze_time="2030-01-01 00:00:00", + ) + assert captured["freeze_time"] == "2030-01-01 00:00:00" + assert captured["config"]["timezone"] == "America/New_York"