mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-04 22:35:08 +00:00
* fix(store): don't read a ZIP-installed plugin's remote from the LEDMatrix repo update_plugin looked up remote.origin.url with `git -C <plugin> config --local` for plugins that are not git checkouts. Under plugin-repos/ git walks up to the enclosing LEDMatrix repository, so the lookup returned LEDMatrix's own URL and a plugin missing from the registry was "reinstalled" from the LEDMatrix repo. Only ask git when the plugin directory has its own .git, the test _get_local_git_info already uses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(schema): report each missing required field once, by name validate_config_against_schema ran its own required-fields loop after Draft7Validator.iter_errors, which already yields one `required` error per missing field, so every missing top-level field was listed twice. The validator's copy also printed the schema's whole `required` list ("Missing required property '['api_key', 'city']'") instead of the field. Drop the loop and take the field name from the error itself. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(store): stop mangling repository URLs that contain ".git" install_from_url and fetch_registry_from_url cleaned URLs with `rstrip('/').replace('.git', '')`, which removes ".git" anywhere: https://github.com/user/my.github.io became .../myhub.io, so installing or browsing that repository asked GitHub for one that does not exist. Add src/plugin_system/repo_urls.py with one anchored normalize_repo_url(), same_repo() for comparisons, github_owner_repo() and github_api_headers(), and use them for the five copies of the owner/repo parsing and GitHub headers in the store and for saved repositories. GitHub URLs are now recognised by urlparse().hostname everywhere: _get_latest_commit_info used a substring test, and _install_from_monorepo_api parsed any host. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(store): install a repository whose only branch is not main/master _install_via_git returned None both when every clone failed and when the last-resort clone of the repository's default branch succeeded. _install_plugin_impl papered over it with `and not plugin_path.exists()`; install_from_url did not, so a repository whose only branch is e.g. `develop` was cloned, then treated as a failure, then "downloaded" from main/master archives that do not exist. After a default-branch clone, return the branch the clone checked out (read from .git/HEAD), so None means failure and nothing else, and give both callers the same `branch_used is None` fallback. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(plugins): judge the memory limit on each call's own growth monitor_call stores `metrics.memory_mb = max(previous, growth)`, and _check_limits compared that high-water mark with max_memory_mb. It never decreases, so once one update() grew the process past the limit every later call raised ResourceLimitExceeded and the circuit breaker kept reopening. Pass the call's own RSS growth to _check_limits; keep the high-water mark for reporting and document what it measures. Remove ResourceMetrics.update_average_execution_time: nothing called it, and it overwrote the running total with the average. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(plugins): reload_plugin re-reads the manifest from the discovered directory reload_plugin read `plugins_dir / plugin_id / "manifest.json"`, ignoring the discovery map and the plugin_dirs rules. For a plugin whose directory name differs from its manifest id the path did not exist, the re-read was skipped without a word, and the reload kept the stale manifest. Resolve the directory with find_plugin_directory, as load_plugin does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(plugins): drop the always-null last_display from plugin state info PluginStateManager reported `last_display` from `_last_display`, which nothing ever wrote, so it was null for every plugin. Recording it in PluginExecutor.execute_display would not help: get_state_info's only reader is the web process, whose PluginManager never calls display(). Remove the field, its dict and get_last_display() (no caller in core, the web UI or the plugin monorepo). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(store): share the rollback and requirements helpers, drop dead code - install_plugin and _reinstall_with_rollback set aside, discard and restore the old copy through _set_aside/_discard_backup/_restore_backup instead of two copies of the same blocks. - The loader and the store run the same pre-pip checks through contained_plugin_dir() and requirements_to_install() in plugin_loader. They still invoke pip differently (sys.executable -m pip vs. the sudo wrapper). `except (BrokenPipeError, OSError)` + `isinstance(e, OSError)` becomes `except OSError` checking errno.EPIPE. - load_module never returns None, so load_plugin's check is gone and the docstring says what it raises. - Remove the always-true JSONSCHEMA_AVAILABLE, the inline re-imports of re and permission_utils, the fake status_result object nobody reads, hasattr(git_error, 'cmd'), a redundant "merge conflict" test and `import traceback` (exc_info=True does it). - Correct comments: install_from_url names the directory for the caller's id when given (not always the manifest id), _get_local_git_info saves one git subprocess (not four), _enrich calls two helpers, search_plugins documents all its arguments, _find_plugin_path states its behaviour instead of a TODO, and history narration is gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(plugins): tidy base_plugin, correct plugin_manager/state comments - base_plugin: drop the unused `import logging`; get_display_duration runs the instance value and the config value through one _positive_seconds() helper instead of two copies of the coercion; the 'static'/'none'/fallback branches of get_vegas_display_mode, which all returned FIXED_SEGMENT, are one; fix the mis-indented validate_config example; say that get_supported_vegas_modes/get_vegas_segment_width are not consulted by core (kept, plugins override them). - schema_manager: import expand_style_elements normally rather than swallowing an ImportError of a core module. - plugin_manager: the plugins directory is the configured one (plugin-repos/ by default), not plugins/; get_config() returns the live dict, not a copy, so the interval cache comments say what it saves. - state_manager: config_version and the file version are not used to detect corruption; say what they are. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(plugins): stop writing data/plugin_operations.json PluginOperationQueue wrote its finished-operation history to data/plugin_operations.json after every operation, and read it back only into its own in-memory list, which only get_operation_history() exposes -- and nothing calls that. The operation-history endpoint reads OperationHistory (data/operation_history.json). No code in src/, web_interface/, scripts/ or test/ reads the file. Drop the history_file/lazy_load parameters and the load/save code; the bounded in-memory history stays. web_interface/app.py and the integration test stop passing the removed arguments. An existing data/plugin_operations.json is left in place (data/* is gitignored). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(changelog): plugin-system Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
235 lines
9.8 KiB
Python
235 lines
9.8 KiB
Python
"""
|
|
Tests for src/plugin_system/resource_monitor.py
|
|
|
|
Focus areas:
|
|
- Execution-time metrics are captured regardless of psutil availability.
|
|
- CPU sampling is non-blocking (regression guard for the previous
|
|
``cpu_percent(interval=0.1)`` call that blocked 100 ms per monitored call).
|
|
- Resource limits are enforced.
|
|
"""
|
|
|
|
import time
|
|
|
|
import pytest
|
|
from unittest.mock import MagicMock, patch
|
|
|
|
from src.plugin_system.resource_monitor import (
|
|
PluginResourceMonitor,
|
|
ResourceLimits,
|
|
ResourceLimitExceeded,
|
|
PSUTIL_AVAILABLE,
|
|
)
|
|
|
|
|
|
def _cache():
|
|
cache = MagicMock()
|
|
cache.get.return_value = None
|
|
return cache
|
|
|
|
|
|
class TestExecutionTimeMetrics:
|
|
def test_monitor_call_returns_value_and_records_call(self):
|
|
mon = PluginResourceMonitor(_cache(), enable_monitoring=False)
|
|
result = mon.monitor_call("p", lambda: 42)
|
|
assert result == 42
|
|
metrics = mon.get_metrics("p")
|
|
assert metrics.call_count == 1
|
|
assert metrics.total_execution_time >= 0.0
|
|
|
|
def test_avg_and_max_execution_time(self):
|
|
mon = PluginResourceMonitor(_cache(), enable_monitoring=False)
|
|
mon.monitor_call("p", lambda: time.sleep(0.01))
|
|
mon.monitor_call("p", lambda: None)
|
|
summary = mon.get_metrics_summary("p")
|
|
assert summary["call_count"] == 2
|
|
assert summary["max_execution_time"] >= summary["avg_execution_time"] >= 0.0
|
|
|
|
def test_exception_propagates_but_is_still_timed(self):
|
|
mon = PluginResourceMonitor(_cache(), enable_monitoring=False)
|
|
|
|
def boom():
|
|
raise ValueError("nope")
|
|
|
|
with pytest.raises(ValueError):
|
|
mon.monitor_call("p", boom)
|
|
# Execution time is still recorded even when the call raised.
|
|
assert mon.get_metrics("p").execution_time >= 0.0
|
|
|
|
|
|
class TestNonBlockingCpu:
|
|
def test_cpu_sampling_is_fast_when_disabled(self):
|
|
mon = PluginResourceMonitor(_cache(), enable_monitoring=False)
|
|
start = time.time()
|
|
for _ in range(50):
|
|
mon._get_process_cpu_percent()
|
|
# The old implementation blocked ~0.1s/call (~5s for 50). Non-blocking
|
|
# must complete near-instantly.
|
|
assert time.time() - start < 0.5
|
|
assert mon._get_process_cpu_percent() == 0.0
|
|
|
|
@pytest.mark.skipif(not PSUTIL_AVAILABLE, reason="psutil not installed")
|
|
def test_cpu_sampling_is_fast_with_psutil(self):
|
|
mon = PluginResourceMonitor(_cache(), enable_monitoring=True)
|
|
assert mon._process is not None
|
|
start = time.time()
|
|
for _ in range(30):
|
|
mon._get_process_cpu_percent()
|
|
# 30 blocking 0.1s samples would be ~3s; non-blocking must be well under.
|
|
assert time.time() - start < 0.5
|
|
|
|
def test_monitor_call_does_not_block_on_cpu_sampling(self):
|
|
mon = PluginResourceMonitor(_cache()) # enable depends on psutil
|
|
start = time.time()
|
|
for _ in range(25):
|
|
mon.monitor_call("p", lambda: None)
|
|
# 25 * 0.1s = 2.5s under the old blocking bug; must be far faster now.
|
|
assert time.time() - start < 1.0
|
|
|
|
|
|
class TestResourceLimits:
|
|
def test_execution_time_limit_raises(self):
|
|
mon = PluginResourceMonitor(_cache(), enable_monitoring=False)
|
|
mon.set_limits("p", ResourceLimits(max_execution_time=0.001))
|
|
with pytest.raises(ResourceLimitExceeded):
|
|
mon.monitor_call("p", lambda: time.sleep(0.02))
|
|
|
|
def test_memory_limit_judges_each_call_on_its_own_growth(self):
|
|
"""One expensive call must not fail every call after it.
|
|
|
|
The check used to compare the stored high-water mark, which never
|
|
decreases, so after one call grew memory past the limit every later
|
|
call raised too and the plugin never updated again.
|
|
"""
|
|
mon = PluginResourceMonitor(_cache(), enable_monitoring=False)
|
|
mon.enable_monitoring = True # measure without needing psutil
|
|
readings = iter([100.0, 200.0, # first call grows RSS by 100 MB
|
|
200.0, 201.0]) # second call grows it by 1 MB
|
|
mon._get_process_memory_mb = lambda: next(readings)
|
|
mon._get_process_cpu_percent = lambda: 0.0
|
|
mon.set_limits("p", ResourceLimits(max_memory_mb=50))
|
|
|
|
with pytest.raises(ResourceLimitExceeded):
|
|
mon.monitor_call("p", lambda: None)
|
|
assert mon.monitor_call("p", lambda: "ok") == "ok"
|
|
# The high-water mark is still reported.
|
|
assert mon.get_metrics("p").memory_mb == 100.0
|
|
|
|
def test_reset_metrics_clears_counts(self):
|
|
cache = _cache()
|
|
mon = PluginResourceMonitor(cache, enable_monitoring=False)
|
|
mon.monitor_call("p", lambda: None)
|
|
assert mon.get_metrics("p").call_count == 1
|
|
mon.reset_metrics("p")
|
|
assert mon.get_metrics("p").call_count == 0
|
|
|
|
|
|
class TestForceReload:
|
|
def test_force_reload_refreshes_stale_snapshot(self):
|
|
"""A read-only consumer must see the writer process's latest persisted
|
|
metrics rather than a pinned first snapshot."""
|
|
cache = MagicMock()
|
|
persisted = {"value": None} # only the metrics key returns data
|
|
|
|
def cache_get(key, max_age=None, memory_ttl=None):
|
|
return persisted["value"] if key.startswith("plugin_metrics:") else None
|
|
|
|
cache.get.side_effect = cache_get
|
|
mon = PluginResourceMonitor(cache, enable_monitoring=False)
|
|
|
|
# First read snapshots empty metrics.
|
|
assert mon.get_metrics_summary("p")["call_count"] == 0
|
|
|
|
# The display service later persists real metrics.
|
|
persisted["value"] = {"call_count": 7, "total_execution_time": 1.4}
|
|
|
|
# Plain read stays stale...
|
|
assert mon.get_metrics_summary("p")["call_count"] == 0
|
|
# ...force_reload picks up the persisted values and bypasses memory.
|
|
fresh = mon.get_metrics_summary("p", force_reload=True)
|
|
assert fresh["call_count"] == 7
|
|
assert any(c.kwargs.get("memory_ttl") == 0 for c in cache.get.call_args_list)
|
|
|
|
|
|
class TestMetricsPersistenceChurn:
|
|
"""Metrics are telemetry; writing them on every call wore the SD card.
|
|
|
|
Each write is a ~350-byte file, which on ext4 costs a 4KB block plus a
|
|
journal entry. At roughly nine calls a minute per plugin across fourteen
|
|
plugins it dominated the device's write volume.
|
|
"""
|
|
|
|
def test_the_first_snapshot_is_written_even_seconds_after_boot(self):
|
|
"""The throttle must key off "have we written?", not process uptime.
|
|
|
|
time.monotonic() is time since boot on Linux, and systemd starts this
|
|
service at boot. With 0.0 as the missing-timestamp default,
|
|
`now - 0.0 < 30` was true for the first half-minute of every run, so
|
|
the very first metrics write -- the one that matters most after a
|
|
restart -- was silently skipped.
|
|
"""
|
|
import src.plugin_system.resource_monitor as rm
|
|
cache = _cache()
|
|
mon = PluginResourceMonitor(cache, enable_monitoring=False)
|
|
# 12 seconds after boot: inside the interval, but nothing written yet.
|
|
with patch.object(rm.time, "monotonic", return_value=12.0):
|
|
mon.monitor_call("p", lambda: None)
|
|
writes = [c for c in cache.set.call_args_list
|
|
if "plugin_metrics:" in str(c)]
|
|
assert writes, \
|
|
"the first snapshot was dropped because the process was young"
|
|
|
|
def test_repeated_calls_persist_once_per_interval(self):
|
|
cache = _cache()
|
|
mon = PluginResourceMonitor(cache, enable_monitoring=False)
|
|
for _ in range(50):
|
|
mon.monitor_call("p", lambda: None)
|
|
writes = [c for c in cache.set.call_args_list
|
|
if c.args and str(c.args[0]).startswith("plugin_metrics:")]
|
|
assert len(writes) == 1, (
|
|
f"50 calls produced {len(writes)} metric writes; expected 1")
|
|
|
|
def test_the_interval_elapsing_allows_the_next_write(self, monkeypatch):
|
|
import src.plugin_system.resource_monitor as rm
|
|
cache = _cache()
|
|
mon = PluginResourceMonitor(cache, enable_monitoring=False)
|
|
mon.monitor_call("p", lambda: None)
|
|
# pretend the interval has passed
|
|
mon._metrics_persisted_at["p"] -= rm._METRICS_PERSIST_INTERVAL + 1
|
|
mon.monitor_call("p", lambda: None)
|
|
writes = [c for c in cache.set.call_args_list
|
|
if c.args and str(c.args[0]).startswith("plugin_metrics:")]
|
|
assert len(writes) == 2
|
|
|
|
def test_in_memory_metrics_stay_exact_while_writes_are_skipped(self):
|
|
mon = PluginResourceMonitor(_cache(), enable_monitoring=False)
|
|
for _ in range(20):
|
|
mon.monitor_call("p", lambda: None)
|
|
assert mon.get_metrics("p").call_count == 20
|
|
|
|
def test_reset_lets_the_next_call_persist_immediately(self):
|
|
cache = _cache()
|
|
mon = PluginResourceMonitor(cache, enable_monitoring=False)
|
|
mon.monitor_call("p", lambda: None)
|
|
mon.reset_metrics("p")
|
|
mon.monitor_call("p", lambda: None)
|
|
writes = [c for c in cache.set.call_args_list
|
|
if c.args and str(c.args[0]).startswith("plugin_metrics:")]
|
|
assert len(writes) == 2, "reset should clear the throttle timestamp"
|
|
|
|
def test_a_failed_write_does_not_buy_the_next_interval_of_silence(self):
|
|
"""A set() that raises must not count as having persisted.
|
|
|
|
Marking the timestamp before the write would leave no snapshot in the
|
|
cache and still suppress the next 30 seconds of attempts.
|
|
"""
|
|
cache = _cache()
|
|
cache.set.side_effect = [OSError("disk full"), None]
|
|
mon = PluginResourceMonitor(cache, enable_monitoring=False)
|
|
with pytest.raises(OSError):
|
|
mon.monitor_call("p", lambda: None)
|
|
# the very next call must try again rather than skip the interval
|
|
mon.monitor_call("p", lambda: None)
|
|
writes = [c for c in cache.set.call_args_list
|
|
if c.args and str(c.args[0]).startswith("plugin_metrics:")]
|
|
assert len(writes) == 2, "a failed write should be retried, not skipped"
|