mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-22 02:48:15 +00:00
* fix(plugins): say when discovery skips a directory A plugin can be enabled in config, enabled in plugin state, present on disk with a valid manifest and an importable entry point -- and simply absent from the running process, with nothing anywhere to say why. That is not hypothetical. hockey-scoreboard on a live rig is enabled in both places, imports cleanly when loaded by hand, and is listed in the Vegas plugin order, but is not among the 22 plugins the process actually holds. Establishing even that much meant comparing cache-file mtimes to find it had last run three days earlier. The journal had nothing, because discovery does not report what it declines to load. Two paths were silent. A directory with no manifest.json was skipped without comment, which is defensible until it is the thing you are trying to explain. Quieter still, a manifest that parsed but carried no "id" was read successfully and then dropped on the floor -- no warning, no trace, and the plugin simply does not exist as far as the rest of the system is concerned. Both now log a warning naming the directory and the reason. This does not explain the rig above; its manifest has an id. It makes the next occurrence diagnosable from the journal instead of from file timestamps. Reverting the change fails both tests. 65 plugin-system tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW * fix(plugins): warn once per directory, not once per scan Self-review catch. Discovery runs on every web UI page load and every config reconcile, so warning unconditionally about an unloadable directory would put a line in the journal each time someone opened a page -- the same log-volume problem this change exists to help diagnose. The skip is now reported once per directory per process. The diagnostic value is unchanged: the reason a plugin is missing still appears in the journal, once, where before it appeared nowhere. Test added covering five consecutive scans producing one warning. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW * fix(plugins): one unusable manifest no longer aborts the whole scan json.load accepts any JSON value, so a manifest.json holding null, [], "text" or 42 parses without complaint and then raises AttributeError on manifest.get('id'). Nothing catches that: the outer handler around the scan takes OSError and PermissionError only. So a single malformed manifest did not skip that one directory -- it aborted _scan_directory_for_plugins outright, and every other plugin on disk, however healthy, silently failed to register. Reproduced with three directories, the middle one holding `null`: SCAN ABORTED -> AttributeError: 'NoneType' object has no attribute 'get' the two valid plugins never registered That is the same failure this PR set out to fix, in its most severe form: a plugin enabled in config, enabled in plugin state, present on disk, and absent from the running process with nothing to say why -- except here it takes every other plugin with it. A manifest that is not a JSON object is now skipped like any other unusable directory, named once, with what it actually was: Skipping bad-null: its manifest.json is NoneType, not a JSON object Skipping bad-list: its manifest.json is list, not a JSON object scan returned: ['aaa-good', 'zzz-good'] Verified: removing the guard fails 6 of the 10 tests. Covers null, list, string, int and bool, and asserts the healthy plugins either side of the bad one still register. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
127 lines
4.8 KiB
Python
127 lines
4.8 KiB
Python
#!/usr/bin/env python3
|
|
"""Discovery must say when it skips a directory.
|
|
|
|
A plugin can be enabled in config, enabled in plugin state, present on disk
|
|
with a valid entry point -- and simply absent from the running process, with
|
|
nothing in the journal to say why. Working that out afterwards meant comparing
|
|
cache-file mtimes to find when it had last run.
|
|
|
|
Two paths were silent. A directory with no manifest.json was ignored, and --
|
|
quieter still -- a manifest that parsed but carried no "id" was read
|
|
successfully and then dropped on the floor.
|
|
"""
|
|
import json
|
|
import logging
|
|
import sys
|
|
from pathlib import Path
|
|
from unittest.mock import MagicMock
|
|
|
|
import pytest
|
|
|
|
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
|
|
|
from src.plugin_system.plugin_manager import PluginManager # noqa: E402
|
|
|
|
|
|
def _manager(tmp_path):
|
|
pm = PluginManager.__new__(PluginManager)
|
|
pm.plugins_dir = tmp_path
|
|
pm.logger = logging.getLogger("test.discovery")
|
|
pm.plugin_manifests = {}
|
|
pm.plugin_directories = {}
|
|
pm._discovery_lock = __import__("threading").RLock()
|
|
pm._skip_reported = set()
|
|
pm.schema_manager = MagicMock()
|
|
return pm
|
|
|
|
|
|
def test_a_directory_without_a_manifest_is_reported(tmp_path, caplog):
|
|
(tmp_path / "not-a-plugin").mkdir()
|
|
pm = _manager(tmp_path)
|
|
with caplog.at_level(logging.WARNING, logger="test.discovery"):
|
|
pm._scan_directory_for_plugins(tmp_path)
|
|
joined = " ".join(r.message for r in caplog.records)
|
|
assert "not-a-plugin" in joined and "manifest" in joined, (
|
|
f"skip was silent; log said: {joined!r}")
|
|
|
|
|
|
def test_a_manifest_without_an_id_is_reported(tmp_path, caplog):
|
|
d = tmp_path / "idless"
|
|
d.mkdir()
|
|
(d / "manifest.json").write_text(json.dumps({"name": "No Id", "version": "1.0.0"}))
|
|
pm = _manager(tmp_path)
|
|
with caplog.at_level(logging.WARNING, logger="test.discovery"):
|
|
pm._scan_directory_for_plugins(tmp_path)
|
|
joined = " ".join(r.message for r in caplog.records)
|
|
assert "idless" in joined and "id" in joined, (
|
|
f"a parsed-but-unusable manifest vanished silently; log said: {joined!r}")
|
|
|
|
|
|
def test_a_good_plugin_still_registers(tmp_path, caplog):
|
|
d = tmp_path / "real-plugin"
|
|
d.mkdir()
|
|
(d / "manifest.json").write_text(json.dumps(
|
|
{"id": "real-plugin", "name": "Real", "version": "1.0.0"}))
|
|
pm = _manager(tmp_path)
|
|
pm._scan_directory_for_plugins(tmp_path)
|
|
assert "real-plugin" in pm.plugin_manifests, "a valid plugin was not registered"
|
|
|
|
|
|
def test_the_warning_does_not_repeat_on_every_scan(tmp_path, caplog):
|
|
"""Discovery runs on every web UI page load and every config reconcile.
|
|
|
|
Warning unconditionally would put a line in the journal each time someone
|
|
opened a page -- the same log-volume problem this is meant to help
|
|
diagnose.
|
|
"""
|
|
(tmp_path / "not-a-plugin").mkdir()
|
|
pm = _manager(tmp_path)
|
|
with caplog.at_level(logging.WARNING, logger="test.discovery"):
|
|
for _ in range(5):
|
|
pm._scan_directory_for_plugins(tmp_path)
|
|
hits = [r for r in caplog.records if "not-a-plugin" in r.message]
|
|
assert len(hits) == 1, f"warned {len(hits)} times across 5 scans"
|
|
|
|
|
|
def _plugin(tmp_path, name, body):
|
|
d = tmp_path / name
|
|
d.mkdir()
|
|
(d / "manifest.json").write_text(json.dumps(body))
|
|
return d
|
|
|
|
|
|
VALID = {"name": "V", "version": "1.0.0", "class_name": "X", "display_modes": ["m"]}
|
|
|
|
|
|
@pytest.mark.parametrize("body", [None, [1, 2], "not an object", 42, True])
|
|
def test_a_manifest_that_is_not_an_object_is_skipped_not_fatal(tmp_path, caplog, body):
|
|
"""json.load accepts any JSON value, not just objects.
|
|
|
|
manifest.get('id') then raised AttributeError, which nothing here caught --
|
|
the outer handler takes OSError/PermissionError only. A single malformed
|
|
manifest aborted the entire scan, so every other plugin on disk, however
|
|
healthy, silently failed to register.
|
|
"""
|
|
_plugin(tmp_path, "aaa-good", dict(VALID, id="aaa-good"))
|
|
_plugin(tmp_path, "mmm-bad", body)
|
|
_plugin(tmp_path, "zzz-good", dict(VALID, id="zzz-good"))
|
|
|
|
pm = _manager(tmp_path)
|
|
with caplog.at_level(logging.WARNING, logger="test.discovery"):
|
|
found = pm._scan_directory_for_plugins(tmp_path)
|
|
|
|
assert sorted(found) == ["aaa-good", "zzz-good"], (
|
|
"one unusable manifest took the healthy plugins down with it")
|
|
joined = " ".join(r.message for r in caplog.records)
|
|
assert "mmm-bad" in joined, f"the skip was silent; log said: {joined!r}"
|
|
|
|
|
|
def test_the_bad_manifest_is_named_with_what_it_actually_was(tmp_path, caplog):
|
|
_plugin(tmp_path, "listy", [1, 2])
|
|
pm = _manager(tmp_path)
|
|
with caplog.at_level(logging.WARNING, logger="test.discovery"):
|
|
pm._scan_directory_for_plugins(tmp_path)
|
|
joined = " ".join(r.message for r in caplog.records)
|
|
assert "listy" in joined and "list" in joined, (
|
|
f"the warning does not say what the manifest was: {joined!r}")
|