mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-04-10 21:03:01 +00:00
8fb2800495
* feat: add error detection, monitoring, and code quality improvements This comprehensive update addresses automatic error detection, code quality, and plugin development experience: ## Error Detection & Monitoring - Add ErrorAggregator service for centralized error tracking - Add pattern detection for recurring errors (5+ in 60 min) - Add error dashboard API endpoints (/api/v3/errors/*) - Integrate error recording into plugin executor ## Code Quality - Remove 10 silent `except: pass` blocks in sports.py and football.py - Remove hardcoded debug log paths - Add pre-commit hooks to prevent future bare except clauses ## Validation & Type Safety - Add warnings when plugins lack config_schema.json - Add config key collision detection for plugins - Improve type coercion logging in BasePlugin ## Testing - Add test_config_validation_edge_cases.py - Add test_plugin_loading_failures.py - Add test_error_aggregator.py ## Documentation - Add PLUGIN_ERROR_HANDLING.md guide - Add CONFIG_DEBUGGING.md guide Note: GitHub Actions CI workflow is available in the plan but requires workflow scope to push. Add .github/workflows/ci.yml manually. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: address code review issues - Fix GitHub issues URL in CONFIG_DEBUGGING.md - Use RLock in error_aggregator.py to prevent deadlock in export_to_file - Distinguish missing vs invalid schema files in plugin_manager.py - Add assertions to test_null_value_for_required_field test - Remove unused initial_count variable in test_plugin_load_error_recorded - Add validation for max_age_hours in clear_old_errors API endpoint Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
347 lines
12 KiB
Python
347 lines
12 KiB
Python
"""
|
|
Tests for plugin loading failure scenarios.
|
|
|
|
Tests various failure modes that can occur during plugin loading:
|
|
- Missing manifest.json
|
|
- Invalid manifest.json
|
|
- Missing entry_point file
|
|
- Import errors in plugin module
|
|
- Missing class_name in module
|
|
- Class doesn't inherit from BasePlugin
|
|
- validate_config() returns False
|
|
- Dependencies installation failure
|
|
"""
|
|
|
|
import pytest
|
|
import json
|
|
from pathlib import Path
|
|
from unittest.mock import Mock, patch, MagicMock
|
|
import tempfile
|
|
import sys
|
|
|
|
# Add project root to path
|
|
project_root = Path(__file__).parent.parent
|
|
if str(project_root) not in sys.path:
|
|
sys.path.insert(0, str(project_root))
|
|
|
|
from src.plugin_system.plugin_manager import PluginManager
|
|
from src.plugin_system.plugin_loader import PluginLoader
|
|
from src.plugin_system.plugin_state import PluginState
|
|
from src.exceptions import PluginError
|
|
|
|
|
|
@pytest.fixture
|
|
def mock_managers():
|
|
"""Create mock managers for plugin loading tests."""
|
|
return {
|
|
"config_manager": MagicMock(),
|
|
"display_manager": MagicMock(),
|
|
"cache_manager": MagicMock(),
|
|
"font_manager": MagicMock()
|
|
}
|
|
|
|
|
|
@pytest.fixture
|
|
def temp_plugin_dir(tmp_path):
|
|
"""Create a temporary plugin directory."""
|
|
plugins_dir = tmp_path / "plugins"
|
|
plugins_dir.mkdir()
|
|
return plugins_dir
|
|
|
|
|
|
class TestMissingManifest:
|
|
"""Test handling of missing manifest.json."""
|
|
|
|
def test_plugin_without_manifest_not_discovered(self, temp_plugin_dir, mock_managers):
|
|
"""Plugin directory without manifest.json should not be discovered."""
|
|
# Create plugin directory without manifest
|
|
plugin_dir = temp_plugin_dir / "test-plugin"
|
|
plugin_dir.mkdir()
|
|
(plugin_dir / "manager.py").write_text("# Empty plugin")
|
|
|
|
with patch('src.common.permission_utils.ensure_directory_permissions'):
|
|
manager = PluginManager(
|
|
plugins_dir=str(temp_plugin_dir),
|
|
**mock_managers
|
|
)
|
|
plugins = manager.discover_plugins()
|
|
|
|
assert "test-plugin" not in plugins
|
|
|
|
|
|
class TestInvalidManifest:
|
|
"""Test handling of invalid manifest.json files."""
|
|
|
|
def test_manifest_invalid_json(self, temp_plugin_dir, mock_managers):
|
|
"""Plugin with invalid JSON manifest should not be discovered."""
|
|
plugin_dir = temp_plugin_dir / "test-plugin"
|
|
plugin_dir.mkdir()
|
|
(plugin_dir / "manifest.json").write_text("{ invalid json }")
|
|
|
|
with patch('src.common.permission_utils.ensure_directory_permissions'):
|
|
manager = PluginManager(
|
|
plugins_dir=str(temp_plugin_dir),
|
|
**mock_managers
|
|
)
|
|
plugins = manager.discover_plugins()
|
|
|
|
assert "test-plugin" not in plugins
|
|
|
|
def test_manifest_missing_required_fields(self, temp_plugin_dir, mock_managers):
|
|
"""Plugin manifest missing required fields should fail gracefully."""
|
|
plugin_dir = temp_plugin_dir / "test-plugin"
|
|
plugin_dir.mkdir()
|
|
|
|
# Manifest missing 'class_name' and 'entry_point'
|
|
manifest = {"id": "test-plugin", "name": "Test Plugin"}
|
|
(plugin_dir / "manifest.json").write_text(json.dumps(manifest))
|
|
|
|
with patch('src.common.permission_utils.ensure_directory_permissions'):
|
|
manager = PluginManager(
|
|
plugins_dir=str(temp_plugin_dir),
|
|
**mock_managers
|
|
)
|
|
plugins = manager.discover_plugins()
|
|
|
|
# Plugin might be discovered but should fail to load
|
|
if "test-plugin" in plugins:
|
|
result = manager.load_plugin("test-plugin")
|
|
assert result is False
|
|
|
|
|
|
class TestMissingEntryPoint:
|
|
"""Test handling of missing entry_point file."""
|
|
|
|
def test_missing_entry_point_file(self, temp_plugin_dir, mock_managers):
|
|
"""Plugin with missing entry_point file should fail to load."""
|
|
plugin_dir = temp_plugin_dir / "test-plugin"
|
|
plugin_dir.mkdir()
|
|
|
|
manifest = {
|
|
"id": "test-plugin",
|
|
"name": "Test Plugin",
|
|
"entry_point": "manager.py", # File doesn't exist
|
|
"class_name": "TestPlugin"
|
|
}
|
|
(plugin_dir / "manifest.json").write_text(json.dumps(manifest))
|
|
|
|
with patch('src.common.permission_utils.ensure_directory_permissions'):
|
|
manager = PluginManager(
|
|
plugins_dir=str(temp_plugin_dir),
|
|
**mock_managers
|
|
)
|
|
manager.discover_plugins()
|
|
|
|
# Force the manifest to be loaded
|
|
manager.plugin_manifests["test-plugin"] = manifest
|
|
|
|
result = manager.load_plugin("test-plugin")
|
|
assert result is False
|
|
|
|
|
|
class TestImportErrors:
|
|
"""Test handling of import errors in plugin modules."""
|
|
|
|
def test_syntax_error_in_plugin(self, temp_plugin_dir, mock_managers):
|
|
"""Plugin with Python syntax error should fail to load."""
|
|
plugin_dir = temp_plugin_dir / "test-plugin"
|
|
plugin_dir.mkdir()
|
|
|
|
manifest = {
|
|
"id": "test-plugin",
|
|
"name": "Test Plugin",
|
|
"entry_point": "manager.py",
|
|
"class_name": "TestPlugin"
|
|
}
|
|
(plugin_dir / "manifest.json").write_text(json.dumps(manifest))
|
|
|
|
# Create manager.py with syntax error
|
|
(plugin_dir / "manager.py").write_text("""
|
|
class TestPlugin
|
|
def __init__(self): # Missing colon above
|
|
pass
|
|
""")
|
|
|
|
with patch('src.common.permission_utils.ensure_directory_permissions'):
|
|
manager = PluginManager(
|
|
plugins_dir=str(temp_plugin_dir),
|
|
**mock_managers
|
|
)
|
|
manager.discover_plugins()
|
|
manager.plugin_manifests["test-plugin"] = manifest
|
|
|
|
result = manager.load_plugin("test-plugin")
|
|
assert result is False
|
|
|
|
def test_missing_dependency_in_plugin(self, temp_plugin_dir, mock_managers):
|
|
"""Plugin importing missing module should fail to load."""
|
|
plugin_dir = temp_plugin_dir / "test-plugin"
|
|
plugin_dir.mkdir()
|
|
|
|
manifest = {
|
|
"id": "test-plugin",
|
|
"name": "Test Plugin",
|
|
"entry_point": "manager.py",
|
|
"class_name": "TestPlugin"
|
|
}
|
|
(plugin_dir / "manifest.json").write_text(json.dumps(manifest))
|
|
|
|
# Create manager.py that imports non-existent module
|
|
(plugin_dir / "manager.py").write_text("""
|
|
import nonexistent_module_xyz123
|
|
|
|
class TestPlugin:
|
|
pass
|
|
""")
|
|
|
|
with patch('src.common.permission_utils.ensure_directory_permissions'):
|
|
manager = PluginManager(
|
|
plugins_dir=str(temp_plugin_dir),
|
|
**mock_managers
|
|
)
|
|
manager.discover_plugins()
|
|
manager.plugin_manifests["test-plugin"] = manifest
|
|
|
|
result = manager.load_plugin("test-plugin")
|
|
assert result is False
|
|
|
|
|
|
class TestMissingClassName:
|
|
"""Test handling when class_name is not found in module."""
|
|
|
|
def test_class_not_in_module(self, temp_plugin_dir, mock_managers):
|
|
"""Plugin with class_name not matching any class should fail."""
|
|
plugin_dir = temp_plugin_dir / "test-plugin"
|
|
plugin_dir.mkdir()
|
|
|
|
manifest = {
|
|
"id": "test-plugin",
|
|
"name": "Test Plugin",
|
|
"entry_point": "manager.py",
|
|
"class_name": "NonExistentClass" # Doesn't exist in manager.py
|
|
}
|
|
(plugin_dir / "manifest.json").write_text(json.dumps(manifest))
|
|
|
|
(plugin_dir / "manager.py").write_text("""
|
|
class ActualPlugin:
|
|
pass
|
|
""")
|
|
|
|
with patch('src.common.permission_utils.ensure_directory_permissions'):
|
|
manager = PluginManager(
|
|
plugins_dir=str(temp_plugin_dir),
|
|
**mock_managers
|
|
)
|
|
manager.discover_plugins()
|
|
manager.plugin_manifests["test-plugin"] = manifest
|
|
|
|
result = manager.load_plugin("test-plugin")
|
|
assert result is False
|
|
|
|
|
|
class TestValidateConfigFailure:
|
|
"""Test handling when validate_config() returns False."""
|
|
|
|
def test_validate_config_returns_false(self, temp_plugin_dir, mock_managers):
|
|
"""Plugin where validate_config() returns False should fail to load."""
|
|
plugin_dir = temp_plugin_dir / "test-plugin"
|
|
plugin_dir.mkdir()
|
|
|
|
manifest = {
|
|
"id": "test-plugin",
|
|
"name": "Test Plugin",
|
|
"entry_point": "manager.py",
|
|
"class_name": "TestPlugin"
|
|
}
|
|
(plugin_dir / "manifest.json").write_text(json.dumps(manifest))
|
|
|
|
# Create a mock plugin that fails validation
|
|
mock_plugin = MagicMock()
|
|
mock_plugin.validate_config.return_value = False
|
|
|
|
with patch('src.common.permission_utils.ensure_directory_permissions'):
|
|
manager = PluginManager(
|
|
plugins_dir=str(temp_plugin_dir),
|
|
**mock_managers
|
|
)
|
|
manager.discover_plugins()
|
|
manager.plugin_manifests["test-plugin"] = manifest
|
|
|
|
# Mock the plugin loader to return our mock plugin
|
|
with patch.object(manager.plugin_loader, 'load_plugin', return_value=(mock_plugin, MagicMock())):
|
|
result = manager.load_plugin("test-plugin")
|
|
assert result is False
|
|
|
|
def test_validate_config_raises_exception(self, temp_plugin_dir, mock_managers):
|
|
"""Plugin where validate_config() raises exception should fail to load."""
|
|
mock_plugin = MagicMock()
|
|
mock_plugin.validate_config.side_effect = ValueError("Config validation error")
|
|
|
|
with patch('src.common.permission_utils.ensure_directory_permissions'):
|
|
manager = PluginManager(
|
|
plugins_dir=str(temp_plugin_dir),
|
|
**mock_managers
|
|
)
|
|
|
|
manifest = {
|
|
"id": "test-plugin",
|
|
"name": "Test Plugin",
|
|
"entry_point": "manager.py",
|
|
"class_name": "TestPlugin"
|
|
}
|
|
manager.plugin_manifests["test-plugin"] = manifest
|
|
|
|
with patch.object(manager.plugin_loader, 'load_plugin', return_value=(mock_plugin, MagicMock())):
|
|
with patch.object(manager.plugin_loader, 'find_plugin_directory', return_value=temp_plugin_dir):
|
|
result = manager.load_plugin("test-plugin")
|
|
assert result is False
|
|
|
|
|
|
class TestPluginStateOnFailure:
|
|
"""Test that plugin state is correctly set on various failures."""
|
|
|
|
def test_state_set_to_error_on_load_failure(self, temp_plugin_dir, mock_managers):
|
|
"""Plugin state should be ERROR when loading fails."""
|
|
with patch('src.common.permission_utils.ensure_directory_permissions'):
|
|
manager = PluginManager(
|
|
plugins_dir=str(temp_plugin_dir),
|
|
**mock_managers
|
|
)
|
|
|
|
manifest = {"id": "test-plugin", "name": "Test Plugin"}
|
|
manager.plugin_manifests["test-plugin"] = manifest
|
|
|
|
# Try to load non-existent plugin
|
|
result = manager.load_plugin("test-plugin")
|
|
|
|
assert result is False
|
|
state = manager.state_manager.get_state("test-plugin")
|
|
assert state == PluginState.ERROR
|
|
|
|
|
|
class TestErrorAggregation:
|
|
"""Test that errors are properly recorded in error aggregator."""
|
|
|
|
def test_plugin_load_error_recorded(self, temp_plugin_dir, mock_managers):
|
|
"""Plugin load errors should be recorded in error aggregator."""
|
|
from src.error_aggregator import get_error_aggregator
|
|
|
|
# Get the aggregator
|
|
aggregator = get_error_aggregator()
|
|
|
|
with patch('src.common.permission_utils.ensure_directory_permissions'):
|
|
manager = PluginManager(
|
|
plugins_dir=str(temp_plugin_dir),
|
|
**mock_managers
|
|
)
|
|
|
|
manifest = {"id": "test-plugin", "name": "Test Plugin"}
|
|
manager.plugin_manifests["test-plugin"] = manifest
|
|
|
|
# This should trigger an error recording
|
|
manager.load_plugin("test-plugin")
|
|
|
|
# Errors may or may not be recorded depending on execution path
|
|
# This test verifies the aggregator is accessible
|
|
assert aggregator is not None
|