Compare commits

...
Author SHA1 Message Date
ChuckBuildsandClaude Opus 5 a997e75c37 test(logging): stop the location assertion matching the clock
test_location_toggle asserted that ":42" -- a bare colon plus the record's
hardcoded lineno -- is absent from a line formatted with include_location=False.
But every formatted line starts with an HH:MM:SS.mmm timestamp, so ":42" also
matches the clock whenever the minute or the second is 42. The test fails for
roughly 3% of runs with nothing wrong:

  2026-08-22 08:05:42.274 - INFO - test.logger - hello
                     ^^^ matches ":42"

Assert on the whole "module.funcName:lineno" token the format string actually
emits ('%(module)s.%(funcName)s:%(lineno)d') instead of a fragment of it. That
cannot collide with a timestamp, and it checks the thing the test is named for.

Confirmed by formatting a record stamped 08:42:42 -- both minute and second
colliding: the old assertion fails, the new one passes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
2026-08-22 08:09:59 -04:00
ChuckandClaude Opus 5 6b74506695 fix(sports): fetch odds for the games shown, not the whole schedule window (#494)
* fix(sports): fetch odds for the games shown, not the whole window

SportsUpcoming.update() walked every upcoming game in the schedule
window and called _fetch_odds() on each one inside that collection
loop, narrowing to upcoming_games_to_show only afterwards. Each call is
a separate sequential ESPN request.

The comment sitting above it said odds were fetched "only for games that
will be displayed". The only narrowing it actually applied was
show_favorite_teams_only, which is not the default, so in the usual
configuration nothing narrowed it at all.

Measured on devpi, where the football plugin has the same shape:

  467 odds requests in one 35s burst, 467 distinct events
  315 NFL + 152 college-football -- roughly a whole season
  plugin football-scoreboard operation timed out after 30.0s

The burst repeats each time the 1h odds TTL expires: 67 -> 327 -> 957 ->
1261 requests/hour across four consecutive hours. Between expiries the
cache works and the rate is zero, so this is a thundering herd on
expiry, not a caching failure.

The fetch now runs after selection, over team_games -- the list already
cut to upcoming_games_to_show. This mirrors the fix the football plugin
already carries; the shared base class never got it.

SportsLive is deliberately left as it is: it walks the raw event list
because it has to find which games are live, but only fetches odds for a
game that has already passed the is_live/is_halftime test, so its
fan-out is bounded by how many games are actually in progress. The test
pins that distinction rather than assuming it.

The test reads the AST rather than the source text, and asserts the full
set of call sites, so a new one has to be classified deliberately
instead of inheriting whichever behaviour it happens to land in. Writing
it that way is what turned up the SportsLive site, which I had missed.

Verified: reverting the fix fails the test with the offending iterable
named ("iterates over 'events'"). 525 passed, 9 skipped across the sports
and odds suites.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW

* test(sports): check the odds guard structurally, not by its text

Review caught that _guards_above() collected an `if` test even when the
call sat in that if's `else`, so moving _fetch_odds() into the else of
the is_live/is_halftime test would still pass -- while fetching odds for
exactly the non-live games the guard exists to exclude.

Verifying that turned up a wider hole in the same assertion. It matched
substrings of the *unparsed source*, so a negated condition satisfied it
too:

    if not (details["is_live"] or details["is_halftime"]):
        self._fetch_odds(details)      # every non-live game

Both names still appear in that text, so `"is_live" in guards` held and
the test passed on code doing the opposite of what it claims to check.

The guard test is now structural. It walks the AST for an enclosing `if`
whose *body* (never its `else`) contains the call, and whose test
references both names without either sitting under a `not`.

Verified by mutation: fetching odds for non-live games now fails with
"does not sit in the true branch of a test requiring the game to be in
progress". Moving the call into the else of the *favourites* test still
passes, which is correct -- the game there is still live, so the
in-progress contract holds and the fan-out stays bounded by how many
games are actually in play.

525 passed, 9 skipped across the sports and odds suites.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-21 16:24:15 -04:00
ChuckandClaude Opus 5 5f29243e87 fix(config): make the device location the default for plugin location fields (#490)
* fix(config): make the device location the default for plugin location fields

A user in Kansas City reported their radar centred on Dallas, TX with
nothing in config.json to explain it.

The radar is the `ledmatrix-weather` plugin's `radar` mode, and it centres
on the same coordinates as every other weather mode: `forecast_data`
lat/lon, geocoded from the plugin's own `location_city` /
`location_state` / `location_country`. Those ship with schema defaults of
Dallas / Texas / US. A user who never opened the weather plugin's config
form therefore has no `location_city` on disk, and `PluginManager` merges
the schema default in at load time — so the whole plugin (not just the
radar) silently runs on Dallas. Radar is just the only mode that draws a
recognisable map and gives the mismatch away.

Meanwhile the device-wide `location` block that General settings writes
was read by nothing at all, despite its own help text promising it was
"used for weather, sunrise/sunset, and other location-based content".

`SchemaManager.generate_default_config()` now substitutes the device
`location` into the three fully-namespaced `location_*` keys before
handing defaults back, so the promise holds:

- Only `location_city` / `location_state` / `location_country` are
  substituted. A bare `state` key is left alone — `ledmatrix-elections`
  uses it for a two-letter code, and rewriting it would break that plugin.
- A value the user saved on the plugin still wins: this replaces the
  schema default, and `merge_with_defaults` puts user config on top.
- The substitution is applied on the way out of the defaults cache rather
  than into it, so changing the device location takes effect immediately.
- No config manager, no `location` block, or an unreadable config all
  fall back to the plugin's own schema defaults.

Every caller benefits: the plugin loader, the config form (which now
pre-fills the user's real city), config save, and reset-to-defaults.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GNLrSZ32FNKpHRaduKEJsg

* docs(web): name the exact plugin keys the device location seeds

Review follow-up. The General settings help text said the device location
was "the default for every plugin that asks for a city", which overstates
what the code does: only the fully-namespaced `location_city` /
`location_state` / `location_country` keys are substituted. A plugin with
a bare `city` key gets nothing — deliberately, since `ledmatrix-elections`
uses `state` for a two-letter code. The tips now name the exact keys.

Worth noting for anyone editing these: `ui.help_tip(...)` takes a
single-quoted Jinja string, so an apostrophe in the tip text has to be
escaped or written around. The wording here avoids them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GNLrSZ32FNKpHRaduKEJsg

---------

Co-authored-by: Claude <noreply@anthropic.com>
2026-08-21 16:22:48 -04:00
ChuckandClaude Opus 5 1fbe244e49 fix(plugins): say when discovery skips a directory (#489)
* fix(plugins): say when discovery skips a directory

A plugin can be enabled in config, enabled in plugin state, present on disk
with a valid manifest and an importable entry point -- and simply absent from
the running process, with nothing anywhere to say why.

That is not hypothetical. hockey-scoreboard on a live rig is enabled in both
places, imports cleanly when loaded by hand, and is listed in the Vegas plugin
order, but is not among the 22 plugins the process actually holds. Establishing
even that much meant comparing cache-file mtimes to find it had last run three
days earlier. The journal had nothing, because discovery does not report what
it declines to load.

Two paths were silent. A directory with no manifest.json was skipped without
comment, which is defensible until it is the thing you are trying to explain.
Quieter still, a manifest that parsed but carried no "id" was read
successfully and then dropped on the floor -- no warning, no trace, and the
plugin simply does not exist as far as the rest of the system is concerned.

Both now log a warning naming the directory and the reason.

This does not explain the rig above; its manifest has an id. It makes the next
occurrence diagnosable from the journal instead of from file timestamps.

Reverting the change fails both tests. 65 plugin-system tests pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW

* fix(plugins): warn once per directory, not once per scan

Self-review catch. Discovery runs on every web UI page load and every config
reconcile, so warning unconditionally about an unloadable directory would put
a line in the journal each time someone opened a page -- the same log-volume
problem this change exists to help diagnose.

The skip is now reported once per directory per process. The diagnostic value
is unchanged: the reason a plugin is missing still appears in the journal,
once, where before it appeared nowhere.

Test added covering five consecutive scans producing one warning.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW

* fix(plugins): one unusable manifest no longer aborts the whole scan

json.load accepts any JSON value, so a manifest.json holding null, [],
"text" or 42 parses without complaint and then raises AttributeError on
manifest.get('id'). Nothing catches that: the outer handler around the
scan takes OSError and PermissionError only.

So a single malformed manifest did not skip that one directory -- it
aborted _scan_directory_for_plugins outright, and every other plugin on
disk, however healthy, silently failed to register. Reproduced with
three directories, the middle one holding `null`:

    SCAN ABORTED -> AttributeError: 'NoneType' object has no attribute 'get'
      the two valid plugins never registered

That is the same failure this PR set out to fix, in its most severe
form: a plugin enabled in config, enabled in plugin state, present on
disk, and absent from the running process with nothing to say why --
except here it takes every other plugin with it.

A manifest that is not a JSON object is now skipped like any other
unusable directory, named once, with what it actually was:

    Skipping bad-null: its manifest.json is NoneType, not a JSON object
    Skipping bad-list: its manifest.json is list, not a JSON object
    scan returned: ['aaa-good', 'zzz-good']

Verified: removing the guard fails 6 of the 10 tests. Covers null, list,
string, int and bool, and asserts the healthy plugins either side of the
bad one still register.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-21 16:19:33 -04:00
10 changed files with 615 additions and 26 deletions
+1 -1
View File
@@ -18,7 +18,7 @@ tooling against it.
| `web_display_autostart` | bool, `true` | Whether the web interface service starts with the system | `scripts/utils/start_web_conditionally.py` | | `web_display_autostart` | bool, `true` | Whether the web interface service starts with the system | `scripts/utils/start_web_conditionally.py` |
| `timezone` | string, `"America/New_York"` | IANA timezone for schedules and displays | `ConfigManager.get_timezone()` | | `timezone` | string, `"America/New_York"` | IANA timezone for schedules and displays | `ConfigManager.get_timezone()` |
| `target_fps` | int, `100` | Frame-rate ceiling for plugin rendering | `src/plugin_system/base_plugin.py`, `src/common/sports_scroll.py` | | `target_fps` | int, `100` | Frame-rate ceiling for plugin rendering | `src/plugin_system/base_plugin.py`, `src/common/sports_scroll.py` |
| `location` | object | `city` / `state` / `country`, offered to plugins that need a location (weather, etc.) | plugins via merged config | | `location` | object | `city` / `state` / `country`. Supplies the **default** for a plugin's own `location_city` / `location_state` / `location_country` setting, so weather, radar and friends follow this device without being configured twice. A value saved on the plugin itself still overrides it. | `SchemaManager.apply_device_location()`, then plugins via merged config |
## `schedule` — display on/off hours ## `schedule` — display on/off hours
+15 -2
View File
@@ -145,8 +145,7 @@ class SportsUpcoming(SportsCore):
if (game['home_abbr'] in self.favorite_teams or if (game['home_abbr'] in self.favorite_teams or
game['away_abbr'] in self.favorite_teams): game['away_abbr'] in self.favorite_teams):
favorite_games_found += 1 favorite_games_found += 1
if self.show_odds: # Odds are NOT fetched here -- see after selection below.
self._fetch_odds(game)
# Enhanced logging for debugging # Enhanced logging for debugging
self.logger.info(f"Found {all_upcoming_games} total upcoming games in data") self.logger.info(f"Found {all_upcoming_games} total upcoming games in data")
@@ -190,6 +189,20 @@ class SportsUpcoming(SportsCore):
# Limit to the specified number of upcoming games # Limit to the specified number of upcoming games
team_games = team_games[:self.upcoming_games_to_show] team_games = team_games[:self.upcoming_games_to_show]
# Odds are fetched here, for the games that survived selection,
# rather than in the loop that collects them. That loop walks every
# upcoming game in the schedule window, and for a college league
# the window is enormous -- a live rig logged 946 upcoming games in
# one cycle and displayed 1 of them. The comment up there claimed
# odds were fetched "only for games that will be displayed", but
# the only narrowing it applied was show_favorite_teams_only, which
# is not the default; in the usual case nothing narrowed it at all
# and every game cost a separate ESPN request on a Pi that is also
# driving the panel.
if self.show_odds:
for game in team_games:
self._fetch_odds(game)
# Log changes or periodically # Log changes or periodically
should_log = ( should_log = (
current_time - self.last_log_time >= self.log_interval or current_time - self.last_log_time >= self.log_interval or
+58 -13
View File
@@ -71,11 +71,15 @@ class PluginManager:
self.plugin_loader = PluginLoader(logger=self.logger) self.plugin_loader = PluginLoader(logger=self.logger)
self.plugin_executor = PluginExecutor(default_timeout=30.0, logger=self.logger) self.plugin_executor = PluginExecutor(default_timeout=30.0, logger=self.logger)
self.state_manager = PluginStateManager(logger=self.logger) self.state_manager = PluginStateManager(logger=self.logger)
self.schema_manager = SchemaManager(plugins_dir=self.plugins_dir, logger=self.logger) self.schema_manager = SchemaManager(plugins_dir=self.plugins_dir, logger=self.logger,
config_manager=self.config_manager)
# Lock protecting plugin_manifests and plugin_directories from # Lock protecting plugin_manifests and plugin_directories from
# concurrent mutation (background reconciliation) and reads (requests). # concurrent mutation (background reconciliation) and reads (requests).
self._discovery_lock = threading.RLock() self._discovery_lock = threading.RLock()
#: Directories already reported as unloadable, so the warning is
#: emitted once rather than on every discovery scan.
self._skip_reported: set = set()
# Lock protecting plugin_last_update from concurrent mutation/iteration. # Lock protecting plugin_last_update from concurrent mutation/iteration.
# It's written from run_scheduled_updates()/update_all_plugins() (main # It's written from run_scheduled_updates()/update_all_plugins() (main
@@ -195,18 +199,59 @@ class PluginManager:
continue continue
manifest_path = item / "manifest.json" manifest_path = item / "manifest.json"
if manifest_path.exists(): if not manifest_path.exists():
try: # Once per directory per process. Discovery runs on every
with open(manifest_path, 'r', encoding='utf-8') as f: # web UI page load and every config reconcile, so warning
manifest = json.load(f) # unconditionally would put a line in the journal each
plugin_id = manifest.get('id') # time someone opened a page -- the same log-volume
if plugin_id: # problem this is meant to help diagnose.
plugin_ids.append(plugin_id) # A directory here that carries no manifest is not a
new_manifests[plugin_id] = manifest # plugin. Said once, because the alternative is a plugin
new_directories[plugin_id] = item # that is enabled in config, enabled in plugin state,
except (json.JSONDecodeError, PermissionError, OSError) as e: # present on disk, and simply absent from the running
self.logger.warning("Error reading manifest from %s: %s", manifest_path, e, exc_info=True) # process with nothing anywhere to say why. Working that
continue # out afterwards means reading cache-file mtimes.
if item.name not in self._skip_reported:
self._skip_reported.add(item.name)
self.logger.warning(
"Skipping %s: no manifest.json, so it cannot be "
"loaded as a plugin", item.name)
continue
try:
with open(manifest_path, 'r', encoding='utf-8') as f:
manifest = json.load(f)
except (json.JSONDecodeError, PermissionError, OSError) as e:
self.logger.warning("Error reading manifest from %s: %s", manifest_path, e, exc_info=True)
continue
# json.load accepts any JSON value, so a manifest holding
# null, [] or "text" parses and then raises AttributeError on
# .get(). Nothing here catches that -- the outer handler takes
# OSError/PermissionError only -- so a single malformed
# manifest aborted the whole scan and every other plugin on
# disk, however healthy, silently failed to register.
if not isinstance(manifest, dict):
if item.name not in self._skip_reported:
self._skip_reported.add(item.name)
self.logger.warning(
"Skipping %s: its manifest.json is %s, not a JSON "
"object", item.name, type(manifest).__name__)
continue
plugin_id = manifest.get('id')
if not plugin_id:
# Parsed but unusable. This was the quietest path of all:
# the manifest is read successfully and then dropped.
if item.name not in self._skip_reported:
self._skip_reported.add(item.name)
self.logger.warning(
"Skipping %s: its manifest.json has no \"id\", so "
"there is nothing to register it under", item.name)
continue
plugin_ids.append(plugin_id)
new_manifests[plugin_id] = manifest
new_directories[plugin_id] = item
except (OSError, PermissionError) as e: except (OSError, PermissionError) as e:
self.logger.error("Error scanning directory %s: %s", directory, e, exc_info=True) self.logger.error("Error scanning directory %s: %s", directory, e, exc_info=True)
+87 -4
View File
@@ -26,7 +26,25 @@ class SchemaManager:
- Cache invalidation on plugin changes - Cache invalidation on plugin changes
""" """
def __init__(self, plugins_dir: Optional[Path] = None, project_root: Optional[Path] = None, logger: Optional[logging.Logger] = None): # Plugin config keys that mean "where this device is". A plugin declaring
# any of these in its schema gets the device-wide ``location`` block from
# config.json as the *default* for that field, instead of whatever city the
# plugin author happened to ship. A value the user set on the plugin itself
# always wins -- this only ever replaces the schema default, so an explicit
# per-plugin location is still honoured.
#
# Only these fully-namespaced keys are substituted. A bare ``state`` or
# ``city`` key is deliberately left alone: plugins use those for unrelated
# things (ledmatrix-elections' ``state`` is a two-letter code, not a place
# name), and silently rewriting them would break those plugins.
DEVICE_LOCATION_KEYS: Dict[str, str] = {
'location_city': 'city',
'location_state': 'state',
'location_country': 'country',
}
def __init__(self, plugins_dir: Optional[Path] = None, project_root: Optional[Path] = None,
logger: Optional[logging.Logger] = None, config_manager: Optional[Any] = None):
""" """
Initialize the Schema Manager. Initialize the Schema Manager.
@@ -34,10 +52,14 @@ class SchemaManager:
plugins_dir: Base plugins directory path plugins_dir: Base plugins directory path
project_root: Project root directory path project_root: Project root directory path
logger: Optional logger instance logger: Optional logger instance
config_manager: Optional config manager, used to resolve the
device-wide ``location`` that seeds plugin location defaults.
Omitting it simply leaves schema defaults untouched.
""" """
self.logger = logger or logging.getLogger(__name__) self.logger = logger or logging.getLogger(__name__)
self.plugins_dir = plugins_dir self.plugins_dir = plugins_dir
self.project_root = project_root or Path.cwd() self.project_root = project_root or Path.cwd()
self.config_manager = config_manager
# Schema cache: plugin_id -> schema dict # Schema cache: plugin_id -> schema dict
self._schema_cache: Dict[str, Dict[str, Any]] = {} self._schema_cache: Dict[str, Dict[str, Any]] = {}
@@ -212,10 +234,70 @@ class SchemaManager:
return defaults return defaults
def get_device_location(self) -> Optional[Dict[str, Any]]:
"""
Return the device-wide ``location`` block from config.json, or None.
This is the City/State/Country the user sets once under General
settings. Returns None when there is no config manager wired, the
config can't be read, or no location has been configured.
"""
if self.config_manager is None:
return None
try:
config = self.config_manager.load_config()
except Exception as e:
# A config that can't be read must never stop defaults being
# generated -- the plugin's own schema defaults still apply.
self.logger.debug(f"Could not read device location from config: {e}")
return None
if not isinstance(config, dict):
return None
location = config.get('location')
return location if isinstance(location, dict) else None
def apply_device_location(self, defaults: Dict[str, Any]) -> Dict[str, Any]:
"""
Replace location-shaped schema defaults with the device's own location.
Without this, a plugin that ships ``"location_city": "Dallas"`` as its
schema default silently reports Dallas weather (and centres its radar
there) for every user who never opened that plugin's config form --
even though they set their real city under General settings. The
substituted value is still only a *default*: ``merge_with_defaults``
lets any per-plugin value the user saved win over it.
Mutates and returns ``defaults`` for convenience.
"""
if not defaults:
return defaults
if not any(key in defaults for key in self.DEVICE_LOCATION_KEYS):
return defaults
location = self.get_device_location()
if not location:
return defaults
for key, field in self.DEVICE_LOCATION_KEYS.items():
if key not in defaults:
continue
value = location.get(field)
# Only a non-empty string is a real answer; a blank or missing
# field means "not configured", which leaves the schema default.
if isinstance(value, str) and value.strip():
defaults[key] = value.strip()
return defaults
def generate_default_config(self, plugin_id: str, use_cache: bool = True) -> Dict[str, Any]: def generate_default_config(self, plugin_id: str, use_cache: bool = True) -> Dict[str, Any]:
""" """
Generate default configuration for a plugin from its schema. Generate default configuration for a plugin from its schema.
Location fields (see ``DEVICE_LOCATION_KEYS``) default to the device's
configured location rather than the plugin author's. That substitution
is applied on the way out rather than being cached, so changing the
device location takes effect without invalidating the defaults cache.
Args: Args:
plugin_id: Plugin identifier plugin_id: Plugin identifier
use_cache: If True, return cached defaults if available use_cache: If True, return cached defaults if available
@@ -225,7 +307,7 @@ class SchemaManager:
""" """
# Check cache first # Check cache first
if use_cache and plugin_id in self._defaults_cache: if use_cache and plugin_id in self._defaults_cache:
return self._defaults_cache[plugin_id].copy() return self.apply_device_location(self._defaults_cache[plugin_id].copy())
schema = self.load_schema(plugin_id, use_cache=use_cache) schema = self.load_schema(plugin_id, use_cache=use_cache)
if not schema: if not schema:
@@ -249,10 +331,11 @@ class SchemaManager:
if 'live_priority' not in defaults: if 'live_priority' not in defaults:
defaults['live_priority'] = schema.get('properties', {}).get('live_priority', {}).get('default', False) defaults['live_priority'] = schema.get('properties', {}).get('live_priority', {}).get('default', False)
# Cache the defaults # Cache the defaults *before* the device location is layered on, so a
# later change to the device location is picked up by the next call.
self._defaults_cache[plugin_id] = defaults.copy() self._defaults_cache[plugin_id] = defaults.copy()
return defaults return self.apply_device_location(defaults)
def validate_config_against_schema(self, config: Dict[str, Any], schema: Dict[str, Any], def validate_config_against_schema(self, config: Dict[str, Any], schema: Dict[str, Any],
plugin_id: Optional[str] = None) -> Tuple[bool, List[str]]: plugin_id: Optional[str] = None) -> Tuple[bool, List[str]]:
+8 -2
View File
@@ -89,11 +89,17 @@ class TestContextualFormatter:
assert "hello" in out assert "hello" in out
def test_location_toggle(self): def test_location_toggle(self):
# Assert on the whole "module.func:lineno" token, not a bare ":42".
# The formatted line starts with an HH:MM:SS timestamp, so a bare
# ":{lineno}" also matches the clock whenever the minute or second
# happens to equal the line number -- about 3% of runs, which is a
# flaky failure with nothing wrong.
record = make_record() record = make_record()
location = f"{record.module}.{record.funcName}:{record.lineno}"
with_loc = ContextualFormatter(include_location=True).format(record) with_loc = ContextualFormatter(include_location=True).format(record)
without = ContextualFormatter(include_location=False).format(record) without = ContextualFormatter(include_location=False).format(record)
assert f":{record.lineno}" in with_loc assert location in with_loc
assert f":{record.lineno}" not in without assert location not in without
def test_record_not_mutated_no_double_prefix(self): def test_record_not_mutated_no_double_prefix(self):
# Regression: a record is formatted once PER HANDLER. The formatter # Regression: a record is formatted once PER HANDLER. The formatter
+126
View File
@@ -0,0 +1,126 @@
#!/usr/bin/env python3
"""Discovery must say when it skips a directory.
A plugin can be enabled in config, enabled in plugin state, present on disk
with a valid entry point -- and simply absent from the running process, with
nothing in the journal to say why. Working that out afterwards meant comparing
cache-file mtimes to find when it had last run.
Two paths were silent. A directory with no manifest.json was ignored, and --
quieter still -- a manifest that parsed but carried no "id" was read
successfully and then dropped on the floor.
"""
import json
import logging
import sys
from pathlib import Path
from unittest.mock import MagicMock
import pytest
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
from src.plugin_system.plugin_manager import PluginManager # noqa: E402
def _manager(tmp_path):
pm = PluginManager.__new__(PluginManager)
pm.plugins_dir = tmp_path
pm.logger = logging.getLogger("test.discovery")
pm.plugin_manifests = {}
pm.plugin_directories = {}
pm._discovery_lock = __import__("threading").RLock()
pm._skip_reported = set()
pm.schema_manager = MagicMock()
return pm
def test_a_directory_without_a_manifest_is_reported(tmp_path, caplog):
(tmp_path / "not-a-plugin").mkdir()
pm = _manager(tmp_path)
with caplog.at_level(logging.WARNING, logger="test.discovery"):
pm._scan_directory_for_plugins(tmp_path)
joined = " ".join(r.message for r in caplog.records)
assert "not-a-plugin" in joined and "manifest" in joined, (
f"skip was silent; log said: {joined!r}")
def test_a_manifest_without_an_id_is_reported(tmp_path, caplog):
d = tmp_path / "idless"
d.mkdir()
(d / "manifest.json").write_text(json.dumps({"name": "No Id", "version": "1.0.0"}))
pm = _manager(tmp_path)
with caplog.at_level(logging.WARNING, logger="test.discovery"):
pm._scan_directory_for_plugins(tmp_path)
joined = " ".join(r.message for r in caplog.records)
assert "idless" in joined and "id" in joined, (
f"a parsed-but-unusable manifest vanished silently; log said: {joined!r}")
def test_a_good_plugin_still_registers(tmp_path, caplog):
d = tmp_path / "real-plugin"
d.mkdir()
(d / "manifest.json").write_text(json.dumps(
{"id": "real-plugin", "name": "Real", "version": "1.0.0"}))
pm = _manager(tmp_path)
pm._scan_directory_for_plugins(tmp_path)
assert "real-plugin" in pm.plugin_manifests, "a valid plugin was not registered"
def test_the_warning_does_not_repeat_on_every_scan(tmp_path, caplog):
"""Discovery runs on every web UI page load and every config reconcile.
Warning unconditionally would put a line in the journal each time someone
opened a page -- the same log-volume problem this is meant to help
diagnose.
"""
(tmp_path / "not-a-plugin").mkdir()
pm = _manager(tmp_path)
with caplog.at_level(logging.WARNING, logger="test.discovery"):
for _ in range(5):
pm._scan_directory_for_plugins(tmp_path)
hits = [r for r in caplog.records if "not-a-plugin" in r.message]
assert len(hits) == 1, f"warned {len(hits)} times across 5 scans"
def _plugin(tmp_path, name, body):
d = tmp_path / name
d.mkdir()
(d / "manifest.json").write_text(json.dumps(body))
return d
VALID = {"name": "V", "version": "1.0.0", "class_name": "X", "display_modes": ["m"]}
@pytest.mark.parametrize("body", [None, [1, 2], "not an object", 42, True])
def test_a_manifest_that_is_not_an_object_is_skipped_not_fatal(tmp_path, caplog, body):
"""json.load accepts any JSON value, not just objects.
manifest.get('id') then raised AttributeError, which nothing here caught --
the outer handler takes OSError/PermissionError only. A single malformed
manifest aborted the entire scan, so every other plugin on disk, however
healthy, silently failed to register.
"""
_plugin(tmp_path, "aaa-good", dict(VALID, id="aaa-good"))
_plugin(tmp_path, "mmm-bad", body)
_plugin(tmp_path, "zzz-good", dict(VALID, id="zzz-good"))
pm = _manager(tmp_path)
with caplog.at_level(logging.WARNING, logger="test.discovery"):
found = pm._scan_directory_for_plugins(tmp_path)
assert sorted(found) == ["aaa-good", "zzz-good"], (
"one unusable manifest took the healthy plugins down with it")
joined = " ".join(r.message for r in caplog.records)
assert "mmm-bad" in joined, f"the skip was silent; log said: {joined!r}"
def test_the_bad_manifest_is_named_with_what_it_actually_was(tmp_path, caplog):
_plugin(tmp_path, "listy", [1, 2])
pm = _manager(tmp_path)
with caplog.at_level(logging.WARNING, logger="test.discovery"):
pm._scan_directory_for_plugins(tmp_path)
joined = " ".join(r.message for r in caplog.records)
assert "listy" in joined and "list" in joined, (
f"the warning does not say what the manifest was: {joined!r}")
+179
View File
@@ -0,0 +1,179 @@
"""
Tests for the device-location default: a plugin that ships a location field in
its schema must default to the device's configured City/State/Country, not to
whatever place the plugin author hard-coded.
The bug this pins: ledmatrix-weather ships ``"location_city": "Dallas"`` as a
schema default, so a user who set Kansas City under General settings but never
opened the weather plugin's own config form got Dallas weather — and a radar
centred on Dallas with nothing in config.json to explain it.
"""
import json
import pytest
from src.plugin_system.schema_manager import SchemaManager
class FakeConfigManager:
"""Minimal stand-in exposing the load_config() SchemaManager relies on."""
def __init__(self, config):
self.config = config
self.load_count = 0
def load_config(self):
self.load_count += 1
return self.config
class ExplodingConfigManager:
def load_config(self):
raise OSError("config.json is unreadable")
WEATHER_SCHEMA = {
"type": "object",
"properties": {
"location_city": {"type": "string", "default": "Dallas"},
"location_state": {"type": "string", "default": "Texas"},
"location_country": {"type": "string", "default": "US"},
"units": {"type": "string", "default": "imperial"},
},
}
def write_plugin(plugins_dir, plugin_id, schema):
plugin_dir = plugins_dir / plugin_id
plugin_dir.mkdir(parents=True, exist_ok=True)
(plugin_dir / "config_schema.json").write_text(json.dumps(schema))
return plugin_dir
@pytest.fixture
def plugins_dir(tmp_path):
d = tmp_path / "plugin-repos"
d.mkdir()
return d
def make_sm(plugins_dir, tmp_path, location):
config = {} if location is None else {"location": location}
cm = FakeConfigManager(config)
sm = SchemaManager(plugins_dir=plugins_dir, project_root=tmp_path,
config_manager=cm)
return sm, cm
class TestDeviceLocationDefaults:
def test_device_location_replaces_plugin_default(self, plugins_dir, tmp_path):
write_plugin(plugins_dir, "ledmatrix-weather", WEATHER_SCHEMA)
sm, _ = make_sm(plugins_dir, tmp_path,
{"city": "Kansas City", "state": "Missouri", "country": "US"})
defaults = sm.generate_default_config("ledmatrix-weather")
assert defaults["location_city"] == "Kansas City"
assert defaults["location_state"] == "Missouri"
assert defaults["location_country"] == "US"
# Non-location defaults are untouched.
assert defaults["units"] == "imperial"
def test_user_set_plugin_value_still_wins(self, plugins_dir, tmp_path):
write_plugin(plugins_dir, "ledmatrix-weather", WEATHER_SCHEMA)
sm, _ = make_sm(plugins_dir, tmp_path,
{"city": "Kansas City", "state": "Missouri", "country": "US"})
defaults = sm.generate_default_config("ledmatrix-weather")
merged = sm.merge_with_defaults({"location_city": "Denver"}, defaults)
assert merged["location_city"] == "Denver"
# Fields the user did not override still follow the device.
assert merged["location_state"] == "Missouri"
def test_blank_and_missing_device_fields_leave_schema_default(self, plugins_dir, tmp_path):
write_plugin(plugins_dir, "ledmatrix-weather", WEATHER_SCHEMA)
sm, _ = make_sm(plugins_dir, tmp_path, {"city": "Kansas City", "state": " "})
defaults = sm.generate_default_config("ledmatrix-weather")
assert defaults["location_city"] == "Kansas City"
assert defaults["location_state"] == "Texas" # blank -> not configured
assert defaults["location_country"] == "US" # absent -> schema default
def test_no_device_location_configured_is_a_no_op(self, plugins_dir, tmp_path):
write_plugin(plugins_dir, "ledmatrix-weather", WEATHER_SCHEMA)
sm, _ = make_sm(plugins_dir, tmp_path, None)
defaults = sm.generate_default_config("ledmatrix-weather")
assert defaults["location_city"] == "Dallas"
def test_no_config_manager_is_a_no_op(self, plugins_dir, tmp_path):
write_plugin(plugins_dir, "ledmatrix-weather", WEATHER_SCHEMA)
sm = SchemaManager(plugins_dir=plugins_dir, project_root=tmp_path)
assert sm.generate_default_config("ledmatrix-weather")["location_city"] == "Dallas"
def test_unreadable_config_falls_back_to_schema_defaults(self, plugins_dir, tmp_path):
write_plugin(plugins_dir, "ledmatrix-weather", WEATHER_SCHEMA)
sm = SchemaManager(plugins_dir=plugins_dir, project_root=tmp_path,
config_manager=ExplodingConfigManager())
assert sm.generate_default_config("ledmatrix-weather")["location_city"] == "Dallas"
class TestScopedToNamespacedKeys:
def test_bare_state_key_is_not_rewritten(self, plugins_dir, tmp_path):
"""ledmatrix-elections' ``state`` is a two-letter code, not a place name."""
write_plugin(plugins_dir, "ledmatrix-elections", {
"type": "object",
"properties": {
"state": {"type": "string", "default": "CA"},
"city": {"type": "string", "default": "Springfield"},
},
})
sm, _ = make_sm(plugins_dir, tmp_path,
{"city": "Kansas City", "state": "Missouri", "country": "US"})
defaults = sm.generate_default_config("ledmatrix-elections")
assert defaults["state"] == "CA"
assert defaults["city"] == "Springfield"
def test_plugin_without_location_fields_never_reads_config(self, plugins_dir, tmp_path):
write_plugin(plugins_dir, "clock-simple", {
"type": "object",
"properties": {"format": {"type": "string", "default": "12h"}},
})
sm, cm = make_sm(plugins_dir, tmp_path, {"city": "Kansas City"})
defaults = sm.generate_default_config("clock-simple")
assert defaults["format"] == "12h"
assert cm.load_count == 0
class TestCachingStaysFresh:
def test_location_change_is_picked_up_through_the_defaults_cache(self, plugins_dir, tmp_path):
write_plugin(plugins_dir, "ledmatrix-weather", WEATHER_SCHEMA)
sm, cm = make_sm(plugins_dir, tmp_path, {"city": "Kansas City"})
assert sm.generate_default_config("ledmatrix-weather")["location_city"] == "Kansas City"
cm.config["location"]["city"] = "Omaha"
# Second call is served from the defaults cache, but must not serve a
# stale location.
assert sm.generate_default_config("ledmatrix-weather")["location_city"] == "Omaha"
def test_cached_defaults_are_not_mutated_by_the_overlay(self, plugins_dir, tmp_path):
write_plugin(plugins_dir, "ledmatrix-weather", WEATHER_SCHEMA)
sm, cm = make_sm(plugins_dir, tmp_path, {"city": "Kansas City"})
sm.generate_default_config("ledmatrix-weather")
assert sm._defaults_cache["ledmatrix-weather"]["location_city"] == "Dallas"
cm.config.pop("location")
assert sm.generate_default_config("ledmatrix-weather")["location_city"] == "Dallas"
+136
View File
@@ -0,0 +1,136 @@
"""Odds must be fetched for the games shown, not every game in the window.
SportsUpcoming.update() collected every upcoming game in the schedule window
and called _fetch_odds() on each one *inside* that collection loop, narrowing
to upcoming_games_to_show only afterwards. The comment there said odds were
fetched "only for games that will be displayed", but the sole narrowing it
applied was show_favorite_teams_only, which is not the default -- so in the
usual configuration nothing narrowed it at all.
Measured on a live rig: a college league produced 946 upcoming games in one
cycle and displayed 1 of them. The same shape on the football plugin produced
a burst of 467 sequential ESPN requests that ran for 35s and blew that
plugin's 30s update budget, and it repeats every time the 1h odds TTL expires.
SportsLive is deliberately different: it walks the raw event list because it
has to find which games are live, but only fetches odds for a game that has
already passed the is_live/is_halftime test, so the fan-out is bounded by how
many games are actually in progress.
"""
import ast
from pathlib import Path
import pytest
MODES = (Path(__file__).resolve().parent.parent
/ "src" / "base_classes" / "sports" / "modes.py")
TREE = ast.parse(MODES.read_text(encoding="utf-8"))
def _fetch_sites():
"""(class name, method name, lineno) for every self._fetch_odds(...) call."""
calls = [n.lineno for n in ast.walk(TREE)
if isinstance(n, ast.Call) and isinstance(n.func, ast.Attribute)
and n.func.attr == "_fetch_odds"]
sites = []
for cls in [n for n in ast.walk(TREE) if isinstance(n, ast.ClassDef)]:
for fn in [n for n in cls.body if isinstance(n, ast.FunctionDef)]:
for lineno in calls:
if fn.lineno <= lineno <= (fn.end_lineno or fn.lineno):
sites.append((cls.name, fn.name, lineno))
assert len(sites) == len(calls), "a _fetch_odds call sits outside any method"
return sites
def _innermost_loop_iterable(lineno):
best = None
for node in ast.walk(TREE):
if isinstance(node, ast.For) and \
node.lineno <= lineno <= (node.end_lineno or node.lineno):
if best is None or node.lineno > best.lineno:
best = node
return None if best is None else ast.unparse(best.iter)
def _spans(body, lineno):
"""True when `lineno` falls inside this list of statements."""
return any(n.lineno <= lineno <= (n.end_lineno or n.lineno) for n in body)
def _parents(tree):
table = {}
for node in ast.walk(tree):
for child in ast.iter_child_nodes(node):
table[child] = node
return table
PARENTS = _parents(TREE)
def _mentions_positively(test, names):
"""True when `test` references every name, none of them under a `not`.
Structural, not textual. Matching the unparsed source would accept
`not (details["is_live"] or details["is_halftime"])` -- which selects
exactly the non-live games this guard exists to exclude -- because the
names still appear in the text.
"""
found = set()
for node in ast.walk(test):
if not (isinstance(node, ast.Constant) and node.value in names):
continue
negated = False
cursor = node
while cursor is not test and cursor in PARENTS:
cursor = PARENTS[cursor]
if isinstance(cursor, ast.UnaryOp) and isinstance(cursor.op, ast.Not):
negated = True
break
if not negated:
found.add(node.value)
return found >= set(names)
def _guarded_by_positive(lineno, names):
"""True when some enclosing `if` runs this line only if `names` hold.
Only the TRUE branch counts: an `if` whose `else` contains the call would
otherwise look like a guard while doing the opposite.
"""
for node in ast.walk(TREE):
if isinstance(node, ast.If) and _spans(node.body, lineno) \
and _mentions_positively(node.test, names):
return True
return False
def test_every_fetch_site_is_accounted_for():
"""A new call site must be classified deliberately, not inherited silently."""
found = {(cls, fn) for cls, fn, _ in _fetch_sites()}
assert found == {("SportsUpcoming", "update"), ("SportsLive", "update")}, (
f"unexpected _fetch_odds call sites: {sorted(found)}. Each one is a "
"sequential ESPN request per game -- classify it here on purpose.")
def test_upcoming_fetches_only_the_selected_games():
for cls, _fn, lineno in _fetch_sites():
if cls != "SportsUpcoming":
continue
iterable = _innermost_loop_iterable(lineno)
assert iterable == "team_games", (
f"SportsUpcoming._fetch_odds at line {lineno} iterates over "
f"{iterable!r}. It must run over team_games -- already narrowed to "
"upcoming_games_to_show -- not over every event in the schedule "
"window. Each item costs one sequential ESPN request.")
def test_live_only_fetches_for_games_actually_in_progress():
for cls, _fn, lineno in _fetch_sites():
if cls != "SportsLive":
continue
assert _guarded_by_positive(lineno, {"is_live", "is_halftime"}), (
f"SportsLive._fetch_odds at line {lineno} does not sit in the true "
"branch of a test requiring the game to be in progress. Without "
"that, it fans out across the whole event list -- one sequential "
"ESPN request per game.")
+2 -1
View File
@@ -118,7 +118,8 @@ saved_repositories_manager = SavedRepositoriesManager()
schema_manager = SchemaManager( schema_manager = SchemaManager(
plugins_dir=plugins_dir, plugins_dir=plugins_dir,
project_root=project_root, project_root=project_root,
logger=None logger=None,
config_manager=config_manager
) )
# Initialize operation queue for plugin operations # Initialize operation queue for plugin operations
@@ -95,7 +95,7 @@
<!-- Location Information --> <!-- Location Information -->
<div class="grid grid-cols-1 md:grid-cols-3 xl:grid-cols-3 2xl:grid-cols-3 gap-4"> <div class="grid grid-cols-1 md:grid-cols-3 xl:grid-cols-3 2xl:grid-cols-3 gap-4">
<div class="form-group" id="setting-general-city" data-setting-key="location.city"> <div class="form-group" id="setting-general-city" data-setting-key="location.city">
<label for="city" class="block text-sm font-medium text-gray-700">City{{ ui.help_tip('City used for weather, sunrise/sunset, and other location-based content.\nExample: Dallas.', 'City') }}</label> <label for="city" class="block text-sm font-medium text-gray-700">City{{ ui.help_tip('City used for weather, sunrise/sunset, radar, and other location-based content.\nExample: Kansas City.\nUsed as the default for the location_city setting on plugins that have one; a value saved on the plugin itself overrides it.', 'City') }}</label>
<input type="text" <input type="text"
id="city" id="city"
name="city" name="city"
@@ -104,7 +104,7 @@
</div> </div>
<div class="form-group" id="setting-general-state" data-setting-key="location.state"> <div class="form-group" id="setting-general-state" data-setting-key="location.state">
<label for="state" class="block text-sm font-medium text-gray-700">State{{ ui.help_tip('State or region for your location.\nExample: Texas. Improves location-lookup accuracy.', 'State') }}</label> <label for="state" class="block text-sm font-medium text-gray-700">State{{ ui.help_tip('State or region for your location.\nExample: Missouri. Improves location-lookup accuracy.\nUsed as the default for the location_state setting on plugins that have one.', 'State') }}</label>
<input type="text" <input type="text"
id="state" id="state"
name="state" name="state"
@@ -113,7 +113,7 @@
</div> </div>
<div class="form-group" id="setting-general-country" data-setting-key="location.country"> <div class="form-group" id="setting-general-country" data-setting-key="location.country">
<label for="country" class="block text-sm font-medium text-gray-700">Country{{ ui.help_tip('Country code or name for your location.\nExample: US. Used with City and State for weather and geolocation.', 'Country') }}</label> <label for="country" class="block text-sm font-medium text-gray-700">Country{{ ui.help_tip('Country code or name for your location.\nExample: US. Used with City and State for weather, radar, and geolocation.\nUsed as the default for the location_country setting on plugins that have one.', 'Country') }}</label>
<input type="text" <input type="text"
id="country" id="country"
name="country" name="country"