diff --git a/src/plugin_system/plugin_manager.py b/src/plugin_system/plugin_manager.py index 22f8b470..9a2512df 100644 --- a/src/plugin_system/plugin_manager.py +++ b/src/plugin_system/plugin_manager.py @@ -12,6 +12,7 @@ import subprocess import time import threading +import types from pathlib import Path from typing import Dict, List, Optional, Any import logging @@ -828,8 +829,18 @@ def run_scheduled_updates(self, current_time: Optional[float] = None) -> None: # If resource monitor exists, wrap the call def monitored_update(): self.resource_monitor.monitor_call(plugin_id, plugin_instance.update) + # SimpleNamespace stores `update` as an *instance* + # attribute, so attribute lookup returns the plain + # function object as-is. A dynamically-built class + # (`type(..., {'update': monitored_update})`) instead + # stores it as a *class* attribute, which the + # descriptor protocol turns into a bound method on + # access -- silently prepending the instance as an + # implicit first argument to a function that takes + # none, raising "monitored_update() takes 0 + # positional arguments but 1 was given" on every call. success = self.plugin_executor.execute_update( - type('obj', (object,), {'update': monitored_update})(), + types.SimpleNamespace(update=monitored_update), plugin_id ) else: diff --git a/test/test_plugin_system.py b/test/test_plugin_system.py index b78a3156..ae44d710 100644 --- a/test/test_plugin_system.py +++ b/test/test_plugin_system.py @@ -3,6 +3,7 @@ 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.""" @@ -74,10 +75,58 @@ def test_load_plugin_missing_manifest(self, mock_config_manager, mock_display_ma # 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()) + + 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 TestPluginLoader: """Test PluginLoader functionality."""