Files
LEDMatrix/test/test_display_controller_plugin_toggle.py
T
7a6bad29fe feat(display-controller): hot-reload plugin enable/disable without a restart (#374)
* feat(display-controller): hot-reload plugin enable/disable without a restart

Enabling or disabling a plugin in config previously required restarting the
display service: the plugin list and available_modes were built once at init
and the run loop never revisited them. (Per-plugin config *values* already
hot-reloaded; only the enabled set was restart-only.)

Now the controller reconciles its running plugins against the config's enabled
set whenever that set changes:

- The ConfigService watcher thread only sets a `_pending_plugin_reconcile`
  flag (via a cheap enabled-set diff). It never mutates loop state.
- The run loop applies the reconcile on its own thread (top of each
  iteration, deferred while on-demand is active), so loading/unloading and
  rebuilding available_modes can't race with rendering.
- `_reconcile_enabled_plugins` diffs desired vs running plugins, unloads the
  removed ones (cleanup + on_disable + config-unsubscribe via the new
  `_unregister_plugin`) and loads the added ones, then clamps the rotation
  index so the current mode stays valid.

The per-plugin registration done at startup is extracted into
`_register_loaded_plugin` and reused by the live-enable path so both build
identical state. Extracting it also fixes a latent late-binding bug: the
per-plugin config-change callbacks were closures over the loop variable, so
every plugin's callback targeted the last-loaded instance; each now binds its
own id/instance.

Adds test/test_display_controller_plugin_toggle.py covering live enable,
live disable, index clamping, no-op when unchanged, and the enabled-set diff.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(display-controller): don't exit on empty available_modes, guard rotation modulo

Hot-reload means available_modes can legitimately be empty at startup (no
plugins enabled yet) and become non-empty later via the web UI, or vice
versa mid-run. Fix four issues found reviewing this PR:

- run() exited the process entirely when available_modes was empty at
  startup instead of idling, permanently defeating the point of live
  enable/disable for anyone who starts with zero plugins enabled.
- The mode-rotation step divided by len(available_modes) unconditionally,
  raising ZeroDivisionError if the last enabled plugin is disabled between
  frames.
- _reconcile_enabled_plugins() called .get('enabled', False) on a config
  section without checking it was a dict first, raising AttributeError on
  a malformed config value.
- Minor: pop the config-change callback only after attempting to
  unsubscribe it, and log the exception in the config-read fallback
  instead of swallowing it silently.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ

* fix(display-controller): address review findings on the hot-reload PR

- Idle-wait tick was a fixed 30s sleep, delaying pickup of a plugin
  enabled via the web UI while no modes were active. Shortened to ~1s so
  it's roughly as responsive as the per-frame check once modes exist.
- _unregister_plugin popped the config-change callback from
  _plugin_config_callbacks even when config_service.unsubscribe() raised,
  losing the only reference to it. Now only pops on a successful
  unsubscribe.
- _pending_plugin_reconcile was cleared before _reconcile_enabled_plugins()
  ran, so a retryable failure (e.g. plugin discovery erroring) silently
  dropped the enable/disable request. _reconcile_enabled_plugins() now
  returns True/False and the caller only clears the flag on True.
- Added a warning log for the malformed-config case (a plugin's config
  section present but not a dict) so it's actually visible, and updated
  the existing test to assert it via caplog.

Left the broad `except Exception` around config_service.unsubscribe() as
Exception -- the current implementation is a simple lock+dict/list op that
doesn't document or realistically raise a narrower type, so this is a
defensive catch-all, not user error handling.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: ChuckBuilds <charlesmynard@gmail.com>
2026-07-06 17:37:01 -04:00

256 lines
11 KiB
Python

"""Tests for live plugin enable/disable hot-reload in DisplayController.
Enabling or disabling a plugin in config used to require a full display
restart because the plugin list and available_modes were built once at init.
These tests cover the reconcile path that loads/unloads plugins and rebuilds
the dispatch maps on the main thread when the enabled set changes.
"""
from unittest.mock import MagicMock
def _make_plugin(modes):
plugin = MagicMock()
plugin.modes = list(modes)
return plugin
def _wire_plugin_manager(controller, plugins, discovered=None):
"""Point the controller's mock plugin_manager at a set of fake plugins.
`plugins` maps plugin_id -> mock instance (with a .modes list).
"""
pm = controller.plugin_manager
pm.discover_plugins.return_value = list(discovered if discovered is not None else plugins.keys())
pm.load_plugin.return_value = True
pm.unload_plugin.return_value = True
pm.plugin_manifests = {}
pm.get_plugin.side_effect = lambda pid: plugins.get(pid)
return pm
def _set_config(controller, cfg):
controller.config_service.get_config = lambda: cfg
class TestPluginEnableDisableHotReload:
def test_enable_plugin_live(self, test_display_controller):
controller = test_display_controller
assert controller.available_modes == []
plugin = _make_plugin(["foo"])
_wire_plugin_manager(controller, {"foo": plugin})
_set_config(controller, {"foo": {"enabled": True}})
controller._reconcile_enabled_plugins()
assert "foo" in controller.plugin_display_modes
assert "foo" in controller.available_modes
assert controller.plugin_modes["foo"] is plugin
assert controller.mode_to_plugin_id["foo"] == "foo"
controller.plugin_manager.load_plugin.assert_any_call("foo")
def test_disable_plugin_live(self, test_display_controller):
controller = test_display_controller
plugin = _make_plugin(["live", "recent"])
_wire_plugin_manager(controller, {"sports": plugin}, discovered=["sports"])
# Enable, then disable.
_set_config(controller, {"sports": {"enabled": True}})
controller._reconcile_enabled_plugins()
assert "sports" in controller.plugin_display_modes
assert "live" in controller.available_modes and "recent" in controller.available_modes
assert "sports" in controller._plugin_config_callbacks
_set_config(controller, {"sports": {"enabled": False}})
controller._reconcile_enabled_plugins()
assert "sports" not in controller.plugin_display_modes
assert "live" not in controller.available_modes
assert "recent" not in controller.available_modes
assert "live" not in controller.plugin_modes
assert "recent" not in controller.mode_to_plugin_id
controller.plugin_manager.unload_plugin.assert_any_call("sports")
assert "sports" not in controller._plugin_config_callbacks
def test_disable_clamps_current_mode_index(self, test_display_controller):
controller = test_display_controller
p1 = _make_plugin(["a"])
p2 = _make_plugin(["b"])
_wire_plugin_manager(controller, {"p1": p1, "p2": p2}, discovered=["p1", "p2"])
_set_config(controller, {"p1": {"enabled": True}, "p2": {"enabled": True}})
controller._reconcile_enabled_plugins()
# Add order across multiple plugins is set-driven (as at init), so
# compare membership, not order.
assert set(controller.available_modes) == {"a", "b"}
# Pretend we're currently showing p2's mode.
controller.current_mode_index = controller.available_modes.index("b")
controller.current_display_mode = "b"
_set_config(controller, {"p1": {"enabled": True}, "p2": {"enabled": False}})
controller._reconcile_enabled_plugins()
assert controller.available_modes == ["a"]
# Index must be back in range and the display mode no longer the removed one.
assert 0 <= controller.current_mode_index < len(controller.available_modes)
assert controller.current_display_mode == "a"
def test_enable_keeps_current_mode(self, test_display_controller):
controller = test_display_controller
p1 = _make_plugin(["a"])
p2 = _make_plugin(["b"])
_wire_plugin_manager(controller, {"p1": p1, "p2": p2}, discovered=["p1", "p2"])
_set_config(controller, {"p1": {"enabled": True}})
controller._reconcile_enabled_plugins()
controller.current_mode_index = 0
controller.current_display_mode = "a"
# Enabling p2 should not disturb the currently-showing mode.
_set_config(controller, {"p1": {"enabled": True}, "p2": {"enabled": True}})
controller._reconcile_enabled_plugins()
assert "b" in controller.available_modes
assert controller.current_display_mode == "a"
assert controller.available_modes[controller.current_mode_index] == "a"
def test_noop_when_enabled_set_unchanged(self, test_display_controller):
controller = test_display_controller
plugin = _make_plugin(["foo"])
_wire_plugin_manager(controller, {"foo": plugin}, discovered=["foo"])
_set_config(controller, {"foo": {"enabled": True}})
controller._reconcile_enabled_plugins()
load_calls = controller.plugin_manager.load_plugin.call_count
unload_calls = controller.plugin_manager.unload_plugin.call_count
# Reconcile again with no change — must not load/unload anything.
controller._reconcile_enabled_plugins()
assert controller.plugin_manager.load_plugin.call_count == load_calls
assert controller.plugin_manager.unload_plugin.call_count == unload_calls
def test_reconcile_ignores_non_dict_config_value(self, test_display_controller, caplog):
"""A malformed config value (e.g. a stray string where a plugin's
section should be a dict) must be treated as disabled, not crash
the reconcile with AttributeError, and should be logged so it's
visible to whoever has to debug the malformed config."""
controller = test_display_controller
plugin = _make_plugin(["foo"])
_wire_plugin_manager(controller, {"foo": plugin}, discovered=["foo"])
_set_config(controller, {"foo": "not-a-dict"})
with caplog.at_level("WARNING"):
controller._reconcile_enabled_plugins() # must not raise
assert "foo" not in controller.plugin_display_modes
assert "foo" not in controller.available_modes
assert any("foo" in r.message and "not a dict" in r.message for r in caplog.records)
def test_disable_keeps_callback_when_unsubscribe_fails(self, test_display_controller):
"""If config_service.unsubscribe() raises, _unregister_plugin must
keep the callback in _plugin_config_callbacks rather than losing the
only reference to it (it still tears down the plugin itself)."""
controller = test_display_controller
plugin = _make_plugin(["live"])
_wire_plugin_manager(controller, {"sports": plugin}, discovered=["sports"])
_set_config(controller, {"sports": {"enabled": True}})
controller._reconcile_enabled_plugins()
assert "sports" in controller._plugin_config_callbacks
controller.config_service.unsubscribe = MagicMock(side_effect=RuntimeError("boom"))
_set_config(controller, {"sports": {"enabled": False}})
controller._reconcile_enabled_plugins()
assert "sports" not in controller.plugin_display_modes
assert "sports" in controller._plugin_config_callbacks
class TestReconcileReturnValue:
"""_reconcile_enabled_plugins() returns True/False so the caller (run()'s
loop) only clears _pending_plugin_reconcile on success, keeping a
retryable failure's request alive instead of silently dropping it."""
def test_returns_true_on_success(self, test_display_controller):
controller = test_display_controller
plugin = _make_plugin(["foo"])
_wire_plugin_manager(controller, {"foo": plugin}, discovered=["foo"])
_set_config(controller, {"foo": {"enabled": True}})
assert controller._reconcile_enabled_plugins() is True
def test_returns_true_for_noop(self, test_display_controller):
controller = test_display_controller
_wire_plugin_manager(controller, {}, discovered=[])
_set_config(controller, {})
assert controller._reconcile_enabled_plugins() is True
def test_returns_false_on_discovery_failure(self, test_display_controller):
controller = test_display_controller
controller.plugin_manager.discover_plugins.side_effect = RuntimeError("boom")
_set_config(controller, {})
assert controller._reconcile_enabled_plugins() is False
def test_returns_true_when_no_plugin_manager(self, test_display_controller):
controller = test_display_controller
controller.plugin_manager = None
assert controller._reconcile_enabled_plugins() is True
class TestRunWithNoModesEnabled:
"""Before hot-reload, an empty available_modes at startup was permanent
-- the display never came back without a restart. Now that a plugin can
be enabled live from the web UI, run() must idle rather than exit."""
def test_idles_instead_of_exiting(self, test_display_controller):
controller = test_display_controller
assert controller.available_modes == []
sleep_calls = []
def fake_sleep(duration, tick_interval=1.0):
sleep_calls.append(duration)
if len(sleep_calls) >= 3:
# Stand in for the process being torn down; run() catches
# this via its broad except + finally, same as any other
# unexpected error during the loop.
raise RuntimeError("stop-test-loop")
controller._sleep_with_plugin_updates = fake_sleep
controller.run()
# Old behavior returned before ever reaching the loop body, so
# _sleep_with_plugin_updates would never have been called. The idle
# tick is short (not a long sleep) so a plugin enabled via the web
# UI while idle is picked up about as promptly as it would be once
# modes exist and the loop is iterating per-frame.
assert sleep_calls == [1, 1, 1]
class TestEnabledSetChanged:
def test_detects_toggle(self, test_display_controller):
c = test_display_controller
assert c._enabled_set_changed({"a": {"enabled": True}}, {"a": {"enabled": False}}) is True
def test_no_change(self, test_display_controller):
c = test_display_controller
cfg = {"a": {"enabled": True}, "b": {"enabled": False}}
assert c._enabled_set_changed(cfg, dict(cfg)) is False
def test_new_enabled_section(self, test_display_controller):
c = test_display_controller
assert c._enabled_set_changed(
{"a": {"enabled": True}},
{"a": {"enabled": True}, "b": {"enabled": True}},
) is True
def test_ignores_non_enabled_value_edits(self, test_display_controller):
c = test_display_controller
assert c._enabled_set_changed(
{"a": {"enabled": True, "duration": 30}},
{"a": {"enabled": True, "duration": 45}},
) is False