From 84afa9d64f2a8c91e55d914571db900bb0b347c6 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Wed, 23 Sep 2026 12:36:26 -0400 Subject: [PATCH] refactor: delete dead Python code in the core (and stop storing Wi-Fi passwords) (#608) * refactor(plugins): remove the no-op PluginHealthMonitor Its monitor loop did nothing (`if callbacks: pass`), register_health_check had no callers and api_v3.health_monitor was never read by any route. The live health data comes from PluginHealthTracker, which is untouched. Co-Authored-By: Claude Opus 5.5 * refactor(store): drop the never-set uninstall tombstones Nothing in production called mark_recently_uninstalled, so the reconciler's was_recently_uninstalled check was always False. The persistent uninstall registry is what actually stops resurrection; the reconciler test now exercises that gate instead. Co-Authored-By: Claude Opus 5.5 * refactor(common): delete unused config/display/game helpers, utils and error_handler Nothing in core, the web UI, scripts or the plugin monorepo imports config_helper, display_helper, game_helper, utils or error_handler; only their own tests did. The error_handler re-exports leave src.common's __all__; APIHelper, TextHelper, ScrollHelper, LogoHelper and the adaptive layout exports are unchanged. Co-Authored-By: Claude Opus 5.5 * refactor(config): drop ConfigService's unused versioning and save API ConfigVersion, get_version/get_version_history/get_version_config, rollback, save_config, reload, get_plugin_config and the backward-compat load_config/get_config_path/get_secrets_path had no callers. The display controller only uses get_config, subscribe, unsubscribe and shutdown, plus the file watcher. Change detection now compares against the current checksum instead of the last history entry. The subscriber tests asserted `callback.called or True`; they now reload the way the watcher does and assert the notification. Co-Authored-By: Claude Opus 5.5 * refactor(plugins): drop unread plugin state history and callbacks plugin_state.PluginStateManager kept a bounded per-plugin transition history that only get_state_history (tests only) read; get_state_info reports a separate lifetime count, which stays. set_error_info and record_display had no callers, and set_state_with_error's `error` argument only fed the history. The web-side state_manager.PluginStateManager loses subscribe_to_state_changes, _notify_callbacks, set_plugin_error and get_state_version, none of which had callers; with no subscribers the old-state copy in update_plugin_state went with them. Co-Authored-By: Claude Opus 5.5 * refactor(plugins): remove unused PluginManager methods and attribute guards update_all_plugins was only called by a test (the display loop uses run_scheduled_updates); get_plugin_health_metrics, get_plugin_resource_metrics and get_plugin_state had no callers; and plugin_modules was written but never read. plugin_directories is now initialised in __init__, so the hasattr() guards around it go. Co-Authored-By: Claude Opus 5.5 * refactor(plugins): remove unused executor, loader, store and package helpers - PluginExecutor.execute_safe: no callers. - PluginLoader._parse_semver: only its own tests; compatibility.parse_semver is the live copy and test_compatibility.py already covers it. - PluginStoreManager.get_installed_plugin_info: no callers. - PluginResourceMonitor._local: never read. - src.plugin_system.get_store_manager and __api_version__: no importers in core, scripts or the plugin monorepo. Co-Authored-By: Claude Opus 5.5 * fix(wifi): stop storing Wi-Fi passwords in wifi_config.json WiFiManager appended every joined network's SSID and password, in plaintext, to saved_networks in config/wifi_config.json, and nothing (web UI, backup restore, scripts) ever read them back: NetworkManager keeps its own credentials. The writes are gone, and loading the config now drops any saved_networks key and rewrites the file, so passwords already on disk are scrubbed. Also removes _check_dnsmasq_conflict (never called) and _detect_trixie, whose result only reached one log line, along with the NM_CONNECTIONS_PATHS constant only it used. Co-Authored-By: Claude Opus 5.5 * refactor(display): remove unreachable and unused DisplayController code - _follower_rebuild_scroll_image: never called. - mode_duration (never read) and last_mode_change (write-only). - The `chosen_cap <= 0` branch: chosen_cap is either the minimum of caps already filtered to > 0 or DEFAULT_DYNAMIC_DURATION_CAP (180). - The `max_duration < min_duration` branch directly after `max_duration = max(min_duration, max_duration)`. - The circuit-breaker branch's `display_result = False` and `manager_to_display = None`: the first is overwritten a few lines later, the second is already None there. - The bool-to-bool conversion of execute_display's result, which is always a bool. - The `loaded_plugins` lookup in _update_modules: PluginManager has no such attribute, so it always fell through to `plugins`. Co-Authored-By: Claude Opus 5.5 * refactor(vegas): remove unused config update, boundary finder and refresh VegasModeConfig.update had no callers outside its own tests (the coordinator rebuilds the config with from_config on a change); geometry.find_item_boundary and StreamManager._refresh_plugin_content had no callers at all. Co-Authored-By: Claude Opus 5.5 * refactor(run): drop the debug block that pretended to import the plugin system In debug mode run.py put src/plugin_system itself on sys.path and printed "Plugin system import successful" without importing anything. Nothing imports plugin_system modules by bare name, so the path entry did nothing either. Co-Authored-By: Claude Opus 5.5 * test: delete tests that test nothing - test/plugins/test_{basketball_scoreboard,calendar,clock_simple, odds_ticker,soccer_scoreboard,text_display}.py skip everywhere the named plugins are not installed, including CI (LEDMATRIX_PLUGINS_DIR holds only the fixture plugin); test_plugin_matrix.py already covers every discovered plugin. Their PluginTestBase and the fixtures only it used (plugins_dir, mock_display_manager, mock_cache_manager, mock_plugin_manager, base_plugin_config in test/plugins/conftest.py) go with them. - test_plugin_system.py: test_discover_plugins (body was `pass`) and test_dependency_check (a comment), plus the test_plugin_manager fixture only the former requested. - test_display_manager.py: test_draw_image asserted that an image it had just assigned was not None. - test_display_controller.py: the rotation and schedule-override tests re-implemented the run-loop arithmetic inline and asserted on their own result without calling the controller. Co-Authored-By: Claude Opus 5.5 * test: expect one plugin_last_update success stamp after update_all_plugins EveryStampRecordsACompletion required at least two success-path stamps; the second was update_all_plugins, removed as test-only. The worker and synchronous paths share the remaining stamp in _execute_update_now, and the check that every stamp calls _note_update_completed is unchanged. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- run.py | 17 - src/common/README.md | 58 +-- src/common/__init__.py | 20 +- src/common/config_helper.py | 361 -------------- src/common/display_helper.py | 303 ------------ src/common/error_handler.py | 220 --------- src/common/game_helper.py | 452 ------------------ src/common/utils.py | 331 ------------- src/config_service.py | 197 +------- src/display_controller.py | 63 +-- src/plugin_system/__init__.py | 13 - src/plugin_system/health_monitor.py | 319 ------------ src/plugin_system/plugin_executor.py | 37 -- src/plugin_system/plugin_loader.py | 15 - src/plugin_system/plugin_manager.py | 123 +---- src/plugin_system/plugin_state.py | 132 +---- src/plugin_system/resource_monitor.py | 3 - src/plugin_system/state_manager.py | 107 +---- src/plugin_system/state_reconciliation.py | 14 +- src/plugin_system/store_manager.py | 47 +- src/vegas_mode/config.py | 82 ---- src/vegas_mode/geometry.py | 39 -- src/vegas_mode/stream_manager.py | 18 - src/wifi_manager.py | 113 +---- test/conftest.py | 30 -- test/plugins/conftest.py | 95 ---- test/plugins/test_basketball_scoreboard.py | 95 ---- test/plugins/test_calendar.py | 63 --- test/plugins/test_clock_simple.py | 103 ---- test/plugins/test_odds_ticker.py | 62 --- test/plugins/test_plugin_base.py | 305 ------------ test/plugins/test_soccer_scoreboard.py | 94 ---- test/plugins/test_text_display.py | 114 ----- test/test_config_helper.py | 253 ---------- test/test_config_service.py | 36 +- test/test_display_controller.py | 54 --- test/test_display_helper.py | 307 ------------ test/test_display_manager.py | 17 - test/test_error_handling.py | 90 +--- test/test_game_helper.py | 317 ------------ test/test_health_monitor.py | 307 ------------ test/test_initial_update_budget.py | 5 +- test/test_loader_compat_warning.py | 11 - test/test_plugin_state_history_cap.py | 166 ------- test/test_plugin_state_history_retention.py | 209 -------- test/test_plugin_state_transition_count.py | 119 +++++ test/test_plugin_system.py | 23 +- test/test_plugin_update_reservation.py | 11 - test/test_store_manager_caches.py | 58 +-- test/test_update_change_reporting.py | 6 +- test/test_utils.py | 329 ------------- test/test_vegas_config.py | 47 +- test/test_vegas_density.py | 5 - test/test_version_consistency.py | 3 +- test/test_wifi_manager_ap.py | 65 ++- .../test_state_reconciliation.py | 7 +- web_interface/app.py | 37 +- 57 files changed, 273 insertions(+), 6254 deletions(-) delete mode 100644 src/common/config_helper.py delete mode 100644 src/common/display_helper.py delete mode 100644 src/common/error_handler.py delete mode 100644 src/common/game_helper.py delete mode 100644 src/common/utils.py delete mode 100644 src/plugin_system/health_monitor.py delete mode 100644 test/plugins/test_basketball_scoreboard.py delete mode 100644 test/plugins/test_calendar.py delete mode 100644 test/plugins/test_clock_simple.py delete mode 100644 test/plugins/test_odds_ticker.py delete mode 100644 test/plugins/test_plugin_base.py delete mode 100644 test/plugins/test_soccer_scoreboard.py delete mode 100644 test/plugins/test_text_display.py delete mode 100644 test/test_config_helper.py delete mode 100644 test/test_display_helper.py delete mode 100644 test/test_game_helper.py delete mode 100644 test/test_health_monitor.py delete mode 100644 test/test_plugin_state_history_cap.py delete mode 100644 test/test_plugin_state_history_retention.py create mode 100644 test/test_plugin_state_transition_count.py delete mode 100644 test/test_utils.py diff --git a/run.py b/run.py index c5e387c6..a1400c68 100755 --- a/run.py +++ b/run.py @@ -41,23 +41,6 @@ if debug_mode: print(f"DEBUG: Current working directory: {os.getcwd()}", flush=True) print(f"DEBUG: EMULATOR mode: {os.environ.get('EMULATOR', 'false')}", flush=True) -# Additional debugging for plugin system (only in debug mode) -if debug_mode: - try: - plugin_system_path = os.path.join(project_dir, 'src', 'plugin_system') - if plugin_system_path not in sys.path: - sys.path.insert(0, plugin_system_path) - print(f"DEBUG: Added plugin_system path to sys.path: {plugin_system_path}", flush=True) - - # Try to import the plugin system directly to get better error info - print("DEBUG: Attempting to import src.plugin_system...", flush=True) - print("DEBUG: Plugin system import successful", flush=True) - except ImportError as e: - print(f"DEBUG: Plugin system import failed: {e}", flush=True) - print(f"DEBUG: Import error details: {type(e).__name__}", flush=True) - except Exception as e: - print(f"DEBUG: Unexpected error during plugin system import: {e}", flush=True) - # Configure logging before importing any other modules # Use centralized logging configuration from src.logging_config import setup_logging diff --git a/src/common/README.md b/src/common/README.md index 4246ccff..a35ee734 100644 --- a/src/common/README.md +++ b/src/common/README.md @@ -24,55 +24,10 @@ fractional scaling), `LayoutContext` (`fit_text`, `fit_image`, `by_tier`, `px`), and composite carvers `scoreboard_regions()` / `media_row()`. Full guide: [docs/ADAPTIVE_LAYOUT.md](../../docs/ADAPTIVE_LAYOUT.md). -## Error Handling (`error_handler.py`) - -Common error handling patterns and utilities: - -- `handle_file_operation()` - Handle file I/O with consistent error handling -- `handle_json_operation()` - Handle JSON operations with error handling -- `safe_execute()` - Safely execute operations with error handling -- `retry_on_failure()` - Decorator for retrying failed operations -- `log_and_continue()` - Log non-critical errors and continue -- `log_and_raise()` - Log errors and raise exceptions - -### Example Usage - -```python -from src.common.error_handler import handle_json_operation, safe_execute - -# Handle JSON loading -config = handle_json_operation( - lambda: json.load(open('config.json')), - "Failed to load config", - logger, - default={} -) - -# Safe execution with error handling -result = safe_execute( - lambda: risky_operation(), - "Operation failed", - logger, - default=None -) -``` - ## API Helpers (`api_helper.py`) Utilities for making HTTP requests and handling API responses. -## Configuration Helpers (`config_helper.py`) - -Utilities for loading, saving, and validating configuration files. - -## Display Helpers (`display_helper.py`) - -Utilities for rendering content to the LED matrix display. - -## Game Helpers (`game_helper.py`) - -Utilities for processing game data and team information. - ## Logo Helpers (`logo_helper.py`) Utilities for loading and managing team logos. @@ -85,14 +40,6 @@ Utilities for text processing and formatting. Utilities for scrolling text on the display. -## General Utilities (`utils.py`) - -General-purpose utility functions: -- Team abbreviation normalization -- Time formatting -- Boolean parsing -- Logger creation (deprecated - use `src.logging_config.get_logger()`) - ## Permission Utilities (`permission_utils.py`) Helpers for ensuring directory permissions and ownership are correct @@ -102,6 +49,5 @@ persistent cache directory). ## Best Practices 1. **Use centralized logging**: Import from `src.logging_config` instead of creating loggers directly -2. **Use error handlers**: Use `error_handler` utilities for consistent error handling -3. **Reuse utilities**: Check existing utilities before creating new ones -4. **Document additions**: Add documentation when adding new utilities +2. **Reuse utilities**: Check existing utilities before creating new ones +3. **Document additions**: Add documentation when adding new utilities diff --git a/src/common/__init__.py b/src/common/__init__.py index 03588175..9b3a8925 100644 --- a/src/common/__init__.py +++ b/src/common/__init__.py @@ -2,25 +2,13 @@ Common utilities and helpers for LEDMatrix. This package provides reusable functionality for plugins and core modules: -- Error handling utilities - API helpers -- Configuration helpers -- Display helpers -- Game/team helpers - Logo helpers - Text/scroll helpers -- General utilities +- Adaptive layout and image helpers """ # Export commonly used utilities -from src.common.error_handler import ( - handle_file_operation, - handle_json_operation, - safe_execute, - retry_on_failure, - log_and_continue, - log_and_raise -) from src.common.api_helper import APIHelper from src.common.scroll_helper import ScrollHelper from src.common import scroll_config @@ -59,12 +47,6 @@ from src.adaptive_images import ( ) __all__ = [ - 'handle_file_operation', - 'handle_json_operation', - 'safe_execute', - 'retry_on_failure', - 'log_and_continue', - 'log_and_raise', 'APIHelper', 'ScrollHelper', 'scroll_config', diff --git a/src/common/config_helper.py b/src/common/config_helper.py deleted file mode 100644 index b3e5aeac..00000000 --- a/src/common/config_helper.py +++ /dev/null @@ -1,361 +0,0 @@ -""" -Config Helper - -Handles configuration management and validation for LED matrix plugins. -Extracted from LEDMatrix core to provide reusable functionality for plugins. -""" - -import copy -import json -import logging -from pathlib import Path -from typing import Any, Dict, List, Optional, Union - - -class ConfigHelper: - """ - Helper class for configuration management and validation. - - Provides functionality for: - - Loading and saving configuration files - - Validating configuration against schemas - - Merging configurations - - Getting configuration values with defaults - - Configuration schema validation - """ - - def __init__(self, logger: Optional[logging.Logger] = None): - """ - Initialize the ConfigHelper. - - Args: - logger: Optional logger instance - """ - self.logger = logger or logging.getLogger(__name__) - - def load_config(self, config_path: Union[str, Path]) -> Dict[str, Any]: - """ - Load configuration from a JSON file. - - Args: - config_path: Path to configuration file - - Returns: - Configuration dictionary - """ - config_path = Path(config_path) - - try: - if not config_path.exists(): - self.logger.warning(f"Configuration file not found: {config_path}") - return {} - - with open(config_path, 'r', encoding='utf-8') as f: - config = json.load(f) - - self.logger.debug(f"Loaded configuration from {config_path}") - return config - - except json.JSONDecodeError as e: - self.logger.error(f"Invalid JSON in configuration file {config_path}: {e}") - return {} - except Exception as e: - self.logger.error(f"Error loading configuration from {config_path}: {e}") - return {} - - def save_config(self, config: Dict[str, Any], config_path: Union[str, Path]) -> bool: - """ - Save configuration to a JSON file. - - Args: - config: Configuration dictionary to save - config_path: Path to save configuration file - - Returns: - True if successful, False otherwise - """ - config_path = Path(config_path) - - try: - # Ensure directory exists - config_path.parent.mkdir(parents=True, exist_ok=True) - - with open(config_path, 'w', encoding='utf-8') as f: - json.dump(config, f, indent=2, ensure_ascii=False) - - self.logger.debug(f"Saved configuration to {config_path}") - return True - - except Exception as e: - self.logger.error(f"Error saving configuration to {config_path}: {e}") - return False - - def get_config_value(self, config: Dict[str, Any], key: str, - default: Any = None, required: bool = False) -> Any: - """ - Get a configuration value with optional default. - - Args: - config: Configuration dictionary - key: Configuration key (supports dot notation like 'display.width') - default: Default value if key not found - required: If True, raise error if key not found - - Returns: - Configuration value or default - """ - try: - # Support dot notation for nested keys - keys = key.split('.') - value = config - - for k in keys: - if isinstance(value, dict) and k in value: - value = value[k] - else: - if required: - raise KeyError(f"Required configuration key not found: {key}") - return default - - return value - - except Exception as e: - if required: - raise - self.logger.warning(f"Error getting config value for {key}: {e}") - return default - - def set_config_value(self, config: Dict[str, Any], key: str, value: Any) -> None: - """ - Set a configuration value. - - Args: - config: Configuration dictionary to modify - key: Configuration key (supports dot notation) - value: Value to set - """ - try: - # Support dot notation for nested keys - keys = key.split('.') - current = config - - # Navigate to parent of target key - for k in keys[:-1]: - if k not in current: - current[k] = {} - current = current[k] - - # Set the value - current[keys[-1]] = value - - except Exception as e: - self.logger.error(f"Error setting config value for {key}: {e}") - - def merge_configs(self, base_config: Dict[str, Any], - override_config: Dict[str, Any]) -> Dict[str, Any]: - """ - Merge two configuration dictionaries. - - Args: - base_config: Base configuration - override_config: Configuration to merge in (takes precedence) - - Returns: - Merged configuration dictionary (fully independent of both - inputs — a shallow copy would alias un-overridden nested dicts, - so mutating the result would mutate the caller's base config). - """ - merged = copy.deepcopy(base_config) - - for key, value in override_config.items(): - if key in merged and isinstance(merged[key], dict) and isinstance(value, dict): - # Recursively merge nested dictionaries - merged[key] = self.merge_configs(merged[key], value) - else: - # Override with new value — deep-copied so mutating the - # merged result can't reach back into override_config. - merged[key] = copy.deepcopy(value) - - return merged - - def validate_config(self, config: Dict[str, Any], - schema: Optional[Dict[str, Any]] = None) -> bool: - """ - Validate configuration against a schema. - - Args: - config: Configuration to validate - schema: Validation schema (optional) - - Returns: - True if valid, False otherwise - """ - if schema is None: - # Basic validation - just check if it's a dictionary - return isinstance(config, dict) - - try: - return self._validate_against_schema(config, schema) - except Exception as e: - self.logger.error(f"Configuration validation error: {e}") - return False - - def get_plugin_config(self, config: Dict[str, Any], plugin_id: str) -> Dict[str, Any]: - """ - Get plugin-specific configuration. - - Args: - config: Full configuration dictionary - plugin_id: Plugin identifier - - Returns: - Plugin-specific configuration - """ - plugin_key = f"{plugin_id}_config" - return config.get(plugin_key, {}) - - def create_default_config(self, plugin_id: str, - default_values: Dict[str, Any]) -> Dict[str, Any]: - """ - Create a default configuration for a plugin. - - Args: - plugin_id: Plugin identifier - default_values: Default configuration values - - Returns: - Default configuration dictionary - """ - return { - f"{plugin_id}_config": default_values - } - - def validate_required_keys(self, config: Dict[str, Any], - required_keys: List[str]) -> List[str]: - """ - Validate that required keys are present in configuration. - - Args: - config: Configuration to validate - required_keys: List of required keys - - Returns: - List of missing keys - """ - missing_keys = [] - - for key in required_keys: - if not self._has_key(config, key): - missing_keys.append(key) - - return missing_keys - - def get_display_config(self, config: Dict[str, Any]) -> Dict[str, Any]: - """ - Get display-related configuration. - - Args: - config: Full configuration dictionary - - Returns: - Display configuration - """ - return config.get('display', {}) - - def get_sports_config(self, config: Dict[str, Any], sport: str) -> Dict[str, Any]: - """ - Get sport-specific configuration. - - Args: - config: Full configuration dictionary - sport: Sport name (e.g., 'basketball', 'football') - - Returns: - Sport-specific configuration - """ - return config.get(f"{sport}_scoreboard", {}) - - def is_plugin_enabled(self, config: Dict[str, Any], plugin_id: str) -> bool: - """ - Check if a plugin is enabled. - - Args: - config: Full configuration dictionary - plugin_id: Plugin identifier - - Returns: - True if plugin is enabled - """ - plugin_config = self.get_plugin_config(config, plugin_id) - return plugin_config.get('enabled', True) - - def get_favorite_teams(self, config: Dict[str, Any], sport: str) -> List[str]: - """ - Get favorite teams for a sport. - - Args: - config: Full configuration dictionary - sport: Sport name - - Returns: - List of favorite team abbreviations - """ - sport_config = self.get_sports_config(config, sport) - return sport_config.get('favorite_teams', []) - - def get_display_modes(self, config: Dict[str, Any], sport: str) -> Dict[str, bool]: - """ - Get display modes for a sport. - - Args: - config: Full configuration dictionary - sport: Sport name - - Returns: - Dictionary of display modes and their enabled status - """ - sport_config = self.get_sports_config(config, sport) - return sport_config.get('display_modes', {}) - - def _validate_against_schema(self, config: Dict[str, Any], - schema: Dict[str, Any]) -> bool: - """Validate configuration against a schema.""" - # This is a simplified schema validation - # In a real implementation, you might use a library like jsonschema - - for key, schema_info in schema.items(): - if key not in config: - if schema_info.get('required', False): - self.logger.error(f"Missing required configuration key: {key}") - return False - continue - - value = config[key] - expected_type = schema_info.get('type') - - if expected_type and not isinstance(value, expected_type): - self.logger.error(f"Configuration key {key} has wrong type. Expected {expected_type}, got {type(value)}") - return False - - # Validate allowed values - allowed_values = schema_info.get('allowed_values') - if allowed_values and value not in allowed_values: - self.logger.error(f"Configuration key {key} has invalid value: {value}. Allowed: {allowed_values}") - return False - - return True - - def _has_key(self, config: Dict[str, Any], key: str) -> bool: - """Check if a key exists in configuration (supports dot notation).""" - try: - keys = key.split('.') - current = config - - for k in keys: - if not isinstance(current, dict) or k not in current: - return False - current = current[k] - - return True - except Exception: - return False diff --git a/src/common/display_helper.py b/src/common/display_helper.py deleted file mode 100644 index c88c9e63..00000000 --- a/src/common/display_helper.py +++ /dev/null @@ -1,303 +0,0 @@ -""" -Display Helper - -Handles common display operations and layouts for LED matrix displays. -Extracted from LEDMatrix core to provide reusable functionality for plugins. -""" - -import logging -from typing import Any, Dict, Optional, Tuple - -from PIL import Image, ImageDraw, ImageFont - - -class DisplayHelper: - """ - Helper class for common display operations and layouts. - - Provides functionality for: - - Creating base images and overlays - - Common layout patterns (scorebug, ticker, etc.) - - Image compositing and manipulation - - Display dimension utilities - """ - - def __init__(self, display_width: int, display_height: int, - logger: Optional[logging.Logger] = None): - """ - Initialize the DisplayHelper. - - Args: - display_width: Width of the LED matrix display - display_height: Height of the LED matrix display - logger: Optional logger instance - """ - self.display_width = display_width - self.display_height = display_height - self.logger = logger or logging.getLogger(__name__) - - def create_base_image(self, background_color: Tuple[int, int, int] = (0, 0, 0), - mode: str = 'RGB') -> Image.Image: - """ - Create a base image for the display. - - Args: - background_color: Background color (R, G, B) - mode: Image mode ('RGB', 'RGBA', etc.) - - Returns: - PIL Image object - """ - return Image.new(mode, (self.display_width, self.display_height), background_color) - - def create_overlay(self, background_color: Tuple[int, int, int, int] = (0, 0, 0, 0)) -> Image.Image: - """ - Create an overlay image for compositing. - - Args: - background_color: Background color with alpha (R, G, B, A) - - Returns: - PIL Image object with alpha channel - """ - return Image.new('RGBA', (self.display_width, self.display_height), background_color) - - def composite_images(self, base_image: Image.Image, overlay_image: Image.Image) -> Image.Image: - """ - Composite overlay onto base image. - - Args: - base_image: Base image (RGB or RGBA) - overlay_image: Overlay image (should be RGBA) - - Returns: - Composited image - """ - if base_image.mode != 'RGBA': - base_image = base_image.convert('RGBA') - - if overlay_image.mode != 'RGBA': - overlay_image = overlay_image.convert('RGBA') - - return Image.alpha_composite(base_image, overlay_image) - - def draw_scorebug_layout(self, game_data: Dict[str, Any], - fonts: Dict[str, ImageFont.ImageFont], - home_logo: Optional[Image.Image] = None, - away_logo: Optional[Image.Image] = None) -> Image.Image: - """ - Draw a standard scorebug layout for sports games. - - Args: - game_data: Dictionary containing game information - fonts: Dictionary of loaded fonts - home_logo: Home team logo (optional) - away_logo: Away team logo (optional) - - Returns: - PIL Image with scorebug layout - """ - # Create base image and overlay - main_img = self.create_base_image() - overlay = self.create_overlay() - draw = ImageDraw.Draw(overlay) - - # Extract game data - home_score = str(game_data.get('home_score', '0')) - away_score = str(game_data.get('away_score', '0')) - home_abbr = game_data.get('home_abbr', 'HOME') - away_abbr = game_data.get('away_abbr', 'AWAY') - status_text = game_data.get('status_text', '') - period_text = game_data.get('period_text', '') - clock = game_data.get('clock', '') - - # Draw logos if provided - if home_logo and away_logo: - self._draw_logos(main_img, home_logo, away_logo) - - # Draw one combined top line (period/status/clock all share y=1 — - # drawing them separately overprinted each other). - top_line = " ".join(p for p in [period_text, status_text, clock] if p) - if top_line: - self._draw_centered_text(draw, top_line, - fonts.get('time', fonts.get('status')), - y_position=1) - - # Draw scores (center) - score_text = f"{away_score}-{home_score}" - self._draw_centered_text(draw, score_text, fonts.get('score'), - y_position=self.display_height // 2 - 3) - - # Draw team abbreviations (bottom) - if away_abbr: - self._draw_text_with_outline(draw, away_abbr, (0, self.display_height - 12), - fonts.get('team')) - if home_abbr: - text_width = draw.textlength(home_abbr, font=fonts.get('team')) - self._draw_text_with_outline(draw, home_abbr, - (self.display_width - text_width, self.display_height - 12), - fonts.get('team')) - - # Composite and return - final_img = self.composite_images(main_img, overlay) - return final_img.convert('RGB') - - def draw_ticker_layout(self, text: str, font: ImageFont.ImageFont, - background_color: Tuple[int, int, int] = (0, 0, 0), - text_color: Tuple[int, int, int] = (255, 255, 255), - scroll_speed: int = 1) -> Image.Image: - """ - Draw a ticker/scrolling text layout. - - Renders a single static frame with the text at the left edge; the - caller advances the scroll by re-rendering or shifting. The - scroll_speed parameter is accepted for API compatibility but does - not affect this frame. (Previously the text was drawn starting at - x=display_width — entirely off-canvas — so every frame was blank.) - - Args: - text: Text to display - font: Font to use - background_color: Background color - text_color: Text color - scroll_speed: Accepted for compatibility; unused per-frame - - Returns: - PIL Image with ticker layout - """ - img = self.create_base_image(background_color) - draw = ImageDraw.Draw(img) - - self._draw_text_with_outline(draw, text, (0, self.display_height // 2 - 6), - font, fill=text_color) - - return img - - def draw_centered_text(self, text: str, font: ImageFont.ImageFont, - background_color: Tuple[int, int, int] = (0, 0, 0), - text_color: Tuple[int, int, int] = (255, 255, 255)) -> Image.Image: - """ - Draw centered text on the display. - - Args: - text: Text to display - font: Font to use - background_color: Background color - text_color: Text color - - Returns: - PIL Image with centered text - """ - img = self.create_base_image(background_color) - draw = ImageDraw.Draw(img) - - # Calculate center position - text_width = draw.textlength(text, font=font) - text_height = 12 # Approximate height - x = (self.display_width - text_width) // 2 - y = (self.display_height - text_height) // 2 - - # Draw text - self._draw_text_with_outline(draw, text, (x, y), font, fill=text_color) - - return img - - def draw_error_message(self, message: str = "Error") -> Image.Image: - """ - Draw a simple error message. - - Args: - message: Error message to display - - Returns: - PIL Image with error message - """ - # Dark red background, white text - font = ImageFont.load_default() - return self.draw_centered_text(message, font, (50, 0, 0), (255, 255, 255)) - - def draw_no_data_message(self, message: str = "No Data") -> Image.Image: - """ - Draw a no data message. - - Args: - message: Message to display - - Returns: - PIL Image with no data message - """ - font = ImageFont.load_default() - return self.draw_centered_text(message, font, (0, 0, 0), (150, 150, 150)) - - def get_display_dimensions(self) -> Tuple[int, int]: - """ - Get display dimensions. - - Returns: - (width, height) tuple - """ - return (self.display_width, self.display_height) - - def is_portrait(self) -> bool: - """ - Check if display is in portrait orientation. - - Returns: - True if height > width - """ - return self.display_height > self.display_width - - def is_landscape(self) -> bool: - """ - Check if display is in landscape orientation. - - Returns: - True if width > height - """ - return self.display_width > self.display_height - - def get_center_position(self) -> Tuple[int, int]: - """ - Get center position of the display. - - Returns: - (x, y) center position - """ - return (self.display_width // 2, self.display_height // 2) - - def _draw_logos(self, img: Image.Image, home_logo: Image.Image, away_logo: Image.Image) -> None: - """Draw team logos on the image.""" - center_y = self.display_height // 2 - - # Home logo (right side) - if home_logo: - home_x = self.display_width - home_logo.width + 10 - home_y = center_y - (home_logo.height // 2) - img.paste(home_logo, (home_x, home_y), home_logo) - - # Away logo (left side) - if away_logo: - away_x = -10 - away_y = center_y - (away_logo.height // 2) - img.paste(away_logo, (away_x, away_y), away_logo) - - def _draw_centered_text(self, draw: ImageDraw.ImageDraw, text: str, - font: ImageFont.ImageFont, y_position: int) -> None: - """Draw centered text at specified y position.""" - text_width = draw.textlength(text, font=font) - x = (self.display_width - text_width) // 2 - self._draw_text_with_outline(draw, text, (x, y_position), font) - - def _draw_text_with_outline(self, draw: ImageDraw.ImageDraw, text: str, - position: Tuple[int, int], font: ImageFont.ImageFont, - fill: Tuple[int, int, int] = (255, 255, 255), - outline_color: Tuple[int, int, int] = (0, 0, 0)) -> None: - """Draw text with outline for better readability.""" - x, y = position - - # Draw outline - for dx, dy in [(-1, -1), (-1, 0), (-1, 1), (0, -1), (0, 1), (1, -1), (1, 0), (1, 1)]: - draw.text((x + dx, y + dy), text, font=font, fill=outline_color) - - # Draw main text - draw.text((x, y), text, font=font, fill=fill) diff --git a/src/common/error_handler.py b/src/common/error_handler.py deleted file mode 100644 index d783d50e..00000000 --- a/src/common/error_handler.py +++ /dev/null @@ -1,220 +0,0 @@ -""" -Error Handling Utilities - -Common error handling patterns and utilities for consistent error handling -across the LEDMatrix codebase. -""" - -import logging -from typing import Any, Callable, Optional, TypeVar, Dict -from functools import wraps -from src.exceptions import LEDMatrixError - -T = TypeVar('T') - - -def handle_file_operation( - operation: Callable[[], T], - error_message: str, - logger: logging.Logger, - default: Optional[T] = None, - context: Optional[Dict[str, Any]] = None -) -> Optional[T]: - """ - Handle file operations with consistent error handling. - - Args: - operation: Function to execute (file read/write) - error_message: Base error message - logger: Logger instance - default: Default value to return on error - context: Optional context dictionary for error details - - Returns: - Result of operation or default value - """ - try: - return operation() - except FileNotFoundError as e: - logger.warning("%s: File not found: %s", error_message, e, exc_info=True) - return default - except PermissionError as e: - logger.error("%s: Permission denied: %s", error_message, e, exc_info=True) - return default - except (IOError, OSError) as e: - logger.error("%s: I/O error: %s", error_message, e, exc_info=True) - return default - except Exception as e: - logger.error("%s: Unexpected error: %s", error_message, e, exc_info=True) - return default - - -def handle_json_operation( - operation: Callable[[], T], - error_message: str, - logger: logging.Logger, - default: Optional[T] = None, - context: Optional[Dict[str, Any]] = None -) -> Optional[T]: - """ - Handle JSON operations with consistent error handling. - - Args: - operation: Function to execute (JSON load/dump) - error_message: Base error message - logger: Logger instance - default: Default value to return on error - context: Optional context dictionary for error details - - Returns: - Result of operation or default value - """ - try: - return operation() - except FileNotFoundError as e: - logger.warning("%s: File not found: %s", error_message, e, exc_info=True) - return default - except PermissionError as e: - logger.error("%s: Permission denied: %s", error_message, e, exc_info=True) - return default - except ValueError as e: - logger.error("%s: Invalid JSON: %s", error_message, e, exc_info=True) - return default - except (IOError, OSError) as e: - logger.error("%s: I/O error: %s", error_message, e, exc_info=True) - return default - except Exception as e: - logger.error("%s: Unexpected error: %s", error_message, e, exc_info=True) - return default - - -def safe_execute( - operation: Callable[[], T], - error_message: str, - logger: logging.Logger, - default: Optional[T] = None, - raise_on_error: bool = False, - exception_type: type = LEDMatrixError -) -> Optional[T]: - """ - Safely execute an operation with error handling. - - Args: - operation: Function to execute - error_message: Base error message - logger: Logger instance - default: Default value to return on error - raise_on_error: If True, raise exception instead of returning default - exception_type: Type of exception to raise if raise_on_error is True - - Returns: - Result of operation or default value (or raises exception) - """ - try: - return operation() - except LEDMatrixError: - # Re-raise LEDMatrix errors as-is - raise - except Exception as e: - logger.error("%s: %s", error_message, e, exc_info=True) - if raise_on_error: - raise exception_type(error_message, context={'original_error': str(e)}) from e - return default - - -def retry_on_failure( - max_attempts: int = 3, - delay: float = 1.0, - backoff: float = 2.0, - exceptions: tuple = (Exception,), - logger: Optional[logging.Logger] = None -): - """ - Decorator to retry a function on failure. - - Args: - max_attempts: Maximum number of retry attempts - delay: Initial delay between retries in seconds - backoff: Multiplier for delay after each retry - exceptions: Tuple of exceptions to catch and retry on - logger: Optional logger instance - - Returns: - Decorator function - """ - def decorator(func: Callable) -> Callable: - @wraps(func) - def wrapper(*args, **kwargs): - current_delay = delay - last_exception = None - - for attempt in range(max_attempts): - try: - return func(*args, **kwargs) - except exceptions as e: - last_exception = e - if attempt < max_attempts - 1: - if logger: - logger.warning( - "%s failed (attempt %d/%d): %s. Retrying in %.1fs...", - func.__name__, attempt + 1, max_attempts, e, current_delay - ) - import time - time.sleep(current_delay) - current_delay *= backoff - else: - if logger: - logger.error( - "%s failed after %d attempts: %s", - func.__name__, max_attempts, e, exc_info=True - ) - - # If we get here, all attempts failed - raise last_exception - - return wrapper - return decorator - - -def log_and_continue( - logger: logging.Logger, - message: str, - level: int = logging.WARNING, - context: Optional[Dict[str, Any]] = None -): - """ - Log a message and continue execution (for non-critical errors). - - Args: - logger: Logger instance - message: Log message - level: Log level (default: WARNING) - context: Optional context dictionary - """ - if context: - logger.log(level, "%s (context: %s)", message, context) - else: - logger.log(level, message) - - -def log_and_raise( - logger: logging.Logger, - message: str, - exception_type: type = LEDMatrixError, - context: Optional[Dict[str, Any]] = None -): - """ - Log an error and raise an exception. - - Args: - logger: Logger instance - message: Error message - exception_type: Type of exception to raise - context: Optional context dictionary - - Raises: - exception_type: The specified exception type - """ - logger.error(message, exc_info=True) - raise exception_type(message, context=context) - diff --git a/src/common/game_helper.py b/src/common/game_helper.py deleted file mode 100644 index 541f4ec3..00000000 --- a/src/common/game_helper.py +++ /dev/null @@ -1,452 +0,0 @@ -""" -Game Helper - -Handles common game data extraction and processing for LED matrix plugins. -Extracted from LEDMatrix core to provide reusable functionality for plugins. -""" - -import logging -from datetime import datetime, timezone, timedelta -from typing import Any, Dict, List, Optional, Tuple -import pytz - - -class GameHelper: - """ - Helper class for game data extraction and processing. - - Provides functionality for: - - Extracting game details from ESPN API responses - - Filtering games by various criteria - - Processing game data for display - - Time zone handling and date formatting - """ - - def __init__(self, timezone_str: str = 'UTC', logger: Optional[logging.Logger] = None): - """ - Initialize the GameHelper. - - Args: - timezone_str: Timezone string for date/time processing - logger: Optional logger instance - """ - self.logger = logger or logging.getLogger(__name__) - self.timezone = self._get_timezone(timezone_str) - - def extract_game_details(self, event: Dict[str, Any], sport: str = None) -> Optional[Dict[str, Any]]: - """ - Extract game details from ESPN event data. - - Args: - event: ESPN event data - sport: Sport type for sport-specific processing - - Returns: - Processed game details or None if extraction fails - """ - if not event: - return None - - try: - competition = event.get("competitions", [{}])[0] - status = competition.get("status", {}) - competitors = competition.get("competitors", []) - game_date_str = event.get("date", "") - - if not competitors or len(competitors) < 2: - self.logger.warning(f"Insufficient competitor data in event: {event.get('id')}") - return None - - # Find home and away teams - home_team = next((c for c in competitors if c.get("homeAway") == "home"), None) - away_team = next((c for c in competitors if c.get("homeAway") == "away"), None) - - if not home_team or not away_team: - self.logger.warning(f"Could not find home/away teams in event: {event.get('id')}") - return None - - # Extract basic team info - home_abbr = self._extract_team_abbreviation(home_team) - away_abbr = self._extract_team_abbreviation(away_team) - - # Parse game time - start_time_utc = self._parse_game_time(game_date_str) - game_time, game_date = self._format_game_time(start_time_utc) - - # Extract records - home_record = self._extract_team_record(home_team) - away_record = self._extract_team_record(away_team) - - # Determine game state - game_state = self._determine_game_state(status) - - # Build game details - details = { - "id": event.get("id"), - "game_time": game_time, - "game_date": game_date, - "start_time_utc": start_time_utc, - "status_text": status.get("type", {}).get("shortDetail", ""), - "is_live": game_state["is_live"], - "is_final": game_state["is_final"], - "is_upcoming": game_state["is_upcoming"], - "is_halftime": game_state["is_halftime"], - "is_period_break": game_state["is_period_break"], - "home_abbr": home_abbr, - "home_id": home_team.get("id"), - "home_score": str(home_team.get("score", "0")), - "home_record": home_record, - "away_abbr": away_abbr, - "away_id": away_team.get("id"), - "away_score": str(away_team.get("score", "0")), - "away_record": away_record, - "is_within_window": True, - } - - # Add sport-specific details - if sport: - details.update(self._extract_sport_specific_details(event, sport)) - - return details - - except Exception as e: - self.logger.error(f"Error extracting game details: {e} from event: {event.get('id')}", exc_info=True) - return None - - def filter_live_games(self, games: List[Dict[str, Any]]) -> List[Dict[str, Any]]: - """ - Filter games to only include live games. - - Args: - games: List of game dictionaries - - Returns: - List of live games - """ - return [game for game in games if game.get('is_live', False)] - - def filter_final_games(self, games: List[Dict[str, Any]]) -> List[Dict[str, Any]]: - """ - Filter games to only include final games. - - Args: - games: List of game dictionaries - - Returns: - List of final games - """ - return [game for game in games if game.get('is_final', False)] - - def filter_upcoming_games(self, games: List[Dict[str, Any]]) -> List[Dict[str, Any]]: - """ - Filter games to only include upcoming games. - - Args: - games: List of game dictionaries - - Returns: - List of upcoming games - """ - return [game for game in games if game.get('is_upcoming', False)] - - def filter_favorite_teams(self, games: List[Dict[str, Any]], - favorite_teams: List[str]) -> List[Dict[str, Any]]: - """ - Filter games to only include games with favorite teams. - - Args: - games: List of game dictionaries - favorite_teams: List of favorite team abbreviations - - Returns: - List of games involving favorite teams - """ - if not favorite_teams: - return games - - return [game for game in games - if game.get('home_abbr') in favorite_teams or - game.get('away_abbr') in favorite_teams] - - def filter_recent_games(self, games: List[Dict[str, Any]], - days_back: int = 7) -> List[Dict[str, Any]]: - """ - Filter games to only include recent games within specified days. - - Args: - games: List of game dictionaries - days_back: Number of days to look back - - Returns: - List of recent games - """ - cutoff_date = datetime.now(timezone.utc) - timedelta(days=days_back) - - recent_games = [] - for game in games: - start_time = game.get('start_time_utc') - if start_time and start_time >= cutoff_date: - recent_games.append(game) - - return recent_games - - def sort_games_by_time(self, games: List[Dict[str, Any]], - reverse: bool = False) -> List[Dict[str, Any]]: - """ - Sort games by start time. - - Args: - games: List of game dictionaries - reverse: If True, sort in descending order (newest first) - - Returns: - Sorted list of games - """ - def get_start_time(game): - start_time = game.get('start_time_utc') - if start_time: - return start_time - # Fallback to current time for games without start time - return datetime.now(timezone.utc) - - return sorted(games, key=get_start_time, reverse=reverse) - - def process_games(self, events: List[Dict[str, Any]], sport: str = None) -> List[Dict[str, Any]]: - """ - Process a list of ESPN events into game details. - - Args: - events: List of ESPN event data - sport: Sport type for processing - - Returns: - List of processed game details - """ - games = [] - - for event in events: - game = self.extract_game_details(event, sport) - if game: - games.append(game) - - return games - - def get_game_summary(self, game: Dict[str, Any]) -> str: - """ - Get a text summary of a game. - - Args: - game: Game dictionary - - Returns: - Text summary of the game - """ - home_abbr = game.get('home_abbr', 'HOME') - away_abbr = game.get('away_abbr', 'AWAY') - home_score = game.get('home_score', '0') - away_score = game.get('away_score', '0') - status = game.get('status_text', '') - - if game.get('is_live'): - return f"{away_abbr} {away_score} @ {home_abbr} {home_score} ({status})" - elif game.get('is_final'): - return f"{away_abbr} {away_score} @ {home_abbr} {home_score} (Final)" - else: - return f"{away_abbr} @ {home_abbr} ({status})" - - def _extract_team_abbreviation(self, team_data: Dict[str, Any]) -> str: - """Extract team abbreviation from team data.""" - try: - return team_data.get("team", {}).get("abbreviation", "") - except (KeyError, AttributeError): - # Fallback to first 3 characters of team name - team_name = team_data.get("team", {}).get("name", "UNK") - return team_name[:3].upper() - - def _extract_team_record(self, team_data: Dict[str, Any]) -> str: - """Extract team record from team data.""" - try: - records = team_data.get('records', []) - if records and len(records) > 0: - record = records[0].get('summary', '') - # Don't show "0-0" records - if record in {"0-0", "0-0-0"}: - return '' - return record - except (KeyError, AttributeError, IndexError): - pass - return '' - - def _parse_game_time(self, game_date_str: str) -> Optional[datetime]: - """Parse game time string to UTC datetime.""" - if not game_date_str: - return None - - try: - # Handle ISO format with Z suffix - if game_date_str.endswith('Z'): - game_date_str = game_date_str.replace('Z', '+00:00') - - dt = datetime.fromisoformat(game_date_str) - # Ensure the datetime is UTC-aware (fromisoformat may create timezone-aware but not pytz.UTC) - if dt.tzinfo is None: - # If naive, assume it's UTC - return dt.replace(tzinfo=pytz.UTC) - else: - # Convert to pytz.UTC for consistency - return dt.astimezone(pytz.UTC) - except ValueError: - self.logger.warning(f"Could not parse game date: {game_date_str}") - return None - - def _format_game_time(self, start_time_utc: Optional[datetime]) -> Tuple[str, str]: - """Format game time for display.""" - if not start_time_utc: - return "", "" - - try: - local_time = start_time_utc.astimezone(self.timezone) - game_time = local_time.strftime("%I:%M%p").lstrip('0') - game_date = local_time.strftime("%B %d") - return game_time, game_date - except Exception as e: - self.logger.error(f"Error formatting game time: {e}") - return "", "" - - def _determine_game_state(self, status: Dict[str, Any]) -> Dict[str, bool]: - """Determine game state from status data.""" - status_type = status.get("type", {}) - state = status_type.get("state", "") - name = status_type.get("name", "").lower() - - return { - "is_live": state == "in", - "is_final": state == "post", - "is_upcoming": state == "pre" or name in ['scheduled', 'pre-game', 'status_scheduled'], - "is_halftime": state == "halftime" or name == "status_halftime", - "is_period_break": name == "status_end_period", - } - - def _extract_sport_specific_details(self, event: Dict[str, Any], sport: str) -> Dict[str, Any]: - """Extract sport-specific game details.""" - details = {} - - if sport == "basketball": - details.update(self._extract_basketball_details(event)) - elif sport == "football": - details.update(self._extract_football_details(event)) - elif sport == "hockey": - details.update(self._extract_hockey_details(event)) - elif sport == "baseball": - details.update(self._extract_baseball_details(event)) - - return details - - def _extract_basketball_details(self, event: Dict[str, Any]) -> Dict[str, Any]: - """Extract basketball-specific details.""" - details = {} - - try: - competition = event.get("competitions", [{}])[0] - status = competition.get("status", {}) - - # Period information - period = status.get("period", 0) - if period > 0: - if period <= 4: - details["period_text"] = f"Q{period}" - else: - details["period_text"] = f"OT{period - 4}" - else: - details["period_text"] = "Start" - - # Clock - details["clock"] = status.get("displayClock", "0:00") - - except (KeyError, IndexError): - pass - - return details - - def _extract_football_details(self, event: Dict[str, Any]) -> Dict[str, Any]: - """Extract football-specific details.""" - details = {} - - try: - competition = event.get("competitions", [{}])[0] - status = competition.get("status", {}) - - # Quarter information - period = status.get("period", 0) - if period > 0: - if period <= 4: - details["period_text"] = f"Q{period}" - else: - details["period_text"] = f"OT{period - 4}" - else: - details["period_text"] = "Start" - - # Clock - details["clock"] = status.get("displayClock", "0:00") - - except (KeyError, IndexError): - pass - - return details - - def _extract_hockey_details(self, event: Dict[str, Any]) -> Dict[str, Any]: - """Extract hockey-specific details.""" - details = {} - - try: - competition = event.get("competitions", [{}])[0] - status = competition.get("status", {}) - - # Period information - period = status.get("period", 0) - if period > 0: - if period <= 3: - details["period_text"] = f"P{period}" - else: - details["period_text"] = f"OT{period - 3}" - else: - details["period_text"] = "Start" - - # Clock - details["clock"] = status.get("displayClock", "0:00") - - except (KeyError, IndexError): - pass - - return details - - def _extract_baseball_details(self, event: Dict[str, Any]) -> Dict[str, Any]: - """Extract baseball-specific details.""" - details = {} - - try: - competition = event.get("competitions", [{}])[0] - status = competition.get("status", {}) - - # Inning information - period = status.get("period", 0) - if period > 0: - details["period_text"] = f"INN {period}" - else: - details["period_text"] = "Start" - - # Clock - details["clock"] = status.get("displayClock", "0:00") - - except (KeyError, IndexError): - pass - - return details - - def _get_timezone(self, timezone_str: str) -> pytz.BaseTzInfo: - """Get timezone object from string.""" - try: - return pytz.timezone(timezone_str) - except pytz.UnknownTimeZoneError: - self.logger.warning(f"Unknown timezone: {timezone_str}, using UTC") - return pytz.utc diff --git a/src/common/utils.py b/src/common/utils.py deleted file mode 100644 index fc2d9613..00000000 --- a/src/common/utils.py +++ /dev/null @@ -1,331 +0,0 @@ -""" -Utility Functions - -Common utility functions for LED matrix plugins. -Extracted from LEDMatrix core to provide reusable functionality for plugins. -""" - -import logging -import re -from datetime import datetime, timezone -from typing import Union -import pytz - - -def normalize_team_abbreviation(team_abbr: str) -> str: - """ - Normalize team abbreviation for consistent usage. - - Args: - team_abbr: Raw team abbreviation - - Returns: - Normalized abbreviation - """ - if not team_abbr: - return "" - - # Remove spaces and convert to uppercase - normalized = team_abbr.strip().upper() - - # Handle special characters - normalized = normalized.replace('&', 'AND') - normalized = normalized.replace(' ', '') - normalized = normalized.replace('-', '') - - return normalized - - -def format_time(dt: datetime, timezone_str: str = 'UTC', - format_str: str = "%I:%M%p") -> str: - """ - Format datetime for display. - - Args: - dt: Datetime object - timezone_str: Target timezone - format_str: Time format string - - Returns: - Formatted time string - """ - try: - if dt.tzinfo is None: - dt = dt.replace(tzinfo=timezone.utc) - - target_tz = pytz.timezone(timezone_str) - local_time = dt.astimezone(target_tz) - - formatted = local_time.strftime(format_str) - # Remove leading zero from hour - if formatted.startswith('0'): - formatted = formatted[1:] - - return formatted - except Exception: - return "" - - -def format_date(dt: datetime, timezone_str: str = 'UTC', - format_str: str = "%B %d") -> str: - """ - Format date for display. - - Args: - dt: Datetime object - timezone_str: Target timezone - format_str: Date format string - - Returns: - Formatted date string - """ - try: - if dt.tzinfo is None: - dt = dt.replace(tzinfo=timezone.utc) - - target_tz = pytz.timezone(timezone_str) - local_time = dt.astimezone(target_tz) - - return local_time.strftime(format_str) - except Exception: - return "" - - -def get_timezone(timezone_str: str) -> pytz.BaseTzInfo: - """ - Get timezone object from string. - - Args: - timezone_str: Timezone string - - Returns: - Timezone object - """ - try: - return pytz.timezone(timezone_str) - except pytz.UnknownTimeZoneError: - logging.getLogger(__name__).warning(f"Unknown timezone: {timezone_str}, using UTC") - return pytz.utc - - -def validate_dimensions(width: int, height: int) -> bool: - """ - Validate display dimensions. - - Args: - width: Display width - height: Display height - - Returns: - True if dimensions are valid - """ - return (isinstance(width, int) and isinstance(height, int) and - width > 0 and height > 0 and width <= 1000 and height <= 1000) - - -def parse_team_abbreviation(text: str) -> str: - """ - Parse team abbreviation from various text formats. - - Args: - text: Text containing team abbreviation - - Returns: - Extracted team abbreviation - """ - if not text: - return "" - - # Remove common prefixes/suffixes - text = re.sub(r'^(Team|Club|FC|SC)\s+', '', text, flags=re.IGNORECASE) - text = re.sub(r'\s+(Team|Club|FC|SC)$', '', text, flags=re.IGNORECASE) - - # Extract abbreviation (usually 2-4 uppercase letters) - match = re.search(r'\b[A-Z]{2,4}\b', text.upper()) - if match: - return match.group() - - # Fallback to first 3 characters - return text[:3].upper() - - -def format_score(home_score: Union[str, int], away_score: Union[str, int]) -> str: - """ - Format score for display. - - Args: - home_score: Home team score - away_score: Away team score - - Returns: - Formatted score string - """ - return f"{away_score}-{home_score}" - - -def format_period(period: int, sport: str = "basketball") -> str: - """ - Format period/quarter/inning for display. - - Args: - period: Period number - sport: Sport type - - Returns: - Formatted period string - """ - if sport == "basketball": - if period <= 4: - return f"Q{period}" - else: - return f"OT{period - 4}" - elif sport == "football": - if period <= 4: - return f"Q{period}" - else: - return f"OT{period - 4}" - elif sport == "hockey": - if period <= 3: - return f"P{period}" - else: - return f"OT{period - 3}" - elif sport == "baseball": - return f"INN {period}" - else: - return f"P{period}" - - -def is_live_game(status: str) -> bool: - """ - Check if game status indicates live play. - - Args: - status: Game status string - - Returns: - True if game is live - """ - live_indicators = ['live', 'in progress', 'halftime', 'overtime', 'ot'] - return any(indicator in status.lower() for indicator in live_indicators) - - -def is_final_game(status: str) -> bool: - """ - Check if game status indicates final. - - Args: - status: Game status string - - Returns: - True if game is final - """ - final_indicators = ['final', 'completed', 'finished', 'ended'] - return any(indicator in status.lower() for indicator in final_indicators) - - -def is_upcoming_game(status: str) -> bool: - """ - Check if game status indicates upcoming. - - Args: - status: Game status string - - Returns: - True if game is upcoming - """ - upcoming_indicators = ['scheduled', 'upcoming', 'pre-game', 'not started'] - return any(indicator in status.lower() for indicator in upcoming_indicators) - - -def sanitize_filename(filename: str) -> str: - """ - Sanitize filename for safe file operations. - - Args: - filename: Original filename - - Returns: - Sanitized filename - """ - # Remove or replace invalid characters - filename = re.sub(r'[<>:"/\\|?*]', '_', filename) - # Remove multiple underscores - filename = re.sub(r'_+', '_', filename) - # Remove leading/trailing underscores and dots - filename = filename.strip('_.') - - return filename - - -def truncate_text(text: str, max_length: int, suffix: str = "...") -> str: - """ - Truncate text to maximum length. - - Args: - text: Text to truncate - max_length: Maximum length - suffix: Suffix to add when truncating - - Returns: - Truncated text - """ - if len(text) <= max_length: - return text - - return text[:max_length - len(suffix)] + suffix - - -def parse_boolean(value: Union[str, bool, int]) -> bool: - """ - Parse various boolean representations. - - Args: - value: Value to parse - - Returns: - Boolean value - """ - if isinstance(value, bool): - return value - - if isinstance(value, int): - return bool(value) - - if isinstance(value, str): - return value.lower() in ('true', '1', 'yes', 'on', 'enabled') - - return False - - -def get_logger(name: str, level: int = logging.INFO) -> logging.Logger: - """ - Get a logger with consistent configuration. - - Note: This function is deprecated. Use src.logging_config.get_logger() instead. - This function is kept for backward compatibility. - - Args: - name: Logger name - level: Log level - - Returns: - Configured logger - """ - # Use centralized logging configuration - try: - from src.logging_config import get_logger as get_logger_centralized - return get_logger_centralized(name) - except ImportError: - # Fallback to basic logging if centralized config not available - logger = logging.getLogger(name) - logger.setLevel(level) - - if not logger.handlers: - handler = logging.StreamHandler() - formatter = logging.Formatter( - '%(asctime)s - %(name)s - %(levelname)s - %(message)s' - ) - handler.setFormatter(formatter) - logger.addHandler(handler) - - return logger diff --git a/src/config_service.py b/src/config_service.py index 234ff84f..a062ba11 100644 --- a/src/config_service.py +++ b/src/config_service.py @@ -1,12 +1,11 @@ """ Configuration Service -Provides centralized configuration management with hot-reload support, -versioning, and change notifications. +Provides centralized configuration management with hot-reload support +and change notifications. This service wraps ConfigManager and adds: - File watching for automatic reload -- Configuration versioning - Change notifications to subscribers - Thread-safe configuration access """ @@ -16,7 +15,6 @@ import time import threading from pathlib import Path from typing import Dict, Any, Optional, List, Callable -from datetime import datetime from collections import defaultdict import logging import hashlib @@ -26,51 +24,20 @@ from src.logging_config import get_logger from src.config_manager import ConfigManager -class ConfigVersion: - """Represents a configuration version snapshot.""" - - def __init__(self, config: Dict[str, Any], version: int, timestamp: datetime, checksum: str): - """ - Initialize a configuration version. - - Args: - config: Configuration dictionary - version: Version number - timestamp: When this version was created - checksum: SHA-256 hex digest of the config (for change detection) - """ - self.config: Dict[str, Any] = config - self.version: int = version - self.timestamp: datetime = timestamp - self.checksum: str = checksum - - def to_dict(self) -> Dict[str, Any]: - """Convert version to dictionary.""" - return { - 'version': self.version, - 'timestamp': self.timestamp.isoformat(), - 'checksum': self.checksum, - 'config_size': len(json.dumps(self.config)) - } - - class ConfigService: """ - Centralized configuration service with hot-reload and versioning. + Centralized configuration service with hot-reload. Features: - Automatic file watching and reload - - Configuration versioning with history - Change notifications to subscribers - Thread-safe access - - Backward compatible with ConfigManager """ def __init__( self, config_manager: Optional[ConfigManager] = None, - enable_hot_reload: bool = True, - max_versions: int = 10 + enable_hot_reload: bool = True ) -> None: """ Initialize the configuration service. @@ -78,24 +45,19 @@ class ConfigService: Args: config_manager: Optional ConfigManager instance (creates new if None) enable_hot_reload: Whether to enable automatic file watching - max_versions: Maximum number of versions to keep in history """ self.logger: logging.Logger = get_logger(__name__) self.config_manager: ConfigManager = config_manager or ConfigManager() self.enable_hot_reload: bool = enable_hot_reload - self.max_versions: int = max_versions # Thread safety self._lock: threading.RLock = threading.RLock() # Current configuration self._current_config: Dict[str, Any] = {} - self._current_version: int = 0 + self._current_checksum: Optional[str] = None self._last_modified: Dict[str, float] = {} - # Version history - self._versions: List[ConfigVersion] = [] - # Subscribers for change notifications # Format: {plugin_id or component_name: [callbacks]} self._subscribers: Dict[str, List[Callable[[Dict[str, Any], Dict[str, Any]], None]]] = defaultdict(list) @@ -130,40 +92,22 @@ class ConfigService: with self._lock: # Check if config actually changed - if self._current_version > 0: - old_checksum = self._versions[-1].checksum if self._versions else "" - if new_checksum == old_checksum: - self.logger.debug("Configuration unchanged, skipping reload") - return False + if new_checksum == self._current_checksum: + self.logger.debug("Configuration unchanged, skipping reload") + return False # Store old config for change detection old_config = self._current_config.copy() - # Create new version - self._current_version += 1 - version = ConfigVersion( - config=new_config.copy(), - version=self._current_version, - timestamp=datetime.now(), - checksum=new_checksum - ) - - # Add to history - self._versions.append(version) - - # Trim history if needed - if len(self._versions) > self.max_versions: - self._versions.pop(0) - # Update current config self._current_config = new_config + self._current_checksum = new_checksum # Notify subscribers self._notify_subscribers(old_config, new_config) self.logger.info( - "Configuration reloaded (version %d, checksum: %s)", - self._current_version, + "Configuration reloaded (checksum: %s)", new_checksum[:8] ) @@ -303,19 +247,6 @@ class ConfigService: with self._lock: return self._current_config.copy() - def get_plugin_config(self, plugin_id: str) -> Dict[str, Any]: - """ - Get configuration for a specific plugin. - - Args: - plugin_id: Plugin identifier - - Returns: - Plugin configuration dictionary - """ - config = self.get_config() - return config.get(plugin_id, {}) - def subscribe( self, callback: Callable[[Dict[str, Any], Dict[str, Any]], None], @@ -354,95 +285,6 @@ class ConfigService: self._subscribers[key].remove(callback) self.logger.debug("Unsubscribed from config changes for %s", key) - def reload(self) -> bool: - """ - Manually reload configuration. - - Returns: - True if reloaded successfully, False otherwise - """ - self.logger.info("Manual configuration reload requested") - return self._load_config() - - def get_version(self) -> int: - """ - Get current configuration version. - - Returns: - Current version number - """ - with self._lock: - return self._current_version - - def get_version_history(self) -> List[Dict[str, Any]]: - """ - Get configuration version history. - - Returns: - List of version dictionaries - """ - with self._lock: - return [v.to_dict() for v in self._versions] - - def get_version_config(self, version: int) -> Optional[Dict[str, Any]]: - """ - Get configuration for a specific version. - - Args: - version: Version number - - Returns: - Configuration dictionary or None if version not found - """ - with self._lock: - for v in self._versions: - if v.version == version: - return v.config.copy() - return None - - def rollback(self, version: int) -> bool: - """ - Rollback to a previous configuration version. - - Args: - version: Version number to rollback to - - Returns: - True if rollback successful, False otherwise - """ - config = self.get_version_config(version) - if config is None: - self.logger.error("Version %d not found in history", version) - return False - - try: - # Save the rolled-back config - self.config_manager.save_config(config) - - # Reload - return self._load_config() - - except Exception as e: - self.logger.error("Error rolling back to version %d: %s", version, e, exc_info=True) - return False - - def save_config(self, new_config: Dict[str, Any]) -> bool: - """ - Save new configuration. - - Args: - new_config: New configuration dictionary - - Returns: - True if saved successfully, False otherwise - """ - try: - self.config_manager.save_config(new_config) - return self._load_config() - except Exception as e: - self.logger.error("Error saving configuration: %s", e, exc_info=True) - return False - def shutdown(self) -> None: """Shutdown the configuration service.""" self.logger.info("Shutting down configuration service") @@ -450,22 +292,3 @@ class ConfigService: with self._lock: self._subscribers.clear() - - # Backward compatibility methods - def load_config(self) -> Dict[str, Any]: - """ - Load configuration (backward compatibility with ConfigManager). - - Returns: - Current configuration dictionary - """ - return self.get_config() - - def get_config_path(self) -> str: - """Get config file path (backward compatibility).""" - return self.config_manager.get_config_path() - - def get_secrets_path(self) -> str: - """Get secrets file path (backward compatibility).""" - return self.config_manager.get_secrets_path() - diff --git a/src/display_controller.py b/src/display_controller.py index 0a976eb9..dea182fa 100644 --- a/src/display_controller.py +++ b/src/display_controller.py @@ -400,8 +400,6 @@ class DisplayController: # Display rotation state self.current_mode_index = 0 self.current_display_mode = None - self.last_mode_change = time.time() - self.mode_duration = 30 # Default duration self.global_dynamic_config = ( self.config.get("display", {}).get("dynamic_duration", {}) or {} ) @@ -828,7 +826,7 @@ class DisplayController: return # Update all loaded plugins - plugins_dict = getattr(self.plugin_manager, 'loaded_plugins', None) or getattr(self.plugin_manager, 'plugins', {}) + plugins_dict = self.plugin_manager.plugins deferred = [] for plugin_id, plugin_instance in plugins_dict.items(): update_timeout = None @@ -960,37 +958,6 @@ class DisplayController: _FOLLOWER_SEND_INTERVAL = 1.0 / 90 # raw bytes are cheap; 90fps > follower render rate - def _follower_rebuild_scroll_image(self) -> None: - """Follower: rebuild the local Vegas scroll image so both Pis render from - the same fresh plugin data. Called at startup (after Vegas initializes) - and each time the leader broadcasts a new-cycle signal. Runs in a daemon - thread so it never blocks the 60fps render loop. - """ - try: - vc = getattr(self, 'vegas_coordinator', None) - if not vc: - logger.warning("Sync: follower has no vegas_coordinator — cannot build scroll image") - return - rp = vc.render_pipeline - if not rp: - logger.warning("Sync: follower vegas_coordinator has no render_pipeline") - return - logger.info("Sync: follower starting scroll image rebuild") - ok = rp.start_new_cycle() - if ok and rp.scroll_helper.cached_image is not None: - logger.info( - "Sync: follower scroll image ready — %dx%d", - rp.scroll_helper.cached_image.width, - rp.scroll_helper.cached_image.height, - ) - else: - logger.warning( - "Sync: follower scroll image rebuild FAILED (ok=%s, cached=%s)", - ok, rp.scroll_helper.cached_image is not None, - ) - except Exception as exc: - logger.warning("Sync: follower scroll image rebuild error: %s", exc, exc_info=True) - def _send_follower_frame(self, plugin_instance) -> None: """Leader: generate and send the follower's portion of the current frame. @@ -2089,9 +2056,6 @@ class DisplayController: should_skip = self.plugin_manager.health_tracker.should_skip_plugin(plugin_id) if should_skip: logger.info("Skipping plugin %s due to circuit breaker (mode: %s)", plugin_id, active_mode) - display_result = False - # Skip to next mode - let existing logic handle it - manager_to_display = None if not should_skip: manager_to_display = plugin_instance @@ -2187,11 +2151,6 @@ class DisplayController: # slips through. _release_display_lock() raise - # execute_display returns bool, convert to expected format - if result: - result = True # Success - else: - result = False # Failed else: # Fallback to direct call if executor not available try: @@ -2281,7 +2240,6 @@ class DisplayController: if next_plugin_id != current_plugin_id: self.current_mode_index = next_index self.current_display_mode = next_mode - self.last_mode_change = time.time() self.force_change = True logger.info("Switching to mode: %s (skipped plugin %s due to exception)", self.current_display_mode, current_plugin_id) @@ -2365,15 +2323,6 @@ class DisplayController: ) min_duration = 15.0 - if chosen_cap <= 0: - logger.warning( - "Invalid dynamic duration cap %s for mode %s, using default %ds", - chosen_cap, - active_mode, - DEFAULT_DYNAMIC_DURATION_CAP, - ) - chosen_cap = DEFAULT_DYNAMIC_DURATION_CAP - # Use plugin-calculated duration if available, capped by max if plugin_cycle_duration is not None and plugin_cycle_duration > 0: # Plugin provided a calculated duration - use it but respect cap @@ -2391,15 +2340,6 @@ class DisplayController: # Ensure max_duration >= min_duration max_duration = max(min_duration, max_duration) - - if max_duration < min_duration: - logger.warning( - "max_duration (%s) < min_duration (%s) for mode %s, adjusting max to min", - max_duration, - min_duration, - active_mode, - ) - max_duration = min_duration else: max_duration = base_duration @@ -2741,7 +2681,6 @@ class DisplayController: if should_rotate and self.available_modes: self.current_mode_index = (self.current_mode_index + 1) % len(self.available_modes) self.current_display_mode = self.available_modes[self.current_mode_index] - self.last_mode_change = time.time() self.force_change = True logger.info("Switching to mode: %s", self.current_display_mode) diff --git a/src/plugin_system/__init__.py b/src/plugin_system/__init__.py index e2772a16..9032c599 100644 --- a/src/plugin_system/__init__.py +++ b/src/plugin_system/__init__.py @@ -3,28 +3,15 @@ LEDMatrix Plugin System This module provides the core plugin infrastructure for the LEDMatrix project. It enables dynamic loading, management, and discovery of display plugins. - -API Version: 1.0.0 """ __version__ = "1.0.0" -__api_version__ = "1.0.0" from .base_plugin import BasePlugin from .plugin_manager import PluginManager -# Import store_manager only when needed to avoid dependency issues -def get_store_manager(): - """Get PluginStoreManager, importing only when needed.""" - try: - from .store_manager import PluginStoreManager - return PluginStoreManager - except ImportError as e: - raise ImportError("PluginStoreManager requires additional dependencies. Install requests: pip install requests") from e - __all__ = [ 'BasePlugin', 'PluginManager', - 'get_store_manager', ] diff --git a/src/plugin_system/health_monitor.py b/src/plugin_system/health_monitor.py deleted file mode 100644 index 37d6e313..00000000 --- a/src/plugin_system/health_monitor.py +++ /dev/null @@ -1,319 +0,0 @@ -""" -Enhanced plugin health monitoring with background checks and auto-recovery. - -Builds on existing PluginHealthTracker to provide: -- Background health checks -- Health status determination (healthy/degraded/unhealthy) -- Auto-recovery suggestions -- Health metrics aggregation -""" - -import threading -import time -from typing import Dict, Any, Optional, List, Callable -from datetime import datetime -from enum import Enum -from dataclasses import dataclass - -from src.logging_config import get_logger - - -class HealthStatus(Enum): - """Overall health status of a plugin.""" - HEALTHY = "healthy" - DEGRADED = "degraded" - UNHEALTHY = "unhealthy" - UNKNOWN = "unknown" - - -@dataclass -class HealthMetrics: - """Health metrics for a plugin.""" - plugin_id: str - status: HealthStatus - last_successful_update: Optional[datetime] - error_rate: float # 0.0 to 1.0 - average_response_time: Optional[float] # seconds - consecutive_failures: int - total_failures: int - total_successes: int - success_rate: float # 0.0 to 1.0 - last_error: Optional[str] - circuit_breaker_state: str - recovery_suggestions: List[str] - - -class PluginHealthMonitor: - """ - Enhanced health monitoring for plugins. - - Provides: - - Background health checks - - Health status determination - - Auto-recovery suggestions - - Health metrics aggregation - """ - - def __init__( - self, - health_tracker, - check_interval: float = 60.0, - degraded_threshold: float = 0.5, # 50% error rate - unhealthy_threshold: float = 0.8, # 80% error rate - max_response_time: float = 5.0 # seconds - ): - """ - Initialize health monitor. - - Args: - health_tracker: PluginHealthTracker instance - check_interval: Interval between background health checks (seconds) - degraded_threshold: Error rate threshold for degraded status - unhealthy_threshold: Error rate threshold for unhealthy status - max_response_time: Maximum acceptable response time (seconds) - """ - self.health_tracker = health_tracker - self.check_interval = check_interval - self.degraded_threshold = degraded_threshold - self.unhealthy_threshold = unhealthy_threshold - self.max_response_time = max_response_time - self.logger = get_logger(__name__) - - # Background check thread - self._monitor_thread: Optional[threading.Thread] = None - self._stop_event = threading.Event() - - # Health check callbacks - self._health_check_callbacks: List[Callable[[str], Dict[str, Any]]] = [] - - def start_monitoring(self) -> None: - """Start background health monitoring.""" - if self._monitor_thread and self._monitor_thread.is_alive(): - return - - self._stop_event.clear() - self._monitor_thread = threading.Thread( - target=self._monitor_loop, - daemon=True, - name="PluginHealthMonitor" - ) - self._monitor_thread.start() - self.logger.info("Started plugin health monitoring") - - def stop_monitoring(self) -> None: - """Stop background health monitoring.""" - self._stop_event.set() - if self._monitor_thread and self._monitor_thread.is_alive(): - self._monitor_thread.join(timeout=5.0) - self.logger.info("Stopped plugin health monitoring") - - def register_health_check(self, callback: Callable[[str], Dict[str, Any]]) -> None: - """ - Register a callback for health checks. - - Callback should accept plugin_id and return dict with health info. - """ - self._health_check_callbacks.append(callback) - - def get_plugin_health_status(self, plugin_id: str) -> HealthStatus: - """ - Determine overall health status for a plugin. - - Args: - plugin_id: Plugin identifier - - Returns: - HealthStatus enum value - """ - if not self.health_tracker: - return HealthStatus.UNKNOWN - - summary = self.health_tracker.get_health_summary(plugin_id) - - if not summary: - return HealthStatus.UNKNOWN - - # Check circuit breaker state - circuit_state = summary.get('circuit_state', 'closed') - if circuit_state == 'open': - return HealthStatus.UNHEALTHY - - # Check error rate - success_rate = summary.get('success_rate', 100.0) - error_rate = 1.0 - (success_rate / 100.0) - - if error_rate >= self.unhealthy_threshold: - return HealthStatus.UNHEALTHY - elif error_rate >= self.degraded_threshold: - return HealthStatus.DEGRADED - else: - return HealthStatus.HEALTHY - - def get_plugin_health_metrics(self, plugin_id: str) -> HealthMetrics: - """ - Get comprehensive health metrics for a plugin. - - Args: - plugin_id: Plugin identifier - - Returns: - HealthMetrics object - """ - if not self.health_tracker: - return HealthMetrics( - plugin_id=plugin_id, - status=HealthStatus.UNKNOWN, - last_successful_update=None, - error_rate=0.0, - average_response_time=None, - consecutive_failures=0, - total_failures=0, - total_successes=0, - success_rate=0.0, - last_error=None, - circuit_breaker_state="unknown", - recovery_suggestions=[] - ) - - summary = self.health_tracker.get_health_summary(plugin_id) - - if not summary: - return HealthMetrics( - plugin_id=plugin_id, - status=HealthStatus.UNKNOWN, - last_successful_update=None, - error_rate=0.0, - average_response_time=None, - consecutive_failures=0, - total_failures=0, - total_successes=0, - success_rate=0.0, - last_error=None, - circuit_breaker_state="unknown", - recovery_suggestions=[] - ) - - # Calculate metrics - success_rate = summary.get('success_rate', 100.0) / 100.0 - error_rate = 1.0 - success_rate - - # Parse last success time - last_success_time = None - if summary.get('last_success_time'): - try: - last_success_time = datetime.fromisoformat(summary['last_success_time']) - except (ValueError, TypeError): - pass - - # Determine status - status = self.get_plugin_health_status(plugin_id) - - # Get recovery suggestions - recovery_suggestions = self._get_recovery_suggestions(plugin_id, summary, status) - - return HealthMetrics( - plugin_id=plugin_id, - status=status, - last_successful_update=last_success_time, - error_rate=error_rate, - average_response_time=None, # Would need resource monitor for this - consecutive_failures=summary.get('consecutive_failures', 0), - total_failures=summary.get('total_failures', 0), - total_successes=summary.get('total_successes', 0), - success_rate=success_rate, - last_error=summary.get('last_error'), - circuit_breaker_state=summary.get('circuit_state', 'closed'), - recovery_suggestions=recovery_suggestions - ) - - def get_all_plugin_health(self) -> Dict[str, HealthMetrics]: - """ - Get health metrics for all tracked plugins. - - Returns: - Dictionary mapping plugin_id to HealthMetrics - """ - if not self.health_tracker: - return {} - - summaries = self.health_tracker.get_all_health_summaries() - health_metrics = {} - - for plugin_id in summaries.keys(): - health_metrics[plugin_id] = self.get_plugin_health_metrics(plugin_id) - - return health_metrics - - def _get_recovery_suggestions( - self, - plugin_id: str, - summary: Dict[str, Any], - status: HealthStatus - ) -> List[str]: - """ - Generate recovery suggestions based on health status. - - Args: - plugin_id: Plugin identifier - summary: Health summary from tracker - status: Current health status - - Returns: - List of suggested recovery actions - """ - suggestions = [] - - if status == HealthStatus.UNHEALTHY: - suggestions.append("Plugin is unhealthy - check plugin logs for errors") - suggestions.append("Verify plugin configuration is correct") - suggestions.append("Check if plugin dependencies are installed") - - if summary.get('circuit_state') == 'open': - suggestions.append("Circuit breaker is open - plugin is being skipped") - suggestions.append("Wait for cooldown period or manually reset health") - - if summary.get('consecutive_failures', 0) > 0: - suggestions.append(f"Plugin has {summary['consecutive_failures']} consecutive failures") - suggestions.append("Consider disabling plugin temporarily") - - elif status == HealthStatus.DEGRADED: - suggestions.append("Plugin is degraded - experiencing intermittent failures") - suggestions.append("Monitor plugin performance") - suggestions.append("Check for resource constraints (CPU, memory)") - - error_rate = (1.0 - (summary.get('success_rate', 100.0) / 100.0)) * 100 - suggestions.append(f"Current error rate: {error_rate:.1f}%") - - elif status == HealthStatus.HEALTHY: - suggestions.append("Plugin is healthy - no action needed") - - # Add specific suggestions based on last error - last_error = summary.get('last_error') - if last_error: - if "timeout" in last_error.lower(): - suggestions.append("Last error was a timeout - plugin may be slow or unresponsive") - elif "import" in last_error.lower() or "module" in last_error.lower(): - suggestions.append("Last error suggests missing dependencies") - elif "permission" in last_error.lower() or "access" in last_error.lower(): - suggestions.append("Last error suggests permission issues") - - return suggestions - - def _monitor_loop(self) -> None: - """Background monitoring loop.""" - while not self._stop_event.is_set(): - try: - # Run health checks for all plugins - if self._health_check_callbacks: - # Get list of plugin IDs (would need plugin manager reference) - # For now, just wait - pass - - # Sleep until next check - self._stop_event.wait(self.check_interval) - - except Exception as e: - self.logger.error(f"Error in health monitor loop: {e}", exc_info=True) - # Continue monitoring even if there's an error - time.sleep(self.check_interval) - diff --git a/src/plugin_system/plugin_executor.py b/src/plugin_system/plugin_executor.py index c1a23b49..6446fca0 100644 --- a/src/plugin_system/plugin_executor.py +++ b/src/plugin_system/plugin_executor.py @@ -238,40 +238,3 @@ class PluginExecutor: ) record_error(e, plugin_id=plugin_id, operation="display") return False - - def execute_safe( - self, - operation: Callable[[], Any], - plugin_id: str, - operation_name: str = "operation", - timeout: Optional[float] = None, - default_return: Any = None - ) -> Any: - """ - Execute an operation safely, returning default on error. - - Args: - operation: Function to execute - plugin_id: Plugin identifier - operation_name: Name of operation for logging - timeout: Timeout in seconds (None = use default) - default_return: Value to return on error - - Returns: - Result of operation or default_return on error - """ - try: - return self.execute_with_timeout( - operation, - timeout=timeout, - plugin_id=plugin_id - ) - except Exception as e: # covers PluginTimeoutError, PluginError, and unexpected errors - self.logger.warning( - "Plugin %s %s failed, using default return: %s", - plugin_id, - operation_name, - e - ) - return default_return - diff --git a/src/plugin_system/plugin_loader.py b/src/plugin_system/plugin_loader.py index 7a4969be..61e80afc 100644 --- a/src/plugin_system/plugin_loader.py +++ b/src/plugin_system/plugin_loader.py @@ -752,21 +752,6 @@ class PluginLoader: self.logger.error(error_msg, exc_info=True) raise PluginError(error_msg, plugin_id=plugin_id) from e - @staticmethod - def _parse_semver(value: Any) -> Optional[Tuple[int, int, int]]: - """Parse 'X.Y.Z' (extra parts/suffixes ignored) into a comparable - 3-tuple, or None when unparseable.""" - if not isinstance(value, str): - return None - parts = value.strip().lstrip('v').split('.') - try: - nums = [int(''.join(ch for ch in p if ch.isdigit()) or 0) for p in parts[:3]] - except ValueError: - return None - while len(nums) < 3: - nums.append(0) - return tuple(nums) # type: ignore[return-value] - def _warn_if_incompatible(self, plugin_id: str, manifest: Dict[str, Any]) -> None: """Log one warning when a plugin declares a minimum LEDMatrix version newer than the running core. Advisory only — never raises — so a diff --git a/src/plugin_system/plugin_manager.py b/src/plugin_system/plugin_manager.py index 2af661a5..9e724e50 100644 --- a/src/plugin_system/plugin_manager.py +++ b/src/plugin_system/plugin_manager.py @@ -86,15 +86,14 @@ class PluginManager: self._skip_reported: set = set() # Lock protecting plugin_last_update from concurrent mutation/iteration. - # It's written from run_scheduled_updates()/update_all_plugins() (main - # loop) and read/diffed by run_scheduled_updates_with_changes(), which + # It's written from run_scheduled_updates() (main loop) and read/diffed by run_scheduled_updates_with_changes(), which # Vegas mode calls from its own background update-tick thread. self._plugin_last_update_lock = threading.RLock() # Active plugins self.plugins: Dict[str, Any] = {} self.plugin_manifests: Dict[str, Dict[str, Any]] = {} - self.plugin_modules: Dict[str, Any] = {} + self.plugin_directories: Dict[str, Path] = {} self.plugin_last_update: Dict[str, float] = {} # Cached data-fetch intervals per plugin_id. @@ -263,10 +262,7 @@ class PluginManager: with self._discovery_lock: self.plugin_manifests.clear() self.plugin_manifests.update(new_manifests) - if not hasattr(self, 'plugin_directories'): - self.plugin_directories = {} - else: - self.plugin_directories.clear() + self.plugin_directories.clear() self.plugin_directories.update(new_directories) return plugin_ids @@ -327,11 +323,10 @@ class PluginManager: self.state_manager.set_state(plugin_id, PluginState.LOADED) # Find plugin directory using PluginLoader - plugin_directories = getattr(self, 'plugin_directories', None) plugin_dir = self.plugin_loader.find_plugin_directory( plugin_id, self.plugins_dir, - plugin_directories + self.plugin_directories ) if plugin_dir is None: @@ -341,9 +336,7 @@ class PluginManager: return False # Update mapping if found via search - if plugin_directories is None or plugin_id not in plugin_directories: - if not hasattr(self, 'plugin_directories'): - self.plugin_directories = {} + if plugin_id not in self.plugin_directories: self.plugin_directories[plugin_id] = plugin_dir # Get plugin config @@ -379,7 +372,7 @@ class PluginManager: config = self.prepare_plugin_config(plugin_id, config, schema=schema) # Use PluginLoader to load plugin - plugin_instance, module = self.plugin_loader.load_plugin( + plugin_instance, _module = self.plugin_loader.load_plugin( plugin_id=plugin_id, manifest=manifest, plugin_dir=plugin_dir, @@ -391,9 +384,6 @@ class PluginManager: plugins_dir=self.plugins_dir, ) - # Store module - self.plugin_modules[plugin_id] = module - # Register plugin-shipped fonts with the FontManager (if any). # Plugin manifests can declare a "fonts" block that ships custom # fonts with the plugin; FontManager.register_plugin_fonts handles @@ -633,9 +623,6 @@ class PluginManager: # Delegate sub-module and cached-module cleanup to the loader self.plugin_loader.unregister_plugin_modules(plugin_id) - # Remove from plugin_modules - self.plugin_modules.pop(plugin_id, None) - # Update state self.state_manager.set_state(plugin_id, PluginState.UNLOADED) self.state_manager.clear_state(plugin_id) @@ -778,7 +765,7 @@ class PluginManager: resolved further: dev plugins are symlinks into ``plugins_dir``. """ with self._discovery_lock: - if hasattr(self, 'plugin_directories') and plugin_id in self.plugin_directories: + if plugin_id in self.plugin_directories: return str(self.plugin_directories[plugin_id]) plugin_id = safe_path_component(plugin_id) @@ -967,7 +954,7 @@ class PluginManager: self.logger.warning("Plugin %s update() failed; will retry after interval", plugin_id) with self._plugin_last_update_lock: self.plugin_last_update[plugin_id] = failure_time - self.state_manager.set_state_with_error(plugin_id, PluginState.ENABLED, error_info, error=err) + self.state_manager.set_state_with_error(plugin_id, PluginState.ENABLED, error_info) if self.health_tracker: self.health_tracker.record_failure(plugin_id, err) @@ -1299,97 +1286,3 @@ class PluginManager: done = sorted(self._completed_updates) self._completed_updates.clear() return done - - def update_all_plugins(self) -> None: - """ - Update all enabled plugins. - Calls update() on each enabled plugin using PluginExecutor. - """ - for plugin_id, plugin_instance in list(self.plugins.items()): - if not getattr(plugin_instance, "enabled", True): - continue - - if not hasattr(plugin_instance, "update"): - continue - - # Eligibility check and the RUNNING transition together, so a - # concurrent scheduler cannot claim the same plugin (see - # _reserve_for_update). - if not self._reserve_for_update(plugin_id): - continue - - try: - success = self.plugin_executor.execute_update(plugin_instance, plugin_id) - if success: - with self._plugin_last_update_lock: - self.plugin_last_update[plugin_id] = time.time() - self._note_update_completed(plugin_id) - self.state_manager.record_update(plugin_id) - self.state_manager.set_state(plugin_id, PluginState.ENABLED) - else: - self._record_update_failure(plugin_id) - except Exception as exc: # pylint: disable=broad-except - self.logger.exception("Error updating plugin %s: %s", plugin_id, exc) - self._record_update_failure(plugin_id, exc=exc) - - def get_plugin_health_metrics(self) -> Dict[str, Any]: - """ - Get health metrics for all plugins. - - Returns: - Dictionary mapping plugin_id to health metrics - """ - metrics = {} - for plugin_id in self.plugins.keys(): - plugin_metrics = {} - - # Get state information - state_info = self.state_manager.get_state_info(plugin_id) - plugin_metrics.update(state_info) - - # Get health tracker metrics if available - if self.health_tracker: - health_info = self.health_tracker.get_health_summary(plugin_id) - plugin_metrics['health'] = health_info - else: - plugin_metrics['health'] = {'status': 'unknown'} - - metrics[plugin_id] = plugin_metrics - return metrics - - def get_plugin_resource_metrics(self) -> Dict[str, Any]: - """ - Get resource usage metrics for all plugins. - - Returns: - Dictionary mapping plugin_id to resource metrics - """ - metrics = {} - for plugin_id in self.plugins.keys(): - plugin_metrics = {} - - # Get state information - state_info = self.state_manager.get_state_info(plugin_id) - plugin_metrics.update(state_info) - - # Get resource monitor metrics if available - if self.resource_monitor: - resource_info = self.resource_monitor.get_metrics_summary(plugin_id) - plugin_metrics['resources'] = resource_info - else: - plugin_metrics['resources'] = {'status': 'unknown'} - - metrics[plugin_id] = plugin_metrics - return metrics - - def get_plugin_state(self, plugin_id: str) -> Dict[str, Any]: - """ - Get comprehensive state information for a plugin. - - Args: - plugin_id: Plugin identifier - - Returns: - Dictionary with state information - """ - return self.state_manager.get_state_info(plugin_id) diff --git a/src/plugin_system/plugin_state.py b/src/plugin_system/plugin_state.py index f9b9d7d0..68087e1d 100644 --- a/src/plugin_system/plugin_state.py +++ b/src/plugin_system/plugin_state.py @@ -6,40 +6,14 @@ with state transitions and queries. """ import threading -import time -from collections import deque from enum import Enum -from typing import Optional, Dict, Any, Deque, List, Tuple +from typing import Optional, Dict, Any from datetime import datetime import logging from src.logging_config import get_logger -# The history is diagnostic only -- nothing reads the entries themselves, just -# their count -- but it is appended to on the hot scheduling path: every update -# cycle records RUNNING on reserve and ENABLED on finish. Unbounded, that is -# 2,880 entries per plugin per day at the default 60s interval, which on a 1 GB -# Pi exhausts memory in weeks. -# -# Two limits, because a single entry count answers the wrong question. What a -# reader wants is "the last couple of hours", and how many transitions that is -# depends entirely on the plugin's update interval -- which on a real board -# spans 2s to 3600s. A flat 200 entries is 4.2 days for the slowest plugin and -# 3.3 minutes for the fastest, so the plugin churning hardest, the one worth -# looking at, keeps the least history. -# -# So: trim by AGE first, which makes the retained window comparable across -# plugins whatever their cadence... -STATE_HISTORY_MAX_AGE_SECONDS = 2 * 60 * 60 - -# ...and cap by COUNT second, purely as a memory ceiling for the fast pollers -# whose age window would otherwise run to thousands of entries. At ~230 bytes -# an entry this is ~0.5 MB per plugin worst case, and only plugins updating -# faster than roughly every 4s can reach it. -MAX_STATE_HISTORY_PER_PLUGIN = 2000 - - class PluginState(Enum): """Plugin state enumeration.""" UNLOADED = "unloaded" # Plugin not loaded @@ -63,39 +37,14 @@ class PluginStateManager: self.logger = logger or get_logger(__name__) self._lock = threading.RLock() self._states: Dict[str, PluginState] = {} - # (monotonic timestamp, transition). The clock is monotonic so a DST - # shift or an NTP step cannot make entries look old and flush the - # history; the human-readable timestamp lives inside the transition. - self._state_history: Dict[str, Deque[Tuple[float, Dict[str, Any]]]] = {} - # Lifetime transition totals, kept separately so the count reported by - # get_state_info() stays truthful once the history above starts rolling. + # Lifetime transition totals, reported by get_state_info(). self._state_transition_counts: Dict[str, int] = {} self._error_info: Dict[str, Dict[str, Any]] = {} self._last_update: Dict[str, datetime] = {} self._last_display: Dict[str, datetime] = {} - def _record_transition( - self, - plugin_id: str, - transition: Dict[str, Any] - ) -> None: - """Append a transition to the plugin's bounded history. - - Callers must already hold ``_lock``. The deque discards its oldest - entry once it is full, so the history cannot grow without bound; the - lifetime total is tracked separately for get_state_info(). - """ - history = self._state_history.get(plugin_id) - if history is None: - history = deque(maxlen=MAX_STATE_HISTORY_PER_PLUGIN) - self._state_history[plugin_id] = history - now = time.monotonic() - history.append((now, transition)) - # Age out first; the deque's maxlen is the backstop for plugins that - # produce more than the ceiling within the window. - cutoff = now - STATE_HISTORY_MAX_AGE_SECONDS - while history and history[0][0] < cutoff: - history.popleft() + def _record_transition(self, plugin_id: str) -> None: + """Count a state transition. Callers must already hold ``_lock``.""" self._state_transition_counts[plugin_id] = ( self._state_transition_counts.get(plugin_id, 0) + 1 ) @@ -117,14 +66,7 @@ class PluginStateManager: with self._lock: old_state = self._states.get(plugin_id, PluginState.UNLOADED) self._states[plugin_id] = state - - transition = { - 'timestamp': datetime.now(), - 'from': old_state.value, - 'to': state.value, - 'error': str(error) if error else None - } - self._record_transition(plugin_id, transition) + self._record_transition(plugin_id) # Store error info if transitioning to ERROR state if state == PluginState.ERROR and error: @@ -181,56 +123,16 @@ class PluginStateManager: state = self.get_state(plugin_id) return state == PluginState.ENABLED - def get_state_history(self, plugin_id: str) -> List[Dict[str, Any]]: - """ - Get state transition history for a plugin. - - Retention is by age first -- transitions older than - STATE_HISTORY_MAX_AGE_SECONDS are dropped -- and by count second, at - MAX_STATE_HISTORY_PER_PLUGIN, which only binds for plugins updating - fast enough to exceed it inside that window. - - Args: - plugin_id: Plugin identifier - - Returns: - List of recent state transitions, oldest first. Both the list and - the transition dicts are copies, so callers cannot mutate the - manager's own history. The values inside a transition are all - immutable, so a shallow copy per entry is enough. - """ - with self._lock: - return [ - dict(transition) - for _stamp, transition in self._state_history.get(plugin_id, ()) - ] - - def set_error_info(self, plugin_id: str, error_info: Dict[str, Any]) -> None: - """ - Persist structured error context without changing plugin state. - - Used for recoverable failures (e.g. update timeout) where the plugin - stays ENABLED but the error details should remain queryable. - - Args: - plugin_id: Plugin identifier - error_info: Arbitrary dict describing the error - """ - with self._lock: - self._error_info[plugin_id] = dict(error_info) - def set_state_with_error( self, plugin_id: str, state: PluginState, error_info: Dict[str, Any], - error: Optional[Exception] = None, ) -> None: """Set plugin state and persist error context atomically. - Unlike calling set_state() then set_error_info() separately, this - method holds ``_lock`` for both writes so no reader can observe the - new state without the accompanying error context. + Holds ``_lock`` for both writes so no reader can observe the new + state without the accompanying error context. Intentionally does not clear ``_error_info`` the way set_state() does for non-ERROR transitions — this is the recoverable-failure path where @@ -240,19 +142,11 @@ class PluginStateManager: plugin_id: Plugin identifier state: New state error_info: Structured error dict to persist alongside the state - error: Optional exception recorded in the transition history """ with self._lock: old_state = self._states.get(plugin_id, PluginState.UNLOADED) self._states[plugin_id] = state - - self._record_transition(plugin_id, { - 'timestamp': datetime.now(), - 'from': old_state.value, - 'to': state.value, - 'error': str(error) if error else None, - }) - + self._record_transition(plugin_id) self._error_info[plugin_id] = dict(error_info) self.logger.debug( @@ -284,10 +178,6 @@ class PluginStateManager: """Record that plugin update() was called.""" self._last_update[plugin_id] = datetime.now() - def record_display(self, plugin_id: str) -> None: - """Record that plugin display() was called.""" - self._last_display[plugin_id] = datetime.now() - def get_last_update(self, plugin_id: str) -> Optional[datetime]: """Get timestamp of last update() call.""" return self._last_update.get(plugin_id) @@ -331,13 +221,13 @@ class PluginStateManager: def clear_state(self, plugin_id: str) -> None: """Clear all state information for a plugin. - Held under ``_lock`` so the five dicts are dropped as one unit: every + Held under ``_lock`` so the dicts are dropped as one unit: every other mutator takes the lock, and without it a concurrent set_state() - could interleave and leave a plugin with history but no state. + could interleave and leave a plugin with a transition count but no + state. """ with self._lock: self._states.pop(plugin_id, None) - self._state_history.pop(plugin_id, None) self._state_transition_counts.pop(plugin_id, None) self._error_info.pop(plugin_id, None) self._last_update.pop(plugin_id, None) diff --git a/src/plugin_system/resource_monitor.py b/src/plugin_system/resource_monitor.py index 1b63d74e..b024e726 100644 --- a/src/plugin_system/resource_monitor.py +++ b/src/plugin_system/resource_monitor.py @@ -94,9 +94,6 @@ class PluginResourceMonitor: # they are rate-limited instead. See _METRICS_PERSIST_INTERVAL. self._metrics_persisted_at: Dict[str, float] = {} - # Thread-local storage for execution tracking - self._local = threading.local() - # Lock for thread-safe access self._lock = threading.Lock() diff --git a/src/plugin_system/state_manager.py b/src/plugin_system/state_manager.py index 38278559..16151f74 100644 --- a/src/plugin_system/state_manager.py +++ b/src/plugin_system/state_manager.py @@ -2,12 +2,12 @@ Centralized plugin state management. Provides a single source of truth for plugin state (installed, enabled, version, etc.) -with state change events and persistence. +with persistence. """ import json import threading -from typing import Dict, Any, Optional, List, Callable +from typing import Dict, Any, Optional from pathlib import Path from datetime import datetime from dataclasses import dataclass, asdict @@ -75,9 +75,7 @@ class PluginStateManager: Provides: - Single source of truth for plugin state - - State change events/notifications - State persistence - - State versioning """ def __init__( @@ -104,9 +102,6 @@ class PluginStateManager: self._states: Dict[str, PluginState] = {} self._state_version = 1 - # State change callbacks - self._callbacks: Dict[str, List[Callable[[str, PluginState, PluginState], None]]] = {} - # Threading self._lock = threading.RLock() @@ -149,8 +144,7 @@ class PluginStateManager: def update_plugin_state( self, plugin_id: str, - updates: Dict[str, Any], - notify: bool = True + updates: Dict[str, Any] ) -> bool: """ Update plugin state. @@ -158,7 +152,6 @@ class PluginStateManager: Args: plugin_id: Plugin identifier updates: Dictionary of state updates - notify: Whether to notify callbacks of changes Returns: True if update successful @@ -174,18 +167,6 @@ class PluginStateManager: enabled=False ) - # Create new state with updates - old_state = PluginState( - plugin_id=current_state.plugin_id, - status=current_state.status, - enabled=current_state.enabled, - version=current_state.version, - installed_at=current_state.installed_at, - last_updated=current_state.last_updated, - config_version=current_state.config_version, - metadata=current_state.metadata.copy() if current_state.metadata else {} - ) - # Apply updates if 'status' in updates: if isinstance(updates['status'], str): @@ -218,10 +199,6 @@ class PluginStateManager: # Store updated state self._states[plugin_id] = current_state - # Notify callbacks - if notify: - self._notify_callbacks(plugin_id, old_state, current_state) - # Auto-save if enabled if self.auto_save: self._save_state() @@ -274,23 +251,6 @@ class PluginStateManager: } ) - def set_plugin_error(self, plugin_id: str, error: Optional[str] = None) -> bool: - """ - Mark plugin as having an error. - - Args: - plugin_id: Plugin identifier - error: Optional error message - - Returns: - True if update successful - """ - updates = {'status': PluginStateStatus.ERROR} - if error: - updates['metadata'] = {'last_error': error} - - return self.update_plugin_state(plugin_id, updates) - def remove_plugin_state(self, plugin_id: str) -> bool: """ Remove plugin state (e.g., after uninstall). @@ -304,12 +264,8 @@ class PluginStateManager: self._ensure_loaded() with self._lock: if plugin_id in self._states: - old_state = self._states[plugin_id] del self._states[plugin_id] - # Notify callbacks - self._notify_callbacks(plugin_id, old_state, None) - # Auto-save if enabled if self.auto_save: self._save_state() @@ -318,58 +274,6 @@ class PluginStateManager: return False - def subscribe_to_state_changes( - self, - callback: Callable[[str, PluginState, Optional[PluginState]], None], - plugin_id: Optional[str] = None - ) -> str: - """ - Subscribe to state changes. - - Args: - callback: Callback function (plugin_id, old_state, new_state) - plugin_id: Optional plugin ID to filter on (None = all plugins) - - Returns: - Subscription ID - """ - import uuid - subscription_id = str(uuid.uuid4()) - - with self._lock: - key = plugin_id or '*' - if key not in self._callbacks: - self._callbacks[key] = [] - self._callbacks[key].append(callback) - - return subscription_id - - def _notify_callbacks( - self, - plugin_id: str, - old_state: PluginState, - new_state: Optional[PluginState] - ) -> None: - """Notify all relevant callbacks of state change.""" - # Get callbacks for this plugin and all plugins - callbacks_to_notify = [] - - if plugin_id in self._callbacks: - callbacks_to_notify.extend(self._callbacks[plugin_id]) - - if '*' in self._callbacks: - callbacks_to_notify.extend(self._callbacks['*']) - - # Call each callback - for callback in callbacks_to_notify: - try: - callback(plugin_id, old_state, new_state) - except Exception as e: - self.logger.error( - f"Error in state change callback: {e}", - exc_info=True - ) - def _save_state(self) -> None: """Save state to file.""" if not self.state_file: @@ -430,8 +334,3 @@ class PluginStateManager: except Exception as e: self.logger.error(f"Error loading plugin state: {e}", exc_info=True) - - def get_state_version(self) -> int: - """Get current state version (for detecting corruption).""" - return self._state_version - diff --git a/src/plugin_system/state_reconciliation.py b/src/plugin_system/state_reconciliation.py index 68d8286c..0359e460 100644 --- a/src/plugin_system/state_reconciliation.py +++ b/src/plugin_system/state_reconciliation.py @@ -425,18 +425,9 @@ class StateReconciliation: # error. The entry is still surfaced as MANUAL_FIX_REQUIRED so the # UI can show it, but no auto-repair will run. previously_unrecoverable = plugin_id in self._unrecoverable_missing_on_disk - # Also refuse to re-install a plugin that the user just uninstalled - # through the UI — prevents a race where the reconciler fires - # between file removal and config cleanup and resurrects the - # plugin the user just deleted. - recently_uninstalled = ( - self.store_manager is not None - and hasattr(self.store_manager, 'was_recently_uninstalled') - and self.store_manager.was_recently_uninstalled(plugin_id) - ) # Also refuse to resurrect a plugin the user has persistently - # uninstalled. Unlike the in-memory race guard above, this record - # survives restarts, so the user's removal sticks across updates. + # uninstalled. The record survives restarts, so the user's + # removal sticks across updates. persistently_uninstalled = ( self.store_manager is not None and hasattr(self.store_manager, 'is_plugin_uninstalled') @@ -445,7 +436,6 @@ class StateReconciliation: can_repair = ( self.store_manager is not None and not previously_unrecoverable - and not recently_uninstalled and not persistently_uninstalled ) inconsistencies.append(Inconsistency( diff --git a/src/plugin_system/store_manager.py b/src/plugin_system/store_manager.py index d2fb5ab2..46174f99 100644 --- a/src/plugin_system/store_manager.py +++ b/src/plugin_system/store_manager.py @@ -91,15 +91,7 @@ class PluginStoreManager: self._token_validation_cache = {} # Cache for token validation results: {token: (is_valid, timestamp, error_message)} self._token_validation_cache_timeout = 300 # 5 minutes cache for token validation - # Per-plugin tombstone timestamps for plugins that were uninstalled - # recently via the UI. Used by the state reconciler to avoid - # resurrecting a plugin the user just deleted when reconciliation - # races against the uninstall operation. Cleared after ``_uninstall_tombstone_ttl``. - self._uninstall_tombstones: Dict[str, float] = {} - self._uninstall_tombstone_ttl = 300 # 5 minutes - - # Persistent record of plugins the user has uninstalled. Unlike the - # in-memory tombstones above (a short-lived race guard), this survives + # Persistent record of plugins the user has uninstalled. It survives # restarts so that a core ``git pull`` update cannot resurrect a # built-in plugin the user removed. Built-in plugins (e.g. # ``web-ui-info``, ``starlark-apps``) are committed into the repo under @@ -189,21 +181,6 @@ class PluginStoreManager: synthetic_ts = time.time() + self._failure_backoff_seconds - cache_timeout cache_dict[cache_key] = (synthetic_ts, payload) - def mark_recently_uninstalled(self, plugin_id: str) -> None: - """Record that ``plugin_id`` was just uninstalled by the user.""" - self._uninstall_tombstones[plugin_id] = time.time() - - def was_recently_uninstalled(self, plugin_id: str) -> bool: - """Return True if ``plugin_id`` has an active uninstall tombstone.""" - ts = self._uninstall_tombstones.get(plugin_id) - if ts is None: - return False - if time.time() - ts > self._uninstall_tombstone_ttl: - # Expired — clean up so the dict doesn't grow unbounded. - self._uninstall_tombstones.pop(plugin_id, None) - return False - return True - def _is_valid_plugin_id(self, plugin_id: Any) -> bool: """Return True if ``plugin_id`` is a safe single-component plugin id. @@ -3269,25 +3246,3 @@ class PluginStoreManager: installed.append(item.name) return installed - - def get_installed_plugin_info(self, plugin_id: str) -> Optional[Dict]: - """ - Get manifest information for an installed plugin. - - Args: - plugin_id: Plugin identifier - - Returns: - Manifest data or None if not found - """ - manifest_path = self.plugins_dir / plugin_id / "manifest.json" - - if not manifest_path.exists(): - return None - - try: - with open(manifest_path, 'r') as f: - return json.load(f) - except Exception as e: - self.logger.error(f"Error reading manifest for {plugin_id}: {e}") - return None diff --git a/src/vegas_mode/config.py b/src/vegas_mode/config.py index a84b3321..6ba61526 100644 --- a/src/vegas_mode/config.py +++ b/src/vegas_mode/config.py @@ -399,85 +399,3 @@ class VegasModeConfig: f"(0 disables the cap), got {self.max_plugin_width_ratio}") return errors - - def update(self, new_config: Dict[str, Any]) -> None: - """ - Update configuration from new values. - - Args: - new_config: New configuration values to apply - """ - vegas_config = new_config.get('display', {}).get('vegas_scroll', {}) - - if 'enabled' in vegas_config: - self.enabled = vegas_config['enabled'] - if 'live_in_ticker' in vegas_config: - self.live_in_ticker = bool(vegas_config['live_in_ticker']) - # Clamped exactly as from_config does: a weight below 1 would drop the - # plugin from the rotation, and a huge one starves everything else. - if 'live_weight' in vegas_config: - self.live_weight = max(1, min(10, int(vegas_config['live_weight']))) - if 'favorite_live_weight' in vegas_config: - self.favorite_live_weight = max( - 1, min(10, int(vegas_config['favorite_live_weight']))) - if 'scroll_speed' in vegas_config: - self.scroll_speed = float(vegas_config['scroll_speed']) - if 'separator_width' in vegas_config: - self.separator_width = int(vegas_config['separator_width']) - if 'intra_plugin_gap' in vegas_config: - self.intra_plugin_gap = int(vegas_config['intra_plugin_gap']) - if 'render_width_pct' in vegas_config: - self.render_width_pct = int(vegas_config['render_width_pct']) - if 'min_content_separation' in vegas_config: - self.min_content_separation = int( - vegas_config['min_content_separation']) - if 'min_cut_gap' in vegas_config: - self.min_cut_gap = int(vegas_config['min_cut_gap']) - if 'smooth_scroll' in vegas_config: - self.smooth_scroll = vegas_config['smooth_scroll'] - if 'continuous_scroll' in vegas_config: - self.continuous_scroll = vegas_config['continuous_scroll'] - if 'extend_threshold_screens' in vegas_config: - self.extend_threshold_screens = float( - vegas_config['extend_threshold_screens']) - if 'auto_trim' in vegas_config: - self.auto_trim = vegas_config['auto_trim'] - if 'trim_threshold' in vegas_config: - self.trim_threshold = int(vegas_config['trim_threshold']) - if 'content_padding' in vegas_config: - self.content_padding = int(vegas_config['content_padding']) - if 'min_plugin_width' in vegas_config: - self.min_plugin_width = int(vegas_config['min_plugin_width']) - if 'lead_in_width' in vegas_config: - self.lead_in_width = int(vegas_config['lead_in_width']) - if 'plugins_per_cycle' in vegas_config: - self.plugins_per_cycle = int(vegas_config['plugins_per_cycle']) - if 'max_plugin_width_ratio' in vegas_config: - self.max_plugin_width_ratio = float( - vegas_config['max_plugin_width_ratio']) - if 'overflow_mode' in vegas_config: - self.overflow_mode = str(vegas_config['overflow_mode']) - if 'plugin_order' in vegas_config: - self.plugin_order = list(vegas_config['plugin_order']) - if 'excluded_plugins' in vegas_config: - self.excluded_plugins = set(vegas_config['excluded_plugins']) - if 'target_fps' in vegas_config: - self.target_fps = int(vegas_config['target_fps']) - if 'buffer_ahead' in vegas_config: - self.buffer_ahead = int(vegas_config['buffer_ahead']) - if 'frame_based_scrolling' in vegas_config: - self.frame_based_scrolling = vegas_config['frame_based_scrolling'] - if 'scroll_delay' in vegas_config: - self.scroll_delay = float(vegas_config['scroll_delay']) - if 'dynamic_duration_enabled' in vegas_config: - self.dynamic_duration_enabled = vegas_config['dynamic_duration_enabled'] - if 'min_cycle_duration' in vegas_config: - self.min_cycle_duration = int(vegas_config['min_cycle_duration']) - if 'max_cycle_duration' in vegas_config: - self.max_cycle_duration = int(vegas_config['max_cycle_duration']) - - # Log config update - logger.info( - "Vegas mode config updated: enabled=%s, speed=%.1f, fps=%d, buffer=%d", - self.enabled, self.scroll_speed, self.target_fps, self.buffer_ahead - ) diff --git a/src/vegas_mode/geometry.py b/src/vegas_mode/geometry.py index 2d6e3b8e..50cff306 100644 --- a/src/vegas_mode/geometry.py +++ b/src/vegas_mode/geometry.py @@ -230,45 +230,6 @@ def blank_runs( return list(zip(starts[long_enough].tolist(), ends[long_enough].tolist())) -def find_item_boundary( - img: Image.Image, - target: int, - min_run: int, - threshold: int = DEFAULT_INK_THRESHOLD, -) -> Optional[int]: - """ - Find the column nearest ``target`` that sits inside a gap between items. - - Used to narrow an oversized segment without cutting through a word. Only - runs of at least ``min_run`` blank columns are considered, so the - single-column gaps between characters are never chosen — cutting there - orphaned the tail of a word into the following cycle, which is how a lone - "y" from "Wednesday" ended up floating between two unrelated plugins. - - Args: - img: Image to cut - target: Preferred cut column - min_run: Minimum blank-run width that counts as an item boundary - threshold: Ink threshold - - Returns: - A column inside a qualifying gap, or None when the image has no such - gap at all — in which case the caller must not cut it. - """ - runs = blank_runs(img, min_run, threshold) - if not runs: - return None - - # Nearest point of the nearest run. For a run left of target that is its - # end (content resumes just after), for a run right of target its start - # (content stopped just before) — the right choice in both directions. - def clamp_to_run(run: Tuple[int, int]) -> int: - start, end = run - return max(start, min(target, end - 1)) - - return min((clamp_to_run(r) for r in runs), key=lambda c: abs(c - target)) - - def find_blank_cut( img: Image.Image, target: int, diff --git a/src/vegas_mode/stream_manager.py b/src/vegas_mode/stream_manager.py index 59bd1134..cb7cc2bc 100644 --- a/src/vegas_mode/stream_manager.py +++ b/src/vegas_mode/stream_manager.py @@ -685,24 +685,6 @@ class StreamManager: self.stats['fetch_errors'] += 1 return None - def _refresh_plugin_content(self, plugin_id: str) -> None: - """ - Refresh content for a specific plugin into staging buffer. - - Args: - plugin_id: Plugin to refresh - """ - # Invalidate cached content - self.plugin_adapter.invalidate_cache(plugin_id) - - # Fetch fresh content - segment = self._fetch_plugin_content(plugin_id) - - if segment: - with self._buffer_lock: - self._staging_buffer.append(segment) - logger.debug("Refreshed content for %s in staging buffer", plugin_id) - def _ensure_buffer_filled(self) -> None: """ Top the buffer back up after segments have been served. diff --git a/src/wifi_manager.py b/src/wifi_manager.py index 2a5fc747..9e3dc0fb 100644 --- a/src/wifi_manager.py +++ b/src/wifi_manager.py @@ -81,12 +81,6 @@ DEFAULT_AP_CHANNEL = 7 # LED status message file (for display_controller integration) LED_STATUS_FILE = None # Will be set dynamically -# NetworkManager connection file locations (Trixie uses /run, Bookworm uses /etc) -NM_CONNECTIONS_PATHS = [ - Path("/etc/NetworkManager/system-connections"), - Path("/run/NetworkManager/system-connections"), # Trixie with Netplan -] - @dataclass class WiFiNetwork: @@ -140,9 +134,6 @@ class WiFiManager: # Discover WiFi interface (don't hardcode wlan0) self._wifi_interface = self._discover_wifi_interface() - # Detect if we're running on Trixie (Netplan-based NetworkManager) - self._is_trixie = self._detect_trixie() - # Initialize disconnected check counter for grace period # This prevents AP mode from enabling on transient network hiccups self._disconnected_checks = 0 @@ -155,7 +146,7 @@ class WiFiManager: logger.info(f"WiFi Manager initialized - nmcli: {self.has_nmcli}, iwlist: {self.has_iwlist}, " f"hostapd: {self.has_hostapd}, dnsmasq: {self.has_dnsmasq}, " - f"interface: {self._wifi_interface}, trixie: {self._is_trixie}") + f"interface: {self._wifi_interface}") # Once per process: remove a stale force-AP flag left by a prior crash. # Guard with a class-level flag so the nmcli AP-state check only runs @@ -301,44 +292,6 @@ class WiFiManager: logger.warning("Could not discover WiFi interface, defaulting to wlan0") return "wlan0" - def _detect_trixie(self) -> bool: - """ - Detect if running on Raspberry Pi OS Trixie (Debian 13). - - Trixie uses Netplan with NetworkManager, which changes behavior: - - Connection files are stored in /run/NetworkManager/system-connections - - nmcli hotspot requires different handling - - PMF (Protected Management Frames) may need to be disabled - """ - try: - # Check for Netplan (primary indicator of Trixie) - netplan_path = Path("/etc/netplan") - if netplan_path.exists() and any(netplan_path.glob("*.yaml")): - logger.debug("Detected Trixie: Netplan configuration found") - return True - - # Check Debian version - os_release = Path("/etc/os-release") - if os_release.exists(): - content = os_release.read_text() - if 'VERSION_CODENAME=trixie' in content or 'VERSION_ID="13"' in content: - logger.debug("Detected Trixie: os-release indicates Debian 13") - return True - - # Check if NM connections are in /run (Trixie behavior) - # NM_CONNECTIONS_PATHS[0] = /etc/..., NM_CONNECTIONS_PATHS[1] = /run/... - etc_nm_path = NM_CONNECTIONS_PATHS[0] # Bookworm location - run_nm_path = NM_CONNECTIONS_PATHS[1] # Trixie location - if run_nm_path.exists() and any(run_nm_path.glob("*.nmconnection")): - if not etc_nm_path.exists() or not any(etc_nm_path.glob("*.nmconnection")): - logger.debug("Detected Trixie: NM connections in /run only") - return True - - except (OSError, PermissionError) as e: - logger.debug(f"Could not detect Trixie: {e}") - - return False - def _load_config(self): """Load WiFi configuration from file""" if self.config_path.exists(): @@ -353,8 +306,7 @@ class WiFiManager: self.config = { "ap_ssid": DEFAULT_AP_SSID, "ap_channel": DEFAULT_AP_CHANNEL, - "auto_enable_ap_mode": True, # Default: auto-enable when no network (safe due to grace period) - "saved_networks": [] + "auto_enable_ap_mode": True # Default: auto-enable when no network (safe due to grace period) } self._save_config() @@ -362,6 +314,12 @@ class WiFiManager: if "auto_enable_ap_mode" not in self.config: self.config["auto_enable_ap_mode"] = True # Default: auto-enable when no network (safe due to grace period) self._save_config() + + # Older versions stored every joined network's password here in + # plaintext and never read it back; scrub it from existing files. + if "saved_networks" in self.config: + del self.config["saved_networks"] + self._save_config() def _save_config(self): """Save WiFi configuration to file""" @@ -1621,9 +1579,6 @@ class WiFiManager: break if connected: - # Save network to config - self._save_network(ssid, password) - ip = status.ip_address or "Unknown" self._show_led_message(f"Connected! {ip}", duration=5) logger.info(f"Successfully connected to {ssid} with IP {ip}") @@ -1635,7 +1590,6 @@ class WiFiManager: # No existing connection or activation failed, create new connection logger.info(f"Creating new connection for {ssid}...") - self._save_network(ssid, password) # Connect using nmcli if password: @@ -1765,8 +1719,6 @@ class WiFiManager: def _connect_wpa_supplicant(self, ssid: str, password: str) -> Tuple[bool, str]: """Connect using wpa_supplicant (fallback)""" try: - self._save_network(ssid, password) - # This would require modifying /etc/wpa_supplicant/wpa_supplicant.conf # For now, return not implemented return False, "wpa_supplicant connection not yet implemented. Please use NetworkManager (nmcli)." @@ -1854,23 +1806,6 @@ class WiFiManager: logger.error(f"Error disconnecting from WiFi: {e}") return False, str(e) - def _save_network(self, ssid: str, password: str): - """Save network credentials to config""" - # Remove existing entry for this SSID - self.config["saved_networks"] = [ - n for n in self.config["saved_networks"] - if n.get("ssid") != ssid - ] - - # Add new entry - self.config["saved_networks"].append({ - "ssid": ssid, - "password": password, - "saved_at": time.time() - }) - - self._save_config() - def _ensure_wifi_radio_enabled(self, max_retries: int = 3) -> bool: """ Ensure WiFi radio is enabled (not soft-blocked) with retry logic and verification. @@ -2592,38 +2527,6 @@ ignore_broadcast_ssid=0 logger.error(f"Error creating hostapd config: {e}") raise - def _check_dnsmasq_conflict(self) -> Tuple[bool, str]: - """ - Check if dnsmasq is already in use for other purposes (e.g., Pi-hole). - - Returns: - Tuple of (conflict_detected, description) - """ - try: - # Check if dnsmasq service is active - result = subprocess.run( - ["systemctl", "is-active", "dnsmasq"], - capture_output=True, - text=True, - timeout=5 - ) - if result.stdout.strip() == "active": - # Check if it's configured for something other than our AP - if DNSMASQ_CONFIG_PATH.exists(): - try: - content = DNSMASQ_CONFIG_PATH.read_text() - # Check for Pi-hole or other common dnsmasq uses - if 'pihole' in content.lower() or 'pi-hole' in content.lower(): - return True, "Pi-hole detected - dnsmasq is in use" - if 'server=' in content and self._wifi_interface not in content: - return True, "dnsmasq appears to be configured for DNS forwarding" - except (OSError, PermissionError): - pass - - return False, "" - except (subprocess.TimeoutExpired, subprocess.SubprocessError): - return False, "" - def _create_dnsmasq_config(self): """ Create dnsmasq drop-in configuration for captive portal DNS redirection. diff --git a/test/conftest.py b/test/conftest.py index 966f5aea..6bfba2e2 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -354,35 +354,6 @@ def test_config_with_plugins(test_config): return config -@pytest.fixture -def test_plugin_manager(mock_config_manager, mock_display_manager, mock_cache_manager): - """Create a test PluginManager instance.""" - from unittest.mock import patch, MagicMock - import tempfile - from pathlib import Path - - # Create temporary plugin directory - with tempfile.TemporaryDirectory() as tmpdir: - plugin_dir = Path(tmpdir) / "plugins" - plugin_dir.mkdir() - - with patch('src.plugin_system.plugin_manager.PluginManager') as MockPM: - pm = MagicMock() - pm.plugins = {} - pm.plugin_manifests = {} - pm.loaded_plugins = {} - pm.plugin_last_update = {} - pm.discover_plugins = MagicMock(return_value=[]) - pm.load_plugin = MagicMock(return_value=True) - pm.unload_plugin = MagicMock(return_value=True) - pm.get_plugin = MagicMock(return_value=None) - pm.plugin_executor = MagicMock() - pm.health_tracker = None - pm.resource_monitor = None - MockPM.return_value = pm - yield pm - - @pytest.fixture def test_display_controller(mock_config_manager, mock_display_manager, mock_cache_manager, test_config_with_plugins, emulator_mode): @@ -406,7 +377,6 @@ def test_display_controller(mock_config_manager, mock_display_manager, mock_cach mock_pm.load_plugin = MagicMock(return_value=True) mock_pm.get_plugin = MagicMock(return_value=None) mock_pm.plugins = {} - mock_pm.loaded_plugins = {} mock_pm.plugin_manifests = {} mock_pm.plugin_last_update = {} mock_pm.plugin_executor = MagicMock() diff --git a/test/plugins/conftest.py b/test/plugins/conftest.py index cff6dd42..d8cf3dd6 100644 --- a/test/plugins/conftest.py +++ b/test/plugins/conftest.py @@ -7,7 +7,6 @@ import os import sys import json from pathlib import Path -from unittest.mock import MagicMock, Mock from typing import Any, Dict # Add project root to path @@ -19,100 +18,6 @@ if str(project_root) not in sys.path: os.environ['EMULATOR'] = 'true' -@pytest.fixture -def plugins_dir() -> Path: - """Get the plugins directory path. - - Honors LEDMATRIX_PLUGINS_DIR (first entry) when set — the same override - test_plugin_matrix.py uses, so CI can point every plugin suite at the - bundled fixture plugins. Otherwise checks plugins/ first, then falls - back to plugin-repos/ for monorepo development environments. - """ - env = os.environ.get('LEDMATRIX_PLUGINS_DIR') - if env: - first = env.split(os.pathsep)[0] - if first: - return Path(first) - - plugins_path = project_root / 'plugins' - plugin_repos_path = project_root / 'plugin-repos' - - # Prefer plugins/ if it has actual plugin directories - if plugins_path.exists(): - try: - has_plugins = any( - p for p in plugins_path.iterdir() - if p.is_dir() and not p.name.startswith('.') - ) - if has_plugins: - return plugins_path - except PermissionError: - pass - if plugin_repos_path.exists(): - return plugin_repos_path - return plugins_path - - -@pytest.fixture -def mock_display_manager() -> Any: - """Create a mock DisplayManager for plugin tests.""" - mock = MagicMock() - mock.width = 128 - mock.height = 32 - mock.clear = Mock() - mock.draw_text = Mock() - mock.draw_image = Mock() - mock.update_display = Mock() - mock.get_font = Mock(return_value=None) - # Some plugins access matrix.width/height - mock.matrix = MagicMock() - mock.matrix.width = 128 - mock.matrix.height = 32 - return mock - - -@pytest.fixture -def mock_cache_manager() -> Any: - """Create a mock CacheManager for plugin tests.""" - mock = MagicMock() - mock._memory_cache = {} - - def mock_get(key: str, max_age: int = 300) -> Any: - return mock._memory_cache.get(key) - - def mock_set(key: str, data: Any, ttl: int = None) -> None: - mock._memory_cache[key] = data - - def mock_clear(key: str = None) -> None: - if key: - mock._memory_cache.pop(key, None) - else: - mock._memory_cache.clear() - - mock.get = Mock(side_effect=mock_get) - mock.set = Mock(side_effect=mock_set) - mock.clear = Mock(side_effect=mock_clear) - return mock - - -@pytest.fixture -def mock_plugin_manager() -> Any: - """Create a mock PluginManager for plugin tests.""" - mock = MagicMock() - mock.plugins = {} - mock.plugin_manifests = {} - return mock - - -@pytest.fixture -def base_plugin_config() -> Dict[str, Any]: - """Base configuration for plugins.""" - return { - 'enabled': True, - 'update_interval': 300 - } - - def load_plugin_manifest(plugin_id: str, plugins_dir: Path) -> Dict[str, Any]: """Load plugin manifest.json.""" manifest_path = plugins_dir / plugin_id / 'manifest.json' diff --git a/test/plugins/test_basketball_scoreboard.py b/test/plugins/test_basketball_scoreboard.py deleted file mode 100644 index b93fb89f..00000000 --- a/test/plugins/test_basketball_scoreboard.py +++ /dev/null @@ -1,95 +0,0 @@ -""" -Integration tests for basketball-scoreboard plugin. - -Requires the real plugin to be installed (plugins/ or plugin-repos/, -or the dir named by LEDMATRIX_PLUGINS_DIR) — on machines without it, -every test here skips by design. CI covers plugin safety with the -bundled fixture plugin via test_plugin_matrix.py instead. -""" - -import pytest -from test.plugins.test_plugin_base import PluginTestBase - - -class TestBasketballScoreboardPlugin(PluginTestBase): - """Test basketball-scoreboard plugin integration.""" - - @pytest.fixture - def plugin_id(self): - return 'basketball-scoreboard' - - def test_manifest_exists(self, plugin_id): - """Test that plugin manifest exists.""" - super().test_manifest_exists(plugin_id) - - def test_manifest_has_required_fields(self, plugin_id): - """Test that manifest has all required fields.""" - super().test_manifest_has_required_fields(plugin_id) - - def test_plugin_can_be_loaded(self, plugin_id): - """Test that plugin module can be loaded.""" - super().test_plugin_can_be_loaded(plugin_id) - - def test_plugin_class_exists(self, plugin_id): - """Test that plugin class exists.""" - super().test_plugin_class_exists(plugin_id) - - def test_plugin_can_be_instantiated(self, plugin_id): - """Test that plugin can be instantiated.""" - super().test_plugin_can_be_instantiated(plugin_id) - - def test_plugin_has_required_methods(self, plugin_id): - """Test that plugin has required methods.""" - super().test_plugin_has_required_methods(plugin_id) - - def test_plugin_update_method(self, plugin_id): - """Test that plugin update() method works.""" - super().test_plugin_update_method(plugin_id) - - def test_plugin_display_method(self, plugin_id): - """Test that plugin display() method works.""" - super().test_plugin_display_method(plugin_id) - - def test_plugin_has_display_modes(self, plugin_id): - """Test that plugin has display modes.""" - manifest = self.load_plugin_manifest(plugin_id) - assert 'display_modes' in manifest - # Manifest uses league-prefixed modes (nba_, wnba_, ncaam_, ncaaw_) - assert 'nba_live' in manifest['display_modes'] - assert 'nba_recent' in manifest['display_modes'] - assert 'nba_upcoming' in manifest['display_modes'] - - def test_plugin_has_get_display_modes(self, plugin_id): - """Test that plugin can return display modes.""" - manifest = self.load_plugin_manifest(plugin_id) - plugin_dir = self.plugins_dir / plugin_id - entry_point = manifest['entry_point'] - class_name = manifest['class_name'] - - module = self.plugin_loader.load_module( - plugin_id=plugin_id, - plugin_dir=plugin_dir, - entry_point=entry_point - ) - - plugin_class = self.plugin_loader.get_plugin_class( - plugin_id=plugin_id, - module=module, - class_name=class_name - ) - - config = self.base_config.copy() - plugin_instance = self.plugin_loader.instantiate_plugin( - plugin_id=plugin_id, - plugin_class=plugin_class, - config=config, - display_manager=self.mock_display_manager, - cache_manager=self.mock_cache_manager, - plugin_manager=self.mock_plugin_manager - ) - - # Check if plugin has get_display_modes method - if hasattr(plugin_instance, 'get_display_modes'): - modes = plugin_instance.get_display_modes() - assert isinstance(modes, list) - assert len(modes) > 0 diff --git a/test/plugins/test_calendar.py b/test/plugins/test_calendar.py deleted file mode 100644 index 4d874940..00000000 --- a/test/plugins/test_calendar.py +++ /dev/null @@ -1,63 +0,0 @@ -""" -Integration tests for calendar plugin. - -Requires the real plugin to be installed (plugins/ or plugin-repos/, -or the dir named by LEDMATRIX_PLUGINS_DIR) — on machines without it, -every test here skips by design. CI covers plugin safety with the -bundled fixture plugin via test_plugin_matrix.py instead. -""" - -import pytest -from test.plugins.test_plugin_base import PluginTestBase - - -class TestCalendarPlugin(PluginTestBase): - """Test calendar plugin integration.""" - - @pytest.fixture - def plugin_id(self): - return 'calendar' - - def test_manifest_exists(self, plugin_id): - """Test that plugin manifest exists.""" - super().test_manifest_exists(plugin_id) - - def test_manifest_has_required_fields(self, plugin_id): - """Test that manifest has all required fields.""" - super().test_manifest_has_required_fields(plugin_id) - - def test_plugin_can_be_loaded(self, plugin_id): - """Test that plugin module can be loaded.""" - super().test_plugin_can_be_loaded(plugin_id) - - def test_plugin_class_exists(self, plugin_id): - """Test that plugin class exists.""" - super().test_plugin_class_exists(plugin_id) - - def test_plugin_can_be_instantiated(self, plugin_id): - """Test that plugin can be instantiated.""" - # Calendar plugin may need credentials, but instantiation should work - super().test_plugin_can_be_instantiated(plugin_id) - - def test_plugin_has_required_methods(self, plugin_id): - """Test that plugin has required methods.""" - super().test_plugin_has_required_methods(plugin_id) - - def test_plugin_update_method(self, plugin_id): - """Test that plugin update() method works.""" - # Calendar requires Google API credentials, so this may skip - super().test_plugin_update_method(plugin_id) - - def test_plugin_display_method(self, plugin_id): - """Test that plugin display() method works.""" - super().test_plugin_display_method(plugin_id) - - def test_plugin_has_display_modes(self, plugin_id): - """Test that plugin has display modes.""" - manifest = self.load_plugin_manifest(plugin_id) - assert 'display_modes' in manifest - assert 'calendar' in manifest['display_modes'] - - def test_config_schema_valid(self, plugin_id): - """Test that config schema is valid.""" - super().test_config_schema_valid(plugin_id) diff --git a/test/plugins/test_clock_simple.py b/test/plugins/test_clock_simple.py deleted file mode 100644 index 9b25a59e..00000000 --- a/test/plugins/test_clock_simple.py +++ /dev/null @@ -1,103 +0,0 @@ -""" -Integration tests for clock-simple plugin. - -Requires the real plugin to be installed (plugins/ or plugin-repos/, -or the dir named by LEDMATRIX_PLUGINS_DIR) — on machines without it, -every test here skips by design. CI covers plugin safety with the -bundled fixture plugin via test_plugin_matrix.py instead. -""" - -import pytest -from test.plugins.test_plugin_base import PluginTestBase - - -class TestClockSimplePlugin(PluginTestBase): - """Test clock-simple plugin integration.""" - - @pytest.fixture - def plugin_id(self): - return 'clock-simple' - - def test_manifest_exists(self, plugin_id): - """Test that plugin manifest exists.""" - super().test_manifest_exists(plugin_id) - - def test_manifest_has_required_fields(self, plugin_id): - """Test that manifest has all required fields.""" - super().test_manifest_has_required_fields(plugin_id) - - def test_plugin_can_be_loaded(self, plugin_id): - """Test that plugin module can be loaded.""" - super().test_plugin_can_be_loaded(plugin_id) - - def test_plugin_class_exists(self, plugin_id): - """Test that plugin class exists.""" - super().test_plugin_class_exists(plugin_id) - - def test_plugin_can_be_instantiated(self, plugin_id): - """Test that plugin can be instantiated.""" - super().test_plugin_can_be_instantiated(plugin_id) - - def test_plugin_has_required_methods(self, plugin_id): - """Test that plugin has required methods.""" - super().test_plugin_has_required_methods(plugin_id) - - def test_plugin_update_method(self, plugin_id): - """Test that plugin update() method works.""" - # Clock doesn't need external APIs, so this should always work - super().test_plugin_update_method(plugin_id) - - def test_plugin_display_method(self, plugin_id): - """Test that plugin display() method works.""" - super().test_plugin_display_method(plugin_id) - - def test_plugin_has_display_modes(self, plugin_id): - """Test that plugin has display modes.""" - manifest = self.load_plugin_manifest(plugin_id) - assert 'display_modes' in manifest - assert 'clock-simple' in manifest['display_modes'] - - def test_clock_displays_time(self, plugin_id): - """Test that clock plugin actually displays time.""" - manifest = self.load_plugin_manifest(plugin_id) - plugin_dir = self.plugins_dir / plugin_id - entry_point = manifest['entry_point'] - class_name = manifest['class_name'] - - module = self.plugin_loader.load_module( - plugin_id=plugin_id, - plugin_dir=plugin_dir, - entry_point=entry_point - ) - - plugin_class = self.plugin_loader.get_plugin_class( - plugin_id=plugin_id, - module=module, - class_name=class_name - ) - - config = self.base_config.copy() - config['timezone'] = 'UTC' - config['time_format'] = '12h' - config['show_date'] = True - - plugin_instance = self.plugin_loader.instantiate_plugin( - plugin_id=plugin_id, - plugin_class=plugin_class, - config=config, - display_manager=self.mock_display_manager, - cache_manager=self.mock_cache_manager, - plugin_manager=self.mock_plugin_manager - ) - - # Update and display - plugin_instance.update() - plugin_instance.display(force_clear=True) - - # Verify time was formatted - assert hasattr(plugin_instance, 'current_time') - assert plugin_instance.current_time is not None - - # Verify display was called - assert self.mock_display_manager.clear.called - assert self.mock_display_manager.update_display.called diff --git a/test/plugins/test_odds_ticker.py b/test/plugins/test_odds_ticker.py deleted file mode 100644 index 231ed7de..00000000 --- a/test/plugins/test_odds_ticker.py +++ /dev/null @@ -1,62 +0,0 @@ -""" -Integration tests for odds-ticker plugin. - -Requires the real plugin to be installed (plugins/ or plugin-repos/, -or the dir named by LEDMATRIX_PLUGINS_DIR) — on machines without it, -every test here skips by design. CI covers plugin safety with the -bundled fixture plugin via test_plugin_matrix.py instead. -""" - -import pytest -from test.plugins.test_plugin_base import PluginTestBase - - -class TestOddsTickerPlugin(PluginTestBase): - """Test odds-ticker plugin integration.""" - - @pytest.fixture - def plugin_id(self): - return 'odds-ticker' - - def test_manifest_exists(self, plugin_id): - """Test that plugin manifest exists.""" - super().test_manifest_exists(plugin_id) - - def test_manifest_has_required_fields(self, plugin_id): - """Test that manifest has all required fields.""" - super().test_manifest_has_required_fields(plugin_id) - - def test_plugin_can_be_loaded(self, plugin_id): - """Test that plugin module can be loaded.""" - super().test_plugin_can_be_loaded(plugin_id) - - def test_plugin_class_exists(self, plugin_id): - """Test that plugin class exists.""" - super().test_plugin_class_exists(plugin_id) - - def test_plugin_can_be_instantiated(self, plugin_id): - """Test that plugin can be instantiated.""" - super().test_plugin_can_be_instantiated(plugin_id) - - def test_plugin_has_required_methods(self, plugin_id): - """Test that plugin has required methods.""" - super().test_plugin_has_required_methods(plugin_id) - - def test_plugin_update_method(self, plugin_id): - """Test that plugin update() method works.""" - # Odds ticker may need API access, but should handle gracefully - super().test_plugin_update_method(plugin_id) - - def test_plugin_display_method(self, plugin_id): - """Test that plugin display() method works.""" - super().test_plugin_display_method(plugin_id) - - def test_plugin_has_display_modes(self, plugin_id): - """Test that plugin has display modes.""" - manifest = self.load_plugin_manifest(plugin_id) - assert 'display_modes' in manifest - assert 'odds_ticker' in manifest['display_modes'] - - def test_config_schema_valid(self, plugin_id): - """Test that config schema is valid.""" - super().test_config_schema_valid(plugin_id) diff --git a/test/plugins/test_plugin_base.py b/test/plugins/test_plugin_base.py deleted file mode 100644 index 43fe8901..00000000 --- a/test/plugins/test_plugin_base.py +++ /dev/null @@ -1,305 +0,0 @@ -""" -Base test class for plugin integration tests. - -Provides common test functionality for all plugins. -""" - -import pytest -import json -from typing import Dict, Any - -from src.plugin_system.plugin_loader import PluginLoader -from src.plugin_system.base_plugin import BasePlugin - - -class PluginTestBase: - """Base class for plugin integration tests.""" - - @pytest.fixture(autouse=True) - def setup_base(self, plugins_dir, mock_display_manager, mock_cache_manager, - mock_plugin_manager, base_plugin_config): - """Setup base fixtures for all plugin tests.""" - self.plugins_dir = plugins_dir - self.mock_display_manager = mock_display_manager - self.mock_cache_manager = mock_cache_manager - self.mock_plugin_manager = mock_plugin_manager - self.base_config = base_plugin_config - self.plugin_loader = PluginLoader() - - def load_plugin_manifest(self, plugin_id: str) -> Dict[str, Any]: - """Load plugin manifest.json.""" - manifest_path = self.plugins_dir / plugin_id / 'manifest.json' - if not manifest_path.exists(): - pytest.skip(f"Manifest not found for {plugin_id}") - - with open(manifest_path, 'r') as f: - return json.load(f) - - def load_plugin_config_schema(self, plugin_id: str) -> Dict[str, Any]: - """Load plugin config_schema.json if it exists.""" - schema_path = self.plugins_dir / plugin_id / 'config_schema.json' - if schema_path.exists(): - with open(schema_path, 'r') as f: - return json.load(f) - return None - - def test_manifest_exists(self, plugin_id: str): - """Test that plugin manifest exists and is valid JSON.""" - manifest = self.load_plugin_manifest(plugin_id) - assert manifest is not None - assert 'id' in manifest - assert manifest['id'] == plugin_id - assert 'class_name' in manifest - # entry_point is optional - default to 'manager.py' if missing - if 'entry_point' not in manifest: - manifest['entry_point'] = 'manager.py' - - def test_manifest_has_required_fields(self, plugin_id: str): - """Test that manifest has all required fields.""" - manifest = self.load_plugin_manifest(plugin_id) - - # Core required fields - required_fields = ['id', 'name', 'description', 'author', 'class_name'] - for field in required_fields: - assert field in manifest, f"Manifest missing required field: {field}" - assert manifest[field], f"Manifest field {field} is empty" - - # entry_point is required but some plugins may not have it explicitly - # If missing, assume it's 'manager.py' - if 'entry_point' not in manifest: - manifest['entry_point'] = 'manager.py' - - def test_plugin_can_be_loaded(self, plugin_id: str): - """Test that plugin module can be loaded.""" - manifest = self.load_plugin_manifest(plugin_id) - plugin_dir = self.plugins_dir / plugin_id - entry_point = manifest.get('entry_point', 'manager.py') - - module = self.plugin_loader.load_module( - plugin_id=plugin_id, - plugin_dir=plugin_dir, - entry_point=entry_point - ) - - assert module is not None - assert hasattr(module, manifest['class_name']) - - def test_plugin_class_exists(self, plugin_id: str): - """Test that plugin class exists in module.""" - manifest = self.load_plugin_manifest(plugin_id) - plugin_dir = self.plugins_dir / plugin_id - entry_point = manifest.get('entry_point', 'manager.py') - class_name = manifest['class_name'] - - module = self.plugin_loader.load_module( - plugin_id=plugin_id, - plugin_dir=plugin_dir, - entry_point=entry_point - ) - - plugin_class = self.plugin_loader.get_plugin_class( - plugin_id=plugin_id, - module=module, - class_name=class_name - ) - - assert plugin_class is not None - assert issubclass(plugin_class, BasePlugin) - - def test_plugin_can_be_instantiated(self, plugin_id: str): - """Test that plugin can be instantiated with mock dependencies.""" - manifest = self.load_plugin_manifest(plugin_id) - plugin_dir = self.plugins_dir / plugin_id - entry_point = manifest.get('entry_point', 'manager.py') - class_name = manifest['class_name'] - - module = self.plugin_loader.load_module( - plugin_id=plugin_id, - plugin_dir=plugin_dir, - entry_point=entry_point - ) - - plugin_class = self.plugin_loader.get_plugin_class( - plugin_id=plugin_id, - module=module, - class_name=class_name - ) - - # Merge base config with plugin-specific defaults - config = self.base_config.copy() - - plugin_instance = self.plugin_loader.instantiate_plugin( - plugin_id=plugin_id, - plugin_class=plugin_class, - config=config, - display_manager=self.mock_display_manager, - cache_manager=self.mock_cache_manager, - plugin_manager=self.mock_plugin_manager - ) - - assert plugin_instance is not None - assert plugin_instance.plugin_id == plugin_id - assert plugin_instance.enabled == config.get('enabled', True) - - def test_plugin_has_required_methods(self, plugin_id: str): - """Test that plugin has required BasePlugin methods.""" - manifest = self.load_plugin_manifest(plugin_id) - plugin_dir = self.plugins_dir / plugin_id - entry_point = manifest.get('entry_point', 'manager.py') - class_name = manifest['class_name'] - - module = self.plugin_loader.load_module( - plugin_id=plugin_id, - plugin_dir=plugin_dir, - entry_point=entry_point - ) - - plugin_class = self.plugin_loader.get_plugin_class( - plugin_id=plugin_id, - module=module, - class_name=class_name - ) - - config = self.base_config.copy() - plugin_instance = self.plugin_loader.instantiate_plugin( - plugin_id=plugin_id, - plugin_class=plugin_class, - config=config, - display_manager=self.mock_display_manager, - cache_manager=self.mock_cache_manager, - plugin_manager=self.mock_plugin_manager - ) - - # Check required methods exist - assert hasattr(plugin_instance, 'update') - assert hasattr(plugin_instance, 'display') - assert callable(plugin_instance.update) - assert callable(plugin_instance.display) - - def test_plugin_update_method(self, plugin_id: str): - """Test that plugin update() method can be called without errors.""" - manifest = self.load_plugin_manifest(plugin_id) - plugin_dir = self.plugins_dir / plugin_id - entry_point = manifest.get('entry_point', 'manager.py') - class_name = manifest['class_name'] - - module = self.plugin_loader.load_module( - plugin_id=plugin_id, - plugin_dir=plugin_dir, - entry_point=entry_point - ) - - plugin_class = self.plugin_loader.get_plugin_class( - plugin_id=plugin_id, - module=module, - class_name=class_name - ) - - config = self.base_config.copy() - plugin_instance = self.plugin_loader.instantiate_plugin( - plugin_id=plugin_id, - plugin_class=plugin_class, - config=config, - display_manager=self.mock_display_manager, - cache_manager=self.mock_cache_manager, - plugin_manager=self.mock_plugin_manager - ) - - # Call update() - should not raise exceptions - # Some plugins may need API keys, but they should handle that gracefully - try: - plugin_instance.update() - except Exception as e: - # If it's a missing API key or similar, that's acceptable for integration tests - error_msg = str(e).lower() - if 'api' in error_msg or 'key' in error_msg or 'auth' in error_msg or 'credential' in error_msg: - pytest.skip(f"Plugin requires API credentials: {e}") - else: - raise - - def test_plugin_display_method(self, plugin_id: str): - """Test that plugin display() method can be called without errors.""" - manifest = self.load_plugin_manifest(plugin_id) - plugin_dir = self.plugins_dir / plugin_id - entry_point = manifest.get('entry_point', 'manager.py') - class_name = manifest['class_name'] - - module = self.plugin_loader.load_module( - plugin_id=plugin_id, - plugin_dir=plugin_dir, - entry_point=entry_point - ) - - plugin_class = self.plugin_loader.get_plugin_class( - plugin_id=plugin_id, - module=module, - class_name=class_name - ) - - config = self.base_config.copy() - plugin_instance = self.plugin_loader.instantiate_plugin( - plugin_id=plugin_id, - plugin_class=plugin_class, - config=config, - display_manager=self.mock_display_manager, - cache_manager=self.mock_cache_manager, - plugin_manager=self.mock_plugin_manager - ) - - # Some plugins need matrix attribute on display_manager (set before update) - if not hasattr(self.mock_display_manager, 'matrix'): - from unittest.mock import MagicMock - self.mock_display_manager.matrix = MagicMock() - self.mock_display_manager.matrix.width = 128 - self.mock_display_manager.matrix.height = 32 - - # Call update() first if needed - try: - plugin_instance.update() - except Exception as e: - error_msg = str(e).lower() - if 'api' in error_msg or 'key' in error_msg or 'auth' in error_msg: - pytest.skip(f"Plugin requires API credentials: {e}") - - # Some plugins need a mode set before display - # Try to set a mode if the plugin has that capability - if hasattr(plugin_instance, 'set_mode') and manifest.get('display_modes'): - try: - first_mode = manifest['display_modes'][0] - plugin_instance.set_mode(first_mode) - except Exception: - pass # If set_mode doesn't exist or fails, continue - - # Call display() - should not raise exceptions - try: - plugin_instance.display(force_clear=True) - except Exception as e: - # Some plugins may need specific setup - if it's a mode issue, that's acceptable - error_msg = str(e).lower() - if 'mode' in error_msg or 'manager' in error_msg: - # This is acceptable - plugin needs proper mode setup - pass - else: - raise - - # Verify display_manager methods were called (if display succeeded) - # Some plugins may not call these if they skip display due to missing data - # So we just verify the method was callable without exceptions - assert hasattr(plugin_instance, 'display') - - def test_plugin_has_display_modes(self, plugin_id: str): - """Test that plugin has display modes defined.""" - manifest = self.load_plugin_manifest(plugin_id) - - assert 'display_modes' in manifest - assert isinstance(manifest['display_modes'], list) - assert len(manifest['display_modes']) > 0 - - def test_config_schema_valid(self, plugin_id: str): - """Test that config schema is valid JSON if it exists.""" - schema = self.load_plugin_config_schema(plugin_id) - - if schema is not None: - assert isinstance(schema, dict) - # Schema should have 'type' field for JSON Schema - assert 'type' in schema or 'properties' in schema diff --git a/test/plugins/test_soccer_scoreboard.py b/test/plugins/test_soccer_scoreboard.py deleted file mode 100644 index d5526fb8..00000000 --- a/test/plugins/test_soccer_scoreboard.py +++ /dev/null @@ -1,94 +0,0 @@ -""" -Integration tests for soccer-scoreboard plugin. - -Requires the real plugin to be installed (plugins/ or plugin-repos/, -or the dir named by LEDMATRIX_PLUGINS_DIR) — on machines without it, -every test here skips by design. CI covers plugin safety with the -bundled fixture plugin via test_plugin_matrix.py instead. -""" - -import pytest -from test.plugins.test_plugin_base import PluginTestBase - - -class TestSoccerScoreboardPlugin(PluginTestBase): - """Test soccer-scoreboard plugin integration.""" - - @pytest.fixture - def plugin_id(self): - return 'soccer-scoreboard' - - def test_manifest_exists(self, plugin_id): - """Test that plugin manifest exists.""" - super().test_manifest_exists(plugin_id) - - def test_manifest_has_required_fields(self, plugin_id): - """Test that manifest has all required fields.""" - super().test_manifest_has_required_fields(plugin_id) - - def test_plugin_can_be_loaded(self, plugin_id): - """Test that plugin module can be loaded.""" - super().test_plugin_can_be_loaded(plugin_id) - - def test_plugin_class_exists(self, plugin_id): - """Test that plugin class exists.""" - super().test_plugin_class_exists(plugin_id) - - def test_plugin_can_be_instantiated(self, plugin_id): - """Test that plugin can be instantiated.""" - super().test_plugin_can_be_instantiated(plugin_id) - - def test_plugin_has_required_methods(self, plugin_id): - """Test that plugin has required methods.""" - super().test_plugin_has_required_methods(plugin_id) - - def test_plugin_update_method(self, plugin_id): - """Test that plugin update() method works.""" - super().test_plugin_update_method(plugin_id) - - def test_plugin_display_method(self, plugin_id): - """Test that plugin display() method works.""" - super().test_plugin_display_method(plugin_id) - - def test_plugin_has_display_modes(self, plugin_id): - """Test that plugin has display modes.""" - manifest = self.load_plugin_manifest(plugin_id) - assert 'display_modes' in manifest - assert 'soccer_live' in manifest['display_modes'] - assert 'soccer_recent' in manifest['display_modes'] - assert 'soccer_upcoming' in manifest['display_modes'] - - def test_plugin_has_get_display_modes(self, plugin_id): - """Test that plugin can return display modes.""" - manifest = self.load_plugin_manifest(plugin_id) - plugin_dir = self.plugins_dir / plugin_id - entry_point = manifest['entry_point'] - class_name = manifest['class_name'] - - module = self.plugin_loader.load_module( - plugin_id=plugin_id, - plugin_dir=plugin_dir, - entry_point=entry_point - ) - - plugin_class = self.plugin_loader.get_plugin_class( - plugin_id=plugin_id, - module=module, - class_name=class_name - ) - - config = self.base_config.copy() - plugin_instance = self.plugin_loader.instantiate_plugin( - plugin_id=plugin_id, - plugin_class=plugin_class, - config=config, - display_manager=self.mock_display_manager, - cache_manager=self.mock_cache_manager, - plugin_manager=self.mock_plugin_manager - ) - - # Check if plugin has get_display_modes method - if hasattr(plugin_instance, 'get_display_modes'): - modes = plugin_instance.get_display_modes() - assert isinstance(modes, list) - assert len(modes) > 0 diff --git a/test/plugins/test_text_display.py b/test/plugins/test_text_display.py deleted file mode 100644 index 34adfa49..00000000 --- a/test/plugins/test_text_display.py +++ /dev/null @@ -1,114 +0,0 @@ -""" -Integration tests for text-display plugin. - -Requires the real plugin to be installed (plugins/ or plugin-repos/, -or the dir named by LEDMATRIX_PLUGINS_DIR) — on machines without it, -every test here skips by design. CI covers plugin safety with the -bundled fixture plugin via test_plugin_matrix.py instead. -""" - -import pytest -from unittest.mock import MagicMock -from test.plugins.test_plugin_base import PluginTestBase - - -class TestTextDisplayPlugin(PluginTestBase): - """Test text-display plugin integration.""" - - @pytest.fixture - def plugin_id(self): - return 'text-display' - - def test_manifest_exists(self, plugin_id): - """Test that plugin manifest exists.""" - super().test_manifest_exists(plugin_id) - - def test_manifest_has_required_fields(self, plugin_id): - """Test that manifest has all required fields.""" - super().test_manifest_has_required_fields(plugin_id) - - def test_plugin_can_be_loaded(self, plugin_id): - """Test that plugin module can be loaded.""" - super().test_plugin_can_be_loaded(plugin_id) - - def test_plugin_class_exists(self, plugin_id): - """Test that plugin class exists.""" - super().test_plugin_class_exists(plugin_id) - - def test_plugin_can_be_instantiated(self, plugin_id): - """Test that plugin can be instantiated.""" - super().test_plugin_can_be_instantiated(plugin_id) - - def test_plugin_has_required_methods(self, plugin_id): - """Test that plugin has required methods.""" - super().test_plugin_has_required_methods(plugin_id) - - def test_plugin_update_method(self, plugin_id): - """Test that plugin update() method works.""" - # Text display doesn't need external APIs - super().test_plugin_update_method(plugin_id) - - def test_plugin_display_method(self, plugin_id): - """Test that plugin display() method works.""" - super().test_plugin_display_method(plugin_id) - - def test_plugin_has_display_modes(self, plugin_id): - """Test that plugin has display modes.""" - manifest = self.load_plugin_manifest(plugin_id) - assert 'display_modes' in manifest - assert 'text_display' in manifest['display_modes'] - - def test_text_display_shows_text(self, plugin_id): - """Test that text display plugin actually displays text.""" - manifest = self.load_plugin_manifest(plugin_id) - plugin_dir = self.plugins_dir / plugin_id - entry_point = manifest.get('entry_point', 'manager.py') - class_name = manifest['class_name'] - - module = self.plugin_loader.load_module( - plugin_id=plugin_id, - plugin_dir=plugin_dir, - entry_point=entry_point - ) - - plugin_class = self.plugin_loader.get_plugin_class( - plugin_id=plugin_id, - module=module, - class_name=class_name - ) - - config = self.base_config.copy() - config['text'] = 'Test Message' - config['scroll'] = False - config['text_color'] = [255, 255, 255] - config['background_color'] = [0, 0, 0] - - # Mock display_manager.matrix to have width/height attributes - if not hasattr(self.mock_display_manager, 'matrix'): - self.mock_display_manager.matrix = MagicMock() - self.mock_display_manager.matrix.width = 128 - self.mock_display_manager.matrix.height = 32 - - plugin_instance = self.plugin_loader.instantiate_plugin( - plugin_id=plugin_id, - plugin_class=plugin_class, - config=config, - display_manager=self.mock_display_manager, - cache_manager=self.mock_cache_manager, - plugin_manager=self.mock_plugin_manager - ) - - # Update and display - plugin_instance.update() - plugin_instance.display(force_clear=True) - - # Verify text was set - assert plugin_instance.text == 'Test Message' - - # Verify display was called (may be called via image assignment) - assert (self.mock_display_manager.update_display.called or - hasattr(self.mock_display_manager, 'image')) - - def test_config_schema_valid(self, plugin_id): - """Test that config schema is valid.""" - super().test_config_schema_valid(plugin_id) diff --git a/test/test_config_helper.py b/test/test_config_helper.py deleted file mode 100644 index 4931fccc..00000000 --- a/test/test_config_helper.py +++ /dev/null @@ -1,253 +0,0 @@ -""" -Tests for src/common/config_helper.py — pins the ConfigHelper contract. - -Covers: load/save round trips (missing/malformed files return {} rather -than raising, non-ASCII preserved via ensure_ascii=False, top-level JSON -lists returned as-is), dot-notation get/set including the silent-failure -contract when an intermediate key holds a non-dict, merge_configs deep -semantics with NO aliasing of the base config (the fixed bug — the old -shallow copy let mutations of the merged result leak into base's nested -dicts), simplified schema validation including the caught-TypeError path -when a schema 'type' is given as a string, plugin config key conventions -('{plugin_id}_config', enabled defaults True), and required-key checks -where a key present with value None counts as present. -""" - -import json - -import pytest - -from src.common.config_helper import ConfigHelper - - -@pytest.fixture -def helper(): - return ConfigHelper() - - -class TestLoadConfig: - def test_missing_file_returns_empty_dict(self, helper, tmp_path): - assert helper.load_config(tmp_path / "nope.json") == {} - - def test_malformed_json_returns_empty_dict(self, helper, tmp_path): - path = tmp_path / "bad.json" - path.write_text("{ this is not json", encoding="utf-8") - assert helper.load_config(path) == {} - - def test_top_level_list_returned_as_is(self, helper, tmp_path): - # load_config does not enforce a dict shape: a JSON list comes - # straight back. Pinned as a characterization of current behavior. - path = tmp_path / "list.json" - path.write_text("[1, 2, 3]", encoding="utf-8") - assert helper.load_config(path) == [1, 2, 3] - - -class TestSaveConfig: - def test_round_trip(self, helper, tmp_path): - path = tmp_path / "config.json" - config = {'display': {'hardware': {'rows': 32}}, 'timezone': 'UTC'} - assert helper.save_config(config, path) is True - assert helper.load_config(path) == config - - def test_creates_parent_directories(self, helper, tmp_path): - path = tmp_path / "deep" / "nested" / "config.json" - assert helper.save_config({'a': 1}, path) is True - assert path.exists() - assert helper.load_config(path) == {'a': 1} - - def test_non_ascii_survives_round_trip(self, helper, tmp_path): - path = tmp_path / "config.json" - config = {'city': 'Zürich', 'note': 'météo ☀'} - assert helper.save_config(config, path) is True - assert helper.load_config(path) == config - # ensure_ascii=False: characters are written raw, not \u-escaped - assert 'Zürich' in path.read_text(encoding='utf-8') - - def test_directory_path_returns_false_not_raise(self, helper, tmp_path): - assert helper.save_config({'a': 1}, tmp_path) is False - - -class TestGetConfigValue: - def test_dot_notation_hit(self, helper): - config = {'display': {'hardware': {'rows': 32}}} - assert helper.get_config_value(config, 'display.hardware.rows') == 32 - - def test_missing_returns_default(self, helper): - sentinel = object() - assert helper.get_config_value({}, 'display.rows', default=sentinel) is sentinel - - def test_intermediate_non_dict_returns_default(self, helper): - config = {'display': 'not-a-dict'} - assert helper.get_config_value(config, 'display.hardware.rows', default=64) == 64 - - def test_required_missing_raises_keyerror(self, helper): - with pytest.raises(KeyError): - helper.get_config_value({}, 'display.rows', required=True) - - -class TestSetConfigValue: - def test_sets_top_level(self, helper): - config = {} - helper.set_config_value(config, 'timezone', 'UTC') - assert config == {'timezone': 'UTC'} - - def test_auto_creates_intermediates(self, helper): - config = {} - helper.set_config_value(config, 'display.hardware.rows', 32) - assert config == {'display': {'hardware': {'rows': 32}}} - - def test_silent_failure_on_non_dict_intermediate(self, helper): - # 'a' exists but holds an int; the assignment attempt raises - # TypeError internally, which set_config_value swallows and logs. - # The config is left unchanged — pinned silent-failure contract. - config = {'a': 5} - helper.set_config_value(config, 'a.b', 1) - assert config == {'a': 5} - - -class TestMergeConfigs: - def test_nested_dicts_merge_recursively(self, helper): - base = {'display': {'rows': 32, 'cols': 64}, 'timezone': 'UTC'} - override = {'display': {'cols': 128, 'brightness': 90}} - merged = helper.merge_configs(base, override) - assert merged == { - 'display': {'rows': 32, 'cols': 128, 'brightness': 90}, - 'timezone': 'UTC', - } - - def test_scalar_override_wins_over_dict(self, helper): - merged = helper.merge_configs({'display': {'rows': 32}}, {'display': 7}) - assert merged['display'] == 7 - - def test_dict_override_wins_over_scalar(self, helper): - merged = helper.merge_configs({'display': 7}, {'display': {'rows': 32}}) - assert merged['display'] == {'rows': 32} - - def test_no_aliasing_of_base(self, helper): - # Post-fix: merge deep-copies base, so mutating the result never - # leaks back into the caller's base config. - base = {'display': {'x': 1}} - merged = helper.merge_configs(base, {}) - assert merged['display'] is not base['display'] - merged['display']['x'] = 99 - assert base['display']['x'] == 1 - - def test_inputs_unchanged(self, helper): - base = {'a': {'b': 1}} - override = {'a': {'c': 2}} - helper.merge_configs(base, override) - assert base == {'a': {'b': 1}} - assert override == {'a': {'c': 2}} - - def test_no_aliasing_of_override_values(self, helper): - # The non-recursive branch must deep-copy the override value too: - # mutating a merged-in list or dict must not reach back into - # override_config. - override = {'teams': ['A', 'B'], 'nested': {'x': [1]}} - merged = helper.merge_configs({}, override) - merged['teams'].append('C') - merged['nested']['x'].append(2) - assert override == {'teams': ['A', 'B'], 'nested': {'x': [1]}} - - -class TestValidateConfig: - def test_no_schema_dict_is_valid(self, helper): - assert helper.validate_config({'a': 1}) is True - - def test_no_schema_list_is_invalid(self, helper): - assert helper.validate_config([1, 2]) is False - - def test_required_key_missing_is_invalid(self, helper): - schema = {'rows': {'required': True, 'type': int}} - assert helper.validate_config({}, schema) is False - - def test_optional_key_missing_is_valid(self, helper): - schema = {'rows': {'required': False, 'type': int}} - assert helper.validate_config({}, schema) is True - - def test_wrong_type_is_invalid(self, helper): - schema = {'rows': {'type': int}} - assert helper.validate_config({'rows': 'thirty-two'}, schema) is False - assert helper.validate_config({'rows': 32}, schema) is True - - def test_allowed_values_violation_is_invalid(self, helper): - schema = {'mode': {'allowed_values': ['clock', 'weather']}} - assert helper.validate_config({'mode': 'stocks'}, schema) is False - assert helper.validate_config({'mode': 'clock'}, schema) is True - - def test_string_type_in_schema_is_invalid_via_typeerror(self, helper): - # 'type' given as the STRING "int" makes isinstance() raise - # TypeError; validate_config catches it and returns False rather - # than raising. Pinned characterization. - schema = {'rows': {'type': 'int'}} - assert helper.validate_config({'rows': 32}, schema) is False - - -class TestPluginConfigHelpers: - def test_get_plugin_config_uses_suffixed_key(self, helper): - plugin_cfg = {'enabled': True, 'display_duration': 30} - assert helper.get_plugin_config({'clock_config': plugin_cfg}, 'clock') == plugin_cfg - - def test_get_plugin_config_bare_id_key_not_found(self, helper): - # Only '{plugin_id}_config' is consulted — a bare 'clock' section - # is invisible to this helper. Pinned key contract. - assert helper.get_plugin_config({'clock': {'enabled': True}}, 'clock') == {} - - def test_create_default_config_wraps_in_suffixed_key(self, helper): - defaults = {'enabled': True} - assert helper.create_default_config('clock', defaults) == {'clock_config': defaults} - - def test_is_plugin_enabled_defaults_true_for_unknown(self, helper): - assert helper.is_plugin_enabled({}, 'clock') is True - - def test_is_plugin_enabled_false_when_disabled(self, helper): - config = {'clock_config': {'enabled': False}} - assert helper.is_plugin_enabled(config, 'clock') is False - - def test_is_plugin_enabled_ignores_bare_id_key(self, helper): - # Disabled under the wrong key -> still reported enabled (default). - config = {'clock': {'enabled': False}} - assert helper.is_plugin_enabled(config, 'clock') is True - - -class TestSportsAndDisplayHelpers: - def test_get_display_config(self, helper): - display = {'hardware': {'rows': 32}} - assert helper.get_display_config({'display': display}) == display - assert helper.get_display_config({}) == {} - - def test_get_sports_config_uses_scoreboard_suffix(self, helper): - sport_cfg = {'favorite_teams': ['TB']} - config = {'football_scoreboard': sport_cfg} - assert helper.get_sports_config(config, 'football') == sport_cfg - assert helper.get_sports_config(config, 'hockey') == {} - - def test_get_favorite_teams(self, helper): - config = {'football_scoreboard': {'favorite_teams': ['TB', 'DAL']}} - assert helper.get_favorite_teams(config, 'football') == ['TB', 'DAL'] - assert helper.get_favorite_teams({}, 'football') == [] - - def test_get_display_modes(self, helper): - modes = {'live': True, 'recent': False} - config = {'football_scoreboard': {'display_modes': modes}} - assert helper.get_display_modes(config, 'football') == modes - assert helper.get_display_modes({}, 'football') == {} - - -class TestValidateRequiredKeys: - def test_returns_missing_subset(self, helper): - config = {'a': 1, 'c': {'d': 2}} - missing = helper.validate_required_keys(config, ['a', 'b', 'c.d', 'c.e']) - assert missing == ['b', 'c.e'] - - def test_dot_notation_present(self, helper): - config = {'display': {'hardware': {'rows': 32}}} - assert helper.validate_required_keys(config, ['display.hardware.rows']) == [] - - def test_empty_requirements(self, helper): - assert helper.validate_required_keys({'a': 1}, []) == [] - - def test_present_with_none_counts_as_present(self, helper): - # _has_key checks key membership, not truthiness — a key set to - # None is NOT reported missing. Pinned semantics. - assert helper.validate_required_keys({'a': None}, ['a']) == [] diff --git a/test/test_config_service.py b/test/test_config_service.py index a0d601a1..d58a93d7 100644 --- a/test/test_config_service.py +++ b/test/test_config_service.py @@ -108,12 +108,13 @@ class TestConfigService: with open(config_path, 'w') as f: json.dump(current_config, f) - # Trigger reload manually - should detect change and notify - service.reload() + # Reload the way the file watcher does - should detect change and notify + assert service._load_config() is True - # Check callback was called (may be called during init or reload) - # The callback should be called if config actually changed - assert callback.called or True # May not be called if checksum matches + callback.assert_called_once() + old_config, new_config = callback.call_args[0] + assert old_config['display']['brightness'] == 50 + assert new_config['display']['brightness'] == 75 def test_plugin_specific_subscriber(self, config_manager): """Test plugin-specific subscriber notification.""" @@ -128,19 +129,17 @@ class TestConfigService: config_path = config_manager.config_path with open(config_path, 'r') as f: current_config = json.load(f) - if 'plugins' not in current_config: - current_config['plugins'] = {} - if 'weather' not in current_config['plugins']: - current_config['plugins']['weather'] = {} - current_config['plugins']['weather']['enabled'] = False # Change value + current_config['weather'] = {'enabled': False} # Change value with open(config_path, 'w') as f: json.dump(current_config, f) - # Trigger reload manually - should detect change and notify - service.reload() + # Reload the way the file watcher does - should detect change and notify + assert service._load_config() is True - # Check callback was called if config changed - assert callback.called or True # May not be called if checksum matches + callback.assert_called_once() + old_plugin_config, new_plugin_config = callback.call_args[0] + assert new_plugin_config['enabled'] is False + assert new_plugin_config['api_key'] == 'secret_key' def test_config_merging(self, config_manager): """Test config merging logic via ConfigService.""" @@ -151,6 +150,15 @@ class TestConfigService: assert "weather" in config assert config["weather"]["api_key"] == "secret_key" + def test_unchanged_config_does_not_notify(self, config_manager): + """Reloading an unchanged config must not notify subscribers.""" + service = ConfigService(config_manager, enable_hot_reload=False) + callback = MagicMock() + service.subscribe(callback) + + assert service._load_config() is False + callback.assert_not_called() + def test_shutdown(self, config_manager): """Test proper shutdown.""" service = ConfigService(config_manager, enable_hot_reload=True) diff --git a/test/test_display_controller.py b/test/test_display_controller.py index da551b93..4f8e1af5 100644 --- a/test/test_display_controller.py +++ b/test/test_display_controller.py @@ -16,46 +16,6 @@ class TestDisplayControllerInitialization: assert test_display_controller.available_modes == [] -class TestDisplayControllerModeRotation: - """Test display mode rotation logic.""" - - def test_basic_rotation(self, test_display_controller): - """Test basic mode rotation.""" - controller = test_display_controller - controller.available_modes = ["mode1", "mode2", "mode3"] - controller.current_mode_index = 0 - controller.current_display_mode = "mode1" - - # Simulate rotation - controller.current_mode_index = (controller.current_mode_index + 1) % len(controller.available_modes) - controller.current_display_mode = controller.available_modes[controller.current_mode_index] - - assert controller.current_display_mode == "mode2" - assert controller.current_mode_index == 1 - - # Rotate again - controller.current_mode_index = (controller.current_mode_index + 1) % len(controller.available_modes) - controller.current_display_mode = controller.available_modes[controller.current_mode_index] - - assert controller.current_display_mode == "mode3" - - # Rotate back to start - controller.current_mode_index = (controller.current_mode_index + 1) % len(controller.available_modes) - controller.current_display_mode = controller.available_modes[controller.current_mode_index] - - assert controller.current_display_mode == "mode1" - - def test_rotation_with_single_mode(self, test_display_controller): - """Test rotation with only one mode.""" - controller = test_display_controller - controller.available_modes = ["mode1"] - controller.current_mode_index = 0 - - controller.current_mode_index = (controller.current_mode_index + 1) % len(controller.available_modes) - - assert controller.current_mode_index == 0 - - class TestDisplayControllerOnDemand: """Test on-demand request handling.""" @@ -93,20 +53,6 @@ class TestDisplayControllerOnDemand: assert controller.on_demand_active is False assert controller.on_demand_mode is None assert controller.on_demand_last_event == "expired" - - def test_on_demand_schedule_override(self, test_display_controller): - """Test that on-demand overrides schedule.""" - controller = test_display_controller - controller.is_display_active = False - controller.on_demand_active = True - - # Logic in run() loop handles this, so we simulate it - if controller.on_demand_active and not controller.is_display_active: - controller.on_demand_schedule_override = True - controller.is_display_active = True - - assert controller.is_display_active is True - assert controller.on_demand_schedule_override is True class TestDisplayControllerLivePriority: diff --git a/test/test_display_helper.py b/test/test_display_helper.py deleted file mode 100644 index b8c9be63..00000000 --- a/test/test_display_helper.py +++ /dev/null @@ -1,307 +0,0 @@ -"""Tests for src/common/display_helper.py (DisplayHelper). - -Pure-PIL tests, no hardware or mocks required. Pixel assertions rely on -getbbox()/getpixel() rather than exact text pixel counts, because the -default-font metrics vary across Pillow versions. - -These tests pin the FIXED behaviors on this branch: -- draw_error_message / draw_no_data_message return a rendered image - (they previously crashed with AttributeError), -- draw_scorebug_layout draws period/status/clock as one combined top - line (previously overprinted at the same y), -- draw_ticker_layout draws at x=0 (previously started at - x=display_width, i.e. entirely off-canvas -> blank frames). -""" - -from PIL import Image, ImageDraw, ImageFont - -from src.common.display_helper import DisplayHelper - - -def default_font(): - return ImageFont.load_default() - - -def make_helper(width=128, height=32): - return DisplayHelper(width, height) - - -class TestCreateBaseImage: - def test_default_is_black_rgb_display_sized(self): - helper = make_helper() - img = helper.create_base_image() - assert img.size == (128, 32) - assert img.mode == 'RGB' - assert img.getpixel((0, 0)) == (0, 0, 0) - assert img.getpixel((127, 31)) == (0, 0, 0) - # Entirely black -> no bounding box in luminance - assert img.convert('L').getbbox() is None - - def test_custom_background_color(self): - helper = make_helper() - img = helper.create_base_image(background_color=(10, 20, 30)) - assert img.getpixel((0, 0)) == (10, 20, 30) - assert img.getpixel((64, 16)) == (10, 20, 30) - - def test_mode_rgba_is_honored(self): - helper = make_helper() - img = helper.create_base_image(mode='RGBA') - assert img.mode == 'RGBA' - assert img.size == (128, 32) - - -class TestCreateOverlay: - def test_overlay_is_transparent_rgba(self): - helper = make_helper() - overlay = helper.create_overlay() - assert overlay.mode == 'RGBA' - assert overlay.size == (128, 32) - assert overlay.getpixel((0, 0)) == (0, 0, 0, 0) - assert overlay.getpixel((127, 31)) == (0, 0, 0, 0) - - -class TestCompositeImages: - def test_rgb_inputs_are_upconverted_and_result_is_rgba(self): - helper = make_helper() - base = Image.new('RGB', (128, 32), (0, 0, 0)) - overlay = Image.new('RGB', (128, 32), (255, 0, 0)) - result = helper.composite_images(base, overlay) - assert result.mode == 'RGBA' - assert result.size == base.size - # RGB->RGBA conversion yields a fully opaque overlay - assert result.getpixel((0, 0)) == (255, 0, 0, 255) - - def test_transparent_overlay_leaves_base_visible(self): - helper = make_helper() - base = Image.new('RGB', (128, 32), (5, 6, 7)) - overlay = helper.create_overlay() - result = helper.composite_images(base, overlay) - assert result.mode == 'RGBA' - assert result.getpixel((64, 16)) == (5, 6, 7, 255) - - -class TestScorebugLayout: - def test_full_game_data_renders(self): - helper = make_helper() - font = default_font() - fonts = {'time': font, 'status': font, 'score': font, 'team': font} - game_data = { - 'home_score': 3, 'away_score': 2, - 'home_abbr': 'NYY', 'away_abbr': 'BOS', - 'status_text': 'LIVE', 'period_text': 'T9', 'clock': '2:30', - } - img = helper.draw_scorebug_layout(game_data, fonts) - assert img.mode == 'RGB' - assert img.size == (128, 32) - assert img.convert('L').getbbox() is not None - - def test_empty_game_data_uses_defaults_without_raising(self): - helper = make_helper() - font = default_font() - fonts = {'time': font, 'status': font, 'score': font, 'team': font} - img = helper.draw_scorebug_layout({}, fonts) - assert img.mode == 'RGB' - assert img.size == (128, 32) - # Defaults '0'/'HOME'/'AWAY' actually render something - assert img.convert('L').getbbox() is not None - - def test_empty_fonts_dict_falls_back_to_default_font(self): - # Pin: fonts={} must not raise — PIL falls back to the default - # font when font=None is passed through. - helper = make_helper() - img = helper.draw_scorebug_layout( - {'status_text': 'FINAL', 'period_text': 'Q4', 'clock': '0:00'}, {}) - assert img.size == (128, 32) - assert img.convert('L').getbbox() is not None - - def test_top_line_is_one_combined_centered_draw(self): - # FIXED behavior: period/status/clock are joined into a single - # top line drawn once at y=1 instead of three overprinted draws. - helper = make_helper() - calls = [] - original = helper._draw_centered_text - - def spy(draw, text, font, y_position): - calls.append({'text': text, 'y_position': y_position}) - original(draw, text, font, y_position) - - helper._draw_centered_text = spy - font = default_font() - fonts = {'time': font, 'status': font, 'score': font, 'team': font} - helper.draw_scorebug_layout( - {'period_text': 'Q4', 'status_text': 'LIVE', 'clock': '2:30'}, - fonts) - - top_calls = [c for c in calls if c['y_position'] == 1] - assert len(top_calls) == 1 - text = top_calls[0]['text'] - assert 'Q4' in text - assert 'LIVE' in text - assert '2:30' in text - - def test_no_top_line_when_all_parts_empty(self): - helper = make_helper() - calls = [] - original = helper._draw_centered_text - - def spy(draw, text, font, y_position): - calls.append(y_position) - original(draw, text, font, y_position) - - helper._draw_centered_text = spy - font = default_font() - helper.draw_scorebug_layout({}, {'score': font, 'team': font}) - assert 1 not in calls # no combined top line drawn - - def test_logo_positions_bleed_off_edges(self): - # Home logo pastes at x = width - logo.width + 10 (right edge, - # bleeding off-screen right); away at x = -10 (bleeding left). - helper = make_helper() - home_logo = Image.new('RGBA', (20, 20), (0, 0, 255, 255)) # blue - away_logo = Image.new('RGBA', (20, 20), (255, 0, 0, 255)) # red - # Empty abbrs/status so text can't land on the probed pixels. - game_data = {'home_abbr': '', 'away_abbr': ''} - font = default_font() - img = helper.draw_scorebug_layout(game_data, {'score': font}, - home_logo=home_logo, - away_logo=away_logo) - # center_y = 16; logos span y 6..25 -> probe y=16 at both edges. - assert img.getpixel((0, 16)) == (255, 0, 0) # away (left edge) - assert img.getpixel((127, 16)) == (0, 0, 255) # home (right edge) - # And the off-screen parts are truly clipped: image is still 128 wide - assert img.size == (128, 32) - - -class TestTickerLayout: - def test_frame_is_not_blank(self): - # FIXED behavior: text now starts at x=0. Previously it was drawn - # at x=display_width, entirely off-canvas, so frames were blank. - helper = make_helper() - img = helper.draw_ticker_layout('HELLO WORLD', default_font()) - assert img.size == (128, 32) - assert img.mode == 'RGB' - assert img.convert('L').getbbox() is not None - - def test_text_starts_at_left_edge(self): - helper = make_helper() - img = helper.draw_ticker_layout('HELLO', default_font()) - bbox = img.convert('L').getbbox() - assert bbox is not None - # Text is positioned at x=0 (outline extends 1px left, clipped), - # so ink begins hugging the left edge. Allow a couple of pixels of - # slack for font-dependent left-side bearing. - assert bbox[0] <= 2 - - def test_scroll_speed_does_not_affect_frame(self): - # Pin: scroll_speed is accepted for API compatibility only. - helper = make_helper() - font = default_font() - img1 = helper.draw_ticker_layout('SCROLLING', font, scroll_speed=1) - img5 = helper.draw_ticker_layout('SCROLLING', font, scroll_speed=5) - assert img1.tobytes() == img5.tobytes() - - def test_custom_colors(self): - helper = make_helper() - img = helper.draw_ticker_layout('X', default_font(), - background_color=(0, 0, 40), - text_color=(0, 255, 0)) - assert img.getpixel((127, 0)) == (0, 0, 40) # background corner - colors = {img.getpixel((x, y)) - for x in range(img.width) for y in range(img.height)} - # Text color appears somewhere (anti-aliasing may blend it, so - # check for a green-dominant pixel rather than the exact color). - assert any(g > 150 and r < 100 for (r, g, b) in colors) - - -class TestCenteredText: - def test_renders_centered_text_on_background(self): - helper = make_helper() - img = helper.draw_centered_text('HI', default_font(), - background_color=(0, 0, 60), - text_color=(255, 255, 0)) - assert img.size == (128, 32) - assert img.convert('L').getbbox() is not None - # Corners stay pure background - assert img.getpixel((0, 0)) == (0, 0, 60) - assert img.getpixel((127, 0)) == (0, 0, 60) - assert img.getpixel((0, 31)) == (0, 0, 60) - assert img.getpixel((127, 31)) == (0, 0, 60) - - -class TestErrorAndNoDataMessages: - def test_draw_error_message_returns_rendered_image(self): - # FIXED behavior: used to crash with AttributeError; now returns - # a rendered image on a dark red background. - helper = make_helper() - img = helper.draw_error_message('Boom') - assert img.size == (128, 32) - assert img.mode == 'RGB' - assert img.convert('L').getbbox() is not None - assert img.getpixel((0, 0)) == (50, 0, 0) # dark red background - - def test_draw_error_message_default_text(self): - helper = make_helper() - img = helper.draw_error_message() - assert img.size == (128, 32) - assert img.getpixel((127, 31)) == (50, 0, 0) - - def test_draw_no_data_message_returns_rendered_image(self): - helper = make_helper() - img = helper.draw_no_data_message() - assert img.size == (128, 32) - assert img.mode == 'RGB' - assert img.convert('L').getbbox() is not None - assert img.getpixel((0, 0)) == (0, 0, 0) # black background - - -class TestDrawTextWithOutline: - def test_fill_color_appears_in_output(self): - helper = make_helper() - img = Image.new('RGB', (40, 20), (0, 0, 255)) - draw = ImageDraw.Draw(img) - helper._draw_text_with_outline(draw, 'X', (5, 2), default_font(), - fill=(255, 0, 0)) - pixels = {img.getpixel((x, y)) - for x in range(img.width) for y in range(img.height)} - # Anti-aliased fonts blend edge pixels, so look for red-dominant - # (fill) and near-black (outline) pixels rather than exact colors. - assert any(r > 150 and g < 50 for (r, g, b) in pixels) # fill - assert any(max(p) < 80 for p in pixels) # outline - - def test_default_fill_is_white(self): - helper = make_helper() - img = Image.new('RGB', (40, 20), (0, 0, 255)) - draw = ImageDraw.Draw(img) - helper._draw_text_with_outline(draw, 'X', (5, 2), default_font()) - pixels = {img.getpixel((x, y)) - for x in range(img.width) for y in range(img.height)} - # White-dominant pixel present (exact white may be anti-aliased) - assert any(r > 200 and g > 200 for (r, g, b) in pixels) - - -class TestOrientationAndDimensions: - def test_landscape_display(self): - helper = DisplayHelper(128, 32) - assert helper.is_landscape() is True - assert helper.is_portrait() is False - - def test_portrait_display(self): - helper = DisplayHelper(32, 128) - assert helper.is_portrait() is True - assert helper.is_landscape() is False - - def test_square_display_is_neither(self): - # Pin: a square display is neither portrait nor landscape. - helper = DisplayHelper(64, 64) - assert helper.is_portrait() is False - assert helper.is_landscape() is False - - def test_get_center_position(self): - assert DisplayHelper(128, 32).get_center_position() == (64, 16) - - def test_get_center_position_floors_odd_dimensions(self): - assert DisplayHelper(65, 33).get_center_position() == (32, 16) - - def test_get_display_dimensions(self): - assert DisplayHelper(128, 32).get_display_dimensions() == (128, 32) - assert DisplayHelper(64, 64).get_display_dimensions() == (64, 64) diff --git a/test/test_display_manager.py b/test/test_display_manager.py index d579ac85..747bdfd8 100644 --- a/test/test_display_manager.py +++ b/test/test_display_manager.py @@ -1,7 +1,6 @@ import os import pytest from unittest.mock import MagicMock, patch -from PIL import ImageDraw # display_manager imports the hardware rgbmatrix module at import time unless # EMULATOR=true. Use the emulator (same convention as @@ -106,22 +105,6 @@ class TestDisplayManagerDrawing: assert dm.image.convert("L").getbbox() is not None, \ "draw_text lit no pixels" - - def test_draw_image(self, test_config, mock_rgb_matrix): - """Test image drawing.""" - with patch.dict('os.environ', {'EMULATOR': 'false'}): - dm = DisplayManager(test_config) - - # DisplayManager doesn't have draw_image method - # It uses SetImage on canvas in update_display() - # Just verify DisplayManager can handle image operations - from PIL import Image - test_image = Image.new('RGB', (64, 32)) - dm.image = test_image - dm.draw = ImageDraw.Draw(dm.image) - - # Verify image was set - assert dm.image is not None class TestDisplayManagerResourceManagement: diff --git a/test/test_error_handling.py b/test/test_error_handling.py index c91e22c3..9fac688c 100644 --- a/test/test_error_handling.py +++ b/test/test_error_handling.py @@ -1,11 +1,5 @@ -import logging -import json from src.exceptions import CacheError, ConfigError, PluginError, DisplayError -from src.common.error_handler import ( - handle_file_operation, - handle_json_operation, - safe_execute -) + class TestCustomExceptions: """Test custom exception classes.""" @@ -37,85 +31,3 @@ class TestCustomExceptions: # DisplayError includes context in string representation assert "Display not found" in str(error) assert error.context.get('display_mode') == 'adafruit' - - -class TestErrorHandlerUtilities: - """Test error handler utilities.""" - - def test_handle_file_operation_read_success(self, tmp_path): - """Test successful file read.""" - test_file = tmp_path / "test.txt" - test_file.write_text("test content") - - result = handle_file_operation( - lambda: test_file.read_text(), - "Read failed", - logging.getLogger(__name__), - default="" - ) - assert result == "test content" - - def test_handle_file_operation_read_failure(self, tmp_path): - """Test file read failure.""" - non_existent = tmp_path / "nonexistent.txt" - - result = handle_file_operation( - lambda: non_existent.read_text(), - "Read failed", - logging.getLogger(__name__), - default="fallback" - ) - assert result == "fallback" - - def test_handle_json_operation_success(self, tmp_path): - """Test successful JSON parse.""" - test_file = tmp_path / "test.json" - test_file.write_text('{"key": "value"}') - - result = handle_json_operation( - lambda: json.loads(test_file.read_text()), - "JSON parse failed", - logging.getLogger(__name__), - default={} - ) - assert result == {"key": "value"} - - def test_handle_json_operation_failure(self, tmp_path): - """Test JSON parse failure.""" - test_file = tmp_path / "invalid.json" - test_file.write_text('invalid json {') - - result = handle_json_operation( - lambda: json.loads(test_file.read_text()), - "JSON parse failed", - logging.getLogger(__name__), - default={"default": True} - ) - assert result == {"default": True} - - def test_safe_execute_success(self): - """Test successful execution with safe_execute.""" - def success_func(): - return "success" - - result = safe_execute( - success_func, - "Execution failed", - logging.getLogger(__name__), - default="failed" - ) - assert result == "success" - - def test_safe_execute_failure(self): - """Test failure handling with safe_execute.""" - def failing_func(): - raise ValueError("Something went wrong") - - result = safe_execute( - failing_func, - "Execution failed", - logging.getLogger(__name__), - default="fallback" - ) - assert result == "fallback" - diff --git a/test/test_game_helper.py b/test/test_game_helper.py deleted file mode 100644 index 9bea5b4c..00000000 --- a/test/test_game_helper.py +++ /dev/null @@ -1,317 +0,0 @@ -""" -Tests for src/common/game_helper.py - -Covers GameHelper: extract_game_details, filter_*, sort_games_by_time, -process_games, get_game_summary, and all private helpers. -""" - -import logging -import pytest -from datetime import datetime, timezone, timedelta - -from src.common.game_helper import GameHelper - - -def _make_logger() -> logging.Logger: - return logging.getLogger("test_game_helper") - - -def _make_espn_event( - state: str = "in", - home_abbr: str = "LAL", - away_abbr: str = "BOS", - home_score: str = "105", - away_score: str = "98", - date_str: str = "2024-01-15T20:00:00Z", - period: int = 4, - status_name: str = "STATUS_IN_PROGRESS", - home_record: str = "30-10", - away_record: str = "25-15", - event_id: str = "game-1", -) -> dict: - return { - "id": event_id, - "date": date_str, - "competitions": [ - { - "status": { - "type": { - "state": state, - "shortDetail": "Q4 2:30", - "name": status_name, - }, - "period": period, - "displayClock": "2:30", - }, - "competitors": [ - { - "homeAway": "home", - "id": "h1", - "team": {"abbreviation": home_abbr, "displayName": f"{home_abbr} Team"}, - "score": home_score, - "records": [{"summary": home_record}], - }, - { - "homeAway": "away", - "id": "a1", - "team": {"abbreviation": away_abbr, "displayName": f"{away_abbr} Team"}, - "score": away_score, - "records": [{"summary": away_record}], - }, - ], - } - ], - } - - -@pytest.fixture -def helper(): - return GameHelper(timezone_str="UTC", logger=_make_logger()) - - -# --------------------------------------------------------------------------- -# extract_game_details -# --------------------------------------------------------------------------- - -class TestExtractGameDetails: - def test_live_game(self, helper): - event = _make_espn_event(state="in") - result = helper.extract_game_details(event) - assert result is not None - assert result["is_live"] is True - assert result["is_final"] is False - assert result["is_upcoming"] is False - - def test_final_game(self, helper): - event = _make_espn_event(state="post") - result = helper.extract_game_details(event) - assert result["is_final"] is True - - def test_upcoming_game(self, helper): - event = _make_espn_event(state="pre") - result = helper.extract_game_details(event) - assert result["is_upcoming"] is True - - def test_halftime_detection(self, helper): - event = _make_espn_event(state="halftime", status_name="STATUS_HALFTIME") - result = helper.extract_game_details(event) - assert result["is_halftime"] is True - - def test_basic_fields_present(self, helper): - event = _make_espn_event() - result = helper.extract_game_details(event) - for key in ("id", "home_abbr", "away_abbr", "home_score", "away_score", - "home_record", "away_record", "start_time_utc"): - assert key in result - - def test_team_abbreviations(self, helper): - event = _make_espn_event(home_abbr="MIA", away_abbr="PHX") - result = helper.extract_game_details(event) - assert result["home_abbr"] == "MIA" - assert result["away_abbr"] == "PHX" - - def test_scores_as_strings(self, helper): - event = _make_espn_event(home_score="110", away_score="99") - result = helper.extract_game_details(event) - assert result["home_score"] == "110" - assert result["away_score"] == "99" - - def test_returns_none_on_empty(self, helper): - assert helper.extract_game_details({}) is None - assert helper.extract_game_details(None) is None - - def test_returns_none_when_no_competitors(self, helper): - event = _make_espn_event() - event["competitions"][0]["competitors"] = [] - assert helper.extract_game_details(event) is None - - def test_date_z_suffix_parsed(self, helper): - event = _make_espn_event(date_str="2024-06-01T19:30:00Z") - result = helper.extract_game_details(event) - assert result["start_time_utc"] is not None - assert result["start_time_utc"].tzinfo is not None - - def test_zero_zero_record_suppressed(self, helper): - event = _make_espn_event(home_record="0-0", away_record="0-0-0") - result = helper.extract_game_details(event) - assert result["home_record"] == "" - assert result["away_record"] == "" - - def test_basketball_sport_fields(self, helper): - event = _make_espn_event(period=3) - result = helper.extract_game_details(event, sport="basketball") - assert result["period_text"] == "Q3" - assert "clock" in result - - def test_basketball_overtime_period(self, helper): - event = _make_espn_event(period=5) - result = helper.extract_game_details(event, sport="basketball") - assert result["period_text"] == "OT1" - - def test_football_sport_fields(self, helper): - event = _make_espn_event(period=2) - result = helper.extract_game_details(event, sport="football") - assert result["period_text"] == "Q2" - - def test_hockey_sport_fields_period_1(self, helper): - event = _make_espn_event(period=1) - result = helper.extract_game_details(event, sport="hockey") - assert result["period_text"] == "P1" - - def test_hockey_sport_fields_ot(self, helper): - event = _make_espn_event(period=4) - result = helper.extract_game_details(event, sport="hockey") - assert result["period_text"] == "OT1" - - def test_baseball_sport_fields(self, helper): - event = _make_espn_event(period=7) - result = helper.extract_game_details(event, sport="baseball") - assert result["period_text"] == "INN 7" - - -# --------------------------------------------------------------------------- -# Filter methods -# --------------------------------------------------------------------------- - -class TestFilterMethods: - def _make_games(self): - now = datetime.now(timezone.utc) - return [ - {"is_live": True, "is_final": False, "is_upcoming": False, "home_abbr": "LAL", "away_abbr": "BOS", "start_time_utc": now}, - {"is_live": False, "is_final": True, "is_upcoming": False, "home_abbr": "MIA", "away_abbr": "PHX", "start_time_utc": now - timedelta(hours=3)}, - {"is_live": False, "is_final": False, "is_upcoming": True, "home_abbr": "DAL", "away_abbr": "CHI", "start_time_utc": now + timedelta(hours=2)}, - ] - - def test_filter_live_games(self, helper): - games = self._make_games() - result = helper.filter_live_games(games) - assert len(result) == 1 - assert result[0]["home_abbr"] == "LAL" - - def test_filter_final_games(self, helper): - games = self._make_games() - result = helper.filter_final_games(games) - assert len(result) == 1 - assert result[0]["home_abbr"] == "MIA" - - def test_filter_upcoming_games(self, helper): - games = self._make_games() - result = helper.filter_upcoming_games(games) - assert len(result) == 1 - assert result[0]["home_abbr"] == "DAL" - - def test_filter_favorite_teams_match(self, helper): - games = self._make_games() - result = helper.filter_favorite_teams(games, ["LAL"]) - assert len(result) == 1 - assert result[0]["home_abbr"] == "LAL" - - def test_filter_favorite_teams_empty_list_returns_all(self, helper): - games = self._make_games() - result = helper.filter_favorite_teams(games, []) - assert len(result) == 3 - - def test_filter_favorite_teams_away_match(self, helper): - games = self._make_games() - result = helper.filter_favorite_teams(games, ["BOS"]) - assert len(result) == 1 - - def test_filter_recent_games_within_window(self, helper): - now = datetime.now(timezone.utc) - games = [ - {"start_time_utc": now - timedelta(days=2), "is_final": True}, - {"start_time_utc": now - timedelta(days=10), "is_final": True}, - ] - result = helper.filter_recent_games(games, days_back=7) - assert len(result) == 1 - - def test_filter_recent_games_all_within(self, helper): - now = datetime.now(timezone.utc) - games = [ - {"start_time_utc": now - timedelta(days=1)}, - {"start_time_utc": now - timedelta(days=3)}, - ] - result = helper.filter_recent_games(games, days_back=7) - assert len(result) == 2 - - def test_sort_games_ascending(self, helper): - now = datetime.now(timezone.utc) - games = [ - {"start_time_utc": now + timedelta(hours=2), "id": "late"}, - {"start_time_utc": now + timedelta(hours=1), "id": "early"}, - ] - result = helper.sort_games_by_time(games) - assert result[0]["id"] == "early" - - def test_sort_games_descending(self, helper): - now = datetime.now(timezone.utc) - games = [ - {"start_time_utc": now + timedelta(hours=1), "id": "early"}, - {"start_time_utc": now + timedelta(hours=2), "id": "late"}, - ] - result = helper.sort_games_by_time(games, reverse=True) - assert result[0]["id"] == "late" - - -# --------------------------------------------------------------------------- -# process_games -# --------------------------------------------------------------------------- - -class TestProcessGames: - def test_processes_valid_events(self, helper): - events = [ - _make_espn_event(event_id="1"), - _make_espn_event(event_id="2"), - ] - result = helper.process_games(events) - assert len(result) == 2 - - def test_skips_invalid_events(self, helper): - events = [ - _make_espn_event(event_id="1"), - {}, # invalid - ] - result = helper.process_games(events) - assert len(result) == 1 - - def test_empty_events(self, helper): - assert helper.process_games([]) == [] - - -# --------------------------------------------------------------------------- -# get_game_summary -# --------------------------------------------------------------------------- - -class TestGetGameSummary: - def test_live_summary(self, helper): - game = { - "home_abbr": "LAL", "away_abbr": "BOS", - "home_score": "105", "away_score": "98", - "status_text": "Q4 2:30", - "is_live": True, "is_final": False, - } - summary = helper.get_game_summary(game) - assert "BOS" in summary - assert "LAL" in summary - assert "98" in summary - assert "105" in summary - - def test_final_summary(self, helper): - game = { - "home_abbr": "LAL", "away_abbr": "BOS", - "home_score": "110", "away_score": "102", - "status_text": "Final", - "is_live": False, "is_final": True, - } - summary = helper.get_game_summary(game) - assert "Final" in summary - - def test_upcoming_summary(self, helper): - game = { - "home_abbr": "LAL", "away_abbr": "BOS", - "home_score": "0", "away_score": "0", - "status_text": "7:30 PM", - "is_live": False, "is_final": False, - } - summary = helper.get_game_summary(game) - assert "7:30 PM" in summary diff --git a/test/test_health_monitor.py b/test/test_health_monitor.py deleted file mode 100644 index 41a6206a..00000000 --- a/test/test_health_monitor.py +++ /dev/null @@ -1,307 +0,0 @@ -""" -Tests for src/plugin_system/health_monitor.py - -Covers PluginHealthMonitor: get_plugin_health_status, get_plugin_health_metrics, -get_all_plugin_health, _get_recovery_suggestions, start/stop_monitoring, -register_health_check. -""" - -import pytest -from unittest.mock import MagicMock, patch -from datetime import datetime - -from src.plugin_system.health_monitor import ( - PluginHealthMonitor, - HealthStatus, - HealthMetrics, -) - - -# --------------------------------------------------------------------------- -# Fixtures -# --------------------------------------------------------------------------- - -def _make_health_tracker( - summary: dict | None = None, - all_summaries: dict | None = None, -): - """Return a mock PluginHealthTracker.""" - tracker = MagicMock() - tracker.get_health_summary.return_value = summary - tracker.get_all_health_summaries.return_value = all_summaries or {} - return tracker - - -def _healthy_summary() -> dict: - return { - "success_rate": 100.0, - "circuit_state": "closed", - "consecutive_failures": 0, - "total_failures": 0, - "total_successes": 50, - "last_success_time": datetime.now().isoformat(), - "last_error": None, - } - - -def _degraded_summary() -> dict: - return { - "success_rate": 40.0, # 60% error rate - "circuit_state": "closed", - "consecutive_failures": 3, - "total_failures": 6, - "total_successes": 4, - "last_success_time": None, - "last_error": "timeout occurred", - } - - -def _unhealthy_summary() -> dict: - return { - "success_rate": 10.0, # 90% error rate - "circuit_state": "open", - "consecutive_failures": 10, - "total_failures": 9, - "total_successes": 1, - "last_success_time": None, - "last_error": "ImportError: missing module", - } - - -@pytest.fixture -def monitor(): - tracker = _make_health_tracker(_healthy_summary()) - return PluginHealthMonitor(health_tracker=tracker) - - -# --------------------------------------------------------------------------- -# get_plugin_health_status -# --------------------------------------------------------------------------- - -class TestGetPluginHealthStatus: - def test_healthy_status(self): - tracker = _make_health_tracker(_healthy_summary()) - monitor = PluginHealthMonitor(tracker) - status = monitor.get_plugin_health_status("plugin_a") - assert status == HealthStatus.HEALTHY - - def test_degraded_status(self): - tracker = _make_health_tracker(_degraded_summary()) - monitor = PluginHealthMonitor(tracker, degraded_threshold=0.5, unhealthy_threshold=0.8) - status = monitor.get_plugin_health_status("plugin_b") - assert status == HealthStatus.DEGRADED - - def test_unhealthy_status(self): - tracker = _make_health_tracker(_unhealthy_summary()) - monitor = PluginHealthMonitor(tracker, unhealthy_threshold=0.8) - status = monitor.get_plugin_health_status("plugin_c") - assert status == HealthStatus.UNHEALTHY - - def test_open_circuit_breaker_is_unhealthy(self): - summary = _healthy_summary() - summary["circuit_state"] = "open" - tracker = _make_health_tracker(summary) - monitor = PluginHealthMonitor(tracker) - status = monitor.get_plugin_health_status("plugin_d") - assert status == HealthStatus.UNHEALTHY - - def test_unknown_when_no_tracker(self): - monitor = PluginHealthMonitor(health_tracker=None) - status = monitor.get_plugin_health_status("plugin_e") - assert status == HealthStatus.UNKNOWN - - def test_unknown_when_no_summary(self): - tracker = _make_health_tracker(None) - monitor = PluginHealthMonitor(tracker) - status = monitor.get_plugin_health_status("plugin_f") - assert status == HealthStatus.UNKNOWN - - -# --------------------------------------------------------------------------- -# get_plugin_health_metrics -# --------------------------------------------------------------------------- - -class TestGetPluginHealthMetrics: - def test_healthy_metrics(self): - tracker = _make_health_tracker(_healthy_summary()) - monitor = PluginHealthMonitor(tracker) - metrics = monitor.get_plugin_health_metrics("plugin_a") - assert isinstance(metrics, HealthMetrics) - assert metrics.status == HealthStatus.HEALTHY - assert metrics.success_rate == pytest.approx(1.0) - assert metrics.error_rate == pytest.approx(0.0) - - def test_degraded_metrics(self): - tracker = _make_health_tracker(_degraded_summary()) - monitor = PluginHealthMonitor(tracker, degraded_threshold=0.5, unhealthy_threshold=0.8) - metrics = monitor.get_plugin_health_metrics("plugin_b") - assert metrics.status == HealthStatus.DEGRADED - assert metrics.consecutive_failures == 3 - - def test_unhealthy_metrics(self): - tracker = _make_health_tracker(_unhealthy_summary()) - monitor = PluginHealthMonitor(tracker, unhealthy_threshold=0.8) - metrics = monitor.get_plugin_health_metrics("plugin_c") - assert metrics.status == HealthStatus.UNHEALTHY - assert metrics.circuit_breaker_state == "open" - assert metrics.last_error is not None - - def test_metrics_without_tracker(self): - monitor = PluginHealthMonitor(health_tracker=None) - metrics = monitor.get_plugin_health_metrics("plugin_d") - assert metrics.status == HealthStatus.UNKNOWN - assert metrics.plugin_id == "plugin_d" - - def test_metrics_without_summary(self): - tracker = _make_health_tracker(None) - monitor = PluginHealthMonitor(tracker) - metrics = monitor.get_plugin_health_metrics("plugin_e") - assert metrics.status == HealthStatus.UNKNOWN - - def test_last_successful_update_parsed(self): - summary = _healthy_summary() - summary["last_success_time"] = "2024-06-01T12:00:00" - tracker = _make_health_tracker(summary) - monitor = PluginHealthMonitor(tracker) - metrics = monitor.get_plugin_health_metrics("plugin_a") - assert metrics.last_successful_update is not None - assert isinstance(metrics.last_successful_update, datetime) - - def test_invalid_last_success_time_handled(self): - summary = _healthy_summary() - summary["last_success_time"] = "not-a-date" - tracker = _make_health_tracker(summary) - monitor = PluginHealthMonitor(tracker) - # Should not raise - metrics = monitor.get_plugin_health_metrics("plugin_a") - assert metrics.last_successful_update is None - - def test_total_successes_failures(self): - tracker = _make_health_tracker(_degraded_summary()) - monitor = PluginHealthMonitor(tracker, degraded_threshold=0.5, unhealthy_threshold=0.8) - metrics = monitor.get_plugin_health_metrics("plugin_b") - assert metrics.total_failures == 6 - assert metrics.total_successes == 4 - - -# --------------------------------------------------------------------------- -# get_all_plugin_health -# --------------------------------------------------------------------------- - -class TestGetAllPluginHealth: - def test_returns_empty_without_tracker(self): - monitor = PluginHealthMonitor(health_tracker=None) - result = monitor.get_all_plugin_health() - assert result == {} - - def test_returns_metrics_for_each_plugin(self): - all_summaries = { - "plugin_a": _healthy_summary(), - "plugin_b": _degraded_summary(), - } - tracker = MagicMock() - tracker.get_all_health_summaries.return_value = all_summaries - tracker.get_health_summary.side_effect = lambda pid: all_summaries.get(pid) - monitor = PluginHealthMonitor(tracker, degraded_threshold=0.5, unhealthy_threshold=0.8) - result = monitor.get_all_plugin_health() - assert "plugin_a" in result - assert "plugin_b" in result - assert isinstance(result["plugin_a"], HealthMetrics) - - def test_returns_empty_when_no_summaries(self): - tracker = _make_health_tracker(all_summaries={}) - monitor = PluginHealthMonitor(tracker) - result = monitor.get_all_plugin_health() - assert result == {} - - -# --------------------------------------------------------------------------- -# _get_recovery_suggestions -# --------------------------------------------------------------------------- - -class TestGetRecoverySuggestions: - def test_healthy_plugin_suggestion(self): - tracker = _make_health_tracker(_healthy_summary()) - monitor = PluginHealthMonitor(tracker) - suggestions = monitor._get_recovery_suggestions("p", _healthy_summary(), HealthStatus.HEALTHY) - assert any("healthy" in s.lower() for s in suggestions) - - def test_unhealthy_suggestions(self): - tracker = _make_health_tracker(_unhealthy_summary()) - monitor = PluginHealthMonitor(tracker, unhealthy_threshold=0.8) - suggestions = monitor._get_recovery_suggestions("p", _unhealthy_summary(), HealthStatus.UNHEALTHY) - assert len(suggestions) > 0 - assert any("unhealthy" in s.lower() for s in suggestions) - - def test_open_circuit_breaker_suggestion(self): - summary = _unhealthy_summary() - summary["circuit_state"] = "open" - tracker = _make_health_tracker(summary) - monitor = PluginHealthMonitor(tracker, unhealthy_threshold=0.8) - suggestions = monitor._get_recovery_suggestions("p", summary, HealthStatus.UNHEALTHY) - assert any("circuit" in s.lower() for s in suggestions) - - def test_timeout_error_suggestion(self): - summary = _degraded_summary() - summary["last_error"] = "connection timeout occurred" - tracker = _make_health_tracker(summary) - monitor = PluginHealthMonitor(tracker, degraded_threshold=0.5, unhealthy_threshold=0.8) - suggestions = monitor._get_recovery_suggestions("p", summary, HealthStatus.DEGRADED) - assert any("timeout" in s.lower() for s in suggestions) - - def test_import_error_suggestion(self): - summary = _unhealthy_summary() - summary["last_error"] = "ImportError: missing module" - tracker = _make_health_tracker(summary) - monitor = PluginHealthMonitor(tracker, unhealthy_threshold=0.8) - suggestions = monitor._get_recovery_suggestions("p", summary, HealthStatus.UNHEALTHY) - assert any("dependencies" in s.lower() or "import" in s.lower() or "missing" in s.lower() - for s in suggestions) - - def test_permission_error_suggestion(self): - summary = _unhealthy_summary() - summary["last_error"] = "permission denied to access resource" - tracker = _make_health_tracker(summary) - monitor = PluginHealthMonitor(tracker, unhealthy_threshold=0.8) - suggestions = monitor._get_recovery_suggestions("p", summary, HealthStatus.UNHEALTHY) - assert any("permission" in s.lower() for s in suggestions) - - def test_degraded_suggestions_include_error_rate(self): - tracker = _make_health_tracker(_degraded_summary()) - monitor = PluginHealthMonitor(tracker, degraded_threshold=0.5, unhealthy_threshold=0.8) - suggestions = monitor._get_recovery_suggestions("p", _degraded_summary(), HealthStatus.DEGRADED) - assert any("%" in s for s in suggestions) - - -# --------------------------------------------------------------------------- -# start / stop monitoring -# --------------------------------------------------------------------------- - -class TestMonitorLifecycle: - def test_start_monitoring(self, monitor): - monitor.start_monitoring() - try: - assert monitor._monitor_thread is not None - assert monitor._monitor_thread.is_alive() - finally: - monitor.stop_monitoring() - - def test_stop_monitoring(self, monitor): - monitor.start_monitoring() - monitor.stop_monitoring() - # Thread should no longer be alive - assert not monitor._monitor_thread.is_alive() - - def test_double_start_no_duplicate_threads(self, monitor): - monitor.start_monitoring() - try: - thread1 = monitor._monitor_thread - monitor.start_monitoring() # should be idempotent - assert monitor._monitor_thread is thread1 - finally: - monitor.stop_monitoring() - - def test_register_health_check(self, monitor): - callback = MagicMock() - monitor.register_health_check(callback) - assert callback in monitor._health_check_callbacks diff --git a/test/test_initial_update_budget.py b/test/test_initial_update_budget.py index 4b74cc8e..889ae457 100644 --- a/test/test_initial_update_budget.py +++ b/test/test_initial_update_budget.py @@ -61,10 +61,7 @@ def tiny_floor(monkeypatch): def _controller(plugin_ids, executor): c = DisplayController.__new__(DisplayController) c.plugin_manager = Mock() - # Both attributes, because _update_modules reads - # `loaded_plugins or plugins` and an empty dict is falsy. - c.plugin_manager.loaded_plugins = {pid: Mock() for pid in plugin_ids} - c.plugin_manager.plugins = dict(c.plugin_manager.loaded_plugins) + c.plugin_manager.plugins = {pid: Mock() for pid in plugin_ids} c.plugin_manager.plugin_executor = executor c.plugin_manager.plugin_last_update = {} c.plugin_manager.health_tracker = None diff --git a/test/test_loader_compat_warning.py b/test/test_loader_compat_warning.py index abe5f29e..a26ed2c6 100644 --- a/test/test_loader_compat_warning.py +++ b/test/test_loader_compat_warning.py @@ -16,17 +16,6 @@ def _warnings(caplog): return [r for r in caplog.records if r.levelno == logging.WARNING] -class TestParseSemver: - def test_basic(self, loader): - assert loader._parse_semver("3.1.0") == (3, 1, 0) - assert loader._parse_semver("v2.0") == (2, 0, 0) - assert loader._parse_semver("2.0.0-beta.1") == (2, 0, 0) - - def test_unparseable(self, loader): - assert loader._parse_semver(None) is None - assert loader._parse_semver(123) is None - - class TestWarnIfIncompatible: def test_warns_when_plugin_needs_newer_core(self, loader, caplog, monkeypatch): import src diff --git a/test/test_plugin_state_history_cap.py b/test/test_plugin_state_history_cap.py deleted file mode 100644 index 1d5e20d5..00000000 --- a/test/test_plugin_state_history_cap.py +++ /dev/null @@ -1,166 +0,0 @@ -"""Plugin state history must not grow without bound. - -`PluginStateManager` recorded every state transition in a per-plugin list and -never trimmed it. The only code that removed entries was `clear_state()`, called -solely from `PluginManager.unload_plugin()`, so a plugin that stays loaded -- -i.e. normal operation -- never released a single entry. - -The list is written on the hot scheduling path. Every update cycle appends -twice: `_reserve_for_update()` sets RUNNING and `_finish()` sets ENABLED back -again. At the default 60-second update interval that is 2,880 entries per -plugin per day, and nothing ever reads the entries -- `get_state_info()` only -takes their `len()`. It is pure dead weight. - -Measured against the unpatched class, ten plugins on a 60s interval retain -864,010 transitions after thirty simulated days, for 231 MB of heap. On a 1 GB -Pi that is fatal on its own, and the failure is not a clean OOM: once -MemAvailable falls far enough, fork() starts returning ENOMEM, so sshd accepts -connections and closes them before its banner while the kernel still answers -pings. The board looks like a hardware fault and needs a power cycle. - -These tests pin the cap, the retention order, and the one piece of behaviour the -cap must not change: `state_history_count` is surfaced through the web API, so -it has to keep reporting the lifetime total rather than plateauing at the cap. -""" - -import os -import sys - -import pytest - -sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) - -from src.plugin_system.plugin_state import ( # noqa: E402 - MAX_STATE_HISTORY_PER_PLUGIN, - PluginState, - PluginStateManager, -) - - -def _cycle_updates(manager, plugin_id, cycles): - """Drive the real scheduling path: RUNNING on reserve, ENABLED on finish.""" - for _ in range(cycles): - manager.set_state(plugin_id, PluginState.RUNNING) - manager.set_state(plugin_id, PluginState.ENABLED) - - -def test_state_history_is_capped(): - """A day of updates must not retain a day of transitions.""" - manager = PluginStateManager() - manager.set_state("clock", PluginState.ENABLED) - - # One simulated day at the default 60s update interval. - _cycle_updates(manager, "clock", 1440) - - history = manager.get_state_history("clock") - assert len(history) <= MAX_STATE_HISTORY_PER_PLUGIN, ( - f"history grew to {len(history)} entries; it is never trimmed" - ) - - -def test_state_history_keeps_the_most_recent_transitions(): - """Trimming drops the oldest entries, not the newest.""" - manager = PluginStateManager() - manager.set_state("clock", PluginState.ENABLED) - _cycle_updates(manager, "clock", MAX_STATE_HISTORY_PER_PLUGIN) - - history = manager.get_state_history("clock") - - # The scheduling cycle ends on ENABLED, so the newest entry is the - # RUNNING -> ENABLED half of the last cycle. - assert history[-1]["from"] == PluginState.RUNNING.value - assert history[-1]["to"] == PluginState.ENABLED.value - - # And the very first ENABLED transition has aged out. - assert history[0]["from"] != PluginState.UNLOADED.value - - -def test_state_history_count_reports_lifetime_total(): - """The count exposed through the API must not plateau at the cap. - - `get_state_info()['state_history_count']` is surfaced by the web UI. Capping - the retained list must not turn it into "entries we happen to still hold". - """ - manager = PluginStateManager() - manager.set_state("clock", PluginState.ENABLED) - total = 1 - - cycles = MAX_STATE_HISTORY_PER_PLUGIN * 2 - _cycle_updates(manager, "clock", cycles) - total += cycles * 2 - - info = manager.get_state_info("clock") - assert info["state_history_count"] == total - assert len(manager.get_state_history("clock")) <= MAX_STATE_HISTORY_PER_PLUGIN - - -def test_error_transitions_are_capped_too(): - """set_state_with_error() appends to the same list and needs the same cap.""" - manager = PluginStateManager() - manager.set_state("clock", PluginState.ENABLED) - - for _ in range(MAX_STATE_HISTORY_PER_PLUGIN * 2): - manager.set_state_with_error( - "clock", - PluginState.ENABLED, - {"reason": "update timeout"}, - error=RuntimeError("boom"), - ) - - assert len(manager.get_state_history("clock")) <= MAX_STATE_HISTORY_PER_PLUGIN - - -def test_history_is_isolated_per_plugin(): - """The cap is per plugin, not shared across the manager.""" - manager = PluginStateManager() - for plugin_id in ("clock", "weather"): - manager.set_state(plugin_id, PluginState.ENABLED) - _cycle_updates(manager, plugin_id, 50) - - assert len(manager.get_state_history("clock")) == 101 - assert len(manager.get_state_history("weather")) == 101 - - -def test_get_state_history_returns_a_copy(): - """Callers must not be able to mutate the manager's internal history.""" - manager = PluginStateManager() - manager.set_state("clock", PluginState.ENABLED) - - history = manager.get_state_history("clock") - history.clear() - - assert len(manager.get_state_history("clock")) == 1 - - -def test_get_state_history_entries_are_copies(): - """Copying the outer list is not enough -- the entries are handed out too. - - A caller holding a returned transition must not be able to rewrite the - manager's record of what happened. - """ - manager = PluginStateManager() - manager.set_state("clock", PluginState.ENABLED) - - entry = manager.get_state_history("clock")[0] - entry["to"] = "tampered" - entry["error"] = "injected" - - stored = manager.get_state_history("clock")[0] - assert stored["to"] == PluginState.ENABLED.value - assert stored["error"] is None - - -def test_clear_state_drops_history(): - """Unloading a plugin still releases everything it accumulated.""" - manager = PluginStateManager() - manager.set_state("clock", PluginState.ENABLED) - _cycle_updates(manager, "clock", 10) - - manager.clear_state("clock") - - assert manager.get_state_history("clock") == [] - assert manager.get_state_info("clock")["state_history_count"] == 0 - - -if __name__ == "__main__": - sys.exit(pytest.main([__file__, "-v"])) diff --git a/test/test_plugin_state_history_retention.py b/test/test_plugin_state_history_retention.py deleted file mode 100644 index a7cf1edf..00000000 --- a/test/test_plugin_state_history_retention.py +++ /dev/null @@ -1,209 +0,0 @@ -"""Retention is bounded by age first and by count second. - -The cap added in the parent change is a flat entry count, and an entry count -answers the wrong question. What a reader wants from this history is "the last -couple of hours"; how many transitions that is depends entirely on the -plugin's update interval, which on a real board spans 2s to 3600s. A flat 200 -entries is 4.2 days of history for the slowest plugin and 3.3 minutes for the -fastest -- so the plugin churning hardest, the one actually worth looking at, -keeps the least. - -Trimming by age makes the retained window comparable whatever the cadence, and -the count then serves only as a memory ceiling for pollers fast enough to -produce thousands of transitions inside that window. -""" - -import time -import pytest - -from src.plugin_system.plugin_state import ( - PluginState, - PluginStateManager, - MAX_STATE_HISTORY_PER_PLUGIN, - STATE_HISTORY_MAX_AGE_SECONDS, -) - - -class FakeClock: - """A monotonic clock the test drives, so no test has to sleep.""" - - def __init__(self): - self.t = 1000.0 - - def __call__(self): - return self.t - - def advance(self, seconds): - self.t += seconds - - -@pytest.fixture -def clock(monkeypatch): - c = FakeClock() - monkeypatch.setattr("src.plugin_system.plugin_state.time.monotonic", c) - return c - - -def _cycle(manager, plugin_id, clock, interval, cycles): - """One update cycle: RUNNING on reserve, ENABLED on finish.""" - for _ in range(cycles): - manager.set_state(plugin_id, PluginState.RUNNING) - manager.set_state(plugin_id, PluginState.ENABLED) - clock.advance(interval) - - -def test_transitions_older_than_the_window_are_dropped(clock): - m = PluginStateManager() - _cycle(m, "clock", clock, interval=60, cycles=10) - assert len(m.get_state_history("clock")) == 20 - - # Nothing happens for longer than the window, then one more cycle. - clock.advance(STATE_HISTORY_MAX_AGE_SECONDS + 1) - _cycle(m, "clock", clock, interval=60, cycles=1) - - assert len(m.get_state_history("clock")) == 2, ( - "only the transitions inside the window should survive") - - -def test_every_plugin_keeps_the_same_WINDOW_not_the_same_COUNT(clock): - """The point of the age policy, stated as the property that distinguishes it. - - Run both plugins for three times the retention window. Under a flat count - cap the slow one would still be holding transitions from hours before the - window, because it never produces enough entries to evict them. Under the - age policy each plugin retains its own last two hours and no more -- - different entry counts, same span of time. - """ - window = STATE_HISTORY_MAX_AGE_SECONDS - m = PluginStateManager() - - _cycle(m, "slow", clock, interval=60, cycles=(3 * window) // 60) - slow = len(m.get_state_history("slow")) - - # Assert the property directly rather than a derived count. The guarantee - # is about the SPAN of retained history, not its age against the current - # clock: trimming happens on append, so a plugin that has gone quiet keeps - # its last window until it writes again. That is intentional -- it is - # bounded either way, and a lazy trim costs nothing on the hot path. - stamps = [stamp for stamp, _ in m._state_history["slow"]] - assert stamps[-1] - stamps[0] <= window, ( - f"retained history spans {stamps[-1] - stamps[0]:.0f}s, " - f"window is {window}s") - assert slow < 2 * ((3 * window) // 60), ( - f"slow plugin kept {slow} entries -- three windows' worth was retained") - - clock.t = 1000.0 - _cycle(m, "fast", clock, interval=2, cycles=(3 * window) // 2) - fast = len(m.get_state_history("fast")) - - # Different counts, and the fast poller keeps more of them -- under a flat - # count cap these would be equal and the fast one would cover minutes. - assert fast > slow, f"fast={fast} slow={slow}" - - -def test_the_count_ceiling_still_bounds_a_fast_poller(clock): - """Age alone would let a 2s plugin hold 7,200 entries.""" - m = PluginStateManager() - _cycle(m, "flights", clock, interval=2, cycles=STATE_HISTORY_MAX_AGE_SECONDS) - assert len(m.get_state_history("flights")) <= MAX_STATE_HISTORY_PER_PLUGIN - - -def test_a_burst_inside_the_window_is_capped_not_kept(clock): - """Transitions with no time between them still cannot grow without bound.""" - m = PluginStateManager() - for _ in range(MAX_STATE_HISTORY_PER_PLUGIN * 3): - m.set_state("flapping", PluginState.RUNNING) # clock never advances - assert len(m.get_state_history("flapping")) <= MAX_STATE_HISTORY_PER_PLUGIN - - -def test_ageing_out_does_not_disturb_the_lifetime_count(clock): - m = PluginStateManager() - _cycle(m, "clock", clock, interval=60, cycles=10) - clock.advance(STATE_HISTORY_MAX_AGE_SECONDS + 1) - _cycle(m, "clock", clock, interval=60, cycles=1) - - assert len(m.get_state_history("clock")) == 2 - assert m.get_state_info("clock")["state_history_count"] == 22, ( - "the lifetime total must survive trimming, it is the flap signal") - - -def test_the_surviving_entries_are_the_recent_ones(clock): - m = PluginStateManager() - _cycle(m, "clock", clock, interval=60, cycles=5) - clock.advance(STATE_HISTORY_MAX_AGE_SECONDS + 1) - m.set_state("clock", PluginState.ERROR) - - history = m.get_state_history("clock") - assert [h["to"] for h in history] == ["error"] - - -def test_a_monotonic_clock_is_used_not_the_wall_clock(clock): - """A DST shift or NTP step must not flush the history. - - The trim reads time.monotonic(); the human-readable datetime inside each - transition is for display only. - """ - m = PluginStateManager() - _cycle(m, "clock", clock, interval=60, cycles=3) - before = len(m.get_state_history("clock")) - - import datetime as real_datetime - - class ShiftedDatetime(real_datetime.datetime): - @classmethod - def now(cls, tz=None): - return real_datetime.datetime(1999, 1, 1) # clock jumps backwards - - import src.plugin_system.plugin_state as ps - original = ps.datetime - ps.datetime = ShiftedDatetime - try: - m.set_state("clock", PluginState.ENABLED) - finally: - ps.datetime = original - - assert len(m.get_state_history("clock")) == before + 1, ( - "a wall-clock jump must not trim anything") - - -def test_get_state_info_is_a_consistent_snapshot(): - """An unload running concurrently must not be observed half-done. - - Each field used to be read under its own lock, so clear_state() could - interleave: 'state' read before the removal, 'state_history_count' after, - handing a caller a plugin that is ENABLED with zero transitions. The whole - payload is now built in one critical section. - """ - import threading - - m = PluginStateManager() - for _ in range(50): - m.set_state("clock", PluginState.RUNNING) - m.set_state("clock", PluginState.ENABLED) - - inconsistent = [] - stop = threading.Event() - - def reader(): - while not stop.is_set(): - info = m.get_state_info("clock") - # Either fully present or fully cleared -- never a live state with - # a wiped count. - if info["state"] != PluginState.UNLOADED.value and \ - info["state_history_count"] == 0: - inconsistent.append(info) - return - - def clearer(): - for _ in range(200): - for _ in range(20): - m.set_state("clock", PluginState.ENABLED) - m.clear_state("clock") - - t = threading.Thread(target=reader, daemon=True) - t.start() - clearer() - stop.set() - t.join(timeout=5) - - assert not inconsistent, f"observed a torn snapshot: {inconsistent[:1]}" diff --git a/test/test_plugin_state_transition_count.py b/test/test_plugin_state_transition_count.py new file mode 100644 index 00000000..ae2ba05f --- /dev/null +++ b/test/test_plugin_state_transition_count.py @@ -0,0 +1,119 @@ +"""Plugin state transitions are counted, not stored. + +`PluginStateManager` used to keep every transition in a per-plugin history on +the hot scheduling path (RUNNING on reserve, ENABLED on finish), but nothing +ever read the entries -- `get_state_info()` only reported how many there were. +It now keeps just that lifetime count, which `state_history_count` surfaces +through the web API. +""" + +import os +import sys +import threading + +import pytest + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) + +from src.plugin_system.plugin_state import ( # noqa: E402 + PluginState, + PluginStateManager, +) + + +def _cycle_updates(manager, plugin_id, cycles): + """Drive the real scheduling path: RUNNING on reserve, ENABLED on finish.""" + for _ in range(cycles): + manager.set_state(plugin_id, PluginState.RUNNING) + manager.set_state(plugin_id, PluginState.ENABLED) + + +def test_state_history_count_reports_lifetime_total(): + """`get_state_info()['state_history_count']` counts every transition.""" + manager = PluginStateManager() + manager.set_state("clock", PluginState.ENABLED) + + cycles = 4000 + _cycle_updates(manager, "clock", cycles) + + info = manager.get_state_info("clock") + assert info["state_history_count"] == 1 + cycles * 2 + + +def test_error_transitions_are_counted(): + """set_state_with_error() is a transition too.""" + manager = PluginStateManager() + manager.set_state("clock", PluginState.ENABLED) + manager.set_state_with_error( + "clock", PluginState.ENABLED, {"reason": "update timeout"} + ) + + info = manager.get_state_info("clock") + assert info["state_history_count"] == 2 + assert info["error_info"] == {"reason": "update timeout"} + + +def test_count_is_isolated_per_plugin(): + manager = PluginStateManager() + manager.set_state("clock", PluginState.ENABLED) + _cycle_updates(manager, "clock", 50) + manager.set_state("weather", PluginState.ENABLED) + + assert manager.get_state_info("clock")["state_history_count"] == 101 + assert manager.get_state_info("weather")["state_history_count"] == 1 + + +def test_clear_state_drops_the_count(): + """Unloading a plugin still releases everything it accumulated.""" + manager = PluginStateManager() + manager.set_state("clock", PluginState.ENABLED) + _cycle_updates(manager, "clock", 10) + + manager.clear_state("clock") + + info = manager.get_state_info("clock") + assert info["state"] == PluginState.UNLOADED.value + assert info["state_history_count"] == 0 + + +def test_get_state_info_is_a_consistent_snapshot(): + """An unload running concurrently must not be observed half-done. + + Each field used to be read under its own lock, so clear_state() could + interleave: 'state' read before the removal, 'state_history_count' after, + handing a caller a plugin that is ENABLED with zero transitions. The whole + payload is now built in one critical section. + """ + m = PluginStateManager() + _cycle_updates(m, "clock", 50) + + inconsistent = [] + stop = threading.Event() + + def reader(): + while not stop.is_set(): + info = m.get_state_info("clock") + # Either fully present or fully cleared -- never a live state with + # a wiped count. + if info["state"] != PluginState.UNLOADED.value and \ + info["state_history_count"] == 0: + inconsistent.append(info) + return + + def clearer(): + for _ in range(200): + for _ in range(20): + m.set_state("clock", PluginState.ENABLED) + m.clear_state("clock") + + t = threading.Thread(target=reader, daemon=True) + t.start() + clearer() + stop.set() + t.join(timeout=5) + + assert not inconsistent, f"observed a torn snapshot: {inconsistent[:1]}" + + +if __name__ == "__main__": + sys.exit(pytest.main([__file__, "-v"])) diff --git a/test/test_plugin_system.py b/test/test_plugin_system.py index f9b539d7..6d48cd86 100644 --- a/test/test_plugin_system.py +++ b/test/test_plugin_system.py @@ -23,17 +23,6 @@ class TestPluginManager: assert pm.cache_manager == mock_cache_manager assert pm.plugins == {} - def test_discover_plugins(self, test_plugin_manager): - """Test plugin discovery.""" - pm = test_plugin_manager - # Mock _scan_directory_for_plugins since we can't easily create real files in fixture - pm._scan_directory_for_plugins = MagicMock(return_value=["plugin1", "plugin2"]) - - # We need to call the real discover_plugins method, not the mock from the fixture - # But the fixture mocks the whole class instance. - # Let's create a real instance with mocked dependencies for this test - pass # Handled by separate test below - 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'), \ @@ -59,7 +48,7 @@ class TestPluginManager: result = pm.load_plugin("test_plugin") assert result is True - assert "test_plugin" in pm.plugin_modules + assert "test_plugin" in pm.plugins # PluginManager sets state to ENABLED after successful load assert pm.state_manager.get_state("test_plugin") == PluginState.ENABLED @@ -136,16 +125,6 @@ class TestPluginManager: assert pm.state_manager.get_state("test_plugin") == PluginState.ENABLED -class TestPluginLoader: - """Test PluginLoader functionality.""" - - def test_dependency_check(self): - """Test dependency checking logic.""" - # Covered by test_plugin_loader.py's install_dependencies tests, - # which exercise requirements_has_real_deps/requirements_are_satisfied - # and the pip subprocess fallback. - - class TestPluginExecutor: """Test PluginExecutor functionality.""" diff --git a/test/test_plugin_update_reservation.py b/test/test_plugin_update_reservation.py index 90ea62ae..e1397329 100644 --- a/test/test_plugin_update_reservation.py +++ b/test/test_plugin_update_reservation.py @@ -183,17 +183,6 @@ class TestNoConcurrentUpdate: f"update() ran {plugin.max_concurrent}x concurrently on the " "synchronous path") - def test_update_all_plugins_never_overlaps(self, pm): - plugin = OverlapDetectingPlugin(update_seconds=0.25) - _install(pm, plugin) - _widen_check_then_act_window(pm) - - _hammer(pm.update_all_plugins, threads=8) - - assert plugin.max_concurrent == 1, ( - f"update() ran {plugin.max_concurrent}x concurrently via " - "update_all_plugins()") - def test_async_path_never_overlaps(self, pm): plugin = OverlapDetectingPlugin(update_seconds=0.2) plugin_id = _install(pm, plugin) diff --git a/test/test_store_manager_caches.py b/test/test_store_manager_caches.py index c3cf190f..9cd48e8a 100644 --- a/test/test_store_manager_caches.py +++ b/test/test_store_manager_caches.py @@ -1,10 +1,8 @@ """ -Tests for the caching and tombstone behaviors added to PluginStoreManager -to fix the plugin-list slowness and the uninstall-resurrection bugs. +Tests for the caching behaviors added to PluginStoreManager to fix the +plugin-list slowness and the uninstall-resurrection bugs. Coverage targets: -- ``mark_recently_uninstalled`` / ``was_recently_uninstalled`` lifecycle and - TTL expiry. - ``_get_local_git_info`` mtime-gated cache: ``git`` subprocesses only run when ``.git/HEAD`` mtime changes. - ``fetch_registry`` stale-cache fallback on network failure. @@ -20,29 +18,6 @@ from unittest.mock import patch, MagicMock from src.plugin_system.store_manager import PluginStoreManager -class TestUninstallTombstone(unittest.TestCase): - def setUp(self): - self._tmp = TemporaryDirectory() - self.addCleanup(self._tmp.cleanup) - self.sm = PluginStoreManager(plugins_dir=self._tmp.name) - - def test_unmarked_plugin_is_not_recent(self): - self.assertFalse(self.sm.was_recently_uninstalled("foo")) - - def test_marking_makes_it_recent(self): - self.sm.mark_recently_uninstalled("foo") - self.assertTrue(self.sm.was_recently_uninstalled("foo")) - - def test_tombstone_expires_after_ttl(self): - self.sm._uninstall_tombstone_ttl = 0.05 - self.sm.mark_recently_uninstalled("foo") - self.assertTrue(self.sm.was_recently_uninstalled("foo")) - time.sleep(0.1) - self.assertFalse(self.sm.was_recently_uninstalled("foo")) - # Expired entry should also be pruned from the dict. - self.assertNotIn("foo", self.sm._uninstall_tombstones) - - class TestPersistentUninstallRegistry(unittest.TestCase): """Regression tests for the persistent uninstall registry that stops a core `git pull` update from resurrecting built-in plugins the user @@ -590,18 +565,13 @@ class TestStaleOnErrorFallbacks(unittest.TestCase): class TestInstallUpdateUninstallInvariants(unittest.TestCase): - """Regression guard: the caching and tombstone work added in this PR - must not break the install / update / uninstall code paths. + """Regression guard: the caching work added in this PR must not break + the install / update / uninstall code paths. Specifically: - ``install_plugin`` bypasses commit/manifest caches via force_refresh, so the 5→30 min TTL bump cannot cause users to install a stale commit. - ``update_plugin`` does the same. - - The uninstall tombstone is only honored by the state reconciler, not - by explicit ``install_plugin`` calls — so a user can uninstall and - immediately reinstall from the store UI without the tombstone getting - in the way. - - ``was_recently_uninstalled`` is not touched by ``install_plugin``. """ def setUp(self): @@ -649,10 +619,8 @@ class TestInstallUpdateUninstallInvariants(unittest.TestCase): self.assertTrue(manifest_calls, "manifest fetch was not called") self.assertTrue(manifest_calls[0][3], "force_refresh=True did not reach _fetch_manifest_from_github") - def test_install_plugin_is_not_blocked_by_tombstone(self): - """A tombstone must only gate the reconciler, not explicit installs. - - Uses a complete, valid manifest stub and a no-op dependency + def test_install_plugin_runs_to_completion(self): + """Uses a complete, valid manifest stub and a no-op dependency installer so ``install_plugin`` runs all the way through to a True return. Anything less (e.g. swallowing exceptions) would hide real regressions in the install path. @@ -664,11 +632,6 @@ class TestInstallUpdateUninstallInvariants(unittest.TestCase): } self.sm.registry_cache_time = time.time() - # Mark it recently uninstalled (simulates a user who just clicked - # uninstall and then immediately clicked install again). - self.sm.mark_recently_uninstalled("bar") - self.assertTrue(self.sm.was_recently_uninstalled("bar")) - # Stub the heavy bits so install_plugin can run without network. self.sm._get_github_repo_info = lambda url: { "default_branch": "main", "stars": 0, @@ -701,17 +664,14 @@ class TestInstallUpdateUninstallInvariants(unittest.TestCase): self.sm._install_via_git = fake_install_via_git - # No exception-swallowing: if install_plugin fails for ANY reason - # unrelated to the tombstone, the test fails loudly. + # No exception-swallowing: if install_plugin fails for ANY reason, + # the test fails loudly. result = self.sm.install_plugin("bar") self.assertTrue( result, - "install_plugin returned False — the tombstone should not gate " - "explicit installs and all other stubs should allow success.", + "install_plugin returned False — all stubs should allow success.", ) - # Tombstone survives install (harmless — nothing reads it for installed plugins). - self.assertTrue(self.sm.was_recently_uninstalled("bar")) class TestRegistryStaleCacheFallback(unittest.TestCase): diff --git a/test/test_update_change_reporting.py b/test/test_update_change_reporting.py index 80445d61..48931f3e 100644 --- a/test/test_update_change_reporting.py +++ b/test/test_update_change_reporting.py @@ -150,9 +150,11 @@ class EveryStampRecordsACompletion(unittest.TestCase): if assigns_time: stamps.append(node) + # The worker and synchronous paths share one stamp, in + # _execute_update_now's _finish(). self.assertGreaterEqual( - len(stamps), 2, - "expected the worker and inline success paths to stamp the time; " + len(stamps), 1, + "expected the update success path to stamp the time; " "if this drops, the search below is looking at the wrong thing") for stamp in stamps: diff --git a/test/test_utils.py b/test/test_utils.py deleted file mode 100644 index 127b743e..00000000 --- a/test/test_utils.py +++ /dev/null @@ -1,329 +0,0 @@ -""" -Tests for src/common/utils.py - -Covers all pure utility functions: normalize_team_abbreviation, format_time, -format_date, get_timezone, validate_dimensions, parse_team_abbreviation, -format_score, format_period, is_live_game, is_final_game, is_upcoming_game, -sanitize_filename, truncate_text, parse_boolean. -""" - -import pytest -from datetime import datetime, timezone -import pytz - -from src.common.utils import ( - normalize_team_abbreviation, - format_time, - format_date, - get_timezone, - validate_dimensions, - parse_team_abbreviation, - format_score, - format_period, - is_live_game, - is_final_game, - is_upcoming_game, - sanitize_filename, - truncate_text, - parse_boolean, -) - - -# --------------------------------------------------------------------------- -# normalize_team_abbreviation -# --------------------------------------------------------------------------- - -class TestNormalizeTeamAbbreviation: - def test_basic_uppercase(self): - assert normalize_team_abbreviation("lal") == "LAL" - - def test_strips_spaces(self): - assert normalize_team_abbreviation(" KC ") == "KC" - - def test_replaces_ampersand(self): - assert normalize_team_abbreviation("TA&M") == "TAANDM" - - def test_removes_internal_spaces(self): - assert normalize_team_abbreviation("A B") == "AB" - - def test_removes_hyphens(self): - assert normalize_team_abbreviation("A-B") == "AB" - - def test_empty_string_returns_empty(self): - assert normalize_team_abbreviation("") == "" - - def test_none_returns_empty(self): - assert normalize_team_abbreviation(None) == "" - - -# --------------------------------------------------------------------------- -# format_time / format_date -# --------------------------------------------------------------------------- - -class TestFormatTime: - def _utc_dt(self, hour=20, minute=30): - return datetime(2024, 1, 15, hour, minute, 0, tzinfo=timezone.utc) - - def test_formats_utc_to_utc(self): - dt = self._utc_dt(20, 30) - result = format_time(dt, timezone_str="UTC") - # 20:30 UTC → "8:30PM" (leading zero stripped) - assert "8:30PM" in result or "8:30 PM" in result or result != "" - - def test_naive_datetime_treated_as_utc(self): - dt = datetime(2024, 1, 15, 12, 0, 0) # naive - result = format_time(dt, timezone_str="UTC") - assert result != "" - - def test_invalid_timezone_returns_empty(self): - dt = self._utc_dt() - result = format_time(dt, timezone_str="Invalid/TZ") - assert result == "" - - def test_eastern_timezone(self): - dt = self._utc_dt(20, 0) # 8 PM UTC = 3 PM ET - result = format_time(dt, timezone_str="America/New_York") - assert result != "" - - -class TestFormatDate: - def test_formats_date(self): - dt = datetime(2024, 6, 15, 18, 0, 0, tzinfo=timezone.utc) - result = format_date(dt, timezone_str="UTC") - assert "June" in result or "15" in result - - def test_naive_datetime(self): - dt = datetime(2024, 3, 10, 12, 0, 0) - result = format_date(dt, timezone_str="UTC") - assert result != "" - - def test_invalid_timezone_returns_empty(self): - dt = datetime(2024, 6, 15, 18, 0, 0, tzinfo=timezone.utc) - result = format_date(dt, timezone_str="BadZone/Here") - assert result == "" - - -# --------------------------------------------------------------------------- -# get_timezone -# --------------------------------------------------------------------------- - -class TestGetTimezone: - def test_valid_timezone(self): - tz = get_timezone("America/New_York") - assert tz is not None - - def test_utc(self): - tz = get_timezone("UTC") - assert tz is pytz.utc or str(tz) == "UTC" - - def test_invalid_returns_utc(self): - tz = get_timezone("Not/ATimezone") - assert tz is pytz.utc - - -# --------------------------------------------------------------------------- -# validate_dimensions -# --------------------------------------------------------------------------- - -class TestValidateDimensions: - def test_valid(self): - assert validate_dimensions(64, 32) is True - - def test_zero_width(self): - assert validate_dimensions(0, 32) is False - - def test_zero_height(self): - assert validate_dimensions(64, 0) is False - - def test_negative(self): - assert validate_dimensions(-1, 32) is False - - def test_too_large(self): - assert validate_dimensions(1001, 32) is False - - def test_max_valid(self): - assert validate_dimensions(1000, 1000) is True - - def test_non_integer(self): - assert validate_dimensions("64", 32) is False # type: ignore[arg-type] - - -# --------------------------------------------------------------------------- -# parse_team_abbreviation -# --------------------------------------------------------------------------- - -class TestParseTeamAbbreviation: - def test_empty_string(self): - assert parse_team_abbreviation("") == "" - - def test_none_returns_empty(self): - assert parse_team_abbreviation(None) == "" - - def test_extracts_uppercase(self): - result = parse_team_abbreviation("LAL") - assert result == "LAL" - - def test_fallback_first_three(self): - # text without recognisable 2-4 char uppercase block - result = parse_team_abbreviation("ab") - assert len(result) <= 3 - - -# --------------------------------------------------------------------------- -# format_score -# --------------------------------------------------------------------------- - -class TestFormatScore: - def test_format_score(self): - assert format_score(14, 7) == "7-14" - - def test_format_score_strings(self): - assert format_score("21", "14") == "14-21" - - def test_zero_zero(self): - assert format_score(0, 0) == "0-0" - - -# --------------------------------------------------------------------------- -# format_period -# --------------------------------------------------------------------------- - -class TestFormatPeriod: - def test_basketball_q1(self): - assert format_period(1, "basketball") == "Q1" - - def test_basketball_q4(self): - assert format_period(4, "basketball") == "Q4" - - def test_basketball_ot1(self): - assert format_period(5, "basketball") == "OT1" - - def test_basketball_ot2(self): - assert format_period(6, "basketball") == "OT2" - - def test_football_q1(self): - assert format_period(1, "football") == "Q1" - - def test_football_ot(self): - assert format_period(5, "football") == "OT1" - - def test_hockey_p1(self): - assert format_period(1, "hockey") == "P1" - - def test_hockey_p3(self): - assert format_period(3, "hockey") == "P3" - - def test_hockey_ot(self): - assert format_period(4, "hockey") == "OT1" - - def test_baseball_inning(self): - assert format_period(7, "baseball") == "INN 7" - - def test_unknown_sport(self): - result = format_period(2, "unknown") - assert "2" in result - - -# --------------------------------------------------------------------------- -# is_live_game / is_final_game / is_upcoming_game -# --------------------------------------------------------------------------- - -class TestGameStatusHelpers: - def test_is_live_game_true(self): - assert is_live_game("In Progress") is True - assert is_live_game("halftime") is True - assert is_live_game("overtime") is True - - def test_is_live_game_false(self): - assert is_live_game("Final") is False - assert is_live_game("Scheduled") is False - - def test_is_final_game_true(self): - assert is_final_game("Final") is True - assert is_final_game("COMPLETED") is True - - def test_is_final_game_false(self): - assert is_final_game("In Progress") is False - - def test_is_upcoming_game_true(self): - assert is_upcoming_game("Scheduled") is True - assert is_upcoming_game("upcoming") is True - - def test_is_upcoming_game_false(self): - assert is_upcoming_game("Final") is False - assert is_upcoming_game("In Progress") is False - - -# --------------------------------------------------------------------------- -# sanitize_filename -# --------------------------------------------------------------------------- - -class TestSanitizeFilename: - def test_removes_invalid_chars(self): - result = sanitize_filename('file<>:"/\\|?*.txt') - assert "<" not in result - assert ">" not in result - assert ":" not in result - - def test_collapses_underscores(self): - result = sanitize_filename("file___name") - assert "__" not in result - - def test_strips_leading_trailing(self): - result = sanitize_filename("_file_") - assert not result.startswith("_") - assert not result.endswith("_") - - def test_normal_filename_unchanged(self): - result = sanitize_filename("my_logo") - assert result == "my_logo" - - -# --------------------------------------------------------------------------- -# truncate_text -# --------------------------------------------------------------------------- - -class TestTruncateText: - def test_no_truncation_needed(self): - assert truncate_text("hello", 10) == "hello" - - def test_truncation_adds_suffix(self): - result = truncate_text("hello world", 8) - assert result.endswith("...") - assert len(result) == 8 - - def test_exact_length(self): - assert truncate_text("hello", 5) == "hello" - - def test_custom_suffix(self): - result = truncate_text("hello world", 8, suffix="~") - assert result.endswith("~") - - -# --------------------------------------------------------------------------- -# parse_boolean -# --------------------------------------------------------------------------- - -class TestParseBoolean: - def test_true_bool(self): - assert parse_boolean(True) is True - - def test_false_bool(self): - assert parse_boolean(False) is False - - def test_int_1(self): - assert parse_boolean(1) is True - - def test_int_0(self): - assert parse_boolean(0) is False - - def test_string_true(self): - for val in ("true", "True", "TRUE", "1", "yes", "on", "enabled"): - assert parse_boolean(val) is True, f"Expected True for {val!r}" - - def test_string_false(self): - for val in ("false", "False", "0", "no", "off", "disabled"): - assert parse_boolean(val) is False, f"Expected False for {val!r}" - - def test_none_returns_false(self): - assert parse_boolean(None) is False # type: ignore[arg-type] diff --git a/test/test_vegas_config.py b/test/test_vegas_config.py index ebacafe7..a639896e 100644 --- a/test/test_vegas_config.py +++ b/test/test_vegas_config.py @@ -2,7 +2,7 @@ Tests for src/vegas_mode/config.py Covers VegasModeConfig: from_config, to_dict, get_frame_interval, -is_plugin_included, get_ordered_plugins, validate, update. +is_plugin_included, get_ordered_plugins, validate. """ import pytest @@ -263,48 +263,3 @@ class TestValidate: cfg = VegasModeConfig(scroll_speed=0.1, target_fps=5) errors = cfg.validate() assert len(errors) >= 2 - - -# --------------------------------------------------------------------------- -# update -# --------------------------------------------------------------------------- - -class TestUpdate: - def _wrap(self, **kwargs) -> dict: - return {"display": {"vegas_scroll": kwargs}} - - def test_update_enabled(self): - cfg = VegasModeConfig(enabled=False) - cfg.update(self._wrap(enabled=True)) - assert cfg.enabled is True - - def test_update_scroll_speed(self): - cfg = VegasModeConfig(scroll_speed=50.0) - cfg.update(self._wrap(scroll_speed=90.0)) - assert cfg.scroll_speed == 90.0 - - def test_update_separator_width(self): - cfg = VegasModeConfig(separator_width=32) - cfg.update(self._wrap(separator_width=8)) - assert cfg.separator_width == 8 - - def test_update_plugin_order(self): - cfg = VegasModeConfig(plugin_order=[]) - cfg.update(self._wrap(plugin_order=["x", "y"])) - assert cfg.plugin_order == ["x", "y"] - - def test_update_excluded_plugins(self): - cfg = VegasModeConfig() - cfg.update(self._wrap(excluded_plugins=["skip_me"])) - assert "skip_me" in cfg.excluded_plugins - - def test_update_ignores_missing_keys(self): - cfg = VegasModeConfig(scroll_speed=50.0) - cfg.update(self._wrap(target_fps=80)) # only fps, not speed - assert cfg.scroll_speed == 50.0 - assert cfg.target_fps == 80 - - def test_empty_update_no_change(self): - cfg = VegasModeConfig(scroll_speed=50.0) - cfg.update({}) - assert cfg.scroll_speed == 50.0 diff --git a/test/test_vegas_density.py b/test/test_vegas_density.py index 2338421f..4c410314 100644 --- a/test/test_vegas_density.py +++ b/test/test_vegas_density.py @@ -626,11 +626,6 @@ class TestConfigSurface: assert restored.trim_threshold == 20 assert restored.lead_in_width == 64 - def test_update_applies_new_keys(self): - cfg = VegasModeConfig() - cfg.update({'display': {'vegas_scroll': {'content_padding': 16}}}) - assert cfg.content_padding == 16 - @pytest.mark.parametrize('overrides,bad_key', [ ({'trim_threshold': 300}, 'trim_threshold'), ({'trim_threshold': -1}, 'trim_threshold'), diff --git a/test/test_version_consistency.py b/test/test_version_consistency.py index 03e638a3..096c0646 100644 --- a/test/test_version_consistency.py +++ b/test/test_version_consistency.py @@ -21,8 +21,7 @@ before tagging: python scripts/check_release_version.py v3.2.0 Note: `src.plugin_system.__version__` is deliberately NOT checked. That module -versions the *plugin API* (it sits beside `__api_version__` and is documented as -such), which moves independently of the core version. +versions the *plugin API*, which moves independently of the core version. """ import re diff --git a/test/test_wifi_manager_ap.py b/test/test_wifi_manager_ap.py index 88e4b309..82e75625 100644 --- a/test/test_wifi_manager_ap.py +++ b/test/test_wifi_manager_ap.py @@ -10,6 +10,7 @@ Scenarios covered: 3. iptables rules and ip_forward are reverted when the AP is torn down. 4. LED matrix message includes the SSID, 'No password', and the setup URL. 5. Known AP profile names are deleted before the new profile is created. +6. Wi-Fi passwords are not written to wifi_config.json, and old ones are scrubbed. """ from __future__ import annotations @@ -62,7 +63,6 @@ def wifi_config(tmp_path: Path) -> Path: "ap_ssid": "LEDMatrix-Setup", "ap_channel": 7, "auto_enable_ap_mode": True, - "saved_networks": [], } p = cfg_dir / "wifi_config.json" p.write_text(json.dumps(cfg)) @@ -75,8 +75,7 @@ def manager(wifi_config: Path, tmp_path: Path) -> WiFiManager: WiFiManager with all system calls stubbed out during construction and the ip_forward save file redirected to a per-test temporary path. """ - with patch("src.wifi_manager.subprocess.run", return_value=_ok(stdout="wlan0\n")), \ - patch.object(WiFiManager, "_detect_trixie", return_value=False): + with patch("src.wifi_manager.subprocess.run", return_value=_ok(stdout="wlan0\n")): mgr = WiFiManager(config_path=wifi_config) # Force clean, deterministic state regardless of what __init__ inferred @@ -85,7 +84,6 @@ def manager(wifi_config: Path, tmp_path: Path) -> WiFiManager: mgr.has_hostapd = False mgr.has_dnsmasq = False mgr.has_iwlist = False - mgr._is_trixie = False # Redirect the ip_forward save file to tmp so tests never share state mgr._IP_FORWARD_SAVE_PATH = tmp_path / "ip_fwd_saved" return mgr @@ -332,3 +330,62 @@ def test_existing_ap_profiles_deleted_before_new_profile_created(manager: WiFiMa assert del_indices, "Expected 'nmcli connection delete' calls" assert max(del_indices) < min(add_indices), \ "All connection deletions must complete before the new profile is created" + + +# --------------------------------------------------------------------------- +# 6. Wi-Fi passwords are not kept in wifi_config.json +# --------------------------------------------------------------------------- + +@pytest.mark.unit +def test_loading_scrubs_plaintext_saved_networks(wifi_config: Path) -> None: + """Older versions wrote every joined network's password to the config in + plaintext and never read it back. Loading must remove it from disk.""" + cfg = json.loads(wifi_config.read_text()) + cfg["saved_networks"] = [ + {"ssid": "HomeNet", "password": "hunter22", "saved_at": 0}, + ] + wifi_config.write_text(json.dumps(cfg)) + + with patch("src.wifi_manager.subprocess.run", return_value=_ok(stdout="wlan0\n")): + mgr = WiFiManager(config_path=wifi_config) + + assert "saved_networks" not in mgr.config + assert "hunter22" not in wifi_config.read_text() + on_disk = json.loads(wifi_config.read_text()) + assert "saved_networks" not in on_disk + # Everything else survives the scrub. + assert on_disk["ap_ssid"] == "LEDMatrix-Setup" + assert on_disk["auto_enable_ap_mode"] is True + + +@pytest.mark.unit +def test_default_config_has_no_saved_networks(tmp_path: Path) -> None: + config_path = tmp_path / "config" / "wifi_config.json" + config_path.parent.mkdir() + + with patch("src.wifi_manager.subprocess.run", return_value=_ok(stdout="wlan0\n")): + WiFiManager(config_path=config_path) + + assert "saved_networks" not in json.loads(config_path.read_text()) + + +@pytest.mark.unit +def test_connecting_does_not_store_the_password(manager: WiFiManager) -> None: + commands = [] + + def fake_run(cmd, *args, **kwargs): + commands.append(cmd) + # No existing profile for the SSID, so a new connection is created. + if cmd[:3] == ["nmcli", "connection", "show"] and "HomeNet" in cmd: + return _fail() + return _ok(stdout="") + + with patch("src.wifi_manager.subprocess.run", side_effect=fake_run), \ + patch("src.wifi_manager.time.sleep"), \ + patch.object(manager, "_show_led_message"): + manager._connect_nmcli("HomeNet", "hunter22") + + assert ["nmcli", "device", "wifi", "connect", "HomeNet", "password", "hunter22"] in commands, \ + "the new-connection path was not reached" + assert "hunter22" not in json.dumps(manager.config) + assert "hunter22" not in manager.config_path.read_text() diff --git a/test/web_interface/test_state_reconciliation.py b/test/web_interface/test_state_reconciliation.py index 7d01b886..b8e92613 100644 --- a/test/web_interface/test_state_reconciliation.py +++ b/test/web_interface/test_state_reconciliation.py @@ -366,7 +366,6 @@ class TestStateReconciliationUnrecoverable(unittest.TestCase): self.store_manager = Mock() self.store_manager.fetch_registry.return_value = {"plugins": []} self.store_manager.install_plugin.return_value = False - self.store_manager.was_recently_uninstalled.return_value = False # A bare Mock() returns a truthy Mock for is_plugin_uninstalled(), # which reads as "persistently uninstalled" and skips auto-repair # entirely — these tests need the repair path to run. @@ -441,9 +440,9 @@ class TestStateReconciliationUnrecoverable(unittest.TestCase): self.assertNotIn("ghost", self.reconciler._unrecoverable_missing_on_disk) self.store_manager.install_plugin.assert_not_called() - def test_recently_uninstalled_skips_auto_repair(self): - """A freshly-uninstalled plugin must not be resurrected by the reconciler.""" - self.store_manager.was_recently_uninstalled.return_value = True + def test_persistently_uninstalled_skips_auto_repair(self): + """A plugin the user uninstalled must not be resurrected by the reconciler.""" + self.store_manager.is_plugin_uninstalled.return_value = True self.store_manager.fetch_registry.return_value = { "plugins": [{"id": "ghost"}] } diff --git a/web_interface/app.py b/web_interface/app.py index fe13a643..72092bae 100644 --- a/web_interface/app.py +++ b/web_interface/app.py @@ -30,7 +30,6 @@ from src.plugin_system.schema_manager import SchemaManager from src.plugin_system.operation_queue import PluginOperationQueue from src.plugin_system.state_manager import PluginStateManager from src.plugin_system.operation_history import OperationHistory -from src.plugin_system.health_monitor import PluginHealthMonitor _JOURNALCTL = shutil.which('journalctl') _SYSTEMCTL = shutil.which('systemctl') @@ -154,11 +153,6 @@ operation_history = OperationHistory( lazy_load=True ) -# Initialize health monitoring (if health tracker is available) -# Deferred until first request to improve startup time -health_monitor = None -_health_monitor_initialized = False - # Plugin discovery is deferred until first API request that needs it # This improves startup time - endpoints will call discover_plugins() when needed @@ -181,7 +175,6 @@ api_v3.schema_manager = schema_manager api_v3.operation_queue = operation_queue api_v3.plugin_state_manager = plugin_state_manager api_v3.operation_history = operation_history -api_v3.health_monitor = health_monitor # Initialize cache manager for API endpoints from src.cache_manager import CacheManager api_v3.cache_manager = CacheManager() @@ -926,28 +919,6 @@ def favicon(): """Return 204 No Content for favicon to avoid 404 errors""" return '', 204 -def _initialize_health_monitor(): - """Initialize health monitoring after server is ready to accept requests.""" - global health_monitor, _health_monitor_initialized - if _health_monitor_initialized: - return - - if health_monitor is None and hasattr(plugin_manager, 'health_tracker') and plugin_manager.health_tracker: - try: - health_monitor = PluginHealthMonitor( - health_tracker=plugin_manager.health_tracker, - check_interval=60.0, # Check every minute - degraded_threshold=0.5, - unhealthy_threshold=0.8, - max_response_time=5.0 - ) - health_monitor.start_monitoring() - print("✓ Plugin health monitoring started") - except Exception as e: - print(f"⚠ Could not start health monitoring: {e}") - - _health_monitor_initialized = True - _reconciliation_done = False _reconciliation_started = False import threading as _threading @@ -1035,13 +1006,11 @@ def _run_startup_reconciliation() -> None: # retrigger reconciliation on every subsequent request. _reconciliation_done = True -# Initialize health monitor and run reconciliation on first request +# Run reconciliation in the background on first request @app.before_request -def check_health_monitor(): - """Ensure health monitor is initialized; launch reconciliation in background.""" +def start_startup_reconciliation(): + """Launch startup reconciliation in the background once.""" global _reconciliation_started - if not _health_monitor_initialized: - _initialize_health_monitor() with _reconciliation_lock: if not _reconciliation_started: _reconciliation_started = True