mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-01 16:58:06 +00:00
fix(testing): stop plugin enabled:false schema defaults from silently disabling harness tests (#408)
check_plugin.py, render_plugin.py, and the pytest plugin matrix each built
config as {"enabled": True} then merged in config_schema.json's defaults on
top, letting a plugin's own enabled:false default (a reasonable choice for
a seasonal/opt-in plugin -- 15 of 23 real plugins ship one) silently win.
Every harness/CI render of those plugins was testing "disabled, do
nothing" rather than real behavior.
Extract build_full_config() into testing/loading.py (already the shared
home for plugin-discovery/config-default logic) and use it from all three
call sites: schema defaults, then a forced enabled=True, then harness.json's
config, then the caller's explicit config -- so a test can still
deliberately disable a plugin on purpose, it just can't happen by accident
via the plugin's own shipped schema default anymore.
This commit is contained in:
@@ -37,7 +37,7 @@ os.environ['EMULATOR'] = 'true'
|
|||||||
|
|
||||||
from src.logging_config import get_logger # noqa: E402
|
from src.logging_config import get_logger # noqa: E402
|
||||||
from src.plugin_system.testing.loading import ( # noqa: E402
|
from src.plugin_system.testing.loading import ( # noqa: E402
|
||||||
find_plugin_dir, load_config_defaults, load_harness_spec, load_manifest,
|
build_full_config, find_plugin_dir, load_harness_spec, load_manifest,
|
||||||
)
|
)
|
||||||
from src.plugin_system.testing.harness import ( # noqa: E402
|
from src.plugin_system.testing.harness import ( # noqa: E402
|
||||||
RenderResult, render_plugin_matrix, compare_to_goldens, write_goldens,
|
RenderResult, render_plugin_matrix, compare_to_goldens, write_goldens,
|
||||||
@@ -97,12 +97,11 @@ def check_one(plugin_id: str, search_dirs: List[str], sizes, mock_data: Dict,
|
|||||||
# matrix path does; explicit CLI flags still override the file.
|
# matrix path does; explicit CLI flags still override the file.
|
||||||
spec = load_harness_spec(plugin_dir)
|
spec = load_harness_spec(plugin_dir)
|
||||||
|
|
||||||
# config_schema defaults (real-install behavior), then harness.json config,
|
# config_schema defaults (real-install behavior, with enabled forced True
|
||||||
# then CLI --config — most specific wins.
|
# so a plugin's own enabled:false default can't accidentally disable
|
||||||
full_config = {"enabled": True}
|
# testing), then harness.json config, then CLI --config — most specific
|
||||||
full_config.update(load_config_defaults(plugin_dir))
|
# wins.
|
||||||
full_config.update(spec.get("config", {}))
|
full_config = build_full_config(plugin_dir, spec, config)
|
||||||
full_config.update(config)
|
|
||||||
|
|
||||||
# Precedence: CLI flag > LEDMATRIX_TEST_SIZES env > harness.json > default.
|
# Precedence: CLI flag > LEDMATRIX_TEST_SIZES env > harness.json > default.
|
||||||
effective_sizes = sizes if sizes else resolve_test_sizes(spec.get("sizes"))
|
effective_sizes = sizes if sizes else resolve_test_sizes(spec.get("sizes"))
|
||||||
|
|||||||
@@ -28,7 +28,7 @@ os.environ['EMULATOR'] = 'true'
|
|||||||
# Import logger after path setup so src.logging_config is importable
|
# Import logger after path setup so src.logging_config is importable
|
||||||
from src.logging_config import get_logger # noqa: E402
|
from src.logging_config import get_logger # noqa: E402
|
||||||
from src.plugin_system.testing.loading import ( # noqa: E402
|
from src.plugin_system.testing.loading import ( # noqa: E402
|
||||||
find_plugin_dir, load_manifest, load_config_defaults,
|
build_full_config, find_plugin_dir, load_manifest,
|
||||||
)
|
)
|
||||||
logger = get_logger("[Render Plugin]")
|
logger = get_logger("[Render Plugin]")
|
||||||
|
|
||||||
@@ -83,16 +83,13 @@ def main() -> int:
|
|||||||
manifest = load_manifest(Path(plugin_dir))
|
manifest = load_manifest(Path(plugin_dir))
|
||||||
|
|
||||||
# Parse config: start with schema defaults, then apply overrides
|
# Parse config: start with schema defaults, then apply overrides
|
||||||
config_defaults = load_config_defaults(Path(plugin_dir))
|
|
||||||
try:
|
try:
|
||||||
user_config = json.loads(args.config)
|
user_config = json.loads(args.config)
|
||||||
except json.JSONDecodeError as e:
|
except json.JSONDecodeError as e:
|
||||||
logger.error("Invalid JSON config: %s", e)
|
logger.error("Invalid JSON config: %s", e)
|
||||||
return 1
|
return 1
|
||||||
|
|
||||||
config = {'enabled': True}
|
config = build_full_config(Path(plugin_dir), cli_config=user_config)
|
||||||
config.update(config_defaults)
|
|
||||||
config.update(user_config)
|
|
||||||
|
|
||||||
# Load mock data if provided
|
# Load mock data if provided
|
||||||
mock_data = {}
|
mock_data = {}
|
||||||
|
|||||||
@@ -88,3 +88,27 @@ def load_harness_spec(plugin_dir: Union[str, Path]) -> Dict[str, Any]:
|
|||||||
with open(mock_path, 'r') as mf:
|
with open(mock_path, 'r') as mf:
|
||||||
spec['mock_data_contents'] = json.load(mf)
|
spec['mock_data_contents'] = json.load(mf)
|
||||||
return spec
|
return spec
|
||||||
|
|
||||||
|
|
||||||
|
def build_full_config(
|
||||||
|
plugin_dir: Union[str, Path],
|
||||||
|
spec: Optional[Dict[str, Any]] = None,
|
||||||
|
cli_config: Optional[Dict[str, Any]] = None,
|
||||||
|
) -> Dict[str, Any]:
|
||||||
|
"""Build the config a plugin sees under test.
|
||||||
|
|
||||||
|
Merge order: config_schema.json defaults, then a forced ``enabled: True``,
|
||||||
|
then harness.json's config overlay, then the caller's explicit config --
|
||||||
|
most specific wins. `enabled` is re-asserted *after* the schema defaults
|
||||||
|
so a plugin that reasonably ships `enabled: false` (e.g. a seasonal or
|
||||||
|
opt-in plugin) can't silently make every harness run test "disabled, do
|
||||||
|
nothing" by accident -- callers that genuinely want to test the disabled
|
||||||
|
path can still do so via `cli_config={"enabled": False}`.
|
||||||
|
"""
|
||||||
|
spec = spec or {}
|
||||||
|
config: Dict[str, Any] = {}
|
||||||
|
config.update(load_config_defaults(plugin_dir))
|
||||||
|
config["enabled"] = True
|
||||||
|
config.update(spec.get("config", {}))
|
||||||
|
config.update(cli_config or {})
|
||||||
|
return config
|
||||||
|
|||||||
@@ -253,3 +253,61 @@ class TestCheckPluginHonorsHarnessJson:
|
|||||||
)
|
)
|
||||||
assert captured["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"
|
assert captured["config"]["timezone"] == "America/New_York"
|
||||||
|
|
||||||
|
|
||||||
|
class TestBuildFullConfigForcesEnabled:
|
||||||
|
"""Regression: a plugin's own config_schema.json may reasonably default
|
||||||
|
enabled to False (e.g. a seasonal or opt-in plugin) -- march-madness and
|
||||||
|
14 other real plugins do. The harness must still test it as enabled
|
||||||
|
unless a caller explicitly asks otherwise, or every render silently
|
||||||
|
becomes a same-shaped "disabled, do nothing" no-op."""
|
||||||
|
|
||||||
|
def _make_plugin_with_disabled_default(self, tmp_path):
|
||||||
|
pdir = tmp_path / "plugins" / "demo-seasonal"
|
||||||
|
pdir.mkdir(parents=True)
|
||||||
|
(pdir / "manifest.json").write_text(json.dumps({
|
||||||
|
"id": "demo-seasonal", "name": "Demo Seasonal", "version": "1.0.0",
|
||||||
|
"author": "test", "entry_point": "manager.py",
|
||||||
|
"class_name": "DemoSeasonal", "display_modes": ["demo-seasonal"],
|
||||||
|
"compatible_versions": ["*"],
|
||||||
|
}))
|
||||||
|
(pdir / "config_schema.json").write_text(json.dumps({
|
||||||
|
"type": "object",
|
||||||
|
"properties": {"enabled": {"type": "boolean", "default": False}},
|
||||||
|
}))
|
||||||
|
return pdir
|
||||||
|
|
||||||
|
def test_schema_disabled_default_does_not_win(self, tmp_path):
|
||||||
|
from src.plugin_system.testing.loading import build_full_config
|
||||||
|
plugin_dir = self._make_plugin_with_disabled_default(tmp_path)
|
||||||
|
config = build_full_config(plugin_dir)
|
||||||
|
assert config["enabled"] is True
|
||||||
|
|
||||||
|
def test_harness_json_config_can_still_disable(self, tmp_path):
|
||||||
|
from src.plugin_system.testing.loading import build_full_config
|
||||||
|
plugin_dir = self._make_plugin_with_disabled_default(tmp_path)
|
||||||
|
config = build_full_config(plugin_dir, spec={"config": {"enabled": False}})
|
||||||
|
assert config["enabled"] is False
|
||||||
|
|
||||||
|
def test_explicit_cli_config_can_still_disable(self, tmp_path):
|
||||||
|
from src.plugin_system.testing.loading import build_full_config
|
||||||
|
plugin_dir = self._make_plugin_with_disabled_default(tmp_path)
|
||||||
|
config = build_full_config(plugin_dir, cli_config={"enabled": False})
|
||||||
|
assert config["enabled"] is False
|
||||||
|
|
||||||
|
def test_check_one_renders_a_schema_disabled_plugin_as_enabled(self, tmp_path, monkeypatch):
|
||||||
|
"""End-to-end: check_plugin.py's check_one() must not blank-render a
|
||||||
|
plugin just because its own schema defaults enabled to False."""
|
||||||
|
mod = _load_check_plugin_cli()
|
||||||
|
plugin_dir = self._make_plugin_with_disabled_default(tmp_path)
|
||||||
|
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-seasonal", search_dirs=[str(tmp_path / "plugins")],
|
||||||
|
sizes=None, mock_data={}, config={}, run_update=True,
|
||||||
|
out_dir=None, update_golden=False, golden_dir_override=None,
|
||||||
|
freeze_time=None,
|
||||||
|
)
|
||||||
|
assert captured["config"]["enabled"] is True
|
||||||
|
|||||||
@@ -22,7 +22,7 @@ import pytest
|
|||||||
from src.plugin_system.testing.harness import (
|
from src.plugin_system.testing.harness import (
|
||||||
render_plugin_matrix, compare_to_goldens,
|
render_plugin_matrix, compare_to_goldens,
|
||||||
)
|
)
|
||||||
from src.plugin_system.testing.loading import load_config_defaults, load_harness_spec
|
from src.plugin_system.testing.loading import build_full_config, load_harness_spec
|
||||||
from src.plugin_system.testing.sizes import resolve_test_sizes
|
from src.plugin_system.testing.sizes import resolve_test_sizes
|
||||||
|
|
||||||
PROJECT_ROOT = Path(__file__).resolve().parents[2]
|
PROJECT_ROOT = Path(__file__).resolve().parents[2]
|
||||||
@@ -81,9 +81,7 @@ def test_plugin_renders_across_sizes_and_screens(plugin_id: str) -> None:
|
|||||||
plugin_dir = _PLUGINS[plugin_id]
|
plugin_dir = _PLUGINS[plugin_id]
|
||||||
spec = load_harness_spec(plugin_dir)
|
spec = load_harness_spec(plugin_dir)
|
||||||
|
|
||||||
config = {"enabled": True}
|
config = build_full_config(plugin_dir, spec)
|
||||||
config.update(load_config_defaults(plugin_dir))
|
|
||||||
config.update(spec.get("config", {}))
|
|
||||||
|
|
||||||
# Sizes: LEDMATRIX_TEST_SIZES env (test on real hardware) wins, then the
|
# Sizes: LEDMATRIX_TEST_SIZES env (test on real hardware) wins, then the
|
||||||
# plugin's own harness.json "sizes", else the default representative sample.
|
# plugin's own harness.json "sizes", else the default representative sample.
|
||||||
|
|||||||
Reference in New Issue
Block a user