mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-04 14:25:08 +00:00
* fix(plugins): drop a plugin's package modules when it unloads A plugin that keeps helpers in a package (providers/feed.py, imported as `from providers.feed import ...`) leaves dotted entries in sys.modules. PluginLoader only tracked bare names: `providers` was namespaced and dropped on unload, `providers.feed` stayed. A reload after a store update imported a fresh `providers`, then got the old `feed` back from the module cache, so the new manager.py ran against the old helpers until the display restarted. A load that failed part-way left them behind the same way. Elections (providers/), flights (enrichment/) and olympics (data/, renderers/) ship packages. The loader now records the dotted modules whose file (or, for a namespace package, every __path__ entry) lies inside the plugin directory. They keep their names while the plugin runs, as before, and unregister_plugin_modules() drops them, only while sys.modules still holds that plugin's module. The failed-load cleanup in load_module() drops them too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(plugins): remove a symlinked dev plugin as a link PluginStoreManager._safe_remove_directory, behind uninstall and behind discarding the set-aside copy after an install or update, handed a symlinked dev plugin (scripts/dev/dev_plugin_setup.sh) to shutil.rmtree, which refuses a symlink. The chmod fallback then walked through the link and set every directory and file in the linked checkout to 0700, and the sudo stage refused the resolved path as outside the plugins directory. The removal failed, the link stayed, and the developer's checkout lost its group/other permissions. A dangling link read as already removed, because exists() follows it, and was left behind. A symlink is now unlinked before any other stage runs, and before the exists() check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(plugins): load a dev plugin linked in under a different name contained_plugin_dir(), the containment check before a plugin's dependencies are installed, resolved the plugin directory and looked for the resolved folder's name among the plugins directory's entries. A dev plugin symlinked in under its id by a name its checkout does not share -- `dev_plugin_setup.sh link-github foo <url>` clones ledmatrix-foo, the repository naming convention, and links it as plugins/foo -- has no such entry, so install_dependencies() returned False and the load failed with "Dependency installation failed", even with no requirements.txt. When the path sits directly in the plugins directory, the entry it names (the link) is looked up first; anything else is resolved and matched by name as before. The answer is still always rebuilt from a name os.scandir() returned for the plugins directory, so a path outside it is still refused. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(plugins): release a plugin whose update() raises a BaseException On the async update worker, the wrapped update() finished its bookkeeping (_finish: release the plugin lock, drop the pending slot, state back to ENABLED) only for an Exception. asyncio.CancelledError and SystemExit derive from BaseException, so one raised from update() skipped _finish: the plugin kept its lock and stayed RUNNING for the life of the process, never rescheduled, with every display() skipped as busy. PluginExecutor caught only Exception as well, so its thread died with the call never marked complete and an immediate failure was logged and recorded as a timeout. _target_update now runs _finish for any BaseException and re-raises it, and the executor's thread stores it like any other exception, so it is reported as the operation's failure (PluginError) on both the async and the synchronous path. _finish and _record_update_failure take a BaseException. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(config): notify config subscribers outside the service lock ConfigService._load_config ran every subscriber while holding _lock. The display's per-plugin subscriber calls PluginManager.apply_config_change, which waits up to PLUGIN_LOCK_TIMEOUT (5 s) for a plugin busy in update(). A save that enables or disables a plugin also flags a reconcile, which the render thread runs: its get_config(), and the unsubscribe() of a plugin it disables, both take _lock, so the panel froze behind every slow callback, up to 5 s per busy plugin. The config is now swapped under _lock and the subscribers are called after it is released, from a copy of the subscriber lists. A separate _notify_lock is held across a whole reload (read, swap, notify), so one reload's notifications still finish before the next one's start. Each callback is checked against the live lists just before it runs, and unsubscribe() waits only for a call of that same callback already in progress (unless it is that callback's own thread), so a callback it removed is not running and will not run once it returns, as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
393 lines
17 KiB
Python
393 lines
17 KiB
Python
import time
|
|
from unittest.mock import MagicMock, patch
|
|
from pathlib import Path
|
|
from src.plugin_system.plugin_manager import PluginManager
|
|
from src.plugin_system.plugin_state import PluginState
|
|
from src.plugin_system.resource_monitor import PluginResourceMonitor
|
|
|
|
class TestPluginManager:
|
|
"""Test PluginManager functionality."""
|
|
|
|
def test_init(self, mock_config_manager, mock_display_manager, mock_cache_manager):
|
|
"""Test PluginManager initialization."""
|
|
with patch('src.plugin_system.plugin_manager.ensure_directory_permissions'):
|
|
pm = PluginManager(
|
|
plugins_dir="plugins",
|
|
config_manager=mock_config_manager,
|
|
display_manager=mock_display_manager,
|
|
cache_manager=mock_cache_manager
|
|
)
|
|
assert pm.plugins_dir == Path("plugins")
|
|
assert pm.config_manager == mock_config_manager
|
|
assert pm.display_manager == mock_display_manager
|
|
assert pm.cache_manager == mock_cache_manager
|
|
assert pm.plugins == {}
|
|
|
|
def test_load_plugin_success(self, mock_config_manager, mock_display_manager, mock_cache_manager):
|
|
"""Test successful plugin loading."""
|
|
with patch('src.plugin_system.plugin_manager.ensure_directory_permissions'), \
|
|
patch('src.plugin_system.plugin_manager.PluginManager._scan_directory_for_plugins'), \
|
|
patch('src.plugin_system.plugin_manager.PluginLoader') as MockLoader, \
|
|
patch('src.plugin_system.plugin_manager.SchemaManager'):
|
|
|
|
pm = PluginManager(
|
|
plugins_dir="plugins",
|
|
config_manager=mock_config_manager,
|
|
display_manager=mock_display_manager,
|
|
cache_manager=mock_cache_manager
|
|
)
|
|
|
|
# Setup mocks
|
|
pm.plugin_manifests = {"test_plugin": {"id": "test_plugin", "name": "Test Plugin"}}
|
|
|
|
mock_loader = MockLoader.return_value
|
|
mock_loader.find_plugin_directory.return_value = Path("plugins/test_plugin")
|
|
mock_loader.load_plugin.return_value = (MagicMock(), MagicMock())
|
|
|
|
# Test loading
|
|
result = pm.load_plugin("test_plugin")
|
|
|
|
assert result is True
|
|
assert "test_plugin" in pm.plugins
|
|
# PluginManager sets state to ENABLED after successful load
|
|
assert pm.state_manager.get_state("test_plugin") == PluginState.ENABLED
|
|
|
|
def test_load_plugin_missing_manifest(self, mock_config_manager, mock_display_manager, mock_cache_manager):
|
|
"""Test loading plugin with missing manifest."""
|
|
with patch('src.plugin_system.plugin_manager.ensure_directory_permissions'):
|
|
pm = PluginManager(
|
|
plugins_dir="plugins",
|
|
config_manager=mock_config_manager,
|
|
display_manager=mock_display_manager,
|
|
cache_manager=mock_cache_manager
|
|
)
|
|
|
|
# No manifest in pm.plugin_manifests
|
|
result = pm.load_plugin("non_existent_plugin")
|
|
|
|
assert result is False
|
|
assert pm.state_manager.get_state("non_existent_plugin") == PluginState.ERROR
|
|
|
|
def test_run_scheduled_updates_calls_update_with_resource_monitor(
|
|
self, mock_config_manager, mock_display_manager, mock_cache_manager
|
|
):
|
|
"""Regression test: run_scheduled_updates() must actually call a
|
|
plugin's update() when self.resource_monitor is set (as it is in
|
|
every real deployment -- display_controller.py and web_interface/
|
|
app.py both assign a real PluginResourceMonitor after construction).
|
|
|
|
Previously, the resource_monitor branch wrapped the call in a
|
|
function stored as a *class* attribute on a dynamically-built type
|
|
(`type('obj', (object,), {'update': monitored_update})()`), which
|
|
the descriptor protocol turns into a bound method on access --
|
|
silently passing the synthetic instance as an implicit first
|
|
argument to monitored_update(), which takes none. Every plugin's
|
|
scheduled update failed with "monitored_update() takes 0 positional
|
|
arguments but 1 was given" and was silently swallowed into a
|
|
circuit-breaker retry loop that never succeeded, so plugin data
|
|
(scores, odds, etc.) never refreshed.
|
|
"""
|
|
with patch('src.plugin_system.plugin_manager.ensure_directory_permissions'):
|
|
pm = PluginManager(
|
|
plugins_dir="plugins",
|
|
config_manager=mock_config_manager,
|
|
display_manager=mock_display_manager,
|
|
cache_manager=mock_cache_manager
|
|
)
|
|
|
|
plugin_instance = MagicMock()
|
|
plugin_instance.enabled = True
|
|
plugin_instance.update = MagicMock()
|
|
|
|
pm.plugins["test_plugin"] = plugin_instance
|
|
pm.plugin_manifests["test_plugin"] = {"update_interval": 10}
|
|
pm.state_manager.set_state("test_plugin", PluginState.ENABLED)
|
|
# Plain MagicMock, not the mock_cache_manager fixture: this test
|
|
# is about run_scheduled_updates() actually invoking update()
|
|
# through the resource-monitor wrapper, not about
|
|
# PluginResourceMonitor's own cache-backed metrics persistence
|
|
# (which calls cache_manager.get(..., memory_ttl=...) --
|
|
# a kwarg the fixture's mock_get() doesn't accept).
|
|
pm.resource_monitor = PluginResourceMonitor(MagicMock())
|
|
|
|
pm.run_scheduled_updates(current_time=time.time())
|
|
|
|
# Updates now execute on the background worker (the scheduler
|
|
# returns immediately) — wait for completion before asserting.
|
|
deadline = time.time() + 5
|
|
while (plugin_instance.update.call_count == 0
|
|
and time.time() < deadline):
|
|
time.sleep(0.02)
|
|
pm.stop_update_worker()
|
|
|
|
plugin_instance.update.assert_called_once()
|
|
assert "test_plugin" in pm.plugin_last_update
|
|
assert pm.state_manager.get_state("test_plugin") == PluginState.ENABLED
|
|
|
|
|
|
class TestPluginExecutor:
|
|
"""Test PluginExecutor functionality."""
|
|
|
|
def test_execute_display_success(self):
|
|
"""Test successful display execution."""
|
|
from src.plugin_system.plugin_executor import PluginExecutor
|
|
executor = PluginExecutor()
|
|
|
|
mock_plugin = MagicMock()
|
|
mock_plugin.display.return_value = True
|
|
|
|
result = executor.execute_display(mock_plugin, "test_plugin")
|
|
|
|
assert result is True
|
|
mock_plugin.display.assert_called_once()
|
|
|
|
def test_execute_display_runs_on_a_thread_named_for_the_plugin(self):
|
|
"""A screen's first frame is presented from this thread, so stack
|
|
dumps (the frame-timing stall watchdog's) should name the plugin."""
|
|
import threading
|
|
from src.plugin_system.plugin_executor import PluginExecutor
|
|
executor = PluginExecutor()
|
|
seen = []
|
|
|
|
class Plugin:
|
|
def display(self, display_mode=None, force_clear=False):
|
|
seen.append((threading.current_thread().name, display_mode))
|
|
return True
|
|
|
|
# Both ways display() is called: without a mode, and with one (most
|
|
# multi-mode plugins, the scoreboards among them).
|
|
assert executor.execute_display(Plugin(), "clock-simple") is True
|
|
assert executor.execute_display(Plugin(), "clock-simple",
|
|
display_mode="clock") is True
|
|
assert seen == [("display-clock-simple", None),
|
|
("display-clock-simple", "clock")]
|
|
|
|
def test_execute_display_exception(self):
|
|
"""Test display execution with exception."""
|
|
from src.plugin_system.plugin_executor import PluginExecutor
|
|
executor = PluginExecutor()
|
|
|
|
mock_plugin = MagicMock()
|
|
mock_plugin.display.side_effect = Exception("Test error")
|
|
|
|
result = executor.execute_display(mock_plugin, "test_plugin")
|
|
|
|
assert result is False
|
|
|
|
def test_execute_update_timeout(self):
|
|
"""Test update execution timeout."""
|
|
# Using a very short timeout for testing
|
|
from src.plugin_system.plugin_executor import PluginExecutor
|
|
executor = PluginExecutor(default_timeout=0.01)
|
|
|
|
mock_plugin = MagicMock()
|
|
def slow_update():
|
|
time.sleep(0.05)
|
|
mock_plugin.update.side_effect = slow_update
|
|
|
|
result = executor.execute_update(mock_plugin, "test_plugin")
|
|
|
|
assert result is False
|
|
|
|
def test_a_base_exception_is_a_failure_not_a_timeout(self):
|
|
"""asyncio.CancelledError derives from BaseException. Uncaught on
|
|
the executor's thread it ended the thread with the call never marked
|
|
complete, so a call that failed at once was reported, and recorded,
|
|
as timing out."""
|
|
import asyncio
|
|
import pytest
|
|
from src.exceptions import PluginError
|
|
from src.plugin_system.plugin_executor import PluginExecutor
|
|
executor = PluginExecutor(default_timeout=5.0)
|
|
|
|
def cancelled():
|
|
raise asyncio.CancelledError()
|
|
|
|
with pytest.raises(PluginError) as raised:
|
|
executor.execute_with_timeout(cancelled, plugin_id="test_plugin")
|
|
assert isinstance(raised.value.__cause__, asyncio.CancelledError)
|
|
|
|
|
|
class TestPluginHealth:
|
|
"""Test plugin health monitoring."""
|
|
|
|
def test_circuit_breaker(self, mock_cache_manager):
|
|
"""Test circuit breaker activation."""
|
|
from src.plugin_system.plugin_health import PluginHealthTracker
|
|
tracker = PluginHealthTracker(cache_manager=mock_cache_manager, failure_threshold=3, cooldown_period=60)
|
|
|
|
plugin_id = "test_plugin"
|
|
|
|
# Initial state
|
|
assert tracker.should_skip_plugin(plugin_id) is False
|
|
|
|
# Failures
|
|
tracker.record_failure(plugin_id, Exception("Error 1"))
|
|
assert tracker.should_skip_plugin(plugin_id) is False
|
|
|
|
tracker.record_failure(plugin_id, Exception("Error 2"))
|
|
assert tracker.should_skip_plugin(plugin_id) is False
|
|
|
|
tracker.record_failure(plugin_id, Exception("Error 3"))
|
|
# Should trip now
|
|
assert tracker.should_skip_plugin(plugin_id) is True
|
|
|
|
# Recovery (simulate timeout - need to update health state correctly)
|
|
if plugin_id in tracker._health_state:
|
|
tracker._health_state[plugin_id]["last_failure"] = time.time() - 61
|
|
tracker._health_state[plugin_id]["circuit_state"] = "closed"
|
|
assert tracker.should_skip_plugin(plugin_id) is False
|
|
|
|
|
|
class TestBasePlugin:
|
|
"""Test BasePlugin functionality."""
|
|
|
|
def test_dynamic_duration_defaults(self, mock_display_manager, mock_cache_manager):
|
|
"""Test default dynamic duration behavior."""
|
|
from src.plugin_system.base_plugin import BasePlugin
|
|
|
|
# Concrete implementation for testing
|
|
class ConcretePlugin(BasePlugin):
|
|
def update(self): pass
|
|
def display(self, force_clear=False): pass
|
|
|
|
config = {"enabled": True}
|
|
plugin = ConcretePlugin("test", config, mock_display_manager, mock_cache_manager, None)
|
|
|
|
assert plugin.supports_dynamic_duration() is False
|
|
assert plugin.get_dynamic_duration_cap() is None
|
|
assert plugin.is_cycle_complete() is True
|
|
|
|
def test_live_priority_config(self, mock_display_manager, mock_cache_manager):
|
|
"""Test live priority configuration."""
|
|
from src.plugin_system.base_plugin import BasePlugin
|
|
|
|
class ConcretePlugin(BasePlugin):
|
|
def update(self): pass
|
|
def display(self, force_clear=False): pass
|
|
|
|
config = {"enabled": True, "live_priority": True}
|
|
plugin = ConcretePlugin("test", config, mock_display_manager, mock_cache_manager, None)
|
|
|
|
assert plugin.has_live_priority() is True
|
|
|
|
|
|
class TestBasePluginGlobalConfig:
|
|
"""global_config exposes device-wide settings that self.config cannot.
|
|
|
|
The sports scoreboards read `getattr(self, 'global_config', {})` to find
|
|
the shared target_fps; before this property existed nothing ever set that
|
|
attribute, so the lookup silently returned {} and the setting could never
|
|
take effect on any core.
|
|
"""
|
|
|
|
@staticmethod
|
|
def _plugin(display_manager, cache_manager, plugin_manager=None):
|
|
from src.plugin_system.base_plugin import BasePlugin
|
|
|
|
class ConcretePlugin(BasePlugin):
|
|
def update(self): pass
|
|
def display(self, force_clear=False): pass
|
|
|
|
return ConcretePlugin(
|
|
"test", {"enabled": True}, display_manager, cache_manager, plugin_manager
|
|
)
|
|
|
|
@staticmethod
|
|
def _manager_with(config):
|
|
"""A stand-in manager exposing config_manager.get_config()."""
|
|
manager = MagicMock()
|
|
manager.config_manager.get_config.return_value = config
|
|
return manager
|
|
|
|
def test_reads_config_from_plugin_manager(self, mock_display_manager, mock_cache_manager):
|
|
plugin = self._plugin(
|
|
mock_display_manager, mock_cache_manager,
|
|
self._manager_with({"target_fps": 100}),
|
|
)
|
|
assert plugin.global_config["target_fps"] == 100
|
|
|
|
def test_falls_back_to_cache_manager(self, mock_display_manager):
|
|
# The core that hangs config_manager off the cache manager instead.
|
|
cache_manager = self._manager_with({"target_fps": 75})
|
|
plugin = self._plugin(mock_display_manager, cache_manager, plugin_manager=None)
|
|
assert plugin.global_config["target_fps"] == 75
|
|
|
|
def test_plugin_manager_wins_over_cache_manager(self, mock_display_manager):
|
|
plugin = self._plugin(
|
|
mock_display_manager,
|
|
self._manager_with({"target_fps": 75}),
|
|
self._manager_with({"target_fps": 100}),
|
|
)
|
|
assert plugin.global_config["target_fps"] == 100
|
|
|
|
def test_empty_plugin_manager_config_falls_through(self, mock_display_manager):
|
|
"""An empty first source means "not loaded yet", not "the answer".
|
|
|
|
Both managers default to the same config/config.json, so falling
|
|
through cannot pick up a different file. Returning {} here instead
|
|
would silently disable every setting read through this property --
|
|
the exact failure this property exists to fix.
|
|
"""
|
|
plugin = self._plugin(
|
|
mock_display_manager,
|
|
self._manager_with({"target_fps": 100}), # cache_manager
|
|
self._manager_with({}), # plugin_manager: empty
|
|
)
|
|
assert plugin.global_config["target_fps"] == 100
|
|
|
|
def test_returns_empty_dict_when_no_config_manager(self, mock_display_manager):
|
|
# Plain objects: no config_manager attribute at all.
|
|
plugin = self._plugin(mock_display_manager, object(), object())
|
|
assert plugin.global_config == {}
|
|
|
|
def test_unreadable_config_does_not_raise(self, mock_display_manager):
|
|
# A plugin must still load when the config on disk is broken.
|
|
broken = MagicMock()
|
|
broken.config_manager.get_config.side_effect = OSError("unreadable")
|
|
plugin = self._plugin(mock_display_manager, broken, broken)
|
|
assert plugin.global_config == {}
|
|
|
|
def test_non_dict_config_is_rejected(self, mock_display_manager):
|
|
# A stub or half-built manager can return a non-mapping; handing that
|
|
# back would blow up later in numeric code, far from the cause.
|
|
plugin = self._plugin(
|
|
mock_display_manager, object(), self._manager_with("not-a-dict")
|
|
)
|
|
assert plugin.global_config == {}
|
|
|
|
def test_missing_property_degrades_to_default(self, mock_display_manager, mock_cache_manager):
|
|
# How plugins actually call it, so a plugin written against this core
|
|
# still loads on one that predates the property.
|
|
plugin = self._plugin(mock_display_manager, mock_cache_manager, object())
|
|
assert getattr(plugin, "global_config", {}).get("target_fps") is None
|
|
|
|
def test_plugin_may_still_assign_global_config(self, mock_display_manager, mock_cache_manager):
|
|
# news, stock-news, ledmatrix-stocks, ledmatrix-elections,
|
|
# ledmatrix-leaderboard and nfl-draft all do exactly this. Without a
|
|
# setter the property raises "has no setter" and those plugins stop
|
|
# loading entirely.
|
|
from src.plugin_system.base_plugin import BasePlugin
|
|
|
|
class AssigningPlugin(BasePlugin):
|
|
def __init__(self, *args, **kwargs):
|
|
super().__init__(*args, **kwargs)
|
|
self.global_config = self.config.get("global", {})
|
|
|
|
def update(self): pass
|
|
def display(self, force_clear=False): pass
|
|
|
|
plugin = AssigningPlugin(
|
|
"news", {"enabled": True, "global": {"scroll_speed": 2}},
|
|
mock_display_manager, mock_cache_manager, self._manager_with({"target_fps": 100}),
|
|
)
|
|
# The plugin's own value wins over the resolved config.
|
|
assert plugin.global_config == {"scroll_speed": 2}
|
|
|
|
def test_template_ships_a_global_target_fps(self):
|
|
# The plumbing is useless if the setting isn't in the shipped config.
|
|
import json
|
|
with open("config/config.template.json") as fh:
|
|
template = json.load(fh)
|
|
assert template.get("target_fps") == 100
|