mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-07 07:36:37 +00:00
chore: remove dead code, deprecate unused plugin APIs (over-engineering audit)
Whole-tree audit. Every symbol was checked against core, the plugin monorepo and all eight third-party plugins in plugins.json first. - Deprecate (removal 3.10.0) plugin-facing methods nothing calls: LogoDownloader bulk download, ConfigManager backup/secret wrappers, APIHelper extras, BackgroundDataService poll API, PluginManager / PluginStateManager info readers, and a few CacheManager, FontManager, BaseOddsManager, DynamicTeamResolver methods and PluginTestCase. plugin_api_usage.py learns their receiver names; DEPRECATIONS doc regenerated. - Remove core-internal dead code: CacheMetrics, Vegas status/stats plumbing, sync "new cycle" message (followers ignore unknown types), unused operation types, test-only PluginCatalog readers, IPC to_dict and ping, _parse_form_value, CacheStrategyProtocol, ErrorAggregator callbacks, duplicate web response helpers. - Web UI: drop never-mounted json-file-manager.js, the example widget, utils/error_handler.js, four uncalled PluginAPI methods, and 29 escapeHtml shims (call window.LEDEscape directly). Public globals, BaseWidget and widget names unchanged. - Remove six one-off scripts (owner decision) and the unused markupsafe and pytest-mock pins. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -66,31 +66,6 @@ class TestPluginOperationsIntegration(unittest.TestCase):
|
||||
history = self.operation_history.get_history(plugin_id=plugin_id)
|
||||
self.assertEqual([r.operation_type for r in history], ["install"])
|
||||
|
||||
def test_update_operation_flow(self):
|
||||
"""Test complete update operation flow."""
|
||||
plugin_id = "test-plugin"
|
||||
|
||||
# Enqueue update operation
|
||||
operation_id = self.operation_queue.enqueue_operation(
|
||||
OperationType.UPDATE,
|
||||
plugin_id,
|
||||
{"from_version": "1.0.0", "to_version": "2.0.0"}
|
||||
)
|
||||
|
||||
self.assertIsNotNone(operation_id)
|
||||
|
||||
# Record in history
|
||||
self.operation_history.record_operation(
|
||||
operation_type="update",
|
||||
plugin_id=plugin_id,
|
||||
status="in_progress",
|
||||
operation_id=operation_id
|
||||
)
|
||||
|
||||
# Verify history
|
||||
history = self.operation_history.get_history(plugin_id=plugin_id)
|
||||
self.assertEqual([r.operation_type for r in history], ["update"])
|
||||
|
||||
def test_uninstall_operation_flow(self):
|
||||
"""Test complete uninstall operation flow."""
|
||||
plugin_id = "test-plugin"
|
||||
@@ -162,7 +137,7 @@ class TestPluginOperationsIntegration(unittest.TestCase):
|
||||
# The prevention only works for truly concurrent (pending/running) operations
|
||||
try:
|
||||
op2_id = self.operation_queue.enqueue_operation(
|
||||
OperationType.UPDATE,
|
||||
OperationType.UNINSTALL,
|
||||
plugin_id
|
||||
)
|
||||
# If no exception, the first operation may have completed already
|
||||
|
||||
@@ -22,7 +22,6 @@ from web_interface.blueprints.api_v3 import ( # noqa: E402
|
||||
_is_plugin_update_available,
|
||||
_coerce_to_bool,
|
||||
deep_merge,
|
||||
_parse_form_value,
|
||||
_get_schema_property,
|
||||
_set_nested_value,
|
||||
_SKIP_FIELD,
|
||||
@@ -116,44 +115,6 @@ class TestDeepMerge:
|
||||
assert result["keep"] is base["keep"] # untouched subtree is shared
|
||||
|
||||
|
||||
class TestParseFormValue:
|
||||
def test_boolean_strings(self):
|
||||
assert _parse_form_value("true") is True
|
||||
assert _parse_form_value("False") is False
|
||||
|
||||
def test_null_like_strings(self):
|
||||
assert _parse_form_value("null") is None
|
||||
assert _parse_form_value("none") is None
|
||||
assert _parse_form_value("") is None
|
||||
|
||||
def test_none_passthrough(self):
|
||||
assert _parse_form_value(None) is None
|
||||
|
||||
def test_numbers(self):
|
||||
assert _parse_form_value("42") == 42
|
||||
assert isinstance(_parse_form_value("42"), int)
|
||||
assert _parse_form_value("3.5") == 3.5
|
||||
assert isinstance(_parse_form_value("3.5"), float)
|
||||
|
||||
def test_json_array_parsed_before_numbers(self):
|
||||
# RGB arrays like "[255, 0, 0]" must come back as lists.
|
||||
assert _parse_form_value("[255, 0, 0]") == [255, 0, 0]
|
||||
|
||||
def test_json_object(self):
|
||||
assert _parse_form_value('{"a": 1}') == {"a": 1}
|
||||
|
||||
def test_malformed_json_falls_back_to_string(self):
|
||||
assert _parse_form_value("[not json") == "[not json"
|
||||
|
||||
def test_plain_string_returned_unstripped(self):
|
||||
# The original value (not the stripped copy) is returned.
|
||||
assert _parse_form_value(" hello ") == " hello "
|
||||
|
||||
def test_non_string_passthrough(self):
|
||||
assert _parse_form_value(7) == 7
|
||||
assert _parse_form_value([1, 2]) == [1, 2]
|
||||
|
||||
|
||||
class TestGetSchemaProperty:
|
||||
SCHEMA: ClassVar[Dict[str, Any]] = {
|
||||
"properties": {
|
||||
|
||||
@@ -1,24 +1,23 @@
|
||||
"""
|
||||
Tests for the response builders in src/web_interface/error_handler.py and
|
||||
the success path in src/web_interface/api_helpers.py.
|
||||
Tests for the response builders in src/web_interface/api_helpers.py.
|
||||
|
||||
describe_exception() in the same module is already covered by
|
||||
describe_exception() in error_handler.py is already covered by
|
||||
test/test_web_error_detail.py and is not duplicated here.
|
||||
|
||||
Regression coverage for one fixed bug: create_success_response used
|
||||
Regression coverage for one fixed bug: the success builder used
|
||||
truthiness for `message` and `metadata` while using `is not None` for
|
||||
`data`, so an explicitly-passed "" or {} was silently dropped —
|
||||
api_helpers.success_response() repeated the same gate, which is the path
|
||||
every api_v3 endpoint actually calls.
|
||||
success_response() repeated the same gate, which is the path every api_v3
|
||||
endpoint actually calls.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from flask import Flask
|
||||
|
||||
from src.web_interface.api_helpers import exception_error_response, success_response
|
||||
from src.web_interface.error_handler import (
|
||||
create_error_response,
|
||||
create_success_response,
|
||||
from src.web_interface.api_helpers import (
|
||||
error_response,
|
||||
exception_error_response,
|
||||
success_response,
|
||||
)
|
||||
from src.web_interface.errors import ErrorCode, WebInterfaceError
|
||||
|
||||
@@ -28,23 +27,23 @@ def app():
|
||||
return Flask(__name__)
|
||||
|
||||
|
||||
class TestCreateErrorResponse:
|
||||
class TestErrorResponse:
|
||||
def test_returns_response_and_status_tuple(self, app):
|
||||
with app.test_request_context():
|
||||
response, status = create_error_response(
|
||||
response, status = error_response(
|
||||
ErrorCode.CONFIG_SAVE_FAILED, "could not save")
|
||||
assert status == 500
|
||||
assert response.get_json()["message"] == "could not save"
|
||||
|
||||
def test_status_code_passthrough(self, app):
|
||||
with app.test_request_context():
|
||||
_, status = create_error_response(
|
||||
_, status = error_response(
|
||||
ErrorCode.INVALID_INPUT, "bad", status_code=400)
|
||||
assert status == 400
|
||||
|
||||
def test_body_matches_the_error_dataclass(self, app):
|
||||
with app.test_request_context():
|
||||
response, _ = create_error_response(
|
||||
response, _ = error_response(
|
||||
ErrorCode.NETWORK_ERROR, "offline",
|
||||
details="connection refused", context={"url": "http://x"})
|
||||
expected = WebInterfaceError(
|
||||
@@ -54,12 +53,12 @@ class TestCreateErrorResponse:
|
||||
|
||||
def test_none_context_produces_no_context_key(self, app):
|
||||
with app.test_request_context():
|
||||
response, _ = create_error_response(ErrorCode.SYSTEM_ERROR, "boom")
|
||||
response, _ = error_response(ErrorCode.SYSTEM_ERROR, "boom")
|
||||
assert "context" not in response.get_json()
|
||||
|
||||
def test_suggested_fixes_passed_through(self, app):
|
||||
with app.test_request_context():
|
||||
response, _ = create_error_response(
|
||||
response, _ = error_response(
|
||||
ErrorCode.SYSTEM_ERROR, "boom", suggested_fixes=["Try again"])
|
||||
assert response.get_json()["suggested_fixes"] == ["Try again"]
|
||||
|
||||
@@ -74,7 +73,6 @@ class TestExceptionErrorResponse:
|
||||
|
||||
@staticmethod
|
||||
def _by_hand(exc, code, with_context):
|
||||
from src.web_interface.api_helpers import error_response
|
||||
error = WebInterfaceError.from_exception(exc, code)
|
||||
if with_context:
|
||||
return error_response(error.error_code, error.message,
|
||||
@@ -117,39 +115,45 @@ class TestExceptionErrorResponse:
|
||||
assert "context" not in response.get_json()
|
||||
|
||||
|
||||
class TestCreateSuccessResponse:
|
||||
def _success_body(**kwargs):
|
||||
"""success_response()'s body, outside any request timing."""
|
||||
with Flask(__name__).test_request_context():
|
||||
return success_response(**kwargs).get_json()
|
||||
|
||||
|
||||
class TestSuccessResponseBody:
|
||||
def test_bare_success(self):
|
||||
assert create_success_response() == {"status": "success"}
|
||||
assert _success_body() == {"status": "success"}
|
||||
|
||||
def test_data_included(self):
|
||||
assert create_success_response(data={"a": 1})["data"] == {"a": 1}
|
||||
assert _success_body(data={"a": 1})["data"] == {"a": 1}
|
||||
|
||||
@pytest.mark.parametrize("falsy", [0, "", False, {}, []])
|
||||
def test_falsy_data_is_still_included(self, falsy):
|
||||
assert create_success_response(data=falsy)["data"] == falsy
|
||||
assert _success_body(data=falsy)["data"] == falsy
|
||||
|
||||
def test_none_data_omitted(self):
|
||||
assert "data" not in create_success_response(data=None)
|
||||
assert "data" not in _success_body(data=None)
|
||||
|
||||
def test_message_included(self):
|
||||
assert create_success_response(message="done")["message"] == "done"
|
||||
assert _success_body(message="done")["message"] == "done"
|
||||
|
||||
def test_empty_message_is_still_included(self):
|
||||
# Regression: `if message:` dropped an explicitly-passed "".
|
||||
assert create_success_response(message="")["message"] == ""
|
||||
assert _success_body(message="")["message"] == ""
|
||||
|
||||
def test_none_message_omitted(self):
|
||||
assert "message" not in create_success_response(message=None)
|
||||
assert "message" not in _success_body(message=None)
|
||||
|
||||
def test_metadata_included(self):
|
||||
assert create_success_response(metadata={"v": 1})["metadata"] == {"v": 1}
|
||||
assert _success_body(metadata={"v": 1})["metadata"] == {"v": 1}
|
||||
|
||||
def test_empty_metadata_is_still_included(self):
|
||||
# Regression: `if metadata:` dropped an explicitly-passed {}.
|
||||
assert create_success_response(metadata={})["metadata"] == {}
|
||||
assert _success_body(metadata={})["metadata"] == {}
|
||||
|
||||
def test_none_metadata_omitted(self):
|
||||
assert "metadata" not in create_success_response(metadata=None)
|
||||
assert "metadata" not in _success_body(metadata=None)
|
||||
|
||||
|
||||
class TestSuccessResponseHelper:
|
||||
@@ -162,8 +166,8 @@ class TestSuccessResponseHelper:
|
||||
|
||||
def test_explicit_empty_metadata_survives_the_wrapper(self, app):
|
||||
# Regression: the wrapper re-gated metadata on truthiness after
|
||||
# create_success_response had already included it, so {} was
|
||||
# dropped again on the way out.
|
||||
# the body builder had already included it, so {} was dropped
|
||||
# again on the way out.
|
||||
with app.test_request_context():
|
||||
body = success_response(data=None, metadata={}).get_json()
|
||||
assert body["metadata"] == {}
|
||||
|
||||
@@ -55,7 +55,7 @@ class TestPluginOperationQueue(unittest.TestCase):
|
||||
# behavior may differ. For this test, we'll verify the mechanism exists.
|
||||
try:
|
||||
self.queue.enqueue_operation(
|
||||
OperationType.UPDATE,
|
||||
OperationType.UNINSTALL,
|
||||
"test-plugin"
|
||||
)
|
||||
# If no exception, the first operation may have completed
|
||||
@@ -64,21 +64,6 @@ class TestPluginOperationQueue(unittest.TestCase):
|
||||
# Expected behavior - concurrent operation prevented
|
||||
pass
|
||||
|
||||
def test_operation_cancellation(self):
|
||||
"""Test cancelling a pending operation."""
|
||||
operation_id = self.queue.enqueue_operation(
|
||||
OperationType.INSTALL,
|
||||
"test-plugin"
|
||||
)
|
||||
|
||||
# Cancel operation
|
||||
success = self.queue.cancel_operation(operation_id)
|
||||
self.assertTrue(success)
|
||||
|
||||
# Check status
|
||||
operation = self.queue.get_operation_status(operation_id)
|
||||
self.assertEqual(operation.status, OperationStatus.CANCELLED)
|
||||
|
||||
def test_operation_history(self):
|
||||
"""Test operation history tracking."""
|
||||
# Enqueue and complete an operation
|
||||
@@ -92,7 +77,7 @@ class TestPluginOperationQueue(unittest.TestCase):
|
||||
time.sleep(0.5)
|
||||
|
||||
# Check history
|
||||
history = self.queue.get_operation_history(limit=10)
|
||||
history = self.queue._operation_history
|
||||
self.assertGreater(len(history), 0)
|
||||
|
||||
# Find our operation in history
|
||||
|
||||
@@ -100,7 +100,6 @@ class TestStateReconciliation(unittest.TestCase):
|
||||
inconsistency = result.inconsistencies_found[0]
|
||||
self.assertEqual(inconsistency.plugin_id, "plugin1")
|
||||
self.assertEqual(inconsistency.inconsistency_type, InconsistencyType.PLUGIN_MISSING_IN_CONFIG)
|
||||
self.assertTrue(inconsistency.can_auto_fix)
|
||||
self.assertEqual(inconsistency.fix_action, FixAction.AUTO_FIX)
|
||||
|
||||
def test_plugin_missing_on_disk(self):
|
||||
@@ -115,7 +114,6 @@ class TestStateReconciliation(unittest.TestCase):
|
||||
inconsistency = result.inconsistencies_found[0]
|
||||
self.assertEqual(inconsistency.plugin_id, "plugin1")
|
||||
self.assertEqual(inconsistency.inconsistency_type, InconsistencyType.PLUGIN_MISSING_ON_DISK)
|
||||
self.assertFalse(inconsistency.can_auto_fix)
|
||||
self.assertEqual(inconsistency.fix_action, FixAction.MANUAL_FIX_REQUIRED)
|
||||
|
||||
def test_enabled_but_not_loaded_is_reported_not_fixed(self):
|
||||
@@ -139,7 +137,6 @@ class TestStateReconciliation(unittest.TestCase):
|
||||
inconsistency = result.inconsistencies_found[0]
|
||||
self.assertEqual(inconsistency.inconsistency_type, InconsistencyType.PLUGIN_ENABLED_MISMATCH)
|
||||
self.assertEqual(inconsistency.fix_action, FixAction.NO_ACTION)
|
||||
self.assertFalse(inconsistency.can_auto_fix)
|
||||
self.assertIn("No module named", inconsistency.description)
|
||||
self.assertEqual(result.inconsistencies_fixed, [])
|
||||
self.assertEqual(result.inconsistencies_manual, [])
|
||||
@@ -410,7 +407,6 @@ class TestStateReconciliationUnrecoverable(unittest.TestCase):
|
||||
# Still one inconsistency, still no install attempt, no new registry fetch
|
||||
self.assertEqual(len(result.inconsistencies_found), 1)
|
||||
inc = result.inconsistencies_found[0]
|
||||
self.assertFalse(inc.can_auto_fix)
|
||||
self.assertEqual(inc.fix_action, FixAction.MANUAL_FIX_REQUIRED)
|
||||
self.store_manager.install_plugin.assert_not_called()
|
||||
self.store_manager.fetch_registry.assert_not_called()
|
||||
@@ -458,7 +454,6 @@ class TestStateReconciliationUnrecoverable(unittest.TestCase):
|
||||
|
||||
self.assertEqual(len(result.inconsistencies_found), 1)
|
||||
inc = result.inconsistencies_found[0]
|
||||
self.assertFalse(inc.can_auto_fix)
|
||||
self.assertEqual(inc.fix_action, FixAction.MANUAL_FIX_REQUIRED)
|
||||
self.store_manager.install_plugin.assert_not_called()
|
||||
|
||||
|
||||
@@ -119,7 +119,7 @@ class Web:
|
||||
self.config_manager.template_path = str(tmp_path / "no-template.json")
|
||||
self.schema_manager = SchemaManager(plugins_dir=self.plugins_dir, project_root=tmp_path,
|
||||
config_manager=self.config_manager)
|
||||
self.catalog = PluginCatalog(self.plugins_dir, self.config_manager, self.schema_manager)
|
||||
self.catalog = PluginCatalog(self.plugins_dir)
|
||||
|
||||
api = self.api = api_v3_module.api_v3
|
||||
api.config_manager = self.config_manager
|
||||
@@ -242,7 +242,8 @@ class TestTheWebProcessNeverRunsAPlugin:
|
||||
_bump_version(web, "1.1.0")
|
||||
body = web.post("/api/v3/plugins/update", {"plugin_id": PLUGIN_ID})
|
||||
assert body["data"]["update_status"] == "updated"
|
||||
assert web.catalog.get_installed_version(PLUGIN_ID) == "1.1.0"
|
||||
# The route rescans the catalog, which now has the new manifest.
|
||||
assert web.catalog.get_manifest(PLUGIN_ID)["version"] == "1.1.0"
|
||||
assert web.ran() == []
|
||||
|
||||
def test_installing_it(self, web):
|
||||
@@ -497,16 +498,15 @@ class TestCatalogReadsWhatIsInstalled:
|
||||
assert expected, f"no plugins under {root}"
|
||||
before = set(sys.modules)
|
||||
schema_manager = SchemaManager(plugins_dir=root, project_root=PROJECT_ROOT)
|
||||
catalog = PluginCatalog(root, schema_manager=schema_manager)
|
||||
catalog = PluginCatalog(root)
|
||||
|
||||
assert set(catalog.discover_plugins()) == set(expected)
|
||||
for plugin_id, (plugin_dir, manifest) in expected.items():
|
||||
assert catalog.get_manifest(plugin_id) == manifest
|
||||
assert catalog.get_plugin_directory(plugin_id) == str(plugin_dir)
|
||||
assert catalog.get_installed_version(plugin_id) == manifest.get("version", "")
|
||||
assert catalog.get_plugin_display_modes(plugin_id) == manifest.get("display_modes", [])
|
||||
if (plugin_dir / "config_schema.json").exists():
|
||||
schema = catalog.get_schema(plugin_id, use_cache=False)
|
||||
schema = schema_manager.load_schema(plugin_id, use_cache=False)
|
||||
assert isinstance(schema, dict) and "properties" in schema, plugin_id
|
||||
|
||||
imported = [name for name in set(sys.modules) - before
|
||||
@@ -537,16 +537,16 @@ class TestCatalogReadsWhatIsInstalled:
|
||||
assert catalog.discover_plugins() == []
|
||||
assert catalog.get_manifest("ci-fixture-plugin") is None
|
||||
|
||||
def test_enabled_follows_the_display_rule(self, tmp_path):
|
||||
def test_enabled_follows_the_display_rule(self, api_v3_module):
|
||||
config = MagicMock()
|
||||
config.load_config.return_value = {"a": {"enabled": True}, "b": {}, "c": "junk"}
|
||||
catalog = PluginCatalog(tmp_path, config_manager=config)
|
||||
assert catalog.is_enabled("a") is True
|
||||
api_v3_module.api_v3.config_manager = config
|
||||
enabled = api_v3_module._plugin_enabled_in_config
|
||||
assert enabled("a") is True
|
||||
# The display runs a plugin only when its section says so.
|
||||
assert catalog.is_enabled("b") is False
|
||||
assert catalog.is_enabled("c") is False
|
||||
assert catalog.is_enabled("missing") is False
|
||||
assert catalog.get_config("c") == {}
|
||||
assert enabled("b") is False
|
||||
assert enabled("c") is False
|
||||
assert enabled("missing") is False
|
||||
|
||||
def test_it_has_nothing_that_runs_a_plugin(self, tmp_path):
|
||||
catalog = PluginCatalog(tmp_path)
|
||||
|
||||
Reference in New Issue
Block a user