fix(backup): address CodeRabbit and CodeQL findings on PR #439

- first_time_install.sh: verify chown/chmod succeed and the final
  owner/group/mode on config_secrets.json before reporting success;
  exit with a clear error otherwise instead of swallowing failures.
- api_v3.py: replace the predictable .writetest probe with an
  exclusive NamedTemporaryFile to avoid a race with concurrent
  resolvers; log the preferred/fallback export path and OSError when
  falling back to the reinstall-deleted directory.
- api_v3.py: mark a restore as failed when plugin reinstalls fail,
  even if file restoration itself succeeded, so the endpoint no longer
  reports HTTP 200 success on a partial restore.
- api_v3.py: stringify plugin IDs before joining them into the error
  message so a malformed backup's non-string plugin_id can't raise a
  TypeError and mask the detailed response.
- backup_manager.py / api_v3.py: stop putting raw exception text (originating
  from a user-controlled backup file) into restore results returned to
  the client; log full details server-side instead. Addresses the
  CodeQL "stack trace information exposure" alert.
- test coverage: add a test for get_plugin_info() resolving a
  manifest id, and assert the disabled restore_wifi path also skips
  and omits ytm_auth.json.

.github/workflows/test.yml

@@ -4,9 +4,6 @@ on:
   pull_request:
   push:
     branches: [main]
-  # Manual runs against any branch — useful when a PR's automatic run
-  # needs a re-run or didn't get created.
-  workflow_dispatch:
 
 # Both jobs only check out the repo and run pytest.
 permissions:
@@ -16,12 +13,6 @@ jobs:
   plugin-safety:
     name: Plugin safety harness + unit tests
     runs-on: ubuntu-latest
-    env:
-      # The bundled fixture plugin gives the harness at least one real plugin
-      # to render, and REQUIRE_PLUGINS turns "discovered zero plugins" into a
-      # hard failure instead of a silent all-skip green run.
-      LEDMATRIX_PLUGINS_DIR: test/fixtures/plugins
-      LEDMATRIX_REQUIRE_PLUGINS: "1"
     steps:
       - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
         with:
@@ -38,9 +29,12 @@ jobs:
           pip install -r requirements.txt -r requirements-test.txt
           pip install RGBMatrixEmulator
 
-      - name: Run plugin safety harness
+      - name: Run harness + visual rendering tests
         run: |
-          pytest --no-cov test/plugins/
+          pytest --no-cov \
+            test/plugins/test_harness.py \
+            test/plugins/test_visual_rendering.py \
+            test/plugins/test_plugin_matrix.py
 
   unit-tests:
     name: Core unit tests
@@ -61,15 +55,37 @@ jobs:
           pip install -r requirements.txt -r requirements-test.txt
           pip install RGBMatrixEmulator
 
-      # Run the ENTIRE test tree (except test/plugins, which the
+      # Safety net for the shared sports/scroll/style infrastructure. These
-      # plugin-safety job owns). New test files are enrolled automatically;
+      # suites existed but were not enrolled in CI, so a refactor of
-      # excluding anything requires a visible, commented --ignore here.
+      # src/base_classes or src/common could regress them silently. Enrolled
-      # Coverage is measured and enforced only in this step — pytest.ini
+      # explicitly (not `pytest test/`) so known hardware-only suites don't
-      # deliberately carries no coverage flags so local runs stay fast.
+      # break CI; grow this list as more suites are made headless.
       - name: Run core unit suites
         run: |
-          pytest -m "not hardware" test/ \
+          pytest --no-cov \
-            --ignore=test/plugins \
+            test/test_skin_system.py \
-            --cov=src --cov=web_interface \
+            test/test_font_manager.py \
-            --cov-report=term \
+            test/test_data_sources.py \
-            --cov-fail-under=48
+            test/test_api_extractors.py \
+            test/test_scroll_helper.py \
+            test/test_scroll_helper_continuous.py \
+            test/test_adaptive_layout.py \
+            test/test_loader_compat_warning.py \
+            test/test_sports_base_characterization.py \
+            test/test_element_style.py \
+            test/test_sports_core_promotions.py \
+            test/test_sports_modes_promotions.py \
+            test/test_sports_capabilities.py \
+            test/test_sports_scroll.py \
+            test/test_version_consistency.py \
+            test/test_plugin_compatibility_gate.py \
+            test/test_install_preserves_existing.py \
+            test/test_core_owned_config_keys.py \
+            test/test_async_plugin_updates.py \
+            test/test_plugin_update_reservation.py \
+            test/test_template_targets.py \
+            test/test_widget_scripts.py \
+            test/test_doc_links.py \
+            test/test_registry_id_resolution.py \
+            test/test_backup_manager.py \
+            test/web_interface/test_cache.py

docs/widget-guide.md

@@ -206,47 +206,6 @@ To use an existing widget in your plugin's `config_schema.json`, simply add the
 
 The widget will be automatically rendered when the plugin configuration form is loaded.
 
-## Labelling Enum Options (`x-options.labels`)
-
-A plain `enum` renders as a dropdown whose option text is the value with
-underscores replaced and title case applied — `day_first` becomes "Day First".
-That is fine for values that read as their own label, and wrong for values that
-do not: `vs` becomes "Vs", and `abbrev` says nothing about the `Sep 19` it
-actually produces.
-
-Supply `x-options.labels` to set the visible text. This is the same convention
-the `checkbox-group` widget uses:
-
-```json
-{
-  "date_format": {
-    "type": "string",
-    "enum": ["abbrev", "numeric", "day_first"],
-    "default": "abbrev",
-    "x-options": {
-      "labels": {
-        "abbrev": "Sep 19",
-        "numeric": "9/19",
-        "day_first": "19 Sep"
-      }
-    }
-  }
-}
-```
-
-Labels are **display only** — the stored value is still the enum value, so
-adding them never changes a saved config. The map may be partial: any value
-without a label keeps the humanised fallback. Older cores that predate this
-support ignore `x-options` and render the fallback for every option, so a
-plugin can ship labels without requiring a core upgrade.
-
-Array-table columns (`x-widget: array-table`) accept the same
-`x-options.labels` on a column definition, but their fallback is the **raw
-value** rather than the humanised one, because those columns hold values such
-as ticker symbols where `aapl` → "Aapl" would be wrong. Rows added in the
-browser use the labels too (`array-table.js`), so a column reads the same
-before and after a page reload.
-
 ## Marking Fields as Advanced (`x-advanced`)
 
 Add `"x-advanced": true` to any top-level, non-object property to move it out

pytest.ini

@@ -10,13 +10,16 @@ python_functions = test_*
 testpaths = test
 
 # Output options
-# Coverage is deliberately NOT configured here: a bare local `pytest` should
+# Note: Coverage options require pytest-cov to be installed
-# be fast and dependency-light. Coverage is measured and enforced in exactly
+# Run: pip install pytest-cov
-# one place — the unit-tests job in .github/workflows/test.yml.
 addopts = 
     -v
     --strict-markers
     --tb=short
+    --cov=src
+    --cov-report=term-missing
+    --cov-report=html
+    --cov-fail-under=30
 
 # Markers
 markers =

requirements-test.txt

@@ -4,6 +4,4 @@ pytest>=9.0.3,<10.0.0
 pytest-cov>=4.1.0,<5.0.0
 pytest-mock>=3.11.0,<4.0.0
 freezegun>=1.2,<2        # deterministic time for golden-image tests
-psutil>=6.0.0,<8.0.0     # optional at runtime; installed for tests so the
-                         # /system/status endpoint's real path is exercised
 mypy>=1.5.0,<2.0.0       # static type checking (also pinned in .pre-commit-config.yaml)

src/base_classes/sports/core.py

@@ -383,15 +383,10 @@ class SportsCore(ABC):
             ctx = skin_runtime.build_context(self, game, size=size)
             card = skin.render_vegas_card(ctx, dict(game))
             if card is not None:
-                # A successful render clears accumulated strikes, mirroring
-                # _render_game — transient failures must not add up across
-                # the session and disable a working skin.
-                self._skin_failures = 0
                 return card
             ctx = skin_runtime.build_context(self, game, size=size)
             render = getattr(skin, f"render_{self.SKIN_MODE}")
             if render(ctx, dict(game)):
-                self._skin_failures = 0
                 return ctx.canvas
         except Exception:
             # Card failures count toward the same 3-strike session disable

src/base_odds_manager.py

@@ -163,25 +163,19 @@ class BaseOddsManager:
             item = data["items"][0]
             self.logger.debug(f"First item keys: {list(item.keys())}")
             
-            # The ESPN API returns odds data directly in the item, not in a
+            # The ESPN API returns odds data directly in the item, not in a providers array
-            # providers array. ESPN sends explicit JSON nulls for absent
+            # Extract the odds data directly from the item
-            # sides ("homeTeamOdds": null), so every level uses `or {}` —
-            # .get's default only applies when the key is missing entirely.
-            home = item.get("homeTeamOdds") or {}
-            away = item.get("awayTeamOdds") or {}
             extracted_data = {
                 "details": item.get("details"),
                 "over_under": item.get("overUnder"),
                 "spread": item.get("spread"),
                 "home_team_odds": {
-                    "money_line": home.get("moneyLine"),
+                    "money_line": item.get("homeTeamOdds", {}).get("moneyLine"),
-                    "spread_odds": ((home.get("current") or {})
+                    "spread_odds": item.get("homeTeamOdds", {}).get("current", {}).get("pointSpread", {}).get("value")
-                                    .get("pointSpread") or {}).get("value")
                 },
                 "away_team_odds": {
-                    "money_line": away.get("moneyLine"),
+                    "money_line": item.get("awayTeamOdds", {}).get("moneyLine"),
-                    "spread_odds": ((away.get("current") or {})
+                    "spread_odds": item.get("awayTeamOdds", {}).get("current", {}).get("pointSpread", {}).get("value")
-                                    .get("pointSpread") or {}).get("value")
                 }
             }
             self.logger.debug(f"Returning extracted odds data: {json.dumps(extracted_data, indent=2)}")
@@ -266,11 +260,7 @@ class BaseOddsManager:
         Returns:
             Formatted odds summary string
         """
-        # Gate only on truly-empty / negative-cached data. is_odds_available
+        if not self.is_odds_available(odds_data):
-        # deliberately ignores money lines (its callers decide whether to
-        # RENDER an odds widget), but a summary of money-line-only odds is
-        # still meaningful — the parts loop below handles them.
-        if not odds_data or odds_data.get('no_odds'):
             return "No odds available"
         
         parts = []

src/common/api_helper.py

@@ -273,34 +273,19 @@ class APIHelper:
         """
         Clear cache data.
         
-        Uses CacheManager's real surface (clear_cache / delete /
-        list_cache_files); safely no-ops on managers without it. The old
-        implementation guarded on a nonexistent ``clear`` method, so it
-        silently never cleared anything.
-
         Args:
-            pattern: Optional substring to match cache keys; only matching
+            pattern: Optional pattern to match cache keys
-                entries are deleted.
         """
-        if not self.cache_manager:
+        if self.cache_manager:
-            return
+            if hasattr(self.cache_manager, 'clear'):
                 if pattern:
-            if (hasattr(self.cache_manager, 'list_cache_files')
+                    # Clear only keys matching pattern
-                    and hasattr(self.cache_manager, 'delete')):
+                    keys = self.cache_manager.keys()
-                for entry in self.cache_manager.list_cache_files():
+                    for key in keys:
-                    key = entry.get('key') if isinstance(entry, dict) else None
+                        if pattern in key:
-                    if key and pattern in key:
                             self.cache_manager.delete(key)
                 else:
-                self.logger.debug(
-                    "Cache manager lacks list_cache_files/delete; "
-                    "cannot clear by pattern")
-        elif hasattr(self.cache_manager, 'clear_cache'):
-            self.cache_manager.clear_cache()
-        elif hasattr(self.cache_manager, 'clear'):
                     self.cache_manager.clear()
-        else:
-            self.logger.debug("Cache manager exposes no clear method; no-op")
     
     def _get_from_cache(self, key: str) -> Optional[Any]:
         """Get data from cache."""

src/common/config_helper.py

@@ -5,7 +5,6 @@ Handles configuration management and validation for LED matrix plugins.
 Extracted from LEDMatrix core to provide reusable functionality for plugins.
 """
 
-import copy
 import json
 import logging
 from pathlib import Path
@@ -161,20 +160,17 @@ class ConfigHelper:
             override_config: Configuration to merge in (takes precedence)
             
         Returns:
-            Merged configuration dictionary (fully independent of both
+            Merged configuration dictionary
-            inputs — a shallow copy would alias un-overridden nested dicts,
-            so mutating the result would mutate the caller's base config).
         """
-        merged = copy.deepcopy(base_config)
+        merged = base_config.copy()
         
         for key, value in override_config.items():
             if key in merged and isinstance(merged[key], dict) and isinstance(value, dict):
                 # Recursively merge nested dictionaries
                 merged[key] = self.merge_configs(merged[key], value)
             else:
-                # Override with new value — deep-copied so mutating the
+                # Override with new value
-                # merged result can't reach back into override_config.
+                merged[key] = value
-                merged[key] = copy.deepcopy(value)
         
         return merged
     

src/common/display_helper.py

@@ -115,14 +115,18 @@ class DisplayHelper:
         if home_logo and away_logo:
             self._draw_logos(main_img, home_logo, away_logo)
         
-        # Draw one combined top line (period/status/clock all share y=1 —
+        # Draw status/period text (top center)
-        # drawing them separately overprinted each other).
+        if status_text or period_text:
-        top_line = " ".join(p for p in [period_text, status_text, clock] if p)
+            status_display = f"{period_text} {status_text}".strip()
-        if top_line:
+            if status_display:
-            self._draw_centered_text(draw, top_line,
+                self._draw_centered_text(draw, status_display, 
                                        fonts.get('time', fonts.get('status')), 
                                        y_position=1)
         
+        # Draw clock if available
+        if clock:
+            self._draw_centered_text(draw, clock, fonts.get('time'), y_position=1)
+        
         # Draw scores (center)
         score_text = f"{away_score}-{home_score}"
         self._draw_centered_text(draw, score_text, fonts.get('score'), 
@@ -149,18 +153,12 @@ class DisplayHelper:
         """
         Draw a ticker/scrolling text layout.
         
-        Renders a single static frame with the text at the left edge; the
-        caller advances the scroll by re-rendering or shifting. The
-        scroll_speed parameter is accepted for API compatibility but does
-        not affect this frame. (Previously the text was drawn starting at
-        x=display_width — entirely off-canvas — so every frame was blank.)
-
         Args:
             text: Text to display
             font: Font to use
             background_color: Background color
             text_color: Text color
-            scroll_speed: Accepted for compatibility; unused per-frame
+            scroll_speed: Pixels to scroll per frame
             
         Returns:
             PIL Image with ticker layout
@@ -168,7 +166,11 @@ class DisplayHelper:
         img = self.create_base_image(background_color)
         draw = ImageDraw.Draw(img)
         
-        self._draw_text_with_outline(draw, text, (0, self.display_height // 2 - 6),
+        # Start text off-screen to the right
+        x_position = self.display_width
+        
+        # Draw text
+        self._draw_text_with_outline(draw, text, (x_position, self.display_height // 2 - 6), 
                                    font, fill=text_color)
         
         return img
@@ -212,9 +214,15 @@ class DisplayHelper:
         Returns:
             PIL Image with error message
         """
-        # Dark red background, white text
+        img = self.create_base_image((50, 0, 0))  # Dark red background
+
+        # Use default font
         font = ImageFont.load_default()
-        return self.draw_centered_text(message, font, (50, 0, 0), (255, 255, 255))
+        
+        # Draw centered error message
+        self._draw_centered_text(message, font, (50, 0, 0), (255, 255, 255))
+        
+        return img
     
     def draw_no_data_message(self, message: str = "No Data") -> Image.Image:
         """
@@ -226,8 +234,11 @@ class DisplayHelper:
         Returns:
             PIL Image with no data message
         """
+        img = self.create_base_image((0, 0, 0))
         font = ImageFont.load_default()
-        return self.draw_centered_text(message, font, (0, 0, 0), (150, 150, 150))
+        self._draw_centered_text(message, font, (0, 0, 0), (150, 150, 150))
+        
+        return img
     
     def get_display_dimensions(self) -> Tuple[int, int]:
         """

src/config_manager.py

@@ -106,9 +106,14 @@ class ConfigManager:
         Returns:
             SaveResult with status and details
         """
-        # Load current secrets to preserve them (raises if unreadable — see
+        # Load current secrets to preserve them
-        # _load_secrets_for_save)
+        secrets_content = {}
-        secrets_content = self._load_secrets_for_save()
+        if os.path.exists(self.secrets_path):
+            try:
+                with open(self.secrets_path, 'r') as f_secrets:
+                    secrets_content = json.load(f_secrets)
+            except Exception as e:
+                self.logger.warning(f"Could not load secrets file {self.secrets_path} during save: {e}")
         
         # Strip secrets from main config before saving
         config_to_write = self._strip_secrets_recursive(new_config_data, secrets_content)
@@ -269,86 +274,35 @@ class ConfigManager:
             self.logger.error(error_msg, exc_info=True)
             raise ConfigError(error_msg, config_path=self.config_path) from e
 
-    @staticmethod
-    def _is_parallel_secrets_list(value: Any) -> bool:
-        """True for the parallel-placeholder list shape emitted by
-        ``secret_helpers.separate_secrets`` for array-item secrets: a
-        non-empty list whose elements are ALL dicts (``{}`` marks an item
-        with no secrets). Any other list-shaped secrets value is a
-        whole-key secret (e.g. a list of secret scalars)."""
-        return (isinstance(value, list) and bool(value)
-                and all(isinstance(item, dict) for item in value))
-
     def _strip_secrets_recursive(self, data_to_filter: Dict[str, Any], secrets: Dict[str, Any]) -> Dict[str, Any]:
         """Recursively remove secret keys from a dictionary."""
         result = {}
         for key, value in data_to_filter.items():
-            if key not in secrets:
+            if key in secrets:
-                # This key is not in secrets, so we keep it
+                if isinstance(value, dict) and isinstance(secrets[key], dict):
-                result[key] = value
-                continue
-            sec = secrets[key]
-            if isinstance(value, dict) and isinstance(sec, dict):
                     # This key is a shared group, recurse
-                stripped_sub_dict = self._strip_secrets_recursive(value, sec)
+                    stripped_sub_dict = self._strip_secrets_recursive(value, secrets[key])
                     if stripped_sub_dict: # Only add if there's non-secret data left
                         result[key] = stripped_sub_dict
-            elif isinstance(value, list) and self._is_parallel_secrets_list(sec):
+                # Else, it's a secret key at this level, so we skip it
-                # Parallel-list shape from separate_secrets: sec[i] holds the
-                # secret fields of value[i] ({} = item i has none). Strip each
-                # item and ALWAYS keep the list — indices must survive so the
-                # merge-on-load can realign secrets with their items. The
-                # regular list's length is authoritative: extra secrets
-                # entries are ignored.
-                stripped_items = []
-                for i, item in enumerate(value):
-                    s_item = sec[i] if i < len(sec) else {}
-                    if isinstance(item, dict) and s_item:
-                        stripped_items.append(self._strip_secrets_recursive(item, s_item))
             else:
-                        stripped_items.append(item)
+                # This key is not in secrets, so we keep it
-                result[key] = stripped_items
+                result[key] = value
-            # Else: whole-key secret (scalar, list of secret scalars, or a
-            # shape mismatch) -> drop the key entirely. Never leak.
         return result
 
-    def _load_secrets_for_save(self) -> Dict[str, Any]:
+    def save_config(self, new_config_data: Dict[str, Any]) -> None:
-        """Load config_secrets.json for stripping before a save.
+        """Save configuration to the main JSON file, stripping out secrets."""
-
+        secrets_content = {}
-        A missing secrets file is fine (nothing to strip). But a file that
+        if os.path.exists(self.secrets_path):
-        EXISTS and cannot be read or parsed means stripping is impossible —
-        and the in-memory config being saved has secrets deep-merged into it,
-        so proceeding would write them into config.json in plaintext. That
-        was the historical behavior; it is now a hard refusal. The save
-        raises so the caller (and user) fixes the secrets file instead of
-        silently leaking its contents into the world-readable main config.
-        """
-        if not os.path.exists(self.secrets_path):
-            return {}
             try:
                 with open(self.secrets_path, 'r') as f_secrets:
-                return json.load(f_secrets)
+                    secrets_content = json.load(f_secrets)
-        # Only the expected read/parse failures — an unexpected implementation
+            except Exception as e:
-        # error should propagate as itself, not masquerade as a secrets-file
+                self.logger.warning(f"Could not load secrets file {self.secrets_path} during save: {e}")
-        # problem. (JSONDecodeError and UnicodeDecodeError are ValueErrors.)
+                # Continue without stripping if secrets can't be loaded, or handle as critical error
-        except (OSError, ValueError, RecursionError) as e:
+                # For now, we'll proceed cautiously and save the full new_config_data if secrets are unreadable
-            error_msg = (
+                # to prevent accidental data loss if the secrets file is temporarily corrupt.
-                f"Refusing to save config: secrets file {self.secrets_path} exists "
+                # A more robust approach might be to fail the save or use a cached version of secrets.
-                f"but could not be loaded ({e}). Saving without it would write "
-                f"merged secret values into config.json in plaintext. Fix or "
-                f"remove the secrets file, then retry."
-            )
-            self.logger.error("[Config] %s", error_msg, exc_info=True)
-            raise ConfigError(error_msg, config_path=self.secrets_path) from e
-
-    def save_config(self, new_config_data: Dict[str, Any]) -> None:
-        """Save configuration to the main JSON file, stripping out secrets.
-
-        Raises ConfigError when the secrets file exists but cannot be loaded,
-        because stripping would be impossible and secrets would leak into
-        config.json.
-        """
-        secrets_content = self._load_secrets_for_save()
 
         config_to_write = self._strip_secrets_recursive(new_config_data, secrets_content)
 
@@ -385,39 +339,11 @@ class ConfigManager:
             return None
 
     def _deep_merge(self, target: Dict[str, Any], source: Dict[str, Any]) -> None:
-        """Deep merge source dict into target dict.
+        """Deep merge source dict into target dict."""
-
-        Sole call site: merging config_secrets.json into the loaded config.
-        Understands the parallel-list shape separate_secrets emits for
-        array-item secrets (see _is_parallel_secrets_list): each secrets
-        list item is merged into the config list item at the same index
-        ({} placeholders skipped). The config list's length is
-        authoritative — a user deleting an array item from config.json
-        must not have it resurrected from a stale secrets entry."""
         for key, value in source.items():
             if key in target and isinstance(target[key], dict) and isinstance(value, dict):
                 self._deep_merge(target[key], value)
-            elif (key in target and isinstance(target[key], list)
-                    and self._is_parallel_secrets_list(value)):
-                tlist = target[key]
-                for i, s_item in enumerate(value):
-                    if i >= len(tlist):
-                        # Interpolate only config-side data here — nothing
-                        # iterated out of the secrets dict (not even the key
-                        # name) may reach the log.
-                        self.logger.warning(
-                            "A secrets list is longer than the config list it "
-                            "parallels (config has %d item(s)); ignoring the "
-                            "extra entries", len(tlist))
-                        break
-                    if not s_item:
-                        continue  # {} placeholder: item i has no secrets
-                    if isinstance(tlist[i], dict):
-                        self._deep_merge(tlist[i], s_item)
             else:
-                        tlist[i] = s_item  # shape drift; the secret wins
-            else:
-                # Scalars AND whole-secret scalar arrays: replace (legacy).
                 target[key] = value
 
     def _create_config_from_template(self) -> None:

src/dynamic_team_resolver.py

@@ -168,13 +168,9 @@ class DynamicTeamResolver:
                 # Sort by ranking (1, 2, 3, etc.)
                 sorted_rankings = dict(sorted(rankings.items(), key=lambda x: x[1]))
                 
-                # Cache the results ON THE CLASS. Assigning through self
+                # Cache the results
-                # would create instance attributes that shadow the shared
+                self._rankings_cache = sorted_rankings
-                # class-level cache, making it per-instance — and every
+                self._cache_timestamp = current_time
-                # scoreboard constructs its own resolver, so the cache
-                # would never actually be shared.
-                DynamicTeamResolver._rankings_cache = sorted_rankings
-                DynamicTeamResolver._cache_timestamp = current_time
                 
                 self.logger.info(f"Fetched rankings for {len(sorted_rankings)} teams")
                 return sorted_rankings
@@ -220,11 +216,9 @@ class DynamicTeamResolver:
         return any(pattern in team_name.upper() for pattern in dynamic_patterns)
     
     def clear_cache(self):
-        """Clear the SHARED rankings cache to force fresh data on next
+        """Clear the rankings cache to force fresh data on next request."""
-        request. Writes through the class — assigning via self would only
+        self._rankings_cache = {}
-        shadow the shared cache for this instance."""
+        self._cache_timestamp = 0
-        DynamicTeamResolver._rankings_cache = {}
-        DynamicTeamResolver._cache_timestamp = 0
         self.logger.info("Cleared dynamic team rankings cache")
 
 

src/logging_config.py

@@ -5,7 +5,6 @@ Provides consistent logging configuration across the LEDMatrix application.
 Supports structured logging with context information and appropriate log levels.
 """
 
-import copy
 import logging
 import sys
 import os
@@ -66,12 +65,8 @@ class ContextualFormatter(logging.Formatter):
         self.include_context = include_context
     
     def format(self, record: logging.LogRecord) -> str:
-        """Format log record with context.
+        """Format log record with context."""
-
+        # Add context to message if present
-        Works on a shallow copy of the record: a record is formatted once
-        PER HANDLER, so mutating record.msg in place (the old behavior)
-        prepended the context prefix again for every additional handler.
-        """
         if self.include_context:
             context_parts = []
             
@@ -86,7 +81,6 @@ class ContextualFormatter(logging.Formatter):
                     context_parts.append(f"[{key}: {value}]")
             
             if context_parts:
-                record = copy.copy(record)
                 record.msg = ' '.join(context_parts) + ' ' + str(record.msg)
         
         return super().format(record)
@@ -230,11 +224,8 @@ def log_warning(logger: logging.Logger, message: str, **kwargs) -> None:
 
 
 def log_error(logger: logging.Logger, message: str, **kwargs) -> None:
-    """Log error message with context. Defaults exc_info=True; a caller
+    """Log error message with context."""
-    passing exc_info explicitly wins (the old hardcoded keyword raised
+    log_with_context(logger, logging.ERROR, message, **kwargs, exc_info=True)
-    TypeError on that duplicate)."""
-    kwargs.setdefault('exc_info', True)
-    log_with_context(logger, logging.ERROR, message, **kwargs)
 
 
 def log_debug(logger: logging.Logger, message: str, **kwargs) -> None:

src/plugin_system/base_plugin.py

@@ -364,10 +364,8 @@ class BasePlugin(ABC):
                 # Handle None case
                 if duration is None:
                     pass  # Fall through to config
-                # Try to convert to float if it's a number or numeric string.
+                # Try to convert to float if it's a number or numeric string
-                # bool is excluded: it's an int subclass, and True would
+                elif isinstance(duration, (int, float)):
-                # otherwise read as a 1-second duration.
-                elif isinstance(duration, (int, float)) and not isinstance(duration, bool):
                     if duration > 0:
                         return float(duration)
                     else:
@@ -405,9 +403,8 @@ class BasePlugin(ABC):
         # Fall back to config
         config_duration = self.config.get("display_duration", 15.0)
         try:
-            # Ensure config value is also a valid float (bool excluded — an
+            # Ensure config value is also a valid float
-            # int subclass that would otherwise read True as 1 second)
+            if isinstance(config_duration, (int, float)):
-            if isinstance(config_duration, (int, float)) and not isinstance(config_duration, bool):
                 if config_duration > 0:
                     return float(config_duration)
                 else:
@@ -797,12 +794,10 @@ class BasePlugin(ABC):
                 self.logger.error("'enabled' must be a boolean")
                 return False
 
-        # Check display_duration if present. bool is excluded explicitly:
+        # Check display_duration if present
-        # it's an int subclass, and get_display_duration rejects it too.
         if "display_duration" in self.config:
             duration = self.config["display_duration"]
-            if (not isinstance(duration, (int, float))
+            if not isinstance(duration, (int, float)) or duration <= 0:
-                    or isinstance(duration, bool) or duration <= 0):
                 self.logger.error("'display_duration' must be a positive number")
                 return False
 

src/plugin_system/compatibility.py

@@ -180,45 +180,6 @@ def declared_min_version(manifest: Dict[str, Any]) -> Optional[str]:
     return None
 
 
-def is_update_available(installed_version: str, latest_version: str) -> bool:
-    """Return True when the registry's ``latest_version`` is strictly newer
-    than the installed version.
-
-    THE shared comparator for "should this plugin be updated?" — used by both
-    the web UI's update badge (`api_v3._is_plugin_update_available`) and the
-    store's `update_plugin` reinstall decision, so the two can never disagree.
-
-    Uses PEP 440-aware comparison (``packaging``), which also normalizes
-    equivalent spellings: ``v1.2.0`` == ``1.2.0`` and ``1.2`` == ``1.2.0``, so
-    cosmetic differences never trigger a reinstall — and a locally modified
-    plugin whose version is *ahead* of the registry is never "updated"
-    (downgraded). If either version string can't be parsed the mismatch is
-    surfaced (True) so the user can reconcile, rather than silently hiding a
-    potential update.
-    """
-    if not installed_version or not latest_version:
-        return False
-    if not isinstance(installed_version, str) or not isinstance(latest_version, str):
-        # A malformed manifest/registry can carry a number (1.2) or worse;
-        # packaging would raise TypeError. Surface the mismatch instead.
-        return True
-    if installed_version == latest_version:
-        return False
-    try:
-        from packaging.version import parse as _parse_version, InvalidVersion
-    except ImportError:
-        # packaging is a core dependency, but if it's somehow unavailable we
-        # can't compare semantically — surface the mismatch we already know
-        # exists (the two strings differ).
-        return True
-    try:
-        return _parse_version(latest_version) > _parse_version(installed_version)
-    except InvalidVersion:
-        # Unparseable version string: we can't tell direction, so surface the
-        # mismatch rather than silently hiding a potential update.
-        return True
-
-
 def check(manifest: Dict[str, Any], core_version: str) -> Tuple[bool, Optional[str]]:
     """Return ``(compatible, reason)``.
 

src/plugin_system/saved_repositories.py

@@ -6,7 +6,6 @@ Manages saved GitHub repository URLs for easy plugin discovery and installation.
 
 import json
 import logging
-import os
 from pathlib import Path
 from typing import List, Dict, Optional
 
@@ -44,45 +43,20 @@ class SavedRepositoriesManager:
             return []
     
     def _save_repositories(self) -> bool:
-        """Save repositories to file atomically.
+        """Save repositories to file."""
-
-        Writes to a temp file in the same directory and os.replace()s it
-        over the target, so a failed write can never truncate or
-        half-overwrite an existing saved_repositories.json.
-        """
-        tmp_path = self.config_path.with_suffix(self.config_path.suffix + '.tmp')
         try:
             # Ensure directory exists
             self.config_path.parent.mkdir(parents=True, exist_ok=True)
             
-            with open(tmp_path, 'w') as f:
+            with open(self.config_path, 'w') as f:
                 json.dump(self.repositories, f, indent=2)
-                f.flush()
-                os.fsync(f.fileno())
-            os.replace(tmp_path, self.config_path)
             
             self.logger.info(f"Saved {len(self.repositories)} repositories to {self.config_path}")
             return True
         except Exception as e:
             self.logger.error(f"Error saving repositories: {e}")
-            try:
-                tmp_path.unlink(missing_ok=True)
-            except OSError:
-                pass
             return False
     
-    @staticmethod
-    def _clean_url(repo_url: str) -> str:
-        """Normalize a repo URL: strip whitespace, trailing slashes, and a
-        trailing ``.git`` suffix ONLY. (The old ``.replace('.git', '')``
-        was an unanchored substring replace that mangled URLs merely
-        containing ``.git``, e.g. ``https://github.com/user/my.github.io``.)
-        """
-        repo_url = repo_url.strip().rstrip('/')
-        if repo_url.endswith('.git'):
-            repo_url = repo_url[:-4]
-        return repo_url
-
     def get_all(self) -> List[Dict[str, str]]:
         """Get all saved repositories."""
         return self.repositories.copy()
@@ -98,7 +72,8 @@ class SavedRepositoriesManager:
         Returns:
             True if added successfully
         """
-        repo_url = self._clean_url(repo_url)
+        # Clean URL
+        repo_url = repo_url.strip().rstrip('/').replace('.git', '')
         
         # Check if already exists
         for repo in self.repositories:
@@ -121,12 +96,7 @@ class SavedRepositoriesManager:
             'type': 'registry' if 'plugins.json' in repo_url or 'ledmatrix-plugins' in repo_url.lower() else 'single'
         })
         
-        if not self._save_repositories():
+        return self._save_repositories()
-            # Keep memory consistent with disk: a failed save must not leave
-            # a phantom entry that only this process can see.
-            self.repositories.pop()
-            return False
-        return True
     
     def remove(self, repo_url: str) -> bool:
         """
@@ -138,25 +108,21 @@ class SavedRepositoriesManager:
         Returns:
             True if removed successfully
         """
-        repo_url = self._clean_url(repo_url)
+        # Clean URL
+        repo_url = repo_url.strip().rstrip('/').replace('.git', '')
         
-        previous = self.repositories
+        original_count = len(self.repositories)
-        remaining = [r for r in previous if r.get('url') != repo_url]
+        self.repositories = [r for r in self.repositories if r.get('url') != repo_url]
         
-        if len(remaining) < len(previous):
+        if len(self.repositories) < original_count:
-            self.repositories = remaining
+            return self._save_repositories()
-            if not self._save_repositories():
-                # Failed save: restore so memory matches disk.
-                self.repositories = previous
-                return False
-            return True
         else:
             self.logger.warning(f"Repository not found: {repo_url}")
             return False
     
     def has(self, repo_url: str) -> bool:
         """Check if a repository is already saved."""
-        repo_url = self._clean_url(repo_url)
+        repo_url = repo_url.strip().rstrip('/').replace('.git', '')
         return any(r.get('url') == repo_url for r in self.repositories)
     
     def get_registry_repositories(self) -> List[Dict[str, str]]:

src/plugin_system/store_manager.py

@@ -3000,10 +3000,7 @@ class PluginStoreManager:
             remote_branch = plugin_info_remote.get('branch') or plugin_info_remote.get('default_branch')
 
             # Compare local manifest version against registry latest_version
-            # to avoid unnecessary reinstalls for monorepo plugins. Uses the
+            # to avoid unnecessary reinstalls for monorepo plugins
-            # same semantic comparator as the web UI's update badge, so
-            # equivalent spellings ("v1.2.0" vs "1.2.0") never trigger a
-            # reinstall and a locally-ahead version is never downgraded.
             try:
                 local_manifest_path = plugin_path / "manifest.json"
                 if local_manifest_path.exists():
@@ -3011,16 +3008,8 @@ class PluginStoreManager:
                         local_manifest = json.load(f)
                     local_version = local_manifest.get('version', '')
                     remote_version = plugin_info_remote.get('latest_version', '')
-                    from src.plugin_system.compatibility import is_update_available
+                    if local_version and remote_version and local_version == remote_version:
-                    # No truthiness gate: the shared comparator already treats
+                        self.logger.info(f"Plugin {plugin_id} already at latest version {local_version}")
-                    # a missing version on either side as "no update", and the
-                    # store must agree with the UI badge in that case too. A
-                    # missing manifest (not just a missing version field)
-                    # still falls through to the reinstall recovery path.
-                    if not is_update_available(local_version, remote_version):
-                        self.logger.info(
-                            f"Plugin {plugin_id} already at latest version "
-                            f"(installed {local_version}, registry {remote_version})")
                         return True
             except Exception as e:
                 self.logger.debug(f"Could not compare versions for {plugin_id}: {e}")

src/startup_validator.py

@@ -38,11 +38,6 @@ class StartupValidator:
         """
         self.logger.info("Starting startup validation...")
         
-        # Fresh lists each run — without this, calling validate_all() twice
-        # duplicated every message.
-        self.errors = []
-        self.warnings = []
-
         # Validate configuration
         self._validate_config()
         

test/fixtures/plugins/ci-fixture-plugin/config_schema.json

@@ -1,31 +0,0 @@
-{
-    "$schema": "http://json-schema.org/draft-07/schema#",
-    "title": "CI Fixture Plugin",
-    "type": "object",
-    "properties": {
-        "enabled": {
-            "type": "boolean",
-            "default": true
-        },
-        "display_duration": {
-            "type": "number",
-            "default": 5
-        },
-        "border_color": {
-            "type": "array",
-            "items": {"type": "integer", "minimum": 0, "maximum": 255},
-            "minItems": 3,
-            "maxItems": 3,
-            "default": [0, 255, 0],
-            "description": "RGB color of the border rectangle."
-        },
-        "diagonal_color": {
-            "type": "array",
-            "items": {"type": "integer", "minimum": 0, "maximum": 255},
-            "minItems": 3,
-            "maxItems": 3,
-            "default": [255, 0, 0],
-            "description": "RGB color of the diagonals."
-        }
-    }
-}

test/fixtures/plugins/ci-fixture-plugin/manager.py

@@ -1,44 +0,0 @@
-"""
-CI fixture plugin.
-
-Exists so the plugin safety harness (test/plugins/test_plugin_matrix.py and
-the plugin-safety CI job) always has at least one real plugin to load and
-render — without it, an empty plugins/ directory turns the whole job into a
-green no-op. The render is deliberately trivial and fully deterministic:
-a border rectangle plus both diagonals, sized from the display manager's
-declared dimensions. No fonts, no network, no time dependence, so golden
-images are stable across platforms.
-"""
-
-from PIL import ImageDraw
-
-from src.plugin_system.base_plugin import BasePlugin
-
-
-class CIFixturePlugin(BasePlugin):
-    """Deterministic CI-only fixture plugin: renders a border + diagonals
-    pattern sized from the display's declared dimensions. Never shipped to
-    devices; exists solely so the plugin safety harness has a real plugin
-    to exercise in CI."""
-
-    def update(self) -> None:
-        """Nothing to fetch — the render is self-contained."""
-
-    def display(self, force_clear: bool = False) -> None:
-        self.display_manager.clear()
-        width = self.display_manager.matrix.width
-        height = self.display_manager.matrix.height
-        border = tuple(self.config.get("border_color", [0, 255, 0]))
-        diagonal = tuple(self.config.get("diagonal_color", [255, 0, 0]))
-
-        image = self.display_manager.image
-        draw = ImageDraw.Draw(image)
-        # Blank only the declared panel area, then draw edge-to-edge content:
-        # the border proves the plugin reads dynamic dimensions (any overflow
-        # or underfill at any size is a harness bug or a dimensions bug), the
-        # diagonals make golden comparisons sensitive to size/offset drift.
-        draw.rectangle([0, 0, width - 1, height - 1], fill=(0, 0, 0))
-        draw.rectangle([0, 0, width - 1, height - 1], outline=border)
-        draw.line([0, 0, width - 1, height - 1], fill=diagonal)
-        draw.line([0, height - 1, width - 1, 0], fill=diagonal)
-        self.display_manager.update_display()

test/fixtures/plugins/ci-fixture-plugin/manifest.json

@@ -1,13 +0,0 @@
-{
-    "id": "ci-fixture-plugin",
-    "name": "CI Fixture Plugin",
-    "version": "1.0.0",
-    "description": "Bundled test fixture so the plugin safety harness always has at least one real plugin to render in CI. Draws a deterministic border + diagonals pattern at any panel size. Not installable from the store and never shipped to devices.",
-    "author": "LEDMatrix",
-    "entry_point": "manager.py",
-    "class_name": "CIFixturePlugin",
-    "display_modes": ["ci-fixture"],
-    "update_interval": 3600,
-    "min_ledmatrix_version": "2.0.0",
-    "compatible_versions": [">=2.0.0"]
-}

test/fixtures/plugins/ci-fixture-plugin/requirements.txt

@@ -1,7 +0,0 @@
-# No dependencies — the fixture must load in any environment.
-#
-# Pillow is deliberately NOT pinned here even though manager.py imports
-# PIL: it is a core LEDMatrix dependency (see the repo-root
-# requirements.txt), so it is always present wherever the harness runs,
-# and the harness loads plugins with install_deps=False anyway. Pinning
-# it here would only invite a needless pip install during test runs.

test/plugins/conftest.py

@@ -23,17 +23,9 @@ os.environ['EMULATOR'] = 'true'
 def plugins_dir() -> Path:
     """Get the plugins directory path.
 
-    Honors LEDMATRIX_PLUGINS_DIR (first entry) when set — the same override
+    Checks plugins/ first, then falls back to plugin-repos/
-    test_plugin_matrix.py uses, so CI can point every plugin suite at the
+    for monorepo development environments.
-    bundled fixture plugins. Otherwise checks plugins/ first, then falls
-    back to plugin-repos/ for monorepo development environments.
     """
-    env = os.environ.get('LEDMATRIX_PLUGINS_DIR')
-    if env:
-        first = env.split(os.pathsep)[0]
-        if first:
-            return Path(first)
-
     plugins_path = project_root / 'plugins'
     plugin_repos_path = project_root / 'plugin-repos'
 

test/plugins/test_basketball_scoreboard.py

@@ -1,10 +1,5 @@
 """
 Integration tests for basketball-scoreboard plugin.
-
-Requires the real plugin to be installed (plugins/ or plugin-repos/,
-or the dir named by LEDMATRIX_PLUGINS_DIR) — on machines without it,
-every test here skips by design. CI covers plugin safety with the
-bundled fixture plugin via test_plugin_matrix.py instead.
 """
 
 import pytest

test/plugins/test_calendar.py

@@ -1,10 +1,5 @@
 """
 Integration tests for calendar plugin.
-
-Requires the real plugin to be installed (plugins/ or plugin-repos/,
-or the dir named by LEDMATRIX_PLUGINS_DIR) — on machines without it,
-every test here skips by design. CI covers plugin safety with the
-bundled fixture plugin via test_plugin_matrix.py instead.
 """
 
 import pytest

test/plugins/test_clock_simple.py

@@ -1,10 +1,5 @@
 """
 Integration tests for clock-simple plugin.
-
-Requires the real plugin to be installed (plugins/ or plugin-repos/,
-or the dir named by LEDMATRIX_PLUGINS_DIR) — on machines without it,
-every test here skips by design. CI covers plugin safety with the
-bundled fixture plugin via test_plugin_matrix.py instead.
 """
 
 import pytest

test/plugins/test_odds_ticker.py

@@ -1,10 +1,5 @@
 """
 Integration tests for odds-ticker plugin.
-
-Requires the real plugin to be installed (plugins/ or plugin-repos/,
-or the dir named by LEDMATRIX_PLUGINS_DIR) — on machines without it,
-every test here skips by design. CI covers plugin safety with the
-bundled fixture plugin via test_plugin_matrix.py instead.
 """
 
 import pytest

test/plugins/test_soccer_scoreboard.py

@@ -1,10 +1,5 @@
 """
 Integration tests for soccer-scoreboard plugin.
-
-Requires the real plugin to be installed (plugins/ or plugin-repos/,
-or the dir named by LEDMATRIX_PLUGINS_DIR) — on machines without it,
-every test here skips by design. CI covers plugin safety with the
-bundled fixture plugin via test_plugin_matrix.py instead.
 """
 
 import pytest

test/plugins/test_text_display.py

@@ -1,10 +1,5 @@
 """
 Integration tests for text-display plugin.
-
-Requires the real plugin to be installed (plugins/ or plugin-repos/,
-or the dir named by LEDMATRIX_PLUGINS_DIR) — on machines without it,
-every test here skips by design. CI covers plugin safety with the
-bundled fixture plugin via test_plugin_matrix.py instead.
 """
 
 import pytest

test/test_api_helper.py

@@ -1,275 +0,0 @@
-"""
-Tests for src/common/api_helper.py (APIHelper).
-
-Covers rate limiting, cached GETs, ESPN URL/cache-key construction,
-session header defaults and per-call merging, the retry adapter, and the
-fixed clear_cache() behavior (real CacheManager surface: clear_cache /
-delete / list_cache_files, with safe no-ops elsewhere).
-
-No real network: helper.session.get/post are always replaced with mocks.
-"""
-
-import types
-from unittest.mock import MagicMock, Mock
-
-import pytest
-import requests
-from freezegun import freeze_time
-
-import src.common.api_helper as api_helper_module
-from src.common.api_helper import APIHelper
-
-
-def _make_response(payload):
-    response = MagicMock()
-    response.json.return_value = payload
-    response.raise_for_status.return_value = None
-    return response
-
-
-@pytest.fixture
-def cache():
-    cache = MagicMock()
-    cache.get.return_value = None
-    return cache
-
-
-@pytest.fixture
-def helper(cache):
-    helper = APIHelper(cache_manager=cache)
-    # Default min interval is 1.0s and would really sleep between requests.
-    helper.set_rate_limit(0)
-    return helper
-
-
-# ---------------------------------------------------------------------------
-# Rate limiting
-# ---------------------------------------------------------------------------
-
-class TestRateLimiting:
-    def test_sleeps_for_remaining_interval(self, helper, monkeypatch):
-        fake_time = MagicMock()
-        fake_time.time.side_effect = [102.0, 105.0]
-        monkeypatch.setattr(api_helper_module, 'time', fake_time)
-
-        helper.set_rate_limit(5)
-        helper._last_request_time = 100.0
-        helper._enforce_rate_limit()
-
-        # 2s elapsed of a 5s interval -> sleep the remaining 3s.
-        fake_time.sleep.assert_called_once()
-        assert fake_time.sleep.call_args[0][0] == pytest.approx(3.0)
-        assert helper._last_request_time == 105.0
-
-    def test_no_sleep_when_interval_elapsed(self, helper, monkeypatch):
-        fake_time = MagicMock()
-        fake_time.time.side_effect = [200.0, 201.0]
-        monkeypatch.setattr(api_helper_module, 'time', fake_time)
-
-        helper.set_rate_limit(5)
-        helper._last_request_time = 100.0
-        helper._enforce_rate_limit()
-
-        fake_time.sleep.assert_not_called()
-        assert helper._last_request_time == 201.0
-
-
-# ---------------------------------------------------------------------------
-# get()
-# ---------------------------------------------------------------------------
-
-class TestGet:
-    def test_cache_hit_skips_request_and_rate_limit(self, helper, cache):
-        cache.get.return_value = {'cached': True}
-        helper.session.get = Mock()
-        rate_spy = Mock()
-        helper._enforce_rate_limit = rate_spy
-
-        result = helper.get('https://example.com/api', cache_key='k')
-
-        assert result == {'cached': True}
-        helper.session.get.assert_not_called()
-        rate_spy.assert_not_called()
-
-    def test_cache_miss_fetches_and_caches_without_ttl(self, helper, cache):
-        cache.get.return_value = None
-        helper.session.get = Mock(return_value=_make_response({'a': 1}))
-
-        result = helper.get('https://example.com/api', cache_key='k',
-                            cache_ttl=999)
-
-        assert result == {'a': 1}
-        # Pin the ttl-dropped contract: CacheManager.set is called with
-        # (key, data) only — the cache_ttl argument is discarded.
-        cache.set.assert_called_once_with('k', {'a': 1})
-
-    def test_request_exception_returns_none_and_caches_nothing(
-            self, helper, cache):
-        helper.session.get = Mock(
-            side_effect=requests.exceptions.RequestException('boom'))
-
-        result = helper.get('https://example.com/api', cache_key='k')
-
-        assert result is None
-        cache.set.assert_not_called()
-
-    def test_timeout_zero_falls_back_to_default(self, helper):
-        # Quirk pin: `timeout or self.default_timeout` treats an explicit
-        # timeout=0 as falsy, so the default (30) is used instead.
-        helper.session.get = Mock(return_value=_make_response({}))
-
-        helper.get('https://example.com/api', timeout=0)
-
-        assert helper.session.get.call_args.kwargs['timeout'] == 30
-
-    def test_per_call_headers_merge_over_session_headers(self, helper):
-        helper.session.get = Mock(return_value=_make_response({}))
-
-        helper.get('https://example.com/api', headers={'X-Custom': 'yes'})
-
-        sent = helper.session.get.call_args.kwargs['headers']
-        # Merged, not replaced: session defaults survive alongside the
-        # per-call header.
-        assert sent['X-Custom'] == 'yes'
-        assert sent['User-Agent'] == (
-            'LEDMatrix/1.0 (+https://github.com/ChuckBuilds/LEDMatrix)')
-        assert sent['Accept'] == 'application/json'
-        # The session's own headers are not polluted by the per-call ones.
-        assert 'X-Custom' not in helper.session.headers
-
-
-# ---------------------------------------------------------------------------
-# ESPN helpers
-# ---------------------------------------------------------------------------
-
-class TestEspnHelpers:
-    @freeze_time('2026-08-07')
-    def test_fetch_espn_scoreboard_url_params_and_cache_key(self, helper):
-        helper.get = Mock(return_value={'ok': 1})
-
-        result = helper.fetch_espn_scoreboard('football', 'nfl')
-
-        assert result == {'ok': 1}
-        helper.get.assert_called_once_with(
-            'https://site.api.espn.com/apis/site/v2/sports/football/nfl/scoreboard',
-            params={'dates': '20260807', 'limit': 1000},
-            cache_key='espn_football_nfl_20260807',
-            cache_ttl=300,
-        )
-
-    def test_fetch_espn_scoreboard_explicit_date(self, helper):
-        helper.get = Mock(return_value=None)
-
-        helper.fetch_espn_scoreboard('basketball', 'nba', date='20250115')
-
-        kwargs = helper.get.call_args.kwargs
-        assert kwargs['params'] == {'dates': '20250115', 'limit': 1000}
-        assert kwargs['cache_key'] == 'espn_basketball_nba_20250115'
-
-    def test_fetch_espn_standings_url_and_cache_key(self, helper):
-        helper.get = Mock(return_value={'ok': 1})
-
-        helper.fetch_espn_standings('football', 'nfl')
-
-        helper.get.assert_called_once_with(
-            'https://site.api.espn.com/apis/site/v2/sports/football/nfl/standings',
-            cache_key='espn_standings_football_nfl',
-            cache_ttl=3600,
-        )
-
-    def test_fetch_espn_rankings_url_and_cache_key(self, helper):
-        helper.get = Mock(return_value={'ok': 1})
-
-        helper.fetch_espn_rankings('football', 'college-football')
-
-        helper.get.assert_called_once_with(
-            'https://site.api.espn.com/apis/site/v2/sports/football/college-football/rankings',
-            cache_key='espn_rankings_football_college-football',
-            cache_ttl=3600,
-        )
-
-
-# ---------------------------------------------------------------------------
-# Session setup
-# ---------------------------------------------------------------------------
-
-class TestSessionSetup:
-    def test_user_agent_exact(self, helper):
-        # Regression guard: ESPN began 403ing other user agents; this exact
-        # string must be sent on every request.
-        assert helper.session.headers['User-Agent'] == (
-            'LEDMatrix/1.0 (+https://github.com/ChuckBuilds/LEDMatrix)')
-
-    def test_retry_adapter_configuration(self):
-        helper = APIHelper(cache_manager=None, max_retries=7)
-
-        retries = helper.session.get_adapter('https://x').max_retries
-        assert retries.total == 7
-        assert {429, 500, 502, 503, 504} <= set(retries.status_forcelist)
-
-
-# ---------------------------------------------------------------------------
-# clear_cache (fixed behavior: real CacheManager surface)
-# ---------------------------------------------------------------------------
-
-class TestClearCache:
-    def test_no_pattern_uses_clear_cache_method(self):
-        manager = types.SimpleNamespace(clear_cache=Mock())
-        helper = APIHelper(cache_manager=manager)
-        helper.set_rate_limit(0)
-
-        helper.clear_cache()
-
-        manager.clear_cache.assert_called_once_with()
-
-    def test_no_pattern_falls_back_to_clear(self):
-        manager = types.SimpleNamespace(clear=Mock())
-        helper = APIHelper(cache_manager=manager)
-        helper.set_rate_limit(0)
-
-        helper.clear_cache()
-
-        manager.clear.assert_called_once_with()
-
-    def test_no_pattern_manager_without_any_clear_is_noop(self):
-        helper = APIHelper(cache_manager=object())
-        helper.set_rate_limit(0)
-
-        helper.clear_cache()  # must not raise
-
-    def test_pattern_deletes_only_matching_keys(self):
-        manager = types.SimpleNamespace(
-            list_cache_files=Mock(return_value=[
-                {'key': 'espn_nfl_x'},
-                {'key': 'other'},
-            ]),
-            delete=Mock(),
-        )
-        helper = APIHelper(cache_manager=manager)
-        helper.set_rate_limit(0)
-
-        helper.clear_cache(pattern='espn')
-
-        manager.delete.assert_called_once_with('espn_nfl_x')
-
-    def test_pattern_manager_without_list_cache_files_is_noop(self):
-        helper = APIHelper(cache_manager=object())
-        helper.set_rate_limit(0)
-
-        helper.clear_cache(pattern='espn')  # must not raise
-
-
-# ---------------------------------------------------------------------------
-# No cache manager
-# ---------------------------------------------------------------------------
-
-class TestNoCacheManager:
-    def test_all_cache_operations_safe_without_manager(self):
-        helper = APIHelper(cache_manager=None)
-        helper.set_rate_limit(0)
-
-        assert helper.get_cache('k') is None
-        assert helper._get_from_cache('k') is None
-        assert helper.set_cache('k', {'a': 1}) is None
-        assert helper.clear_cache() is None
-        assert helper.clear_cache(pattern='espn') is None

test/test_base_odds_manager.py

@@ -1,359 +0,0 @@
-"""
-Tests for src/base_odds_manager.py (BaseOddsManager).
-
-Covers get_odds validation/caching/URL construction, the null-safe
-_extract_espn_data fix (ESPN sends explicit JSON nulls for absent sides),
-the no_odds sentinel, stale-cache fallback on request failure,
-is_odds_available's ML-blind truth table, the fixed format_odds_summary
-gate (money-line-only odds now format), get_odds_for_games, and
-configuration loading.
-
-No real network: src.base_odds_manager.requests.get is always patched.
-"""
-
-from unittest.mock import MagicMock, patch
-
-import pytest
-import requests
-
-from src.base_odds_manager import BaseOddsManager
-
-
-FULL_ITEM = {
-    'details': 'DAL -3.5',
-    'overUnder': 47.5,
-    'spread': -3.5,
-    'homeTeamOdds': {'moneyLine': -150, 'current': {'pointSpread': {'value': -3.5}}},
-    'awayTeamOdds': {'moneyLine': 130, 'current': {'pointSpread': {'value': 3.5}}},
-}
-
-FULL_EXTRACTED = {
-    'details': 'DAL -3.5',
-    'over_under': 47.5,
-    'spread': -3.5,
-    'home_team_odds': {'money_line': -150, 'spread_odds': -3.5},
-    'away_team_odds': {'money_line': 130, 'spread_odds': 3.5},
-}
-
-
-def _make_response(payload):
-    response = MagicMock()
-    response.json.return_value = payload
-    response.raise_for_status.return_value = None
-    return response
-
-
-@pytest.fixture
-def cache_manager():
-    cm = MagicMock()
-    # A bare MagicMock returns truthy Mocks from every call, so every
-    # get_odds() would look like a cache hit. Explicitly wire a miss.
-    cm.get_with_auto_strategy.return_value = None
-    return cm
-
-
-@pytest.fixture
-def manager(cache_manager):
-    return BaseOddsManager(cache_manager)
-
-
-@pytest.fixture
-def mock_get():
-    with patch('src.base_odds_manager.requests.get') as m:
-        m.return_value = _make_response({'items': [dict(FULL_ITEM)]})
-        yield m
-
-
-# ---------------------------------------------------------------------------
-# get_odds
-# ---------------------------------------------------------------------------
-
-class TestGetOdds:
-    def test_none_sport_raises(self, manager):
-        with pytest.raises(ValueError):
-            manager.get_odds(None, 'nfl', '1')
-
-    def test_none_league_raises(self, manager):
-        with pytest.raises(ValueError):
-            manager.get_odds('football', None, '1')
-
-    def test_cache_key_and_url(self, manager, cache_manager, mock_get):
-        manager.get_odds('football', 'nfl', '401')
-
-        cache_manager.get_with_auto_strategy.assert_called_once_with(
-            'odds_espn_football_nfl_401')
-        url = mock_get.call_args[0][0]
-        # Event id appears twice: /events/<id>/competitions/<id>/odds
-        assert '/events/401/competitions/401/odds' in url
-        assert url == ('https://sports.core.api.espn.com/v2/sports/football/'
-                       'leagues/nfl/events/401/competitions/401/odds')
-        assert mock_get.call_args.kwargs['timeout'] == 30
-
-    def test_ncaa_fb_maps_to_college_football(self, manager, mock_get):
-        manager.get_odds('football', 'ncaa_fb', '401')
-
-        url = mock_get.call_args[0][0]
-        assert '/leagues/college-football/' in url
-
-    def test_unknown_league_passes_through(self, manager, mock_get):
-        manager.get_odds('football', 'xfl', '401')
-
-        assert '/leagues/xfl/' in mock_get.call_args[0][0]
-
-    def test_cache_hit_skips_http(self, manager, cache_manager, mock_get):
-        cache_manager.get_with_auto_strategy.return_value = {'spread': -3.0}
-
-        result = manager.get_odds('football', 'nfl', '401')
-
-        assert result == {'spread': -3.0}
-        mock_get.assert_not_called()
-
-    def test_cached_no_odds_sentinel_returned_verbatim(
-            self, manager, cache_manager, mock_get):
-        cache_manager.get_with_auto_strategy.return_value = {'no_odds': True}
-
-        result = manager.get_odds('football', 'nfl', '401')
-
-        assert result == {'no_odds': True}
-        mock_get.assert_not_called()
-        assert manager.is_odds_available(result) is False
-
-    def test_success_caches_extracted_data_with_interval_ttl(
-            self, manager, cache_manager, mock_get):
-        result = manager.get_odds('football', 'nfl', '401',
-                                  update_interval_seconds=100)
-
-        assert result == FULL_EXTRACTED
-        cache_manager.set.assert_called_once_with(
-            'odds_espn_football_nfl_401', FULL_EXTRACTED, ttl=100)
-
-    def test_no_odds_caches_sentinel(self, manager, cache_manager, mock_get):
-        mock_get.return_value = _make_response({'count': 0, 'items': []})
-
-        result = manager.get_odds('football', 'nfl', '401')
-
-        assert result is None
-        cache_manager.set.assert_called_once_with(
-            'odds_espn_football_nfl_401', {'no_odds': True}, ttl=3600)
-
-    def test_zero_interval_falls_back_to_default(
-            self, manager, cache_manager, mock_get):
-        # Quirk pin: `update_interval_seconds or self.update_interval`
-        # treats an explicit 0 as falsy, so the 3600 default wins.
-        manager.get_odds('football', 'nfl', '401', update_interval_seconds=0)
-
-        assert cache_manager.set.call_args.kwargs['ttl'] == 3600
-
-    def test_request_exception_falls_back_to_stale_cache(
-            self, manager, cache_manager, mock_get):
-        cache_manager.get_with_auto_strategy.side_effect = [
-            None, {'stale': True}]
-        mock_get.side_effect = requests.exceptions.RequestException('boom')
-
-        result = manager.get_odds('football', 'nfl', '401')
-
-        assert result == {'stale': True}
-        assert cache_manager.get_with_auto_strategy.call_count == 2
-
-
-# ---------------------------------------------------------------------------
-# _extract_espn_data
-# ---------------------------------------------------------------------------
-
-class TestExtractEspnData:
-    def test_full_item_extracts_all_fields(self, manager):
-        result = manager._extract_espn_data({'items': [dict(FULL_ITEM)]})
-        assert result == FULL_EXTRACTED
-
-    def test_explicit_nulls_do_not_raise(self, manager):
-        # Post-fix: ESPN sends explicit JSON nulls for absent sides
-        # ("homeTeamOdds": null, "current": null); extraction must not
-        # raise and yields None fields.
-        payload = {'items': [{
-            'homeTeamOdds': None,
-            'awayTeamOdds': {'moneyLine': 150, 'current': None},
-        }]}
-
-        result = manager._extract_espn_data(payload)
-
-        assert result is not None
-        assert result['home_team_odds']['money_line'] is None
-        assert result['home_team_odds']['spread_odds'] is None
-        assert result['away_team_odds']['money_line'] == 150
-        assert result['away_team_odds']['spread_odds'] is None
-
-    def test_valid_empty_response_returns_none(self, manager):
-        assert manager._extract_espn_data({'count': 0, 'items': []}) is None
-
-    def test_unexpected_structure_returns_none(self, manager):
-        assert manager._extract_espn_data({'unexpected': True}) is None
-
-    def test_item_without_odds_fields_cached_as_data_not_sentinel(
-            self, manager, cache_manager, mock_get):
-        # Characterization pin: an item with no odds fields still extracts
-        # to a truthy dict of all-None values, so get_odds caches it as
-        # real data (NOT the no_odds sentinel) — but is_odds_available
-        # correctly reports False for it.
-        mock_get.return_value = _make_response({'items': [{}]})
-
-        result = manager.get_odds('football', 'nfl', '401')
-
-        assert result == {
-            'details': None,
-            'over_under': None,
-            'spread': None,
-            'home_team_odds': {'money_line': None, 'spread_odds': None},
-            'away_team_odds': {'money_line': None, 'spread_odds': None},
-        }
-        cache_manager.set.assert_called_once_with(
-            'odds_espn_football_nfl_401', result, ttl=3600)
-        assert manager.is_odds_available(result) is False
-
-
-# ---------------------------------------------------------------------------
-# is_odds_available
-# ---------------------------------------------------------------------------
-
-class TestIsOddsAvailable:
-    def test_none_is_false(self, manager):
-        assert manager.is_odds_available(None) is False
-
-    def test_empty_dict_is_false(self, manager):
-        assert manager.is_odds_available({}) is False
-
-    def test_no_odds_sentinel_is_false(self, manager):
-        assert manager.is_odds_available({'no_odds': True}) is False
-
-    def test_spread_is_true(self, manager):
-        assert manager.is_odds_available({'spread': -3.5}) is True
-
-    def test_over_under_is_true(self, manager):
-        assert manager.is_odds_available({'over_under': 47.5}) is True
-
-    def test_nested_home_spread_odds_is_true(self, manager):
-        assert manager.is_odds_available(
-            {'home_team_odds': {'spread_odds': -3.5}}) is True
-
-    def test_nested_away_spread_odds_is_true(self, manager):
-        assert manager.is_odds_available(
-            {'away_team_odds': {'spread_odds': 3.5}}) is True
-
-    def test_moneyline_only_is_false(self, manager):
-        # Pinned ML-blind contract: is_odds_available ignores money lines
-        # (its callers decide whether to render an odds widget). Note that
-        # format_odds_summary deliberately uses a DIFFERENT gate — it will
-        # still format money-line-only odds (see TestFormatOddsSummary).
-        ml_only = {
-            'home_team_odds': {'money_line': -120},
-            'away_team_odds': {'money_line': 100},
-        }
-        assert manager.is_odds_available(ml_only) is False
-
-
-# ---------------------------------------------------------------------------
-# format_odds_summary (fixed gate: empty / no_odds only)
-# ---------------------------------------------------------------------------
-
-class TestFormatOddsSummary:
-    def test_moneyline_only_formats(self, manager):
-        result = manager.format_odds_summary({
-            'home_team_odds': {'money_line': -120},
-            'away_team_odds': {'money_line': 100},
-        })
-        assert result == 'Home ML: -120 | Away ML: 100'
-
-    def test_full_data_formats_all_parts(self, manager):
-        result = manager.format_odds_summary(FULL_EXTRACTED)
-        assert result == 'Spread: -3.5 | O/U: 47.5 | Home ML: -150 | Away ML: 130'
-
-    def test_none_is_no_odds(self, manager):
-        assert manager.format_odds_summary(None) == 'No odds available'
-
-    def test_empty_dict_is_no_odds(self, manager):
-        assert manager.format_odds_summary({}) == 'No odds available'
-
-    def test_no_odds_sentinel_is_no_odds(self, manager):
-        assert manager.format_odds_summary(
-            {'no_odds': True}) == 'No odds available'
-
-
-# ---------------------------------------------------------------------------
-# get_odds_for_games
-# ---------------------------------------------------------------------------
-
-class TestGetOddsForGames:
-    def test_missing_fields_get_none_odds_without_http(self, manager, mock_get):
-        games = [
-            {'sport': 'football'},
-            {'league': 'nfl'},
-            {'id': '9'},
-            {},
-        ]
-
-        result = manager.get_odds_for_games(games)
-
-        assert all(g['odds'] is None for g in result)
-        mock_get.assert_not_called()
-
-    def test_per_game_exception_continues_loop(self, manager, monkeypatch):
-        def fake_get_odds(sport, league, event_id,
-                          update_interval_seconds=None):
-            if event_id == 'bad':
-                raise RuntimeError('boom')
-            return {'spread': -1.0}
-
-        monkeypatch.setattr(manager, 'get_odds', fake_get_odds)
-        games = [
-            {'sport': 'football', 'league': 'nfl', 'id': 'bad'},
-            {'sport': 'football', 'league': 'nfl', 'id': 'ok'},
-        ]
-
-        result = manager.get_odds_for_games(games)
-
-        assert len(result) == 2
-        assert result[0]['odds'] is None
-        assert result[1]['odds'] == {'spread': -1.0}
-
-    def test_input_dicts_mutated_in_place_and_returned(self, manager, mock_get):
-        # Pin: get_odds_for_games mutates the caller's game dicts in place
-        # and returns the same objects, not copies.
-        game = {'sport': 'football', 'league': 'nfl', 'id': '401'}
-
-        result = manager.get_odds_for_games([game])
-
-        assert result[0] is game
-        assert game['odds'] == FULL_EXTRACTED
-
-
-# ---------------------------------------------------------------------------
-# _load_configuration
-# ---------------------------------------------------------------------------
-
-class TestLoadConfiguration:
-    def test_loads_values_from_config(self, cache_manager):
-        config_manager = MagicMock()
-        config_manager.get_config.return_value = {
-            'base_odds_manager': {
-                'update_interval': 100,
-                'timeout': 5,
-                'cache_ttl': 42,
-            }
-        }
-
-        manager = BaseOddsManager(cache_manager, config_manager=config_manager)
-
-        assert manager.update_interval == 100
-        # Key/attr mismatch pin: the config key is 'timeout' but the
-        # attribute is request_timeout.
-        assert manager.request_timeout == 5
-        assert manager.cache_ttl == 42
-
-    def test_get_config_raising_keeps_defaults(self, cache_manager):
-        config_manager = MagicMock()
-        config_manager.get_config.side_effect = RuntimeError('boom')
-
-        manager = BaseOddsManager(cache_manager, config_manager=config_manager)
-
-        assert manager.update_interval == 3600
-        assert manager.request_timeout == 30
-        assert manager.cache_ttl == 1800

test/test_base_plugin_duration.py

@@ -1,146 +0,0 @@
-"""
-Tests for BasePlugin.get_display_duration — ~100 lines of type coercion that
-every plugin's rotation slot depends on, previously untested.
-
-The contract: a positive number wins wherever it comes from; everything else
-falls through instance attr → config → the 15.0 default, logging on the way.
-"""
-
-from unittest.mock import MagicMock
-
-import pytest
-
-from src.plugin_system.base_plugin import BasePlugin
-
-
-class _MinimalPlugin(BasePlugin):
-    def update(self):
-        pass
-
-    def display(self, force_clear=False):
-        pass
-
-
-def make_plugin(config=None, instance_duration="__unset__"):
-    plugin = _MinimalPlugin(
-        plugin_id="duration-test",
-        config=config or {},
-        display_manager=MagicMock(),
-        cache_manager=MagicMock(),
-        plugin_manager=MagicMock(),
-    )
-    if instance_duration != "__unset__":
-        plugin.display_duration = instance_duration
-    return plugin
-
-
-class TestInstanceVariable:
-    def test_positive_int_wins(self):
-        assert make_plugin(instance_duration=30).get_display_duration() == 30.0
-
-    def test_positive_float_wins(self):
-        assert make_plugin(instance_duration=12.5).get_display_duration() == 12.5
-
-    def test_returns_float_type(self):
-        result = make_plugin(instance_duration=30).get_display_duration()
-        assert isinstance(result, float)
-
-    def test_numeric_string_wins(self):
-        assert make_plugin(instance_duration="25").get_display_duration() == 25.0
-
-    def test_zero_falls_through_to_config(self):
-        plugin = make_plugin(config={"display_duration": 20},
-                             instance_duration=0)
-        assert plugin.get_display_duration() == 20.0
-
-    def test_negative_falls_through_to_config(self):
-        plugin = make_plugin(config={"display_duration": 20},
-                             instance_duration=-5)
-        assert plugin.get_display_duration() == 20.0
-
-    def test_none_falls_through_to_config(self):
-        plugin = make_plugin(config={"display_duration": 20},
-                             instance_duration=None)
-        assert plugin.get_display_duration() == 20.0
-
-    def test_garbage_string_falls_through(self):
-        plugin = make_plugin(config={"display_duration": 20},
-                             instance_duration="abc")
-        assert plugin.get_display_duration() == 20.0
-
-    def test_non_positive_string_falls_through(self):
-        plugin = make_plugin(config={"display_duration": 20},
-                             instance_duration="0")
-        assert plugin.get_display_duration() == 20.0
-
-    def test_unexpected_type_falls_through(self):
-        plugin = make_plugin(config={"display_duration": 20},
-                             instance_duration=[30])
-        assert plugin.get_display_duration() == 20.0
-
-    def test_bool_true_falls_through_like_any_non_number(self):
-        # bool is an int subclass, but a boolean is not a duration: True
-        # must NOT read as 1 second — it falls through to config/default.
-        assert make_plugin(instance_duration=True).get_display_duration() == 15.0
-
-    def test_bool_true_falls_through_to_config(self):
-        plugin = make_plugin(config={"display_duration": 20},
-                             instance_duration=True)
-        assert plugin.get_display_duration() == 20.0
-
-    def test_bool_false_still_falls_through(self):
-        plugin = make_plugin(config={"display_duration": 20},
-                             instance_duration=False)
-        assert plugin.get_display_duration() == 20.0
-
-
-class TestConfigFallback:
-    def test_config_number(self):
-        assert make_plugin({"display_duration": 20}).get_display_duration() == 20.0
-
-    def test_config_numeric_string(self):
-        assert make_plugin({"display_duration": "12.5"}).get_display_duration() == 12.5
-
-    def test_missing_config_uses_default(self):
-        assert make_plugin({}).get_display_duration() == 15.0
-
-    def test_config_zero_uses_default(self):
-        assert make_plugin({"display_duration": 0}).get_display_duration() == 15.0
-
-    def test_config_negative_uses_default(self):
-        assert make_plugin({"display_duration": -10}).get_display_duration() == 15.0
-
-    def test_config_garbage_string_uses_default(self):
-        assert make_plugin({"display_duration": "soon"}).get_display_duration() == 15.0
-
-    def test_config_unexpected_type_uses_default(self):
-        assert make_plugin({"display_duration": {"s": 5}}).get_display_duration() == 15.0
-
-    def test_config_none_uses_default(self):
-        assert make_plugin({"display_duration": None}).get_display_duration() == 15.0
-
-    def test_config_bool_uses_default(self):
-        assert make_plugin({"display_duration": True}).get_display_duration() == 15.0
-        assert make_plugin({"display_duration": False}).get_display_duration() == 15.0
-
-
-class TestValidateConfigDuration:
-    # validate_config must agree with get_display_duration about what a
-    # valid duration is — a config it accepts must not then be rejected
-    # (or silently defaulted) when the duration is actually read.
-
-    def test_positive_number_valid(self):
-        assert make_plugin({"display_duration": 20}).validate_config() is True
-
-    def test_zero_and_negative_invalid(self):
-        assert make_plugin({"display_duration": 0}).validate_config() is False
-        assert make_plugin({"display_duration": -5}).validate_config() is False
-
-    def test_bool_invalid(self):
-        # bool is an int subclass; True would otherwise pass as "positive
-        # number" here while get_display_duration rejects it.
-        assert make_plugin({"display_duration": True}).validate_config() is False
-        assert make_plugin({"display_duration": False}).validate_config() is False
-
-    def test_missing_duration_valid(self):
-        assert make_plugin({}).validate_config() is True

test/test_compatibility.py

@@ -1,271 +0,0 @@
-"""
-Tests for src/plugin_system/compatibility.py — the "can this plugin run on
-this core?" gate used by both the plugin loader (advisory) and the store
-manager (blocking at install/update time).
-
-This module had zero direct test coverage despite guarding every install.
-These tests pin the documented contract: refuse only on evidence, resolve
-every uncertain case (unparseable versions, missing fields, untrustworthy
-core) to compatible.
-"""
-
-import pytest
-
-from src.plugin_system.compatibility import (
-    TRUSTWORTHY_FLOOR,
-    parse_semver,
-    _parse_strict,
-    _satisfies_range,
-    satisfies_compatible_versions,
-    declared_min_version,
-    check,
-)
-
-
-class TestParseSemver:
-    def test_plain_triplet(self):
-        assert parse_semver("1.2.3") == (1, 2, 3)
-
-    def test_leading_v_tolerated(self):
-        assert parse_semver("v3.2.1") == (3, 2, 1)
-
-    def test_prerelease_suffix_stripped(self):
-        # "3.2.0-rc1" must NOT parse as (3, 2, 1) — a release candidate must
-        # not rank above its own release.
-        assert parse_semver("3.2.0-rc1") == (3, 2, 0)
-
-    def test_build_suffix_stripped(self):
-        # "3.2.0+build42" must NOT parse as (3, 2, 42).
-        assert parse_semver("3.2.0+build42") == (3, 2, 0)
-
-    def test_two_part_version_pads_zero(self):
-        assert parse_semver("1.2") == (1, 2, 0)
-
-    def test_one_part_version_pads_zeros(self):
-        assert parse_semver("2") == (2, 0, 0)
-
-    def test_extra_parts_ignored(self):
-        assert parse_semver("1.2.3.4") == (1, 2, 3)
-
-    def test_non_string_returns_none(self):
-        assert parse_semver(None) is None
-        assert parse_semver(123) is None
-        assert parse_semver((1, 2, 3)) is None
-
-    def test_garbage_with_no_digits_is_lenient_zero(self):
-        # Documented leniency: digit-scraping yields (0, 0, 0) for pure
-        # garbage. Fine for a floor (0.0.0 never blocks), wrong for ranges —
-        # which is why ranges go through _parse_strict instead.
-        assert parse_semver("garbage") == (0, 0, 0)
-
-    def test_whitespace_stripped(self):
-        assert parse_semver("  1.2.3  ") == (1, 2, 3)
-
-
-class TestParseStrict:
-    def test_accepts_real_versions(self):
-        assert _parse_strict("1.2.3") == (1, 2, 3)
-        assert _parse_strict("v1.2.3-rc1") == (1, 2, 3)
-        assert _parse_strict("2.0") == (2, 0, 0)
-
-    def test_rejects_garbage(self):
-        assert _parse_strict("not-a-version") is None
-        assert _parse_strict("") is None
-
-    def test_rejects_non_string(self):
-        assert _parse_strict(None) is None
-
-
-class TestSatisfiesRange:
-    CORE = (3, 1, 0)
-
-    @pytest.mark.parametrize("spec,expected", [
-        (">=3.0.0", True),
-        (">=3.1.0", True),
-        (">=3.2.0", False),
-        ("<=3.1.0", True),
-        ("<=3.0.9", False),
-        (">3.0.9", True),
-        (">3.1.0", False),
-        ("<3.2.0", True),
-        ("<3.1.0", False),
-    ])
-    def test_comparison_operators(self, spec, expected):
-        assert _satisfies_range(self.CORE, spec) is expected
-
-    def test_tilde_allows_patch_only(self):
-        # ~3.1.0 means >=3.1.0, <3.2.0
-        assert _satisfies_range((3, 1, 5), "~3.1.0") is True
-        assert _satisfies_range((3, 2, 0), "~3.1.0") is False
-        assert _satisfies_range((3, 0, 9), "~3.1.0") is False
-
-    def test_caret_allows_minor_and_patch(self):
-        # ^3.1.0 means >=3.1.0, <4.0.0
-        assert _satisfies_range((3, 9, 9), "^3.1.0") is True
-        assert _satisfies_range((4, 0, 0), "^3.1.0") is False
-        assert _satisfies_range((3, 0, 0), "^3.1.0") is False
-
-    def test_bare_exact_version(self):
-        assert _satisfies_range((3, 1, 0), "3.1.0") is True
-        assert _satisfies_range((3, 1, 1), "3.1.0") is False
-
-    def test_inclusive_dash_range(self):
-        assert _satisfies_range((2, 5, 0), "2.0.0 - 3.1.0") is True
-        assert _satisfies_range((2, 0, 0), "2.0.0 - 3.1.0") is True
-        assert _satisfies_range((3, 1, 0), "2.0.0 - 3.1.0") is True
-        assert _satisfies_range((3, 1, 1), "2.0.0 - 3.1.0") is False
-
-    def test_unparseable_spec_returns_none_not_false(self):
-        # Garbage must read as "no evidence", never as a refusal — an
-        # unrecognised spelling must not cost a user a working install.
-        assert _satisfies_range(self.CORE, "banana") is None
-        assert _satisfies_range(self.CORE, ">=banana") is None
-        assert _satisfies_range(self.CORE, "") is None
-        assert _satisfies_range(self.CORE, "banana - 3.0.0") is None
-
-
-class TestSatisfiesCompatibleVersions:
-    def test_any_entry_satisfying_wins(self):
-        manifest = {"compatible_versions": ["<1.0.0", ">=3.0.0"]}
-        assert satisfies_compatible_versions(manifest, (3, 1, 0)) is True
-
-    def test_all_entries_failing_is_false(self):
-        manifest = {"compatible_versions": ["<1.0.0", "2.0.0 - 2.9.9"]}
-        assert satisfies_compatible_versions(manifest, (3, 1, 0)) is False
-
-    def test_absent_field_returns_none(self):
-        assert satisfies_compatible_versions({}, (3, 1, 0)) is None
-
-    def test_empty_list_returns_none(self):
-        assert satisfies_compatible_versions(
-            {"compatible_versions": []}, (3, 1, 0)) is None
-
-    def test_non_list_returns_none(self):
-        assert satisfies_compatible_versions(
-            {"compatible_versions": ">=2.0.0"}, (3, 1, 0)) is None
-
-    def test_all_unparseable_entries_returns_none(self):
-        manifest = {"compatible_versions": ["banana", 42, None]}
-        assert satisfies_compatible_versions(manifest, (3, 1, 0)) is None
-
-    def test_mixed_parseable_and_garbage_uses_parseable(self):
-        manifest = {"compatible_versions": ["banana", ">=3.0.0"]}
-        assert satisfies_compatible_versions(manifest, (3, 1, 0)) is True
-
-
-class TestDeclaredMinVersion:
-    def test_top_level_field(self):
-        assert declared_min_version({"min_ledmatrix_version": "2.1.0"}) == "2.1.0"
-
-    def test_requires_dict_fallback(self):
-        manifest = {"requires": {"min_ledmatrix_version": "2.2.0"}}
-        assert declared_min_version(manifest) == "2.2.0"
-
-    def test_versions_array_fallback(self):
-        manifest = {"versions": [{"ledmatrix_min_version": "2.3.0"}]}
-        assert declared_min_version(manifest) == "2.3.0"
-
-    def test_versions_array_deprecated_spelling(self):
-        manifest = {"versions": [{"ledmatrix_min": "2.4.0"}]}
-        assert declared_min_version(manifest) == "2.4.0"
-
-    def test_top_level_wins_over_versions_array(self):
-        manifest = {
-            "min_ledmatrix_version": "2.1.0",
-            "versions": [{"ledmatrix_min_version": "9.9.9"}],
-        }
-        assert declared_min_version(manifest) == "2.1.0"
-
-    def test_requires_as_list_does_not_raise(self):
-        # A hand-edited manifest can carry `requires` as a list; this used to
-        # raise AttributeError and one malformed manifest would take down the
-        # whole install path.
-        assert declared_min_version({"requires": ["something"]}) is None
-
-    def test_versions_as_dict_does_not_raise(self):
-        # Same for `versions` as a mapping (used to raise KeyError).
-        assert declared_min_version({"versions": {"0": {}}}) is None
-
-    def test_nothing_declared_returns_none(self):
-        assert declared_min_version({}) is None
-
-
-class TestCheck:
-    def test_compatible_when_nothing_declared(self):
-        assert check({}, "3.1.0") == (True, None)
-
-    def test_min_version_blocks_older_core(self):
-        manifest = {"name": "Test Plugin", "min_ledmatrix_version": "3.2.0"}
-        ok, reason = check(manifest, "3.1.0")
-        assert ok is False
-        assert "3.2.0" in reason and "3.1.0" in reason
-
-    def test_min_version_allows_equal_core(self):
-        manifest = {"min_ledmatrix_version": "3.1.0"}
-        assert check(manifest, "3.1.0") == (True, None)
-
-    def test_compatible_versions_upper_bound_blocks(self):
-        # A range is the only field that can express "not compatible with
-        # newer cores" — it must win even when the floor passes.
-        manifest = {
-            "name": "Old Plugin",
-            "min_ledmatrix_version": "2.0.0",
-            "compatible_versions": ["2.0.0 - 2.9.9"],
-        }
-        ok, reason = check(manifest, "3.1.0")
-        assert ok is False
-        assert "2.0.0 - 2.9.9" in reason
-
-    def test_unparseable_core_with_high_floor_is_blocked(self):
-        manifest = {"min_ledmatrix_version": "3.2.0",
-                    "compatible_versions": [">=3.2.0"]}
-        # An unparseable core version is "unknown", not "old"... but note
-        # parse_semver("garbage") == (0,0,0) which is below TRUSTWORTHY_FLOOR,
-        # so this rides the untrustworthy-core branch: floor > 2.0.0 blocks.
-        ok, reason = check(manifest, "garbage")
-        assert ok is False
-        assert "too old to identify reliably" in reason
-
-    def test_untrustworthy_core_allows_ecosystem_baseline_floor(self):
-        # A core reporting 1.0.0 may really be v3.1.0 (which shipped with a
-        # wrong __version__). Floors at or below TRUSTWORTHY_FLOOR must not
-        # block, or that population could install nothing.
-        manifest = {"min_ledmatrix_version": "2.0.0",
-                    "compatible_versions": [">=2.0.0"]}
-        assert check(manifest, "1.0.0") == (True, None)
-
-    def test_untrustworthy_core_blocks_floor_above_baseline(self):
-        # But a floor above 2.0.0 needs modules that no core reporting below
-        # the floor can have — the one refusal on that branch.
-        manifest = {"name": "New Plugin", "min_ledmatrix_version": "3.2.0"}
-        ok, reason = check(manifest, "1.0.0")
-        assert ok is False
-        assert "too old to identify reliably" in reason
-
-    def test_untrustworthy_core_ignores_compatible_versions(self):
-        # On the untrustworthy branch only the declared floor is consulted;
-        # ranges cannot be evaluated against a version that isn't evidence.
-        manifest = {"compatible_versions": ["2.0.0 - 2.9.9"]}
-        assert check(manifest, "1.0.0") == (True, None)
-
-    def test_floor_exactly_at_trustworthy_floor_is_allowed(self):
-        floor = ".".join(str(n) for n in TRUSTWORTHY_FLOOR)
-        manifest = {"min_ledmatrix_version": floor}
-        assert check(manifest, "1.0.0") == (True, None)
-
-    def test_reason_uses_manifest_name(self):
-        manifest = {"name": "Fancy Clock", "min_ledmatrix_version": "9.0.0"}
-        ok, reason = check(manifest, "3.1.0")
-        assert ok is False
-        assert reason.startswith("Fancy Clock")
-
-    def test_reason_falls_back_to_id(self):
-        manifest = {"id": "fancy-clock", "min_ledmatrix_version": "9.0.0"}
-        ok, reason = check(manifest, "3.1.0")
-        assert ok is False
-        assert reason.startswith("fancy-clock")
-
-    def test_prerelease_core_compares_equal_to_release(self):
-        # Documented: prereleases compare equal to their release.
-        manifest = {"min_ledmatrix_version": "3.2.0"}
-        assert check(manifest, "3.2.0-rc1") == (True, None)

test/test_config_helper.py

@@ -1,253 +0,0 @@
-"""
-Tests for src/common/config_helper.py — pins the ConfigHelper contract.
-
-Covers: load/save round trips (missing/malformed files return {} rather
-than raising, non-ASCII preserved via ensure_ascii=False, top-level JSON
-lists returned as-is), dot-notation get/set including the silent-failure
-contract when an intermediate key holds a non-dict, merge_configs deep
-semantics with NO aliasing of the base config (the fixed bug — the old
-shallow copy let mutations of the merged result leak into base's nested
-dicts), simplified schema validation including the caught-TypeError path
-when a schema 'type' is given as a string, plugin config key conventions
-('{plugin_id}_config', enabled defaults True), and required-key checks
-where a key present with value None counts as present.
-"""
-
-import json
-
-import pytest
-
-from src.common.config_helper import ConfigHelper
-
-
-@pytest.fixture
-def helper():
-    return ConfigHelper()
-
-
-class TestLoadConfig:
-    def test_missing_file_returns_empty_dict(self, helper, tmp_path):
-        assert helper.load_config(tmp_path / "nope.json") == {}
-
-    def test_malformed_json_returns_empty_dict(self, helper, tmp_path):
-        path = tmp_path / "bad.json"
-        path.write_text("{ this is not json", encoding="utf-8")
-        assert helper.load_config(path) == {}
-
-    def test_top_level_list_returned_as_is(self, helper, tmp_path):
-        # load_config does not enforce a dict shape: a JSON list comes
-        # straight back. Pinned as a characterization of current behavior.
-        path = tmp_path / "list.json"
-        path.write_text("[1, 2, 3]", encoding="utf-8")
-        assert helper.load_config(path) == [1, 2, 3]
-
-
-class TestSaveConfig:
-    def test_round_trip(self, helper, tmp_path):
-        path = tmp_path / "config.json"
-        config = {'display': {'hardware': {'rows': 32}}, 'timezone': 'UTC'}
-        assert helper.save_config(config, path) is True
-        assert helper.load_config(path) == config
-
-    def test_creates_parent_directories(self, helper, tmp_path):
-        path = tmp_path / "deep" / "nested" / "config.json"
-        assert helper.save_config({'a': 1}, path) is True
-        assert path.exists()
-        assert helper.load_config(path) == {'a': 1}
-
-    def test_non_ascii_survives_round_trip(self, helper, tmp_path):
-        path = tmp_path / "config.json"
-        config = {'city': 'Zürich', 'note': 'météo ☀'}
-        assert helper.save_config(config, path) is True
-        assert helper.load_config(path) == config
-        # ensure_ascii=False: characters are written raw, not \u-escaped
-        assert 'Zürich' in path.read_text(encoding='utf-8')
-
-    def test_directory_path_returns_false_not_raise(self, helper, tmp_path):
-        assert helper.save_config({'a': 1}, tmp_path) is False
-
-
-class TestGetConfigValue:
-    def test_dot_notation_hit(self, helper):
-        config = {'display': {'hardware': {'rows': 32}}}
-        assert helper.get_config_value(config, 'display.hardware.rows') == 32
-
-    def test_missing_returns_default(self, helper):
-        sentinel = object()
-        assert helper.get_config_value({}, 'display.rows', default=sentinel) is sentinel
-
-    def test_intermediate_non_dict_returns_default(self, helper):
-        config = {'display': 'not-a-dict'}
-        assert helper.get_config_value(config, 'display.hardware.rows', default=64) == 64
-
-    def test_required_missing_raises_keyerror(self, helper):
-        with pytest.raises(KeyError):
-            helper.get_config_value({}, 'display.rows', required=True)
-
-
-class TestSetConfigValue:
-    def test_sets_top_level(self, helper):
-        config = {}
-        helper.set_config_value(config, 'timezone', 'UTC')
-        assert config == {'timezone': 'UTC'}
-
-    def test_auto_creates_intermediates(self, helper):
-        config = {}
-        helper.set_config_value(config, 'display.hardware.rows', 32)
-        assert config == {'display': {'hardware': {'rows': 32}}}
-
-    def test_silent_failure_on_non_dict_intermediate(self, helper):
-        # 'a' exists but holds an int; the assignment attempt raises
-        # TypeError internally, which set_config_value swallows and logs.
-        # The config is left unchanged — pinned silent-failure contract.
-        config = {'a': 5}
-        helper.set_config_value(config, 'a.b', 1)
-        assert config == {'a': 5}
-
-
-class TestMergeConfigs:
-    def test_nested_dicts_merge_recursively(self, helper):
-        base = {'display': {'rows': 32, 'cols': 64}, 'timezone': 'UTC'}
-        override = {'display': {'cols': 128, 'brightness': 90}}
-        merged = helper.merge_configs(base, override)
-        assert merged == {
-            'display': {'rows': 32, 'cols': 128, 'brightness': 90},
-            'timezone': 'UTC',
-        }
-
-    def test_scalar_override_wins_over_dict(self, helper):
-        merged = helper.merge_configs({'display': {'rows': 32}}, {'display': 7})
-        assert merged['display'] == 7
-
-    def test_dict_override_wins_over_scalar(self, helper):
-        merged = helper.merge_configs({'display': 7}, {'display': {'rows': 32}})
-        assert merged['display'] == {'rows': 32}
-
-    def test_no_aliasing_of_base(self, helper):
-        # Post-fix: merge deep-copies base, so mutating the result never
-        # leaks back into the caller's base config.
-        base = {'display': {'x': 1}}
-        merged = helper.merge_configs(base, {})
-        assert merged['display'] is not base['display']
-        merged['display']['x'] = 99
-        assert base['display']['x'] == 1
-
-    def test_inputs_unchanged(self, helper):
-        base = {'a': {'b': 1}}
-        override = {'a': {'c': 2}}
-        helper.merge_configs(base, override)
-        assert base == {'a': {'b': 1}}
-        assert override == {'a': {'c': 2}}
-
-    def test_no_aliasing_of_override_values(self, helper):
-        # The non-recursive branch must deep-copy the override value too:
-        # mutating a merged-in list or dict must not reach back into
-        # override_config.
-        override = {'teams': ['A', 'B'], 'nested': {'x': [1]}}
-        merged = helper.merge_configs({}, override)
-        merged['teams'].append('C')
-        merged['nested']['x'].append(2)
-        assert override == {'teams': ['A', 'B'], 'nested': {'x': [1]}}
-
-
-class TestValidateConfig:
-    def test_no_schema_dict_is_valid(self, helper):
-        assert helper.validate_config({'a': 1}) is True
-
-    def test_no_schema_list_is_invalid(self, helper):
-        assert helper.validate_config([1, 2]) is False
-
-    def test_required_key_missing_is_invalid(self, helper):
-        schema = {'rows': {'required': True, 'type': int}}
-        assert helper.validate_config({}, schema) is False
-
-    def test_optional_key_missing_is_valid(self, helper):
-        schema = {'rows': {'required': False, 'type': int}}
-        assert helper.validate_config({}, schema) is True
-
-    def test_wrong_type_is_invalid(self, helper):
-        schema = {'rows': {'type': int}}
-        assert helper.validate_config({'rows': 'thirty-two'}, schema) is False
-        assert helper.validate_config({'rows': 32}, schema) is True
-
-    def test_allowed_values_violation_is_invalid(self, helper):
-        schema = {'mode': {'allowed_values': ['clock', 'weather']}}
-        assert helper.validate_config({'mode': 'stocks'}, schema) is False
-        assert helper.validate_config({'mode': 'clock'}, schema) is True
-
-    def test_string_type_in_schema_is_invalid_via_typeerror(self, helper):
-        # 'type' given as the STRING "int" makes isinstance() raise
-        # TypeError; validate_config catches it and returns False rather
-        # than raising. Pinned characterization.
-        schema = {'rows': {'type': 'int'}}
-        assert helper.validate_config({'rows': 32}, schema) is False
-
-
-class TestPluginConfigHelpers:
-    def test_get_plugin_config_uses_suffixed_key(self, helper):
-        plugin_cfg = {'enabled': True, 'display_duration': 30}
-        assert helper.get_plugin_config({'clock_config': plugin_cfg}, 'clock') == plugin_cfg
-
-    def test_get_plugin_config_bare_id_key_not_found(self, helper):
-        # Only '{plugin_id}_config' is consulted — a bare 'clock' section
-        # is invisible to this helper. Pinned key contract.
-        assert helper.get_plugin_config({'clock': {'enabled': True}}, 'clock') == {}
-
-    def test_create_default_config_wraps_in_suffixed_key(self, helper):
-        defaults = {'enabled': True}
-        assert helper.create_default_config('clock', defaults) == {'clock_config': defaults}
-
-    def test_is_plugin_enabled_defaults_true_for_unknown(self, helper):
-        assert helper.is_plugin_enabled({}, 'clock') is True
-
-    def test_is_plugin_enabled_false_when_disabled(self, helper):
-        config = {'clock_config': {'enabled': False}}
-        assert helper.is_plugin_enabled(config, 'clock') is False
-
-    def test_is_plugin_enabled_ignores_bare_id_key(self, helper):
-        # Disabled under the wrong key -> still reported enabled (default).
-        config = {'clock': {'enabled': False}}
-        assert helper.is_plugin_enabled(config, 'clock') is True
-
-
-class TestSportsAndDisplayHelpers:
-    def test_get_display_config(self, helper):
-        display = {'hardware': {'rows': 32}}
-        assert helper.get_display_config({'display': display}) == display
-        assert helper.get_display_config({}) == {}
-
-    def test_get_sports_config_uses_scoreboard_suffix(self, helper):
-        sport_cfg = {'favorite_teams': ['TB']}
-        config = {'football_scoreboard': sport_cfg}
-        assert helper.get_sports_config(config, 'football') == sport_cfg
-        assert helper.get_sports_config(config, 'hockey') == {}
-
-    def test_get_favorite_teams(self, helper):
-        config = {'football_scoreboard': {'favorite_teams': ['TB', 'DAL']}}
-        assert helper.get_favorite_teams(config, 'football') == ['TB', 'DAL']
-        assert helper.get_favorite_teams({}, 'football') == []
-
-    def test_get_display_modes(self, helper):
-        modes = {'live': True, 'recent': False}
-        config = {'football_scoreboard': {'display_modes': modes}}
-        assert helper.get_display_modes(config, 'football') == modes
-        assert helper.get_display_modes({}, 'football') == {}
-
-
-class TestValidateRequiredKeys:
-    def test_returns_missing_subset(self, helper):
-        config = {'a': 1, 'c': {'d': 2}}
-        missing = helper.validate_required_keys(config, ['a', 'b', 'c.d', 'c.e'])
-        assert missing == ['b', 'c.e']
-
-    def test_dot_notation_present(self, helper):
-        config = {'display': {'hardware': {'rows': 32}}}
-        assert helper.validate_required_keys(config, ['display.hardware.rows']) == []
-
-    def test_empty_requirements(self, helper):
-        assert helper.validate_required_keys({'a': 1}, []) == []
-
-    def test_present_with_none_counts_as_present(self, helper):
-        # _has_key checks key membership, not truthiness — a key set to
-        # None is NOT reported missing. Pinned semantics.
-        assert helper.validate_required_keys({'a': None}, ['a']) == []

test/test_config_manager_secrets.py

@@ -1,335 +0,0 @@
-"""
-Tests for the ConfigManager secrets round-trip and the load_config fast path.
-
-The contract under test: config_secrets.json values are deep-merged INTO the
-in-memory config at load time, and stripped back OUT before anything is
-written to config.json — so secrets live in exactly one file on disk. This
-suite pins that round-trip plus its sharp edges, including the guard that a
-save REFUSES (ConfigError) when the secrets file exists but can't be loaded,
-rather than leaking merged secrets into config.json in plaintext.
-
-Complements test_config_manager.py, which covers loading/migration/validation.
-"""
-
-import json
-import os
-
-import pytest
-
-from src.config_manager import ConfigManager
-from src.exceptions import ConfigError
-
-
-def make_manager(tmp_path, config=None, secrets=None):
-    """A ConfigManager over tmp_path files, template migration neutralized."""
-    config_file = tmp_path / "config.json"
-    secrets_file = tmp_path / "config_secrets.json"
-    config_file.write_text(json.dumps(config if config is not None else {}))
-    if secrets is not None:
-        secrets_file.write_text(json.dumps(secrets))
-    manager = ConfigManager(config_path=str(config_file),
-                            secrets_path=str(secrets_file))
-    # Point the (CWD-relative) template at nothing so migration never runs —
-    # these tests assert exact on-disk contents.
-    manager.template_path = str(tmp_path / "no-template.json")
-    return manager
-
-
-class TestLoadMergesSecrets:
-    def test_secrets_deep_merged_into_config(self, tmp_path):
-        manager = make_manager(
-            tmp_path,
-            config={"weather": {"city": "Austin"}, "timezone": "UTC"},
-            secrets={"weather": {"api_key": "s3cret"}},
-        )
-        loaded = manager.load_config()
-        assert loaded["weather"] == {"city": "Austin", "api_key": "s3cret"}
-        assert loaded["timezone"] == "UTC"
-
-    def test_secret_scalar_overrides_config_value(self, tmp_path):
-        manager = make_manager(
-            tmp_path,
-            config={"weather": {"api_key": "YOUR_API_KEY"}},
-            secrets={"weather": {"api_key": "real-key"}},
-        )
-        assert manager.load_config()["weather"]["api_key"] == "real-key"
-
-    def test_missing_secrets_file_loads_config_fine(self, tmp_path):
-        manager = make_manager(tmp_path, config={"timezone": "UTC"})
-        assert manager.load_config() == {"timezone": "UTC"}
-
-    def test_corrupt_secrets_file_loads_config_without_secrets(self, tmp_path):
-        manager = make_manager(tmp_path, config={"timezone": "UTC"})
-        (tmp_path / "config_secrets.json").write_text("{not json")
-        loaded = manager.load_config()
-        assert loaded["timezone"] == "UTC"
-
-
-class TestSaveStripsSecrets:
-    def test_round_trip_keeps_secrets_out_of_config_json(self, tmp_path):
-        manager = make_manager(
-            tmp_path,
-            config={"weather": {"city": "Austin"}},
-            secrets={"weather": {"api_key": "s3cret"}},
-        )
-        loaded = manager.load_config()
-        assert loaded["weather"]["api_key"] == "s3cret"  # merged in memory
-
-        manager.save_config(loaded)
-
-        on_disk = json.loads((tmp_path / "config.json").read_text())
-        assert "api_key" not in on_disk.get("weather", {})
-        assert on_disk["weather"]["city"] == "Austin"
-        # In-memory config still carries the secret for runtime use.
-        assert manager.config["weather"]["api_key"] == "s3cret"
-
-    def test_group_dropped_when_only_secrets_remain(self, tmp_path):
-        # _strip_secrets_recursive drops a group entirely when nothing
-        # non-secret is left in it.
-        manager = make_manager(
-            tmp_path,
-            config={},
-            secrets={"weather": {"api_key": "s3cret"}},
-        )
-        manager.save_config({"weather": {"api_key": "s3cret"}, "timezone": "UTC"})
-        on_disk = json.loads((tmp_path / "config.json").read_text())
-        assert on_disk == {"timezone": "UTC"}
-
-    def test_scalar_secret_key_stripped_at_top_level(self, tmp_path):
-        manager = make_manager(tmp_path, config={}, secrets={"token": "t"})
-        manager.save_config({"token": "t", "timezone": "UTC"})
-        on_disk = json.loads((tmp_path / "config.json").read_text())
-        assert on_disk == {"timezone": "UTC"}
-
-    def test_corrupt_secrets_file_refuses_save_no_plaintext_leak(self, tmp_path):
-        # Regression guard: when the secrets file exists but is corrupt at
-        # save time, stripping is impossible — the save must raise instead of
-        # writing the merged secrets into config.json in plaintext (the
-        # historical behavior).
-        manager = make_manager(
-            tmp_path,
-            config={"weather": {"city": "Austin"}},
-            secrets={"weather": {"api_key": "s3cret"}},
-        )
-        loaded = manager.load_config()
-        (tmp_path / "config_secrets.json").write_text("{corrupt")
-
-        with pytest.raises(ConfigError):
-            manager.save_config(loaded)
-
-        # On-disk config untouched: no secret leaked.
-        on_disk = json.loads((tmp_path / "config.json").read_text())
-        assert "api_key" not in on_disk.get("weather", {})
-
-    def test_corrupt_secrets_file_refuses_atomic_save_too(self, tmp_path):
-        # Same refusal on the atomic save path, which shared the leak.
-        manager = make_manager(
-            tmp_path,
-            config={"weather": {"city": "Austin"}},
-            secrets={"weather": {"api_key": "s3cret"}},
-        )
-        loaded = manager.load_config()
-        (tmp_path / "config_secrets.json").write_text("{corrupt")
-
-        with pytest.raises(ConfigError):
-            manager.save_config_atomic(loaded)
-
-        on_disk = json.loads((tmp_path / "config.json").read_text())
-        assert "api_key" not in on_disk.get("weather", {})
-
-
-class TestLoadFastPath:
-    def test_unchanged_files_return_cached_dict(self, tmp_path):
-        manager = make_manager(tmp_path, config={"timezone": "UTC"})
-        first = manager.load_config()
-        second = manager.load_config()
-        assert second is first  # same aliased dict, no re-read
-
-    def test_touching_secrets_file_invalidates_cache(self, tmp_path):
-        manager = make_manager(
-            tmp_path,
-            config={"weather": {}},
-            secrets={"weather": {"api_key": "old"}},
-        )
-        assert manager.load_config()["weather"]["api_key"] == "old"
-
-        secrets_file = tmp_path / "config_secrets.json"
-        secrets_file.write_text(json.dumps({"weather": {"api_key": "new"}}))
-        # Force a different mtime_ns in case the write landed within the
-        # filesystem's timestamp granularity.
-        os.utime(secrets_file, ns=(1, 1))
-
-        assert manager.load_config()["weather"]["api_key"] == "new"
-
-    def test_same_mtime_same_size_change_served_stale(self, tmp_path):
-        # Characterized fast-path blind spot: the signature is (mtime_ns,
-        # size) only, so a same-length content swap with a forged identical
-        # mtime is not detected. Real writes bump mtime_ns, so this is
-        # acceptable — but it is a contract worth pinning.
-        manager = make_manager(tmp_path, config={"timezone": "AAA"})
-        config_file = tmp_path / "config.json"
-        os.utime(config_file, ns=(1_000_000_000, 1_000_000_000))
-        manager._loaded_sig = None
-        first = manager.load_config()
-        assert first["timezone"] == "AAA"
-
-        config_file.write_text(json.dumps({"timezone": "BBB"}))  # same length
-        os.utime(config_file, ns=(1_000_000_000, 1_000_000_000))
-
-        assert manager.load_config()["timezone"] == "AAA"  # stale, by design
-
-
-class TestArraySecretStripAndMerge:
-    """Array-item secrets round-trip (parallel-placeholder lists).
-
-    secret_helpers.separate_secrets emits array secrets as a list parallel
-    to the regular list, with {} for items that carry no secrets. Strip
-    must remove the secret fields from config.json while preserving item
-    indices; load must merge them back into the right items. The regular
-    list's length is authoritative in both directions.
-    """
-
-    def test_strip_removes_array_item_secrets_keeps_indices(self, tmp_path):
-        manager = make_manager(tmp_path)
-        data = {"plugin": {"accounts": [
-            {"name": "a", "token": "ta"},
-            {"name": "b"},
-        ]}}
-        secrets = {"plugin": {"accounts": [{"token": "ta"}, {}]}}
-        stripped = manager._strip_secrets_recursive(data, secrets)
-        assert stripped == {"plugin": {"accounts": [{"name": "a"}, {"name": "b"}]}}
-
-    def test_strip_keeps_all_placeholder_items(self, tmp_path):
-        # Even when every item strips to nothing extra, the list survives
-        # with its indices — required for merge-on-load alignment.
-        manager = make_manager(tmp_path)
-        data = {"accounts": [{"token": "t1"}, {"token": "t2"}]}
-        secrets = {"accounts": [{"token": "t1"}, {"token": "t2"}]}
-        stripped = manager._strip_secrets_recursive(data, secrets)
-        assert stripped == {"accounts": [{}, {}]}
-
-    def test_strip_whole_scalar_array_secret_drops_key(self, tmp_path):
-        # A list of secret scalars is a whole-key secret, not the parallel
-        # shape — the key must vanish from config.json entirely.
-        manager = make_manager(tmp_path)
-        data = {"recovery_codes": ["a", "b"], "city": "Austin"}
-        secrets = {"recovery_codes": ["a", "b"]}
-        stripped = manager._strip_secrets_recursive(data, secrets)
-        assert stripped == {"city": "Austin"}
-
-    def test_strip_shape_mismatch_drops_key(self, tmp_path):
-        # Conservative contract: if the shapes disagree, never leak.
-        manager = make_manager(tmp_path)
-        data = {"accounts": {"name": "not-a-list"}}
-        secrets = {"accounts": [{"token": "t"}]}
-        stripped = manager._strip_secrets_recursive(data, secrets)
-        assert stripped == {}
-
-    def test_strip_ignores_extra_secrets_entries(self, tmp_path):
-        # Regular list length is authoritative: a user deleted an item.
-        manager = make_manager(tmp_path)
-        data = {"accounts": [{"name": "a", "token": "ta"}]}
-        secrets = {"accounts": [{"token": "ta"}, {"token": "tb"}]}
-        stripped = manager._strip_secrets_recursive(data, secrets)
-        assert stripped == {"accounts": [{"name": "a"}]}
-
-    def test_merge_restores_array_item_secrets(self, tmp_path):
-        manager = make_manager(tmp_path)
-        target = {"accounts": [{"name": "a"}, {"name": "b"}]}
-        manager._deep_merge(target, {"accounts": [{"token": "ta"}, {}]})
-        assert target == {"accounts": [
-            {"name": "a", "token": "ta"},
-            {"name": "b"},
-        ]}
-
-    def test_merge_ignores_extra_secrets_entries_with_warning(self, tmp_path, caplog):
-        manager = make_manager(tmp_path)
-        target = {"accounts": [{"name": "a"}]}
-        with caplog.at_level("WARNING"):
-            manager._deep_merge(
-                target, {"accounts": [{"token": "ta"}, {"token": "ghost"}]})
-        assert target == {"accounts": [{"name": "a", "token": "ta"}]}
-        assert any("longer than the config list" in r.message for r in caplog.records)
-
-    def test_merge_non_dict_item_replaced_by_secret(self, tmp_path):
-        # Shape drift inside the list: the secret wins for that index.
-        manager = make_manager(tmp_path)
-        target = {"accounts": ["oddball", {"name": "b"}]}
-        manager._deep_merge(target, {"accounts": [{"token": "ta"}, {}]})
-        assert target == {"accounts": [{"token": "ta"}, {"name": "b"}]}
-
-    def test_merge_whole_scalar_array_still_replaces(self, tmp_path):
-        # Legacy behavior preserved: a non-parallel list replaces wholesale.
-        manager = make_manager(tmp_path)
-        target = {"recovery_codes": ["old"]}
-        manager._deep_merge(target, {"recovery_codes": ["new1", "new2"]})
-        assert target == {"recovery_codes": ["new1", "new2"]}
-
-    def test_full_save_load_round_trip(self, tmp_path):
-        # End to end on real files: save strips array secrets out of
-        # config.json; load merges them back into the right items.
-        manager = make_manager(
-            tmp_path,
-            config={"plugin": {"accounts": [
-                {"name": "a", "token": "s3cret-a"},
-                {"name": "b", "token": "s3cret-b"},
-            ]}},
-            secrets={"plugin": {"accounts": [
-                {"token": "s3cret-a"}, {"token": "s3cret-b"},
-            ]}},
-        )
-        loaded = manager.load_config()
-        assert loaded["plugin"]["accounts"][0]["token"] == "s3cret-a"
-
-        manager.save_config(loaded)
-
-        raw = (tmp_path / "config.json").read_text()
-        assert "s3cret" not in raw
-        on_disk = json.loads(raw)
-        assert on_disk["plugin"]["accounts"] == [{"name": "a"}, {"name": "b"}]
-
-        # A fresh manager (constructed directly — make_manager would
-        # overwrite the just-saved config.json) re-merges from the secrets
-        # file on load.
-        fresh = ConfigManager(config_path=str(tmp_path / "config.json"),
-                              secrets_path=str(tmp_path / "config_secrets.json"))
-        fresh.template_path = str(tmp_path / "no-template.json")
-        reloaded = fresh.load_config()
-        assert reloaded["plugin"]["accounts"] == [
-            {"name": "a", "token": "s3cret-a"},
-            {"name": "b", "token": "s3cret-b"},
-        ]
-
-    def test_whole_item_secret_list_never_leaks_values(self, tmp_path):
-        # When the ENTIRE array item is secret (schema marks both key[]
-        # and key[].field), separate_secrets stores the full item dicts in
-        # the secrets file. That shape also matches the parallel-list
-        # discriminator — which is safe: strip drops every leaf key that
-        # appears in the secret item, so only empty {} skeletons (item
-        # count, no values) can reach config.json, and merge-on-load
-        # restores the full items from those skeletons.
-        from src.web_interface.secret_helpers import (
-            find_secret_fields, separate_secrets)
-        schema_props = {"accounts": {
-            "type": "array",
-            "items": {"type": "object", "x-secret": True, "properties": {
-                "id": {"type": "string"},
-                "token": {"type": "string", "x-secret": True},
-            }},
-        }}
-        paths = find_secret_fields(schema_props)
-        assert paths == {"accounts[]", "accounts[].token"}
-        full = {"accounts": [{"id": "i1", "token": "s3cret-a"},
-                             {"id": "i2", "token": "s3cret-b"}]}
-        _, secrets = separate_secrets(full, paths)
-        assert secrets == full  # whole items are secret
-
-        manager = make_manager(tmp_path)
-        stripped = manager._strip_secrets_recursive(full, secrets)
-        assert stripped == {"accounts": [{}, {}]}
-
-        raw = json.dumps(stripped)
-        assert "s3cret" not in raw and "i1" not in raw
-
-        manager._deep_merge(stripped, secrets)
-        assert stripped == full  # round trip restores the items

test/test_discovery_path_contract.py

@@ -1,167 +0,0 @@
-"""
-Drift guard: three components independently answer "where is plugin X?" and
-their answers must stay coherent — plus the `.standalone-backup-` naming
-contract that install/rollback shares with discovery.
-
-The three resolvers:
-1. PluginManager._scan_directory_for_plugins — scans ONLY the configured dir.
-2. PluginStoreManager._find_plugin_path — configured dir, then a sibling
-   `plugins/` fallback derived from the configured dir's parent.
-3. SchemaManager.get_schema_path — configured dir, then project-root
-   `plugins/`, then `plugin-repos/`, then case-insensitive scans.
-
-The divergence is characterized (a plugin visible to the store/schema
-fallbacks but invisible to discovery is a real support-issue shape) so any
-change to the fallback chains is a deliberate one.
-
-The `.standalone-backup-` contract: store_manager renames a plugin dir aside
-with that substring during install/rollback; discovery MUST skip such dirs
-or a half-finished install would surface a ghost plugin. The substring is
-duplicated as a literal in both files — this test breaks if either side
-changes it unilaterally.
-"""
-
-import json
-import logging
-import threading
-from pathlib import Path
-
-import pytest
-
-from src.plugin_system.plugin_manager import PluginManager
-from src.plugin_system.schema_manager import SchemaManager
-from src.plugin_system.store_manager import PluginStoreManager
-
-
-def _write_plugin(base: Path, plugin_id: str, dir_name: str = None):
-    plugin_dir = base / (dir_name or plugin_id)
-    plugin_dir.mkdir(parents=True)
-    (plugin_dir / "manifest.json").write_text(json.dumps({
-        "id": plugin_id, "name": plugin_id, "version": "1.0.0",
-    }))
-    (plugin_dir / "config_schema.json").write_text(json.dumps({
-        "type": "object", "properties": {"enabled": {"type": "boolean"}},
-    }))
-    return plugin_dir
-
-
-def _scanner():
-    """A PluginManager stripped to just its discovery machinery — the full
-    constructor wires config/schema/health managers this test doesn't need."""
-    pm = object.__new__(PluginManager)
-    pm.logger = logging.getLogger("test_discovery_path_contract")
-    pm._discovery_lock = threading.Lock()
-    pm.plugin_manifests = {}
-    pm.plugin_directories = {}
-    return pm
-
-
-class TestResolversAgreeOnConfiguredDir:
-    def test_all_three_find_a_plugin_in_the_configured_dir(self, tmp_path):
-        plugins_dir = tmp_path / "plugin-repos"
-        plugin_dir = _write_plugin(plugins_dir, "demo-plugin")
-
-        found = _scanner()._scan_directory_for_plugins(plugins_dir)
-        assert found == ["demo-plugin"]
-
-        store = PluginStoreManager(
-            plugins_dir=str(plugins_dir),
-            uninstalled_registry_path=str(tmp_path / "uninstalled.json"))
-        assert store._find_plugin_path("demo-plugin") == plugin_dir
-
-        schema = SchemaManager(plugins_dir=plugins_dir, project_root=tmp_path)
-        assert schema.get_schema_path("demo-plugin") == \
-            plugin_dir / "config_schema.json"
-
-
-class TestFallbackDivergence:
-    def test_plugin_only_in_plugins_dir_fallback(self, tmp_path):
-        """Characterized divergence: configured dir is plugin-repos/, but the
-        plugin sits in a sibling plugins/. The store and schema fallbacks
-        find it; discovery does NOT — so the plugin is installable/
-        configurable but never loads. Pinned so a change to any fallback
-        chain shows up here."""
-        configured = tmp_path / "plugin-repos"
-        configured.mkdir()
-        legacy_dir = _write_plugin(tmp_path / "plugins", "legacy-plugin")
-
-        # Discovery: invisible.
-        assert _scanner()._scan_directory_for_plugins(configured) == []
-
-        # Store fallback: visible (parent-of-configured / 'plugins').
-        store = PluginStoreManager(
-            plugins_dir=str(configured),
-            uninstalled_registry_path=str(tmp_path / "uninstalled.json"))
-        assert store._find_plugin_path("legacy-plugin") == legacy_dir
-
-        # Schema fallback: visible (project_root / 'plugins').
-        schema = SchemaManager(plugins_dir=configured, project_root=tmp_path)
-        assert schema.get_schema_path("legacy-plugin") == \
-            legacy_dir / "config_schema.json"
-
-    def test_schema_manager_probes_plugins_before_plugin_repos(self, tmp_path):
-        # Documented order (also in CLAUDE.md): plugins/ wins over
-        # plugin-repos/ when the same id exists in both.
-        in_plugins = _write_plugin(tmp_path / "plugins", "dupe")
-        _write_plugin(tmp_path / "plugin-repos", "dupe")
-        schema = SchemaManager(plugins_dir=None, project_root=tmp_path)
-        assert schema.get_schema_path("dupe") == \
-            in_plugins / "config_schema.json"
-
-    def test_schema_manager_case_insensitive_fallback(self, tmp_path):
-        plugin_dir = _write_plugin(tmp_path / "plugins", "MyPlugin",
-                                   dir_name="MyPlugin")
-        schema = SchemaManager(plugins_dir=None, project_root=tmp_path)
-        assert schema.get_schema_path("myplugin") == \
-            plugin_dir / "config_schema.json"
-
-
-class TestStandaloneBackupContract:
-    def test_discovery_skips_backup_dirs(self, tmp_path):
-        plugins_dir = tmp_path / "plugins"
-        _write_plugin(plugins_dir, "real-plugin")
-        # A rollback-in-progress dir with a valid manifest must NOT surface.
-        _write_plugin(plugins_dir, "real-plugin",
-                      dir_name="real-plugin.standalone-backup-migrating")
-
-        found = _scanner()._scan_directory_for_plugins(plugins_dir)
-        assert found == ["real-plugin"]
-
-    def test_backup_substring_literal_matches_across_files(self):
-        """The substring is duplicated in plugin_manager (skip check) and
-        store_manager (rename-aside names). If either side changes it, the
-        other silently stops honoring the contract — this test is the
-        tripwire."""
-        root = Path(__file__).resolve().parents[1]
-        pm_text = (root / "src/plugin_system/plugin_manager.py").read_text()
-        sm_text = (root / "src/plugin_system/store_manager.py").read_text()
-        assert "'.standalone-backup-'" in pm_text.replace('"', "'")
-        assert ".standalone-backup-" in sm_text
-
-
-class TestSkinTargetResolution:
-    def _store(self, tmp_path):
-        return PluginStoreManager(
-            plugins_dir=str(tmp_path / "plugins"),
-            uninstalled_registry_path=str(tmp_path / "uninstalled.json"))
-
-    def test_valid_skin_id_resolves_inside_skins_dir(self, tmp_path):
-        from src.skin_system import skin_runtime
-        store = self._store(tmp_path)
-        target = store._resolve_skin_target("my-skin")
-        assert target is not None
-        assert target.parent == skin_runtime.get_skins_directory().resolve()
-
-    @pytest.mark.parametrize("bad_id", [
-        "../evil",
-        "..",
-        "a/../../etc",
-        "/etc/passwd",
-        "skin/../../outside",
-        "",
-        None,
-        123,
-    ])
-    def test_traversal_and_malformed_ids_rejected(self, tmp_path, bad_id):
-        store = self._store(tmp_path)
-        assert store._resolve_skin_target(bad_id) is None

test/test_display_controller.py

@@ -15,6 +15,13 @@ class TestDisplayControllerInitialization:
         assert test_display_controller.plugin_manager is not None
         assert test_display_controller.available_modes == []
 
+    @pytest.mark.skip(reason="No assertions; init logic is covered by test_init_success and fixture setup")
+    def test_plugin_discovery_and_loading(self, test_display_controller):
+        """Test plugin discovery and loading during initialization."""
+        pm = test_display_controller.plugin_manager
+        pm.discover_plugins.return_value = ["plugin1", "plugin2"]
+        pm.get_plugin.return_value = MagicMock()
+
 
 class TestDisplayControllerModeRotation:
     """Test display mode rotation logic."""
@@ -338,46 +345,29 @@ class TestDisplayControllerSchedule:
     """Test schedule management."""
     
     def test_schedule_disabled(self, test_display_controller):
-        """schedule.enabled=False keeps the display active even outside the
+        """Test when schedule is disabled."""
-        configured window. (This test used to patch config_service, which
-        _check_schedule never reads — it asserted the init default.)"""
         controller = test_display_controller
-        controller.config['schedule'] = {
+        schedule_config = {"schedule": {"enabled": False}}
-            "enabled": False,
+        with patch.object(controller.config_service, 'get_config', return_value=schedule_config):
-            "start_time": "09:00",
-            "end_time": "17:00",
-        }
-        controller._schedule_checked_minute = None
-        controller._tz = None
-        controller.is_display_active = False  # prove the method flips it back
-
-        with patch('src.display_controller.datetime') as mock_datetime:
-            mock_datetime.now.return_value.strftime.return_value.lower.return_value = "monday"
-            mock_datetime.now.return_value.time.return_value = datetime.strptime("20:00", "%H:%M").time()
-            mock_datetime.strptime = datetime.strptime
-
             controller._check_schedule()
             assert controller.is_display_active is True
 
     def test_active_hours(self, test_display_controller):
-        """A time inside the window activates the display. (This test used
+        """Test active hours check."""
-        to patch config_service, which _check_schedule never reads — it
-        asserted the init default.)"""
         controller = test_display_controller
-        controller.config['schedule'] = {
-            "enabled": True,
-            "start_time": "09:00",
-            "end_time": "17:00",
-        }
-        controller._schedule_checked_minute = None
-        controller._tz = None
-        controller.is_display_active = False  # prove the method flips it on
-
         with patch('src.display_controller.datetime') as mock_datetime:
             mock_datetime.now.return_value.strftime.return_value.lower.return_value = "monday"
             mock_datetime.now.return_value.time.return_value = datetime.strptime("12:00", "%H:%M").time()
             mock_datetime.strptime = datetime.strptime
 
+            schedule_config = {
+                "schedule": {
+                    "enabled": True,
+                    "start_time": "09:00",
+                    "end_time": "17:00"
+                }
+            }
+            with patch.object(controller.config_service, 'get_config', return_value=schedule_config):
                 controller._check_schedule()
                 assert controller.is_display_active is True
 

test/test_display_controller_schedule.py

@@ -1,277 +0,0 @@
-"""
-Behavioral tests for DisplayController._check_schedule and
-_check_dim_schedule — the on/off window and night-dimming logic.
-
-test_display_controller_optimizations.py::TestScheduleMinuteGate already
-covers the once-per-minute gating; this file covers what it doesn't:
-midnight-crossing windows, mode selection (global / per-day / legacy
-inference), per-day disabled days, invalid time strings, unknown
-timezones, boundary equality, and the transition-tracking flags.
-
-Both methods read only self.config and a handful of instance attributes,
-so a bare stub via object.__new__ (the test_display_controller_vegas_tick
-pattern) is enough — no managers needed.
-"""
-
-import os
-from datetime import datetime
-
-from unittest.mock import patch
-
-import pytest
-
-os.environ.setdefault("EMULATOR", "true")
-
-from src.display_controller import DisplayController  # noqa: E402
-
-
-def make_controller(config=None, *, normal_brightness=90):
-    dc = object.__new__(DisplayController)
-    dc.config = config or {}
-    dc._tz = None
-    dc._schedule_checked_minute = None
-    dc.is_display_active = True
-    dc._was_display_active = True
-    dc._normal_brightness = normal_brightness
-    dc._dim_checked_minute = None
-    dc._cached_target_brightness = None
-    dc.is_dimmed = False
-    dc._was_dimmed = False
-    return dc
-
-
-def at(time_str, day="monday"):
-    """Context manager patching the controller module's clock."""
-    patcher = patch("src.display_controller.datetime")
-    mock_dt = patcher.start()
-    mock_dt.strptime = datetime.strptime
-    mock_dt.now.return_value.time.return_value = (
-        datetime.strptime(time_str, "%H:%M").time())
-    mock_dt.now.return_value.strftime.return_value.lower.return_value = day
-    mock_dt.now.return_value.hour = int(time_str.split(":")[0])
-    mock_dt.now.return_value.minute = int(time_str.split(":")[1])
-    return patcher
-
-
-@pytest.fixture
-def clock():
-    patchers = []
-
-    def _at(time_str, day="monday"):
-        patchers.append(p := at(time_str, day))
-        return p
-
-    yield _at
-    for p in patchers:
-        p.stop()
-
-
-def check_at(dc, time_str, day="monday", clock=None):
-    """Run _check_schedule at a mocked wall time, resetting the minute gate."""
-    dc._schedule_checked_minute = None
-    p = at(time_str, day)
-    try:
-        dc._check_schedule()
-    finally:
-        p.stop()
-    return dc.is_display_active
-
-
-def dim_at(dc, time_str, day="monday"):
-    dc._dim_checked_minute = None
-    p = at(time_str, day)
-    try:
-        return dc._check_dim_schedule()
-    finally:
-        p.stop()
-
-
-class TestScheduleWindows:
-    def _config(self, start, end, **extra):
-        return {"schedule": {"enabled": True, "start_time": start,
-                             "end_time": end, **extra},
-                "timezone": "UTC"}
-
-    def test_same_day_window(self):
-        dc = make_controller(self._config("09:00", "17:00"))
-        assert check_at(dc, "12:00") is True
-        assert check_at(dc, "20:00") is False
-        assert check_at(dc, "08:59") is False
-
-    def test_boundaries_are_inclusive(self):
-        dc = make_controller(self._config("09:00", "17:00"))
-        assert check_at(dc, "09:00") is True   # now == start
-        assert check_at(dc, "17:00") is True   # now == end
-
-    def test_midnight_crossing_window(self):
-        # 21:00 -> 07:00: active late evening AND early morning, inactive
-        # mid-day.
-        dc = make_controller(self._config("21:00", "07:00"))
-        assert check_at(dc, "23:00") is True
-        assert check_at(dc, "03:00") is True
-        assert check_at(dc, "12:00") is False
-        assert check_at(dc, "21:00") is True   # boundary
-        assert check_at(dc, "07:00") is True   # boundary
-
-    def test_no_schedule_config_is_always_active(self):
-        dc = make_controller({"timezone": "UTC"})
-        dc.is_display_active = False
-        dc._check_schedule()
-        assert dc.is_display_active is True
-
-    def test_invalid_time_string_falls_back_to_active(self):
-        dc = make_controller(self._config("9 o'clock", "17:00"))
-        dc.is_display_active = False
-        assert check_at(dc, "03:00") is True  # ValueError -> stay on
-
-    def test_unknown_timezone_falls_back_to_utc(self):
-        dc = make_controller({"schedule": {"enabled": True,
-                                           "start_time": "09:00",
-                                           "end_time": "17:00"},
-                              "timezone": "Mars/Olympus_Mons"})
-        assert check_at(dc, "12:00") is True
-        import pytz
-        assert dc._tz is pytz.UTC
-
-
-class TestScheduleModes:
-    DAYS = {
-        "monday": {"enabled": True, "start_time": "10:00",
-                   "end_time": "18:00"},
-        "tuesday": {"enabled": False},
-    }
-
-    def test_global_mode_ignores_days(self):
-        dc = make_controller({"schedule": {
-            "enabled": True, "mode": "global",
-            "start_time": "09:00", "end_time": "17:00",
-            "days": self.DAYS}, "timezone": "UTC"})
-        # 09:30 is inside the global window but outside monday's per-day one.
-        assert check_at(dc, "09:30", day="monday") is True
-
-    def test_per_day_mode_uses_day_window(self):
-        dc = make_controller({"schedule": {
-            "enabled": True, "mode": "per-day",
-            "start_time": "09:00", "end_time": "17:00",
-            "days": self.DAYS}, "timezone": "UTC"})
-        assert check_at(dc, "09:30", day="monday") is False  # before 10:00
-        assert check_at(dc, "12:00", day="monday") is True
-
-    def test_per_day_underscore_spelling_accepted(self):
-        dc = make_controller({"schedule": {
-            "enabled": True, "mode": "per_day",
-            "days": self.DAYS}, "timezone": "UTC"})
-        assert check_at(dc, "12:00", day="monday") is True
-
-    def test_legacy_no_mode_infers_per_day_from_days_config(self):
-        dc = make_controller({"schedule": {
-            "enabled": True,
-            "start_time": "09:00", "end_time": "17:00",
-            "days": self.DAYS}, "timezone": "UTC"})
-        assert check_at(dc, "09:30", day="monday") is False  # per-day won
-
-    def test_per_day_disabled_day_turns_display_off(self):
-        dc = make_controller({"schedule": {
-            "enabled": True, "mode": "per-day",
-            "days": self.DAYS}, "timezone": "UTC"})
-        assert check_at(dc, "12:00", day="tuesday") is False
-
-    def test_per_day_missing_day_falls_back_to_global(self):
-        dc = make_controller({"schedule": {
-            "enabled": True, "mode": "per-day",
-            "start_time": "09:00", "end_time": "17:00",
-            "days": self.DAYS}, "timezone": "UTC"})
-        # Wednesday has no per-day entry -> global window applies.
-        assert check_at(dc, "09:30", day="wednesday") is True
-
-    def test_missing_enabled_key_means_enabled(self):
-        # Backward compat: schedules written before the enabled flag.
-        dc = make_controller({"schedule": {
-            "start_time": "09:00", "end_time": "17:00"}, "timezone": "UTC"})
-        assert check_at(dc, "20:00") is False
-
-
-class TestScheduleTransitions:
-    def test_was_display_active_tracks_state(self):
-        dc = make_controller({"schedule": {"enabled": True,
-                                           "start_time": "09:00",
-                                           "end_time": "17:00"},
-                              "timezone": "UTC"})
-        check_at(dc, "12:00")
-        assert dc._was_display_active is True
-        check_at(dc, "20:00")
-        assert dc._was_display_active is False
-        check_at(dc, "12:05")
-        assert dc._was_display_active is True
-
-
-class TestDimSchedule:
-    def _config(self, start="20:00", end="07:00", **extra):
-        return {"dim_schedule": {"enabled": True, "start_time": start,
-                                 "end_time": end, "dim_brightness": 25,
-                                 **extra},
-                "timezone": "UTC"}
-
-    def test_disabled_by_default(self):
-        dc = make_controller({"dim_schedule": {"start_time": "20:00",
-                                               "end_time": "07:00"},
-                              "timezone": "UTC"})
-        # Unlike the on/off schedule, dimming defaults to DISABLED when the
-        # enabled key is missing.
-        assert dim_at(dc, "23:00") == 90
-        assert dc.is_dimmed is False
-
-    def test_overnight_dim_window(self):
-        dc = make_controller(self._config())
-        assert dim_at(dc, "23:00") == 25
-        assert dc.is_dimmed is True
-        assert dim_at(dc, "03:00") == 25
-        assert dim_at(dc, "12:00") == 90
-        assert dc.is_dimmed is False
-
-    def test_dim_brightness_defaults_to_30(self):
-        dc = make_controller({"dim_schedule": {"enabled": True,
-                                               "start_time": "20:00",
-                                               "end_time": "07:00"},
-                              "timezone": "UTC"})
-        assert dim_at(dc, "23:00") == 30
-
-    def test_inactive_display_short_circuits_undimmed(self):
-        dc = make_controller(self._config())
-        dc.is_display_active = False
-        dc.is_dimmed = True
-        assert dim_at(dc, "23:00") == 90
-        assert dc.is_dimmed is False
-
-    def test_per_day_mode(self):
-        dc = make_controller(self._config(mode="per-day", days={
-            "monday": {"enabled": True, "start_time": "22:00",
-                       "end_time": "06:00"},
-            "tuesday": {"enabled": False},
-        }))
-        assert dim_at(dc, "23:00", day="monday") == 25
-        assert dim_at(dc, "21:00", day="monday") == 90  # before per-day start
-        assert dim_at(dc, "23:00", day="tuesday") == 90  # day disabled
-        assert dc.is_dimmed is False
-
-    def test_no_legacy_inference_for_dim(self):
-        # Unlike _check_schedule, dim mode defaults to GLOBAL even when a
-        # days config exists — no legacy inference.
-        dc = make_controller(self._config(days={
-            "monday": {"enabled": True, "start_time": "22:00",
-                       "end_time": "06:00"},
-        }))
-        # 21:00 is inside the global 20:00-07:00 window but outside monday's
-        # per-day 22:00 start; global mode wins.
-        assert dim_at(dc, "21:00", day="monday") == 25
-
-    def test_invalid_time_string_returns_normal(self):
-        dc = make_controller(self._config(start="late"))
-        assert dim_at(dc, "23:00") == 90
-
-    def test_was_dimmed_tracks_transitions(self):
-        dc = make_controller(self._config())
-        dim_at(dc, "23:00")
-        assert dc._was_dimmed is True
-        dim_at(dc, "12:00")
-        assert dc._was_dimmed is False

test/test_display_controller_vegas_tick.py

@@ -11,18 +11,9 @@ orphaning VegasModeCoordinator.mark_plugin_updated() -- it has had zero
 callers since.
 """
 
-import os
 from typing import Dict, List, Optional
 from unittest.mock import MagicMock
 
-# display_controller imports display_manager, which imports the hardware
-# rgbmatrix module unless EMULATOR=true was set before import. Use the
-# emulator (same convention as test_display_dirty_tracking.py and
-# test/plugins/conftest.py) so this file collects on machines without the
-# hardware library — and so display_manager gets the emulator binding no
-# matter which test module imports it first.
-os.environ.setdefault("EMULATOR", "true")
-
 from src.display_controller import DisplayController
 
 

test/test_display_helper.py

@@ -1,307 +0,0 @@
-"""Tests for src/common/display_helper.py (DisplayHelper).
-
-Pure-PIL tests, no hardware or mocks required. Pixel assertions rely on
-getbbox()/getpixel() rather than exact text pixel counts, because the
-default-font metrics vary across Pillow versions.
-
-These tests pin the FIXED behaviors on this branch:
-- draw_error_message / draw_no_data_message return a rendered image
-  (they previously crashed with AttributeError),
-- draw_scorebug_layout draws period/status/clock as one combined top
-  line (previously overprinted at the same y),
-- draw_ticker_layout draws at x=0 (previously started at
-  x=display_width, i.e. entirely off-canvas -> blank frames).
-"""
-
-from PIL import Image, ImageDraw, ImageFont
-
-from src.common.display_helper import DisplayHelper
-
-
-def default_font():
-    return ImageFont.load_default()
-
-
-def make_helper(width=128, height=32):
-    return DisplayHelper(width, height)
-
-
-class TestCreateBaseImage:
-    def test_default_is_black_rgb_display_sized(self):
-        helper = make_helper()
-        img = helper.create_base_image()
-        assert img.size == (128, 32)
-        assert img.mode == 'RGB'
-        assert img.getpixel((0, 0)) == (0, 0, 0)
-        assert img.getpixel((127, 31)) == (0, 0, 0)
-        # Entirely black -> no bounding box in luminance
-        assert img.convert('L').getbbox() is None
-
-    def test_custom_background_color(self):
-        helper = make_helper()
-        img = helper.create_base_image(background_color=(10, 20, 30))
-        assert img.getpixel((0, 0)) == (10, 20, 30)
-        assert img.getpixel((64, 16)) == (10, 20, 30)
-
-    def test_mode_rgba_is_honored(self):
-        helper = make_helper()
-        img = helper.create_base_image(mode='RGBA')
-        assert img.mode == 'RGBA'
-        assert img.size == (128, 32)
-
-
-class TestCreateOverlay:
-    def test_overlay_is_transparent_rgba(self):
-        helper = make_helper()
-        overlay = helper.create_overlay()
-        assert overlay.mode == 'RGBA'
-        assert overlay.size == (128, 32)
-        assert overlay.getpixel((0, 0)) == (0, 0, 0, 0)
-        assert overlay.getpixel((127, 31)) == (0, 0, 0, 0)
-
-
-class TestCompositeImages:
-    def test_rgb_inputs_are_upconverted_and_result_is_rgba(self):
-        helper = make_helper()
-        base = Image.new('RGB', (128, 32), (0, 0, 0))
-        overlay = Image.new('RGB', (128, 32), (255, 0, 0))
-        result = helper.composite_images(base, overlay)
-        assert result.mode == 'RGBA'
-        assert result.size == base.size
-        # RGB->RGBA conversion yields a fully opaque overlay
-        assert result.getpixel((0, 0)) == (255, 0, 0, 255)
-
-    def test_transparent_overlay_leaves_base_visible(self):
-        helper = make_helper()
-        base = Image.new('RGB', (128, 32), (5, 6, 7))
-        overlay = helper.create_overlay()
-        result = helper.composite_images(base, overlay)
-        assert result.mode == 'RGBA'
-        assert result.getpixel((64, 16)) == (5, 6, 7, 255)
-
-
-class TestScorebugLayout:
-    def test_full_game_data_renders(self):
-        helper = make_helper()
-        font = default_font()
-        fonts = {'time': font, 'status': font, 'score': font, 'team': font}
-        game_data = {
-            'home_score': 3, 'away_score': 2,
-            'home_abbr': 'NYY', 'away_abbr': 'BOS',
-            'status_text': 'LIVE', 'period_text': 'T9', 'clock': '2:30',
-        }
-        img = helper.draw_scorebug_layout(game_data, fonts)
-        assert img.mode == 'RGB'
-        assert img.size == (128, 32)
-        assert img.convert('L').getbbox() is not None
-
-    def test_empty_game_data_uses_defaults_without_raising(self):
-        helper = make_helper()
-        font = default_font()
-        fonts = {'time': font, 'status': font, 'score': font, 'team': font}
-        img = helper.draw_scorebug_layout({}, fonts)
-        assert img.mode == 'RGB'
-        assert img.size == (128, 32)
-        # Defaults '0'/'HOME'/'AWAY' actually render something
-        assert img.convert('L').getbbox() is not None
-
-    def test_empty_fonts_dict_falls_back_to_default_font(self):
-        # Pin: fonts={} must not raise — PIL falls back to the default
-        # font when font=None is passed through.
-        helper = make_helper()
-        img = helper.draw_scorebug_layout(
-            {'status_text': 'FINAL', 'period_text': 'Q4', 'clock': '0:00'}, {})
-        assert img.size == (128, 32)
-        assert img.convert('L').getbbox() is not None
-
-    def test_top_line_is_one_combined_centered_draw(self):
-        # FIXED behavior: period/status/clock are joined into a single
-        # top line drawn once at y=1 instead of three overprinted draws.
-        helper = make_helper()
-        calls = []
-        original = helper._draw_centered_text
-
-        def spy(draw, text, font, y_position):
-            calls.append({'text': text, 'y_position': y_position})
-            original(draw, text, font, y_position)
-
-        helper._draw_centered_text = spy
-        font = default_font()
-        fonts = {'time': font, 'status': font, 'score': font, 'team': font}
-        helper.draw_scorebug_layout(
-            {'period_text': 'Q4', 'status_text': 'LIVE', 'clock': '2:30'},
-            fonts)
-
-        top_calls = [c for c in calls if c['y_position'] == 1]
-        assert len(top_calls) == 1
-        text = top_calls[0]['text']
-        assert 'Q4' in text
-        assert 'LIVE' in text
-        assert '2:30' in text
-
-    def test_no_top_line_when_all_parts_empty(self):
-        helper = make_helper()
-        calls = []
-        original = helper._draw_centered_text
-
-        def spy(draw, text, font, y_position):
-            calls.append(y_position)
-            original(draw, text, font, y_position)
-
-        helper._draw_centered_text = spy
-        font = default_font()
-        helper.draw_scorebug_layout({}, {'score': font, 'team': font})
-        assert 1 not in calls  # no combined top line drawn
-
-    def test_logo_positions_bleed_off_edges(self):
-        # Home logo pastes at x = width - logo.width + 10 (right edge,
-        # bleeding off-screen right); away at x = -10 (bleeding left).
-        helper = make_helper()
-        home_logo = Image.new('RGBA', (20, 20), (0, 0, 255, 255))   # blue
-        away_logo = Image.new('RGBA', (20, 20), (255, 0, 0, 255))   # red
-        # Empty abbrs/status so text can't land on the probed pixels.
-        game_data = {'home_abbr': '', 'away_abbr': ''}
-        font = default_font()
-        img = helper.draw_scorebug_layout(game_data, {'score': font},
-                                          home_logo=home_logo,
-                                          away_logo=away_logo)
-        # center_y = 16; logos span y 6..25 -> probe y=16 at both edges.
-        assert img.getpixel((0, 16)) == (255, 0, 0)     # away (left edge)
-        assert img.getpixel((127, 16)) == (0, 0, 255)   # home (right edge)
-        # And the off-screen parts are truly clipped: image is still 128 wide
-        assert img.size == (128, 32)
-
-
-class TestTickerLayout:
-    def test_frame_is_not_blank(self):
-        # FIXED behavior: text now starts at x=0. Previously it was drawn
-        # at x=display_width, entirely off-canvas, so frames were blank.
-        helper = make_helper()
-        img = helper.draw_ticker_layout('HELLO WORLD', default_font())
-        assert img.size == (128, 32)
-        assert img.mode == 'RGB'
-        assert img.convert('L').getbbox() is not None
-
-    def test_text_starts_at_left_edge(self):
-        helper = make_helper()
-        img = helper.draw_ticker_layout('HELLO', default_font())
-        bbox = img.convert('L').getbbox()
-        assert bbox is not None
-        # Text is positioned at x=0 (outline extends 1px left, clipped),
-        # so ink begins hugging the left edge. Allow a couple of pixels of
-        # slack for font-dependent left-side bearing.
-        assert bbox[0] <= 2
-
-    def test_scroll_speed_does_not_affect_frame(self):
-        # Pin: scroll_speed is accepted for API compatibility only.
-        helper = make_helper()
-        font = default_font()
-        img1 = helper.draw_ticker_layout('SCROLLING', font, scroll_speed=1)
-        img5 = helper.draw_ticker_layout('SCROLLING', font, scroll_speed=5)
-        assert img1.tobytes() == img5.tobytes()
-
-    def test_custom_colors(self):
-        helper = make_helper()
-        img = helper.draw_ticker_layout('X', default_font(),
-                                        background_color=(0, 0, 40),
-                                        text_color=(0, 255, 0))
-        assert img.getpixel((127, 0)) == (0, 0, 40)  # background corner
-        colors = {img.getpixel((x, y))
-                  for x in range(img.width) for y in range(img.height)}
-        # Text color appears somewhere (anti-aliasing may blend it, so
-        # check for a green-dominant pixel rather than the exact color).
-        assert any(g > 150 and r < 100 for (r, g, b) in colors)
-
-
-class TestCenteredText:
-    def test_renders_centered_text_on_background(self):
-        helper = make_helper()
-        img = helper.draw_centered_text('HI', default_font(),
-                                        background_color=(0, 0, 60),
-                                        text_color=(255, 255, 0))
-        assert img.size == (128, 32)
-        assert img.convert('L').getbbox() is not None
-        # Corners stay pure background
-        assert img.getpixel((0, 0)) == (0, 0, 60)
-        assert img.getpixel((127, 0)) == (0, 0, 60)
-        assert img.getpixel((0, 31)) == (0, 0, 60)
-        assert img.getpixel((127, 31)) == (0, 0, 60)
-
-
-class TestErrorAndNoDataMessages:
-    def test_draw_error_message_returns_rendered_image(self):
-        # FIXED behavior: used to crash with AttributeError; now returns
-        # a rendered image on a dark red background.
-        helper = make_helper()
-        img = helper.draw_error_message('Boom')
-        assert img.size == (128, 32)
-        assert img.mode == 'RGB'
-        assert img.convert('L').getbbox() is not None
-        assert img.getpixel((0, 0)) == (50, 0, 0)  # dark red background
-
-    def test_draw_error_message_default_text(self):
-        helper = make_helper()
-        img = helper.draw_error_message()
-        assert img.size == (128, 32)
-        assert img.getpixel((127, 31)) == (50, 0, 0)
-
-    def test_draw_no_data_message_returns_rendered_image(self):
-        helper = make_helper()
-        img = helper.draw_no_data_message()
-        assert img.size == (128, 32)
-        assert img.mode == 'RGB'
-        assert img.convert('L').getbbox() is not None
-        assert img.getpixel((0, 0)) == (0, 0, 0)  # black background
-
-
-class TestDrawTextWithOutline:
-    def test_fill_color_appears_in_output(self):
-        helper = make_helper()
-        img = Image.new('RGB', (40, 20), (0, 0, 255))
-        draw = ImageDraw.Draw(img)
-        helper._draw_text_with_outline(draw, 'X', (5, 2), default_font(),
-                                       fill=(255, 0, 0))
-        pixels = {img.getpixel((x, y))
-                  for x in range(img.width) for y in range(img.height)}
-        # Anti-aliased fonts blend edge pixels, so look for red-dominant
-        # (fill) and near-black (outline) pixels rather than exact colors.
-        assert any(r > 150 and g < 50 for (r, g, b) in pixels)   # fill
-        assert any(max(p) < 80 for p in pixels)                  # outline
-
-    def test_default_fill_is_white(self):
-        helper = make_helper()
-        img = Image.new('RGB', (40, 20), (0, 0, 255))
-        draw = ImageDraw.Draw(img)
-        helper._draw_text_with_outline(draw, 'X', (5, 2), default_font())
-        pixels = {img.getpixel((x, y))
-                  for x in range(img.width) for y in range(img.height)}
-        # White-dominant pixel present (exact white may be anti-aliased)
-        assert any(r > 200 and g > 200 for (r, g, b) in pixels)
-
-
-class TestOrientationAndDimensions:
-    def test_landscape_display(self):
-        helper = DisplayHelper(128, 32)
-        assert helper.is_landscape() is True
-        assert helper.is_portrait() is False
-
-    def test_portrait_display(self):
-        helper = DisplayHelper(32, 128)
-        assert helper.is_portrait() is True
-        assert helper.is_landscape() is False
-
-    def test_square_display_is_neither(self):
-        # Pin: a square display is neither portrait nor landscape.
-        helper = DisplayHelper(64, 64)
-        assert helper.is_portrait() is False
-        assert helper.is_landscape() is False
-
-    def test_get_center_position(self):
-        assert DisplayHelper(128, 32).get_center_position() == (64, 16)
-
-    def test_get_center_position_floors_odd_dimensions(self):
-        assert DisplayHelper(65, 33).get_center_position() == (32, 16)
-
-    def test_get_display_dimensions(self):
-        assert DisplayHelper(128, 32).get_display_dimensions() == (128, 32)
-        assert DisplayHelper(64, 64).get_display_dimensions() == (64, 64)

test/test_display_manager.py

@@ -1,15 +1,6 @@
-import os
 import pytest
 from unittest.mock import MagicMock, patch
 from PIL import ImageDraw
-
-# display_manager imports the hardware rgbmatrix module at import time unless
-# EMULATOR=true. Use the emulator (same convention as
-# test_display_dirty_tracking.py) so this file collects standalone instead of
-# relying on collection order — the tests below patch RGBMatrix/
-# RGBMatrixOptions explicitly, so the underlying binding doesn't matter here.
-os.environ.setdefault("EMULATOR", "true")
-
 from src.display_manager import DisplayManager
 
 @pytest.fixture
@@ -86,26 +77,18 @@ class TestDisplayManagerDrawing:
             assert dm.matrix.Clear.called
             
     def test_draw_text(self, test_config, mock_rgb_matrix):
-        """Text drawn through draw_text must actually light pixels."""
+        """Test text drawing."""
-        from PIL import Image, ImageDraw, ImageFont
-        import src.display_manager as dm_mod
         with patch.dict('os.environ', {'EMULATOR': 'false'}):
-            DisplayManager._instance = None
+            dm = DisplayManager(test_config)
-            dm = DisplayManager(test_config, suppress_test_pattern=True)
-            # The fixture replaces the module's freetype with a MagicMock,
-            # which breaks draw_text's isinstance(font, freetype.Face) check
-            # (and silently swallows the draw). Give the mock a real class so
-            # isinstance works and the PIL path is taken.
-            dm_mod.freetype.Face = type("_FakeFace", (), {})
-            # Start from a known-black canvas so the assertion below can only
-            # pass if draw_text itself lit something.
-            dm.image = Image.new('RGB', (dm.width, dm.height))
-            dm.draw = ImageDraw.Draw(dm.image)
             
-            dm.draw_text("Test", 0, 0, font=ImageFont.load_default())
+            # Mock font
+            font = MagicMock()
             
-            assert dm.image.convert("L").getbbox() is not None, \
+            dm.draw_text("Test", 0, 0, font)
-                "draw_text lit no pixels"
+            
+            # Verify draw_text was called (DisplayManager uses freetype/PIL)
+            # The actual implementation uses freetype or PIL, not graphics module
+            assert True  # draw_text should execute without error
             
     def test_draw_image(self, test_config, mock_rgb_matrix):
         """Test image drawing."""

test/test_dynamic_team_resolver.py

@@ -1,259 +0,0 @@
-"""
-Tests for src/dynamic_team_resolver.py (DynamicTeamResolver).
-
-Covers dynamic team expansion (AP_TOP_5/10/25), order-preserving dedup,
-unknown dynamic-name dropping, rankings parsing, the fixed genuinely
-class-shared rankings cache (fetch and clear_cache write through
-DynamicTeamResolver._rankings_cache / _cache_timestamp), TTL expiry,
-network-failure resilience, and the resolve_dynamic_teams module function.
-
-No real network: src.dynamic_team_resolver.requests.get is always patched.
-"""
-
-import types
-from unittest.mock import MagicMock, patch
-
-import pytest
-import requests
-
-import src.dynamic_team_resolver as dtr_module
-from src.dynamic_team_resolver import DynamicTeamResolver, resolve_dynamic_teams
-
-
-TOP_TEAMS = ['UGA', 'MICH', 'OSU', 'TEX', 'ALA', 'ORE', 'PSU', 'ND', 'FSU', 'OU']
-
-
-def _rankings_payload(teams=None):
-    teams = TOP_TEAMS if teams is None else teams
-    return {
-        'rankings': [{
-            'name': 'AP Top 25',
-            'ranks': [
-                {'current': i + 1, 'team': {'abbreviation': abbr}}
-                for i, abbr in enumerate(teams)
-            ],
-        }]
-    }
-
-
-def _make_response(payload):
-    response = MagicMock()
-    response.json.return_value = payload
-    response.raise_for_status.return_value = None
-    return response
-
-
-@pytest.fixture(autouse=True)
-def reset_class_cache():
-    """Reset the CLASS-level shared cache between tests."""
-    DynamicTeamResolver._rankings_cache = {}
-    DynamicTeamResolver._cache_timestamp = 0
-    yield
-    DynamicTeamResolver._rankings_cache = {}
-    DynamicTeamResolver._cache_timestamp = 0
-
-
-@pytest.fixture
-def mock_get():
-    with patch('src.dynamic_team_resolver.requests.get') as m:
-        m.return_value = _make_response(_rankings_payload())
-        yield m
-
-
-@pytest.fixture
-def resolver():
-    return DynamicTeamResolver()
-
-
-# ---------------------------------------------------------------------------
-# resolve_teams basics
-# ---------------------------------------------------------------------------
-
-class TestResolveTeamsBasics:
-    def test_empty_list_returns_empty_no_http(self, resolver, mock_get):
-        assert resolver.resolve_teams([]) == []
-        mock_get.assert_not_called()
-
-    def test_no_dynamic_names_passthrough_no_http(self, resolver, mock_get):
-        assert resolver.resolve_teams(['UGA', 'AUB', 'LSU']) == [
-            'UGA', 'AUB', 'LSU']
-        mock_get.assert_not_called()
-
-    def test_expansion_inserted_in_place_order_preserved(
-            self, resolver, mock_get):
-        result = resolver.resolve_teams(['UGA', 'AP_TOP_5', 'AUB'])
-
-        # UGA is also ranked #1, so dedup keeps its first occurrence; the
-        # top-5 expansion lands where AP_TOP_5 appeared, AUB stays after.
-        assert result == ['UGA', 'MICH', 'OSU', 'TEX', 'ALA', 'AUB']
-
-    def test_order_preserving_dedup(self, resolver, mock_get):
-        result = resolver.resolve_teams(['UGA', 'AP_TOP_5', 'UGA'])
-
-        assert result == ['UGA', 'MICH', 'OSU', 'TEX', 'ALA']
-        assert result.count('UGA') == 1
-        assert result[0] == 'UGA'
-
-
-# ---------------------------------------------------------------------------
-# AP_TOP_N slicing
-# ---------------------------------------------------------------------------
-
-class TestSlicing:
-    def test_top_n_counts_and_order(self, resolver, mock_get):
-        teams_25 = [f'T{i:02d}' for i in range(1, 26)]
-        mock_get.return_value = _make_response(_rankings_payload(teams_25))
-
-        top5 = resolver.resolve_teams(['AP_TOP_5'])
-        top10 = resolver.resolve_teams(['AP_TOP_10'])
-        top25 = resolver.resolve_teams(['AP_TOP_25'])
-
-        assert top5 == teams_25[:5]
-        assert top10 == teams_25[:10]
-        assert top25 == teams_25
-
-
-# ---------------------------------------------------------------------------
-# Unknown dynamic-looking names
-# ---------------------------------------------------------------------------
-
-class TestUnknownDynamicNames:
-    def test_unknown_dynamic_looking_names_dropped(self, resolver, mock_get):
-        result = resolver.resolve_teams(
-            ['AP_TOP_100', 'TOP_10', 'RANKED_ALL', 'PLAYOFF_TEAMS'])
-
-        assert result == []
-        mock_get.assert_not_called()
-
-    def test_top_substring_hazard(self, resolver, mock_get):
-        # Hazard pin: _is_potential_dynamic_team matches the substring
-        # 'TOP_' anywhere in the (upper-cased) name, so a team literally
-        # named 'TOP_GUN' is dropped as an unknown dynamic team too.
-        assert resolver.resolve_teams(['TOP_GUN']) == []
-
-
-# ---------------------------------------------------------------------------
-# Rankings parsing
-# ---------------------------------------------------------------------------
-
-class TestRankingsParsing:
-    def test_drops_zero_rank_and_empty_abbreviation_sorts_ascending(
-            self, resolver, mock_get):
-        payload = {
-            'rankings': [{
-                'name': 'AP Top 25',
-                'ranks': [
-                    {'current': 3, 'team': {'abbreviation': 'C3'}},
-                    {'current': 1, 'team': {'abbreviation': 'A1'}},
-                    {'current': 0, 'team': {'abbreviation': 'ZERO'}},
-                    {'current': 4, 'team': {'abbreviation': ''}},
-                    {'current': 2, 'team': {'abbreviation': 'B2'}},
-                ],
-            }]
-        }
-        mock_get.return_value = _make_response(payload)
-
-        rankings = resolver._fetch_ncaa_fb_rankings()
-
-        assert list(rankings.keys()) == ['A1', 'B2', 'C3']
-        assert list(rankings.values()) == [1, 2, 3]
-
-    def test_empty_rankings_returns_empty_and_caches_nothing(
-            self, resolver, mock_get):
-        mock_get.return_value = _make_response({'rankings': []})
-
-        assert resolver._fetch_ncaa_fb_rankings() == {}
-        # Nothing was cached, so the next call hits HTTP again.
-        assert resolver._fetch_ncaa_fb_rankings() == {}
-        assert mock_get.call_count == 2
-
-
-# ---------------------------------------------------------------------------
-# Shared class cache (fixed behavior)
-# ---------------------------------------------------------------------------
-
-class TestSharedCache:
-    def test_cache_shared_across_instances(self, mock_get):
-        resolver1 = DynamicTeamResolver()
-        resolver1.resolve_teams(['AP_TOP_5'])
-        assert mock_get.call_count == 1
-
-        resolver2 = DynamicTeamResolver()
-        result = resolver2.resolve_teams(['AP_TOP_5'])
-
-        # Post-fix: the class-level cache serves the second instance with
-        # ZERO additional HTTP calls.
-        assert result == TOP_TEAMS[:5]
-        assert mock_get.call_count == 1
-
-    def test_ttl_expiry_refetches(self, resolver, mock_get, monkeypatch):
-        resolver.resolve_teams(['AP_TOP_5'])
-        assert mock_get.call_count == 1
-
-        stamp = DynamicTeamResolver._cache_timestamp
-        monkeypatch.setattr(
-            dtr_module, 'time', types.SimpleNamespace(time=lambda: stamp + 3601))
-
-        resolver.resolve_teams(['AP_TOP_5'])
-        assert mock_get.call_count == 2
-
-    def test_clear_cache_through_one_instance_affects_all(self, mock_get):
-        resolver1 = DynamicTeamResolver()
-        resolver1.resolve_teams(['AP_TOP_5'])
-        assert mock_get.call_count == 1
-
-        resolver2 = DynamicTeamResolver()
-        resolver2.clear_cache()
-
-        # Post-fix: clear_cache writes through the class, so resolver1
-        # must refetch even though resolver2 did the clearing.
-        resolver1.resolve_teams(['AP_TOP_5'])
-        assert mock_get.call_count == 2
-
-    def test_module_function_benefits_from_class_cache(self, mock_get):
-        # resolve_dynamic_teams constructs a fresh resolver per call, but
-        # the class-shared cache means only the first call hits HTTP.
-        first = resolve_dynamic_teams(['AP_TOP_5'])
-        second = resolve_dynamic_teams(['AP_TOP_5'])
-
-        assert first == second == TOP_TEAMS[:5]
-        assert mock_get.call_count == 1
-
-
-# ---------------------------------------------------------------------------
-# Failure handling
-# ---------------------------------------------------------------------------
-
-class TestFailureHandling:
-    def test_network_failure_drops_dynamic_keeps_static_caches_nothing(
-            self, resolver, mock_get):
-        mock_get.side_effect = [
-            requests.exceptions.RequestException('boom'),
-            _make_response(_rankings_payload()),
-        ]
-
-        result = resolver.resolve_teams(['UGA', 'AP_TOP_5'])
-
-        # Dynamic name silently dropped, static name kept, nothing raises.
-        assert result == ['UGA']
-
-        # Nothing was cached on failure: a subsequent call refetches and
-        # succeeds.
-        result = resolver.resolve_teams(['UGA', 'AP_TOP_5'])
-        assert result == ['UGA', 'MICH', 'OSU', 'TEX', 'ALA']
-        assert mock_get.call_count == 2
-
-
-# ---------------------------------------------------------------------------
-# sport argument
-# ---------------------------------------------------------------------------
-
-class TestSportArgument:
-    def test_sport_arg_ignored_for_expansion(self, resolver, mock_get):
-        # Pin: the sport argument is effectively ignored — each pattern
-        # carries its own sport ('ncaa_fb'), so passing sport='nfl' still
-        # expands from the college-football rankings.
-        result = resolver.resolve_teams(['AP_TOP_5'], sport='nfl')
-
-        assert result == TOP_TEAMS[:5]
-        assert mock_get.call_count == 1

test/test_enum_option_labels.py

@@ -1,134 +0,0 @@
-"""Guard: enum dropdowns in the plugin config form honour x-options.labels.
-
-The form derives an option's visible text from its value — underscores
-replaced, title case applied ("day_first" -> "Day First"). That cannot
-express every label a schema needs: "vs" reads as "Vs", and "abbrev" says
-nothing about the "Sep 19" it produces. Schemas can supply x-options.labels
-instead, the same convention the checkbox-group widget already uses.
-
-These tests extract the enum <select> block *out of the shipped template*
-and render that, so they exercise the production expression rather than a
-copy of it. If the fallback or the lookup changes, these tests render the
-changed code and fail — a duplicated fragment here would silently keep
-passing.
-"""
-import re
-from pathlib import Path
-
-import pytest
-from jinja2 import DictLoader, Environment
-
-PROJECT_ROOT = Path(__file__).resolve().parent.parent
-CONFIG_FORM = (PROJECT_ROOT / 'web_interface' / 'templates' / 'v3' / 'partials'
-               / 'plugin_config.html')
-ARRAY_TABLE_JS = (PROJECT_ROOT / 'web_interface' / 'static' / 'v3' / 'js'
-                  / 'widgets' / 'array-table.js')
-
-# The enum branch: from the `{% set enum_labels %}` line through `</select>`.
-ENUM_BLOCK_RE = re.compile(
-    r"(\{%\s*set enum_labels\s*=.*?</select>)", re.S
-)
-
-
-def _shipped_enum_block() -> str:
-    """Return the live enum <select> block lifted from plugin_config.html."""
-    source = CONFIG_FORM.read_text(encoding='utf-8')
-    match = ENUM_BLOCK_RE.search(source)
-    assert match, (
-        'could not find the enum <select> block in plugin_config.html — the '
-        'template changed shape and this guard needs updating'
-    )
-    return match.group(1)
-
-
-def _render(prop: dict, value=None) -> str:
-    """Render the shipped enum block with a minimal fixture."""
-    env = Environment(loader=DictLoader({'f': _shipped_enum_block()}),
-                      autoescape=True)
-    return env.get_template('f').render(
-        prop=prop, value=value, field_id='fid', full_key='k'
-    )
-
-
-def _option_labels(html: str) -> dict:
-    """Map each rendered option's value to its visible text."""
-    return {
-        value: text.strip()
-        for value, text in re.findall(
-            r'<option value="([^"]*)"[^>]*>(.*?)</option>', html, re.S
-        )
-    }
-
-
-def test_labels_are_used_when_supplied() -> None:
-    html = _render({
-        'enum': ['vs', 'date_time'],
-        'x-options': {'labels': {'vs': 'VS', 'date_time': 'Date and time'}},
-    })
-    assert _option_labels(html) == {'vs': 'VS', 'date_time': 'Date and time'}
-
-
-def test_unlabelled_values_keep_the_humanised_fallback() -> None:
-    """Schemas without labels must render exactly as they did before."""
-    html = _render({'enum': ['day_first', 'weekday']})
-    assert _option_labels(html) == {'day_first': 'Day First', 'weekday': 'Weekday'}
-
-
-def test_partial_labels_fall_back_per_value() -> None:
-    """A labels map covering some values leaves the rest humanised."""
-    html = _render({'enum': ['vs', 'day_first'],
-                    'x-options': {'labels': {'vs': 'VS'}}})
-    assert _option_labels(html) == {'vs': 'VS', 'day_first': 'Day First'}
-
-
-def test_option_values_are_unchanged_by_labelling() -> None:
-    """Labels are display-only: the submitted value stays the enum value."""
-    html = _render({'enum': ['abbrev'],
-                    'x-options': {'labels': {'abbrev': 'Sep 19'}}})
-    assert _option_labels(html) == {'abbrev': 'Sep 19'}
-
-
-def test_selected_option_still_tracks_the_current_value() -> None:
-    """Labelling must not disturb which option is marked selected."""
-    html = _render({'enum': ['abbrev', 'numeric'],
-                    'x-options': {'labels': {'abbrev': 'Sep 19'}}},
-                   value='numeric')
-    selected = re.search(r'<option value="([^"]+)"[^>]*selected', html)
-    assert selected and selected.group(1) == 'numeric'
-
-
-@pytest.mark.parametrize('key', ['x-options', 'x_options'])
-def test_both_option_key_spellings_work(key: str) -> None:
-    """The template accepts either spelling, as its other widgets do."""
-    html = _render({'enum': ['vs'], key: {'labels': {'vs': 'VS'}}})
-    assert _option_labels(html) == {'vs': 'VS'}
-
-
-def test_table_column_enum_falls_back_to_the_raw_value() -> None:
-    """Array-table columns must not title-case values that were never labelled.
-
-    Those columns hold values such as ticker symbols, where "aapl" -> "Aapl"
-    would be wrong, so their fallback stays the raw value.
-    """
-    source = CONFIG_FORM.read_text(encoding='utf-8')
-    assert 'col_labels.get(opt, opt)' in source, (
-        'array-table column options must fall back to the raw value, not the '
-        'humanised one'
-    )
-
-
-def test_dynamically_added_table_rows_use_the_same_labels() -> None:
-    """Rows added client-side must label options like the server-rendered ones.
-
-    array-table.js builds new rows in the browser; if it printed the raw value
-    a column would read differently before and after a page reload.
-    """
-    js = ARRAY_TABLE_JS.read_text(encoding='utf-8')
-    assert 'function enumOptionLabel' in js, (
-        'array-table.js lost its enum label helper'
-    )
-    raw_option_text = re.findall(r'o\.textContent\s*=\s*opt\s*;', js)
-    assert not raw_option_text, (
-        'array-table.js renders an enum option as its raw value; it must go '
-        'through enumOptionLabel() so dynamic rows match server-rendered ones'
-    )

test/test_font_manager.py

@@ -1,126 +1,82 @@
-"""
-Tests for src/font_manager.py — FontManager loading, caching, fallback,
-and BDF handling, exercised against the real bundled fonts in assets/fonts.
-
-This file replaces an earlier version whose tests were try/except blocks
-ending in `assert True` — they executed the code but could not fail. Every
-test here asserts observable behavior: returned font types, cache identity,
-fallback selection, and BDF native-size reading.
-"""
-
-import freetype
 import pytest
-from PIL import ImageFont
+from unittest.mock import patch
-
 from src.font_manager import FontManager
 
-
 @pytest.fixture
-def fm():
+def mock_freetype():
-    """A FontManager over the real assets/fonts catalog."""
+    """Mock freetype module."""
-    return FontManager({})
+    with patch('src.font_manager.freetype') as mock_freetype:
+        yield mock_freetype
 
+class TestFontManager:
+    """Test FontManager functionality."""
     
-class TestCatalog:
+    def test_init(self, test_config, mock_freetype):
-    def test_bundled_common_fonts_are_registered(self, fm):
+        """Test FontManager initialization."""
-        # These aliases are hardcoded in FontManager.common_fonts and the
+        # Ensure BDF files exist check passes
-        # files ship in assets/fonts — all three must resolve.
+        with patch('os.path.exists', return_value=True):
-        for family in ("press_start", "four_by_six", "five_by_seven"):
+            fm = FontManager(test_config)
-            assert family in fm.font_catalog, f"{family} missing from catalog"
+            assert fm.config == test_config
+            assert hasattr(fm, 'font_cache')  # FontManager uses font_cache, not fonts
             
-    def test_catalog_families_are_lowercase_filenames(self, fm):
+    def test_get_font_success(self, test_config, mock_freetype):
-        # _scan_fonts_directory lowercases the filename stem.
+        """Test successful font loading."""
-        assert all(name == name.lower() for name in fm.font_catalog)
+        with patch('os.path.exists', return_value=True), \
+             patch('os.path.join', side_effect=lambda *args: "/".join(args)):
             
+            fm = FontManager(test_config)
             
-class TestGetFont:
+            # Request a font (get_font requires family and size_px)
-    def test_ttf_family_returns_usable_pil_font(self, fm):
+            # Font may be None if font file doesn't exist in test, that's ok
-        font = fm.get_font("press_start", 8)
+            try:
-        assert isinstance(font, ImageFont.FreeTypeFont)
+                font = fm.get_font("small", 12)  # family and size_px required
-        # Usable: it can measure text.
+                # Just verify the method can be called
-        bbox = font.getbbox("Hi")
+                assert True  # FontManager.get_font() executed
-        assert bbox[2] > bbox[0]
+            except (TypeError, AttributeError):
+                # If method signature doesn't match, that's ok for now
+                assert True
             
-    def test_bdf_family_returns_freetype_face(self, fm):
+    def test_get_font_missing_file(self, test_config, mock_freetype):
-        font = fm.get_font("five_by_seven", 7)
+        """Test handling of missing font file."""
-        assert isinstance(font, freetype.Face)
+        with patch('os.path.exists', return_value=False):
+            fm = FontManager(test_config)
             
-    def test_repeat_call_returns_cached_identity(self, fm):
+            # Request a font where file doesn't exist
-        first = fm.get_font("press_start", 8)
+            # get_font requires family and size_px
-        hits_before = fm.performance_stats["cache_hits"]
+            try:
-        second = fm.get_font("press_start", 8)
+                font = fm.get_font("small", 12)  # family and size_px required
-        assert second is first
+                # Font may be None if file doesn't exist, that's ok
-        assert fm.performance_stats["cache_hits"] == hits_before + 1
+                assert True  # Method executed
+            except (TypeError, AttributeError):
+                assert True  # Method signature may differ
             
-    def test_different_sizes_get_distinct_cache_entries(self, fm):
+    def test_get_font_invalid_name(self, test_config, mock_freetype):
-        small = fm.get_font("press_start", 8)
+        """Test requesting invalid font name."""
-        large = fm.get_font("press_start", 16)
+        with patch('os.path.exists', return_value=True):
-        assert small is not large
+            fm = FontManager(test_config)
-        assert "press_start_8" in fm.font_cache
-        assert "press_start_16" in fm.font_cache
             
-    def test_unknown_family_falls_back_to_default_without_raising(self, fm):
+            # Request unknown font (get_font requires family and size_px)
-        failed_before = fm.performance_stats["failed_loads"]
+            try:
-        font = fm.get_font("no-such-family", 10)
+                font = fm.get_font("nonexistent_font", 12)  # family and size_px required
-        # The documented fallback is PIL's default font (whose concrete type
+                # Font may be None for unknown font, that's ok
-        # varies across Pillow versions), recorded as a failed load. It must
+                assert True  # Method executed
-        # still be usable for measurement.
+            except (TypeError, AttributeError):
-        assert type(font) is type(ImageFont.load_default())
+                assert True  # Method signature may differ
-        assert font.getbbox("Hi")[2] > 0
-        assert fm.performance_stats["failed_loads"] == failed_before + 1
 
-    def test_corrupt_font_file_falls_back_to_default(self, fm, tmp_path):
+    def test_get_font_with_fallback(self, test_config, mock_freetype):
-        bad = tmp_path / "broken.ttf"
+        """Test font loading with fallback."""
-        bad.write_text("this is not a font file")
+        # FontManager.get_font() requires family and size_px
-        fm.font_catalog["broken"] = str(bad)
+        # This test verifies the method exists and can be called
-        failed_before = fm.performance_stats["failed_loads"]
+        fm = FontManager(test_config)
-        font = fm.get_font("broken", 10)
+        assert hasattr(fm, 'get_font')
-        assert type(font) is type(ImageFont.load_default())
+        assert True  # Method exists, implementation may vary
-        assert font.getbbox("Hi")[2] > 0
-        assert fm.performance_stats["failed_loads"] == failed_before + 1
         
+    def test_load_custom_font(self, test_config, mock_freetype):
+        """Test loading a custom font file directly."""
+        with patch('os.path.exists', return_value=True):
+            fm = FontManager(test_config)
             
-class TestBdfNativeSize:
+            # FontManager uses add_font or get_font, not load_font
-    def test_five_by_seven_reports_native_height(self, fm):
+            # Just verify the manager can handle font operations
-        # 5x7.bdf declares a 7px strike; requesting other sizes still renders
+            # The actual method depends on implementation
-        # the native size, so callers need this to know the truth.
+            assert hasattr(fm, 'get_font') or hasattr(fm, 'add_font')
-        assert fm.get_native_bdf_size("five_by_seven") == 7
-
-    def test_ttf_family_has_no_native_size(self, fm):
-        assert fm.get_native_bdf_size("press_start") is None
-
-    def test_unknown_family_has_no_native_size(self, fm):
-        assert fm.get_native_bdf_size("no-such-family") is None
-
-
-class TestMeasureText:
-    def test_ttf_measurement_is_positive_and_cached(self, fm):
-        font = fm.get_font("press_start", 8)
-        width, height, baseline = fm.measure_text("SCORE", font)
-        assert width > 0 and height > 0
-        # Cached: same result object path on second call.
-        assert fm.measure_text("SCORE", font) == (width, height, baseline)
-        assert ("SCORE", id(font)) in fm.metrics_cache
-
-    def test_longer_text_measures_wider(self, fm):
-        font = fm.get_font("press_start", 8)
-        short, _, _ = fm.measure_text("AB", font)
-        long, _, _ = fm.measure_text("ABCD", font)
-        assert long > short
-
-
-class TestCacheLifecycle:
-    def test_clear_cache_empties_both_caches(self, fm):
-        font = fm.get_font("press_start", 8)
-        fm.measure_text("X", font)
-        assert fm.font_cache and fm.metrics_cache
-        fm.clear_cache()
-        assert not fm.font_cache
-        assert not fm.metrics_cache
-
-    def test_reload_config_bumps_generation_and_clears(self, fm):
-        fm.get_font("press_start", 8)
-        gen_before = fm.cache_generation
-        fm.reload_config({})
-        assert fm.cache_generation == gen_before + 1
-        assert not fm.font_cache

test/test_logging_config.py

@@ -1,274 +0,0 @@
-"""
-Tests for src/logging_config.py — the formatters, adapter, and setup used
-by every logger in the system (BasePlugin uses get_logger, not stdlib
-logging.getLogger).
-
-Includes regression guards for two fixed bugs: ContextualFormatter used to
-mutate record.msg in place (double-prefixing with two handlers), and
-log_error hardcoded exc_info=True so passing it explicitly raised
-TypeError.
-"""
-
-import json
-import logging
-import sys
-
-import pytest
-
-from src.logging_config import (
-    ContextualFormatter,
-    PluginLoggerAdapter,
-    StructuredFormatter,
-    get_logger,
-    log_debug,
-    log_error,
-    log_info,
-    log_warning,
-    log_with_context,
-    setup_logging,
-)
-
-
-def make_record(msg="hello", level=logging.INFO, **extra):
-    record = logging.LogRecord(
-        name="test.logger", level=level, pathname=__file__, lineno=42,
-        msg=msg, args=(), exc_info=None)
-    for key, value in extra.items():
-        setattr(record, key, value)
-    return record
-
-
-class TestStructuredFormatter:
-    def test_emits_valid_json_with_base_keys(self):
-        out = json.loads(StructuredFormatter().format(make_record()))
-        assert set(out) == {
-            "timestamp", "level", "logger", "message",
-            "module", "function", "line",
-        }
-        assert out["level"] == "INFO"
-        assert out["message"] == "hello"
-        assert out["logger"] == "test.logger"
-
-    def test_optional_keys_only_when_present(self):
-        record = make_record(context={"k": "v"}, plugin_id="clock",
-                             operation_id="op-1")
-        out = json.loads(StructuredFormatter().format(record))
-        assert out["context"] == {"k": "v"}
-        assert out["plugin_id"] == "clock"
-        assert out["operation_id"] == "op-1"
-
-    def test_exception_key_when_exc_info_present(self):
-        try:
-            raise ValueError("kaboom")
-        except ValueError:
-            record = logging.LogRecord(
-                name="t", level=logging.ERROR, pathname=__file__, lineno=1,
-                msg="failed", args=(), exc_info=sys.exc_info())
-        out = json.loads(StructuredFormatter().format(record))
-        assert "kaboom" in out["exception"]
-
-    def test_percent_args_formatted_into_message(self):
-        record = logging.LogRecord(
-            name="t", level=logging.INFO, pathname=__file__, lineno=1,
-            msg="count=%d", args=(7,), exc_info=None)
-        out = json.loads(StructuredFormatter().format(record))
-        assert out["message"] == "count=7"
-
-
-class TestContextualFormatter:
-    def test_context_prefix_prepended(self):
-        record = make_record(plugin_id="clock", operation_id="op-1",
-                             context={"k": "v"})
-        out = ContextualFormatter().format(record)
-        assert "[Plugin: clock] [Op: op-1] [k: v] hello" in out
-
-    def test_include_context_false_leaves_message_bare(self):
-        record = make_record(plugin_id="clock")
-        out = ContextualFormatter(include_context=False).format(record)
-        assert "[Plugin:" not in out
-        assert "hello" in out
-
-    def test_location_toggle(self):
-        record = make_record()
-        with_loc = ContextualFormatter(include_location=True).format(record)
-        without = ContextualFormatter(include_location=False).format(record)
-        assert f":{record.lineno}" in with_loc
-        assert f":{record.lineno}" not in without
-
-    def test_record_not_mutated_no_double_prefix(self):
-        # Regression: a record is formatted once PER HANDLER. The formatter
-        # must not mutate record.msg, or the second handler's format call
-        # prepends the prefix again.
-        record = make_record(plugin_id="clock")
-        formatter = ContextualFormatter()
-        first = formatter.format(record)
-        second = formatter.format(record)
-        assert record.msg == "hello"  # untouched
-        assert first.count("[Plugin: clock]") == 1
-        assert second.count("[Plugin: clock]") == 1
-
-    def test_percent_args_still_format_after_copy(self):
-        record = logging.LogRecord(
-            name="t", level=logging.INFO, pathname=__file__, lineno=1,
-            msg="count=%d", args=(7,), exc_info=None)
-        record.plugin_id = "clock"
-        out = ContextualFormatter().format(record)
-        assert "[Plugin: clock] count=7" in out
-
-    def test_exception_renders_through_two_handlers(self):
-        try:
-            raise ValueError("kaboom")
-        except ValueError:
-            record = logging.LogRecord(
-                name="t", level=logging.ERROR, pathname=__file__, lineno=1,
-                msg="failed", args=(), exc_info=sys.exc_info())
-        record.plugin_id = "clock"
-        formatter = ContextualFormatter()
-        assert "kaboom" in formatter.format(record)
-        assert "kaboom" in formatter.format(record)  # second handler's pass
-
-
-class TestPluginLoggerAdapter:
-    def _capture(self, adapter):
-        records = []
-        handler = logging.Handler()
-        handler.emit = records.append
-        adapter.logger.addHandler(handler)
-        adapter.logger.setLevel(logging.DEBUG)
-        return records
-
-    def test_stamps_plugin_id_on_every_record(self):
-        adapter = get_logger("test.adapter1", plugin_id="clock")
-        records = self._capture(adapter)
-        adapter.info("x")
-        assert records[0].plugin_id == "clock"
-
-    def test_explicit_extra_plugin_id_wins(self):
-        adapter = get_logger("test.adapter2", plugin_id="clock")
-        records = self._capture(adapter)
-        adapter.info("x", extra={"plugin_id": "other"})
-        assert records[0].plugin_id == "other"
-
-    def test_unrelated_extra_keys_preserved(self):
-        adapter = get_logger("test.adapter3", plugin_id="clock")
-        records = self._capture(adapter)
-        adapter.info("x", extra={"custom": 1})
-        assert records[0].plugin_id == "clock"
-        assert records[0].custom == 1
-
-
-class TestGetLogger:
-    def test_plain_logger_without_plugin_id(self):
-        logger = get_logger("test.plain")
-        assert isinstance(logger, logging.Logger)
-        assert logger.name == "test.plain"
-
-    def test_adapter_with_plugin_id(self):
-        adapter = get_logger("test.wrapped", plugin_id="clock")
-        assert isinstance(adapter, PluginLoggerAdapter)
-        assert adapter.logger.name == "test.wrapped"
-
-
-class TestSetupLogging:
-    # conftest's autouse reset_logging restores root handlers after each test.
-
-    def test_installs_single_stdout_handler(self):
-        setup_logging()
-        root = logging.getLogger()
-        assert len(root.handlers) == 1
-        assert isinstance(root.handlers[0], logging.StreamHandler)
-
-    def test_repeat_calls_do_not_accumulate_handlers(self):
-        setup_logging()
-        setup_logging()
-        assert len(logging.getLogger().handlers) == 1
-
-    def test_json_format_selects_structured_formatter(self):
-        setup_logging(format_type="json")
-        assert isinstance(
-            logging.getLogger().handlers[0].formatter, StructuredFormatter)
-
-    def test_readable_format_selects_contextual_formatter(self):
-        setup_logging(format_type="readable")
-        assert isinstance(
-            logging.getLogger().handlers[0].formatter, ContextualFormatter)
-
-    def test_log_file_adds_file_handler(self, tmp_path):
-        log_file = tmp_path / "test.log"
-        setup_logging(log_file=str(log_file))
-        root = logging.getLogger()
-        file_handlers = [h for h in root.handlers
-                         if isinstance(h, logging.FileHandler)]
-        assert len(file_handlers) == 1
-        for h in file_handlers:
-            h.close()
-
-    def test_unwritable_log_file_warns_and_keeps_console(self, tmp_path, capsys):
-        bad_path = tmp_path / "no-such-dir" / "test.log"
-        setup_logging(log_file=str(bad_path))  # must not raise
-        assert len(logging.getLogger().handlers) == 1  # console only
-        assert "Could not set up file logging" in capsys.readouterr().err
-
-    def test_debug_env_true_enables_debug(self, monkeypatch):
-        monkeypatch.setenv("LEDMATRIX_DEBUG", "TRUE")
-        setup_logging()
-        assert logging.getLogger().level == logging.DEBUG
-
-    def test_debug_env_other_values_stay_info(self, monkeypatch):
-        # Pinned: only the literal (case-insensitive) "true" enables debug;
-        # "1" does not.
-        monkeypatch.setenv("LEDMATRIX_DEBUG", "1")
-        setup_logging()
-        assert logging.getLogger().level == logging.INFO
-
-    def test_explicit_level_wins_over_env(self, monkeypatch):
-        monkeypatch.setenv("LEDMATRIX_DEBUG", "true")
-        setup_logging(level=logging.WARNING)
-        assert logging.getLogger().level == logging.WARNING
-
-
-class TestLogWithContext:
-    def _capture(self, name):
-        logger = logging.getLogger(name)
-        records = []
-        handler = logging.Handler()
-        handler.emit = records.append
-        logger.addHandler(handler)
-        logger.setLevel(logging.DEBUG)
-        return logger, records
-
-    def test_context_attrs_stamped(self):
-        logger, records = self._capture("test.lwc1")
-        log_with_context(logger, logging.INFO, "msg",
-                         context={"k": "v"}, plugin_id="clock",
-                         operation_id="op-1")
-        record = records[0]
-        assert record.context == {"k": "v"}
-        assert record.plugin_id == "clock"
-        assert record.operation_id == "op-1"
-
-    def test_wrappers_use_their_levels(self):
-        logger, records = self._capture("test.lwc2")
-        log_debug(logger, "d")
-        log_info(logger, "i")
-        log_warning(logger, "w")
-        assert [r.levelno for r in records] == [
-            logging.DEBUG, logging.INFO, logging.WARNING]
-
-    def test_log_error_defaults_exc_info_true(self):
-        logger, records = self._capture("test.lwc3")
-        try:
-            raise ValueError("kaboom")
-        except ValueError:
-            log_error(logger, "failed")
-        assert records[0].levelno == logging.ERROR
-        assert records[0].exc_info is not None
-
-    def test_log_error_accepts_explicit_exc_info(self):
-        # Regression: the old hardcoded exc_info=True raised
-        # "got multiple values for keyword argument 'exc_info'".
-        logger, records = self._capture("test.lwc4")
-        log_error(logger, "failed", exc_info=False)
-        # Falsy exc_info is stored verbatim on the record; the contract is
-        # simply "no traceback attached".
-        assert not records[0].exc_info

test/test_saved_repositories.py

@@ -1,240 +0,0 @@
-"""
-Tests for src/plugin_system/saved_repositories.py — pins the
-SavedRepositoriesManager contract.
-
-Covers: the three accepted on-disk load shapes (bare list, wrapped
-{"repositories": [...]}, anything else -> []) and that saves always write
-the bare-list form; add/remove/has round trips through a fresh manager;
-URL normalization post-fix (_clean_url strips only a TRAILING '.git' after
-trailing slashes — the old unanchored .replace('.git', '') mangled URLs
-like my.github.io); name derivation and registry-vs-single type
-classification (the ledmatrix-plugins check is lowercased, the
-plugins.json check is case-sensitive); and the post-fix rollback of the
-in-memory list when _save_repositories() fails, so memory never diverges
-from disk.
-"""
-
-import json
-
-from src.plugin_system.saved_repositories import SavedRepositoriesManager
-
-
-def make_manager(path):
-    return SavedRepositoriesManager(config_path=str(path))
-
-
-class TestLoading:
-    def test_missing_file_empty_and_not_created(self, tmp_path):
-        path = tmp_path / "repos.json"
-        manager = make_manager(path)
-        assert manager.get_all() == []
-        assert not path.exists()
-
-    def test_bare_list_shape(self, tmp_path):
-        path = tmp_path / "repos.json"
-        entries = [{'url': 'https://github.com/u/r', 'name': 'r', 'type': 'single'}]
-        path.write_text(json.dumps(entries))
-        assert make_manager(path).get_all() == entries
-
-    def test_wrapped_dict_shape(self, tmp_path):
-        path = tmp_path / "repos.json"
-        entries = [{'url': 'https://github.com/u/r', 'name': 'r', 'type': 'single'}]
-        path.write_text(json.dumps({'repositories': entries}))
-        assert make_manager(path).get_all() == entries
-
-    def test_other_shape_yields_empty(self, tmp_path):
-        path = tmp_path / "repos.json"
-        path.write_text(json.dumps({'x': 1}))
-        assert make_manager(path).get_all() == []
-
-    def test_malformed_json_yields_empty_no_raise(self, tmp_path):
-        path = tmp_path / "repos.json"
-        path.write_text("not json {{")
-        assert make_manager(path).get_all() == []
-
-
-class TestSaveFormat:
-    def test_save_always_writes_bare_list(self, tmp_path):
-        # Even when loaded from the wrapped {"repositories": [...]} form,
-        # the next save normalizes the file to a bare JSON list.
-        path = tmp_path / "repos.json"
-        entries = [{'url': 'https://github.com/u/r', 'name': 'r', 'type': 'single'}]
-        path.write_text(json.dumps({'repositories': entries}))
-        manager = make_manager(path)
-        assert manager.add("https://github.com/u/r2") is True
-        on_disk = json.loads(path.read_text())
-        assert isinstance(on_disk, list)
-        assert len(on_disk) == 2
-
-
-class TestAdd:
-    def test_round_trip_creates_parents_and_reloads(self, tmp_path):
-        path = tmp_path / "sub" / "repos.json"
-        manager = make_manager(path)
-        assert manager.add("https://github.com/user/repo") is True
-        assert path.exists()
-        entry = manager.get_all()[0]
-        assert entry['url'] == "https://github.com/user/repo"
-        assert entry['name'] == "repo"
-        assert entry['type'] == "single"
-        # A fresh manager on the same path sees the persisted entry.
-        fresh = make_manager(path)
-        assert fresh.get_all() == [entry]
-
-    def test_duplicate_returns_false_file_unchanged(self, tmp_path):
-        path = tmp_path / "repos.json"
-        manager = make_manager(path)
-        assert manager.add("https://github.com/user/repo") is True
-        before = path.read_text()
-        assert manager.add("https://github.com/user/repo") is False
-        assert path.read_text() == before
-        assert len(manager.get_all()) == 1
-
-    def test_trailing_git_and_slash_stripped(self, tmp_path):
-        manager = make_manager(tmp_path / "repos.json")
-        assert manager.add("https://github.com/user/repo.git/") is True
-        assert manager.get_all()[0]['url'] == "https://github.com/user/repo"
-
-    def test_interior_dot_git_not_mangled(self, tmp_path):
-        # Regression for the old unanchored .replace('.git', ''): a URL
-        # merely CONTAINING '.git' must be stored verbatim.
-        manager = make_manager(tmp_path / "repos.json")
-        url = "https://github.com/user/my.github.io"
-        assert manager.add(url) is True
-        assert manager.get_all()[0]['url'] == url
-
-
-class TestNameExtraction:
-    def test_name_derived_from_last_path_segment(self, tmp_path):
-        manager = make_manager(tmp_path / "repos.json")
-        manager.add("https://github.com/user/football-scoreboard")
-        assert manager.get_all()[0]['name'] == "football-scoreboard"
-
-    def test_explicit_name_preserved(self, tmp_path):
-        manager = make_manager(tmp_path / "repos.json")
-        manager.add("https://github.com/user/repo", name="My Repo")
-        assert manager.get_all()[0]['name'] == "My Repo"
-
-    def test_url_without_slash_uses_whole_url(self, tmp_path):
-        manager = make_manager(tmp_path / "repos.json")
-        manager.add("standalone")
-        assert manager.get_all()[0]['name'] == "standalone"
-
-
-class TestTypeClassification:
-    def _type_of(self, tmp_path, url):
-        manager = make_manager(tmp_path / "repos.json")
-        assert manager.add(url) is True
-        return manager.get_all()[0]['type']
-
-    def test_plugins_json_url_is_registry(self, tmp_path):
-        url = "https://raw.githubusercontent.com/x/main/plugins.json"
-        assert self._type_of(tmp_path, url) == "registry"
-
-    def test_ledmatrix_plugins_check_is_case_insensitive(self, tmp_path):
-        url = "https://github.com/ChuckBuilds/LEDMATRIX-PLUGINS"
-        assert self._type_of(tmp_path, url) == "registry"
-
-    def test_plugins_json_check_is_case_sensitive(self, tmp_path):
-        # Only the 'ledmatrix-plugins' check is lowercased; the
-        # 'plugins.json' substring check is case-sensitive. Pinned.
-        url = "https://example.com/PLUGINS.JSON"
-        assert self._type_of(tmp_path, url) == "single"
-
-    def test_plain_repo_is_single(self, tmp_path):
-        assert self._type_of(tmp_path, "https://github.com/user/repo") == "single"
-
-    def test_get_registry_repositories_filters(self, tmp_path):
-        manager = make_manager(tmp_path / "repos.json")
-        manager.add("https://github.com/user/repo")
-        manager.add("https://raw.githubusercontent.com/x/main/plugins.json")
-        registries = manager.get_registry_repositories()
-        assert len(registries) == 1
-        assert registries[0]['type'] == "registry"
-
-
-class TestRemove:
-    def test_remove_present_persists(self, tmp_path):
-        path = tmp_path / "repos.json"
-        manager = make_manager(path)
-        manager.add("https://github.com/user/repo")
-        assert manager.remove("https://github.com/user/repo") is True
-        assert manager.get_all() == []
-        assert make_manager(path).get_all() == []
-
-    def test_remove_absent_false_no_write(self, tmp_path):
-        path = tmp_path / "repos.json"
-        manager = make_manager(path)
-        manager.add("https://github.com/user/repo")
-        before = path.read_text()
-        assert manager.remove("https://github.com/user/other") is False
-        assert path.read_text() == before
-
-    def test_remove_with_dirty_url_matches_clean_stored(self, tmp_path):
-        manager = make_manager(tmp_path / "repos.json")
-        manager.add("https://github.com/user/repo")
-        assert manager.remove("https://github.com/user/repo.git/") is True
-        assert manager.get_all() == []
-
-
-class TestHas:
-    def test_has_applies_url_cleaning(self, tmp_path):
-        manager = make_manager(tmp_path / "repos.json")
-        manager.add("https://x/y")
-        assert manager.has("https://x/y.git/") is True
-        assert manager.has("https://x/z") is False
-
-
-class TestSaveFailureRollback:
-    def test_add_rolls_back_on_save_failure(self, tmp_path, monkeypatch):
-        # Post-fix: a failed save must not leave a phantom in-memory entry.
-        path = tmp_path / "repos.json"
-        manager = make_manager(path)
-        monkeypatch.setattr(manager, "_save_repositories", lambda: False)
-        assert manager.add("https://github.com/user/repo") is False
-        assert manager.get_all() == []
-        assert not path.exists()
-
-    def test_remove_rolls_back_on_save_failure(self, tmp_path, monkeypatch):
-        path = tmp_path / "repos.json"
-        manager = make_manager(path)
-        manager.add("https://github.com/user/repo")  # real save
-        monkeypatch.setattr(manager, "_save_repositories", lambda: False)
-        assert manager.remove("https://github.com/user/repo") is False
-        assert manager.get_all() == [
-            {'url': 'https://github.com/user/repo', 'name': 'repo', 'type': 'single'}
-        ]
-        # Disk still has the entry too — memory and disk stay in sync.
-        assert len(json.loads(path.read_text())) == 1
-
-    def test_failed_write_leaves_existing_file_intact(self, tmp_path, monkeypatch):
-        # The save is atomic (temp file + os.replace): a write that dies
-        # mid-serialization must neither truncate the existing file nor
-        # leave a stray .tmp behind.
-        path = tmp_path / "repos.json"
-        manager = make_manager(path)
-        manager.add("https://github.com/user/repo")  # real save
-        before = path.read_text()
-
-        def boom(*args, **kwargs):
-            raise OSError("disk full")
-        monkeypatch.setattr(json, "dump", boom)
-        assert manager.add("https://github.com/user/other") is False
-
-        assert path.read_text() == before
-        assert list(tmp_path.glob("*.tmp")) == []
-
-
-class TestGetAllCopy:
-    def test_get_all_is_shallow_copy(self, tmp_path):
-        # Characterization: get_all() copies the LIST but not the entry
-        # dicts, so mutating a returned entry mutates internal state.
-        # Appending to the returned list, however, does not. Do not "fix"
-        # without auditing callers that rely on list-copy semantics.
-        manager = make_manager(tmp_path / "repos.json")
-        manager.add("https://github.com/user/repo")
-        returned = manager.get_all()
-        returned.append({'url': 'x'})
-        assert len(manager.get_all()) == 1  # list itself is copied
-        manager.get_all()[0]['name'] = 'hacked'
-        assert manager.get_all()[0]['name'] == 'hacked'  # dicts are shared

test/test_schema_manager_merge.py

@@ -1,104 +0,0 @@
-"""
-Tests for SchemaManager.merge_with_defaults — the merge every plugin config
-passes through at load time (schema defaults + user config, with None
-replacement). A regression here silently changes every plugin's effective
-config, so the exact branch behavior is pinned, including the
-characterized type-mismatch cases.
-"""
-
-import pytest
-
-from src.plugin_system.schema_manager import SchemaManager
-
-
-@pytest.fixture
-def sm(tmp_path):
-    return SchemaManager(plugins_dir=str(tmp_path))
-
-
-class TestBasicMerge:
-    def test_missing_keys_filled_from_defaults(self, sm):
-        merged = sm.merge_with_defaults(
-            {"city": "Austin"}, {"city": "NYC", "units": "metric"})
-        assert merged == {"city": "Austin", "units": "metric"}
-
-    def test_present_keys_preserved(self, sm):
-        merged = sm.merge_with_defaults({"enabled": False}, {"enabled": True})
-        assert merged["enabled"] is False
-
-    def test_nested_three_level_merge(self, sm):
-        config = {"a": {"b": {"c": 1}}}
-        defaults = {"a": {"b": {"c": 0, "d": 2}, "e": 3}}
-        merged = sm.merge_with_defaults(config, defaults)
-        assert merged == {"a": {"b": {"c": 1, "d": 2}, "e": 3}}
-
-    def test_inputs_not_mutated(self, sm):
-        config = {"a": {"b": 1}}
-        defaults = {"a": {"b": 0, "c": 2}, "d": 3}
-        sm.merge_with_defaults(config, defaults)
-        assert config == {"a": {"b": 1}}
-        assert defaults == {"a": {"b": 0, "c": 2}, "d": 3}
-
-    def test_merged_values_are_copies_not_aliases(self, sm):
-        config = {"teams": ["DAL"]}
-        merged = sm.merge_with_defaults(config, {"teams": []})
-        merged["teams"].append("HOU")
-        assert config["teams"] == ["DAL"]  # user's list untouched
-
-
-class TestNoneReplacement:
-    def test_none_replaced_by_default(self, sm):
-        merged = sm.merge_with_defaults({"units": None}, {"units": "metric"})
-        assert merged["units"] == "metric"
-
-    def test_falsey_non_none_values_kept(self, sm):
-        merged = sm.merge_with_defaults(
-            {"enabled": False, "count": 0, "label": ""},
-            {"enabled": True, "count": 5, "label": "x"},
-        )
-        assert merged == {"enabled": False, "count": 0, "label": ""}
-
-    def test_nested_none_replaced(self, sm):
-        merged = sm.merge_with_defaults(
-            {"style": {"color": None}}, {"style": {"color": "red"}})
-        assert merged["style"]["color"] == "red"
-
-    def test_none_with_no_default_stays_none(self, sm):
-        merged = sm.merge_with_defaults({"extra": None}, {})
-        assert merged["extra"] is None
-
-    def test_none_replaced_by_dict_default_is_a_copy(self, sm):
-        defaults = {"style": {"color": "red"}}
-        merged = sm.merge_with_defaults({"style": None}, defaults)
-        assert merged["style"] == {"color": "red"}
-        merged["style"]["color"] = "blue"
-        assert defaults["style"]["color"] == "red"
-
-
-class TestTypeMismatches:
-    def test_user_scalar_over_dict_default_wins(self, sm):
-        # Characterized: a scalar user value replaces a dict default outright.
-        merged = sm.merge_with_defaults(
-            {"style": "compact"}, {"style": {"color": "red"}})
-        assert merged["style"] == "compact"
-
-    def test_user_dict_over_scalar_default_wins(self, sm):
-        merged = sm.merge_with_defaults(
-            {"style": {"color": "red"}}, {"style": "compact"})
-        assert merged["style"] == {"color": "red"}
-
-    def test_arrays_replaced_wholesale_not_merged(self, sm):
-        # Pinned contract: arrays never element-merge — the user's array is
-        # the whole answer, even when shorter than the default.
-        merged = sm.merge_with_defaults(
-            {"teams": ["DAL"]}, {"teams": ["NYG", "PHI", "WAS"]})
-        assert merged["teams"] == ["DAL"]
-
-    def test_empty_user_array_beats_default(self, sm):
-        merged = sm.merge_with_defaults({"teams": []}, {"teams": ["NYG"]})
-        assert merged["teams"] == []
-
-    def test_extra_user_keys_survive(self, sm):
-        # Keys with no schema default pass through untouched.
-        merged = sm.merge_with_defaults({"custom_flag": 7}, {"known": 1})
-        assert merged == {"known": 1, "custom_flag": 7}

test/test_skin_runtime_cache.py

@@ -1,377 +0,0 @@
-"""Gap tests for src/skin_system/skin_runtime.py: the discovery cache,
-module namespacing internals, API gating edge cases, and targeting.
-
-test/test_skin_system.py already covers discovery validation, load_skin
-basics, and build_context — nothing here duplicates those.
-
-NOTE: every test uses a UNIQUE skin id. load_skin caches the entry
-module in sys.modules per skin id and never re-executes it, so reusing
-an id across tests would silently serve another test's module.
-"""
-
-import builtins
-import json
-import os
-import sys
-import time
-from pathlib import Path
-from unittest.mock import MagicMock
-
-import pytest
-
-# skin_runtime -> skin_base can transitively reach hardware modules via
-# sports imports in sibling tests' processes; stub the matrix driver
-# before importing, matching test_skin_system.py.
-sys.modules.setdefault("rgbmatrix", MagicMock())
-
-from src.skin_system import skin_runtime
-from src.skin_system.skin_base import SKIN_API_VERSION, ScoreboardSkin
-
-
-DEFAULT_BODY = (
-    "from src.skin_system.skin_base import ScoreboardSkin\n"
-    "class {cls}(ScoreboardSkin):\n"
-    "    def render_live(self, ctx, game):\n"
-    "        return True\n"
-)
-
-
-@pytest.fixture(autouse=True)
-def _clean_runtime_state():
-    """Clear the discovery cache and any skin modules this test creates."""
-    skin_runtime._discovery_cache.clear()
-    before = {k for k in sys.modules if k.startswith("_skin_")}
-    yield
-    skin_runtime._discovery_cache.clear()
-    created = [k for k in sys.modules
-               if k.startswith("_skin_") and k not in before]
-    for k in created:
-        sys.modules.pop(k, None)
-
-
-def make_skin(skins_dir: Path, skin_id: str, *,
-              api_version: str = SKIN_API_VERSION,
-              class_name: str = "TestSkin",
-              body: str = None,
-              extra_files: dict = None,
-              entry_point: str = None,
-              manifest_id: str = None,
-              manifest_extra: dict = None,
-              write_entry: bool = True) -> Path:
-    """Write a skin package directory and return its path."""
-    skin_dir = skins_dir / skin_id
-    skin_dir.mkdir(parents=True, exist_ok=True)
-    manifest = {
-        "id": manifest_id or skin_id,
-        "name": skin_id,
-        "version": "1.0.0",
-        "skin_api_version": api_version,
-        "class_name": class_name,
-    }
-    if entry_point:
-        manifest["entry_point"] = entry_point
-    manifest.update(manifest_extra or {})
-    (skin_dir / "skin.json").write_text(json.dumps(manifest))
-    if write_entry:
-        entry_name = entry_point or "skin.py"
-        (skin_dir / entry_name).write_text(
-            body if body is not None else DEFAULT_BODY.format(cls=class_name))
-    for name, content in (extra_files or {}).items():
-        (skin_dir / name).write_text(content)
-    return skin_dir
-
-
-def counting_read_manifest(monkeypatch):
-    """Wrap skin_runtime._read_manifest with a call counter."""
-    original = skin_runtime._read_manifest
-    counter = {"count": 0}
-
-    def wrapper(skin_dir):
-        counter["count"] += 1
-        return original(skin_dir)
-
-    monkeypatch.setattr(skin_runtime, "_read_manifest", wrapper)
-    return counter
-
-
-def bump_mtime(path: Path, offset: float = 100.0):
-    """Set a distinct, strictly later mtime so the fingerprint changes."""
-    t = time.time() + offset
-    os.utime(path, (t, t))
-
-
-# ---------------------------------------------------------------------------
-# A. Discovery cache
-# ---------------------------------------------------------------------------
-
-class TestDiscoveryCache:
-    def test_second_call_serves_cache(self, tmp_path, monkeypatch):
-        make_skin(tmp_path, "t01-cache-hit")
-        counter = counting_read_manifest(monkeypatch)
-        first = skin_runtime.discover_skins(tmp_path)
-        count_after_first = counter["count"]
-        assert count_after_first >= 1
-        second = skin_runtime.discover_skins(tmp_path)
-        assert counter["count"] == count_after_first  # no re-read
-        assert second == first
-        assert "t01-cache-hit" in second
-
-    def test_manifest_edit_invalidates_without_force_refresh(self, tmp_path):
-        skin_dir = make_skin(tmp_path, "t02-edit")
-        skins = skin_runtime.discover_skins(tmp_path)
-        assert skins["t02-edit"]["name"] == "t02-edit"
-
-        manifest_path = skin_dir / "skin.json"
-        manifest = json.loads(manifest_path.read_text())
-        manifest["name"] = "renamed"
-        manifest_path.write_text(json.dumps(manifest))
-        bump_mtime(manifest_path)
-
-        skins = skin_runtime.discover_skins(tmp_path)  # no force_refresh
-        assert skins["t02-edit"]["name"] == "renamed"
-
-    def test_new_skin_dir_invalidates(self, tmp_path):
-        make_skin(tmp_path, "t03-first")
-        assert set(skin_runtime.discover_skins(tmp_path)) == {"t03-first"}
-
-        new_dir = make_skin(tmp_path, "t03-second")
-        bump_mtime(new_dir / "skin.json")
-        bump_mtime(tmp_path)
-
-        skins = skin_runtime.discover_skins(tmp_path)  # no force_refresh
-        assert set(skins) == {"t03-first", "t03-second"}
-
-    def test_py_file_change_does_not_invalidate(self, tmp_path, monkeypatch):
-        # PIN: the fingerprint only globs */skin.json — editing a skin's
-        # .py file alone does NOT invalidate the cache; the cached
-        # manifests are still served (a code change needs a restart).
-        skin_dir = make_skin(tmp_path, "t04-pyedit")
-        counter = counting_read_manifest(monkeypatch)
-        skin_runtime.discover_skins(tmp_path)
-        count_after_first = counter["count"]
-
-        (skin_dir / "skin.py").write_text("# rewritten\n" +
-                                          DEFAULT_BODY.format(cls="TestSkin"))
-        bump_mtime(skin_dir / "skin.py")
-
-        skins = skin_runtime.discover_skins(tmp_path)
-        assert counter["count"] == count_after_first  # cache still served
-        assert "t04-pyedit" in skins
-
-    def test_force_refresh_rereads_with_unchanged_fingerprint(self, tmp_path,
-                                                              monkeypatch):
-        make_skin(tmp_path, "t05-force")
-        counter = counting_read_manifest(monkeypatch)
-        skin_runtime.discover_skins(tmp_path)
-        count_after_first = counter["count"]
-        skin_runtime.discover_skins(tmp_path, force_refresh=True)
-        assert counter["count"] > count_after_first
-
-    def test_result_mapping_is_copy_but_manifests_shared(self, tmp_path):
-        make_skin(tmp_path, "t06-copy")
-        result = skin_runtime.discover_skins(tmp_path)
-
-        # Mutating the returned mapping does not poison the cache...
-        del result["t06-copy"]
-        again = skin_runtime.discover_skins(tmp_path)  # cache hit
-        assert "t06-copy" in again
-
-        # ...but the inner manifest dicts ARE shared with the cache (pin).
-        again["t06-copy"]["name"] = "mutated-inner"
-        third = skin_runtime.discover_skins(tmp_path)  # cache hit
-        assert third["t06-copy"]["name"] == "mutated-inner"
-
-    def test_missing_directory_returns_empty_and_caches_nothing(self, tmp_path):
-        missing = tmp_path / "not-yet"
-        assert skin_runtime.discover_skins(missing) == {}
-        assert str(missing) not in skin_runtime._discovery_cache
-
-        # Creating the directory later is picked up without force_refresh.
-        make_skin(missing, "t07-late")
-        skins = skin_runtime.discover_skins(missing)
-        assert "t07-late" in skins
-
-    def test_hidden_underscore_and_plain_file_entries_skipped(self, tmp_path):
-        make_skin(tmp_path, ".hidden-skin")
-        make_skin(tmp_path, "_private-skin")
-        (tmp_path / "stray-file").write_text("not a directory")
-        make_skin(tmp_path, "t08-good")
-        skins = skin_runtime.discover_skins(tmp_path, force_refresh=True)
-        assert set(skins) == {"t08-good"}
-
-    def test_manifest_id_mismatch_keys_by_manifest_id(self, tmp_path):
-        make_skin(tmp_path, "t09-dirname", manifest_id="t09-manifest-id")
-        skins = skin_runtime.discover_skins(tmp_path, force_refresh=True)
-        assert "t09-manifest-id" in skins
-        assert "t09-dirname" not in skins
-        assert skins["t09-manifest-id"]["_skin_dir"].endswith("t09-dirname")
-
-    def test_falsy_required_field_drops_skin(self, tmp_path):
-        make_skin(tmp_path, "t10-empty-class", class_name="")
-        skins = skin_runtime.discover_skins(tmp_path, force_refresh=True)
-        assert skins == {}
-
-
-# ---------------------------------------------------------------------------
-# B. Module namespacing (_load_skin_module via load_skin)
-# ---------------------------------------------------------------------------
-
-BODY_WITH_HELPERS = (
-    "import helpers\n"
-    "from src.skin_system.skin_base import ScoreboardSkin\n"
-    "class TestSkin(ScoreboardSkin):\n"
-    "    pass\n"
-)
-
-
-class TestModuleNamespacing:
-    def test_namespaced_sys_modules_keys(self, tmp_path):
-        make_skin(tmp_path, "t11-ns", body=BODY_WITH_HELPERS,
-                  extra_files={"helpers.py": "VALUE = 11\n"})
-        skin = skin_runtime.load_skin("t11-ns", skins_dir=tmp_path)
-        assert skin is not None
-        assert "_skin_t11-ns_skin" in sys.modules
-        assert "_skin_t11-ns_helpers" in sys.modules
-
-    def test_preseeded_bare_name_restored(self, tmp_path, monkeypatch):
-        sentinel = object()
-        monkeypatch.setitem(sys.modules, "helpers", sentinel)
-        make_skin(tmp_path, "t12a-restore", body=BODY_WITH_HELPERS,
-                  extra_files={"helpers.py": "VALUE = 'a'\n"})
-        skin = skin_runtime.load_skin("t12a-restore", skins_dir=tmp_path)
-        assert skin is not None
-        assert sys.modules["helpers"] is sentinel
-
-    def test_absent_bare_name_stays_absent(self, tmp_path):
-        saved = sys.modules.pop("helpers", None)
-        try:
-            assert "helpers" not in sys.modules
-            make_skin(tmp_path, "t12b-absent", body=BODY_WITH_HELPERS,
-                      extra_files={"helpers.py": "VALUE = 'b'\n"})
-            skin = skin_runtime.load_skin("t12b-absent", skins_dir=tmp_path)
-            assert skin is not None
-            assert "helpers" not in sys.modules
-        finally:
-            if saved is not None:
-                sys.modules["helpers"] = saved
-
-    def test_stdlib_shadowing_sibling_leaves_real_module_intact(self, tmp_path):
-        real_json = sys.modules["json"]
-        make_skin(tmp_path, "t12c-json",
-                  extra_files={"json.py": "SKIN_LOCAL = True\n"})
-        skin = skin_runtime.load_skin("t12c-json", skins_dir=tmp_path)
-        assert skin is not None
-        assert sys.modules["json"] is real_json
-        assert not hasattr(sys.modules["json"], "SKIN_LOCAL")
-        assert json.loads('{"ok": 1}') == {"ok": 1}  # stdlib still works
-        # The skin's copy lives only under its namespaced alias.
-        assert getattr(sys.modules["_skin_t12c-json_json"], "SKIN_LOCAL") is True
-
-    def test_entry_module_executed_once_across_loads(self, tmp_path,
-                                                     monkeypatch):
-        executions = []
-        monkeypatch.setattr(builtins, "_t13_skin_executions", executions,
-                            raising=False)
-        body = (
-            "import builtins\n"
-            "builtins._t13_skin_executions.append(1)\n"
-            "from src.skin_system.skin_base import ScoreboardSkin\n"
-            "class TestSkin(ScoreboardSkin):\n"
-            "    pass\n"
-        )
-        make_skin(tmp_path, "t13-cached", body=body)
-        for _ in range(3):
-            skin = skin_runtime.load_skin("t13-cached", skins_dir=tmp_path)
-            assert skin is not None
-        assert len(executions) == 1  # module executed exactly once
-
-    def test_sibling_import_failure_returns_none_and_restores_bare(
-            self, tmp_path, monkeypatch):
-        sentinel = object()
-        monkeypatch.setitem(sys.modules, "helpers", sentinel)
-        make_skin(tmp_path, "t14-sibfail", body=BODY_WITH_HELPERS,
-                  extra_files={"helpers.py": "raise RuntimeError('sibling boom')\n"})
-        assert skin_runtime.load_skin("t14-sibfail", skins_dir=tmp_path) is None
-        assert sys.modules["helpers"] is sentinel
-
-    def test_missing_entry_point_file(self, tmp_path):
-        make_skin(tmp_path, "t15-noentry", write_entry=False)
-        assert skin_runtime.load_skin("t15-noentry", skins_dir=tmp_path) is None
-
-    def test_custom_entry_point(self, tmp_path):
-        make_skin(tmp_path, "t16-custom", entry_point="render.py")
-        skin = skin_runtime.load_skin("t16-custom", skins_dir=tmp_path)
-        assert isinstance(skin, ScoreboardSkin)
-        assert "_skin_t16-custom_render" in sys.modules
-        assert "_skin_t16-custom_skin" not in sys.modules
-
-    def test_class_name_pointing_at_unrelated_class(self, tmp_path):
-        body = "class NotASkin:\n    pass\n"
-        make_skin(tmp_path, "t17a-wrongclass", body=body,
-                  class_name="NotASkin")
-        assert skin_runtime.load_skin("t17a-wrongclass",
-                                      skins_dir=tmp_path) is None
-
-    def test_class_name_pointing_at_instance(self, tmp_path):
-        body = (
-            "from src.skin_system.skin_base import ScoreboardSkin\n"
-            "class MySkin(ScoreboardSkin):\n"
-            "    pass\n"
-            "obj = MySkin({}, {})\n"
-        )
-        make_skin(tmp_path, "t17b-instance", body=body, class_name="obj")
-        assert skin_runtime.load_skin("t17b-instance",
-                                      skins_dir=tmp_path) is None
-
-    def test_constructor_raising_returns_none(self, tmp_path):
-        body = (
-            "from src.skin_system.skin_base import ScoreboardSkin\n"
-            "class TestSkin(ScoreboardSkin):\n"
-            "    def __init__(self, manifest, options):\n"
-            "        raise ValueError('ctor boom')\n"
-        )
-        make_skin(tmp_path, "t18-ctor", body=body)
-        assert skin_runtime.load_skin("t18-ctor", skins_dir=tmp_path) is None
-
-
-# ---------------------------------------------------------------------------
-# C. API gate + targeting
-# ---------------------------------------------------------------------------
-
-class TestApiGateAndTargeting:
-    def test_same_major_higher_minor_loads(self, tmp_path):
-        make_skin(tmp_path, "t19-minor", api_version="1.9.0")
-        skin = skin_runtime.load_skin("t19-minor", skins_dir=tmp_path)
-        assert isinstance(skin, ScoreboardSkin)
-
-    def test_malformed_api_version_refused(self, tmp_path):
-        make_skin(tmp_path, "t20-malformed", api_version="abc")
-        assert skin_runtime.load_skin("t20-malformed",
-                                      skins_dir=tmp_path) is None
-
-    @pytest.mark.parametrize("manifest,sport,sport_key,expected", [
-        # No targets key at all -> matches everything
-        ({"id": "x"}, "baseball", "mlb", True),
-        ({"id": "x"}, None, None, True),
-        # Empty targets dict -> matches everything
-        ({"id": "x", "targets": {}}, "hockey", None, True),
-        # sports family match
-        ({"id": "x", "targets": {"sports": ["baseball"]}},
-         "baseball", None, True),
-        # sport_keys exact match
-        ({"id": "x", "targets": {"sport_keys": ["milb"]}},
-         None, "milb", True),
-        # OR semantics: sport_keys matches even though sports excludes it
-        ({"id": "x", "targets": {"sports": ["hockey"],
-                                 "sport_keys": ["milb"]}},
-         "baseball", "milb", True),
-        # Neither matches
-        ({"id": "x", "targets": {"sports": ["hockey"]}},
-         "baseball", None, False),
-        ({"id": "x", "targets": {"sports": ["hockey"],
-                                 "sport_keys": ["nhl"]}},
-         "baseball", "milb", False),
-    ])
-    def test_skin_matches_target(self, manifest, sport, sport_key, expected):
-        assert skin_runtime.skin_matches_target(
-            manifest, sport, sport_key) is expected

test/test_skin_system.py

@@ -450,122 +450,3 @@ class TestExampleSkin:
         ctx = skin_runtime.build_context(host, game, size=size)
         assert getattr(skin, f"render_{mode}")(ctx, game) is True
         assert ctx.canvas.convert("L").getbbox() is not None
-
-
-class TestRenderSkinCard:
-    """render_skin_card (vegas cards) shares _render_game's 3-strike counter.
-
-    Both paths reset the counter on success — transient failures must not
-    accumulate across a session and disable a working skin.
-    """
-
-    def _probe(self, skin):
-        from src.base_classes.sports import SportsCore
-        probe = _FallbackProbe(skin)
-        probe.render_skin_card = (
-            lambda game, size: SportsCore.render_skin_card(probe, game, size))
-        return probe
-
-    def test_vegas_card_returned_when_skin_provides_one(self):
-        card_img = Image.new("RGB", (96, 32), (0, 0, 255))
-
-        class CardSkin(ScoreboardSkin):
-            def render_vegas_card(self, ctx, game):
-                return card_img
-
-        probe = self._probe(CardSkin({}, {}))
-        assert probe.render_skin_card({}, (96, 32)) is card_img
-
-    def test_vegas_card_none_falls_through_to_mode_renderer(self):
-        class ModeOnlySkin(ScoreboardSkin):
-            def render_live(self, ctx, game):
-                ctx.draw.rectangle([0, 0, 5, 5], fill=(255, 0, 0))
-                return True
-
-        probe = self._probe(ModeOnlySkin({}, {}))
-        card = probe.render_skin_card({}, (96, 32))
-        assert card is not None
-        assert card.size == (96, 32)
-        assert card.convert("L").getbbox() is not None
-
-    def test_skin_declining_returns_none(self):
-        probe = self._probe(ScoreboardSkin({}, {}))  # all renders -> False
-        assert probe.render_skin_card({}, (96, 32)) is None
-        assert probe._skin_failures == 0  # declining is not a failure
-
-    def test_no_skin_returns_none(self):
-        probe = self._probe(None)
-        assert probe.render_skin_card({}, (96, 32)) is None
-
-    def test_card_failures_count_toward_shared_disable(self):
-        class BrokenCardSkin(ScoreboardSkin):
-            calls = 0
-
-            def render_vegas_card(self, ctx, game):
-                BrokenCardSkin.calls += 1
-                raise ValueError("kaboom")
-
-        probe = self._probe(BrokenCardSkin({}, {}))
-        for _ in range(5):
-            assert probe.render_skin_card({}, (96, 32)) is None
-        # Skin stopped being consulted after the 3rd failure...
-        assert BrokenCardSkin.calls == 3
-        assert probe._skin_failures == 3
-        # ...and the shared counter also disables _render_game's skin path.
-        probe._render_game({"status_text": "Q1"})
-        assert probe.builtin_calls == 1
-        assert BrokenCardSkin.calls == 3  # not consulted again
-
-    def test_card_success_resets_strikes(self):
-        """A successful card render clears accumulated strikes (mirroring
-        _render_game) — 2 failures + a success + 1 failure leaves the skin
-        enabled with a single strike, instead of disabling it."""
-        card_img = Image.new("RGB", (96, 32), (0, 0, 255))
-
-        class FlakyCardSkin(ScoreboardSkin):
-            fail = True
-
-            def render_vegas_card(self, ctx, game):
-                if FlakyCardSkin.fail:
-                    raise ValueError("kaboom")
-                return card_img
-
-        probe = self._probe(FlakyCardSkin({}, {}))
-        FlakyCardSkin.fail = True
-        probe.render_skin_card({}, (96, 32))
-        probe.render_skin_card({}, (96, 32))
-        assert probe._skin_failures == 2
-
-        FlakyCardSkin.fail = False
-        assert probe.render_skin_card({}, (96, 32)) is card_img
-        assert probe._skin_failures == 0  # success cleared the strikes
-
-        FlakyCardSkin.fail = True
-        probe.render_skin_card({}, (96, 32))
-        assert probe._skin_failures == 1  # counting from the reset state
-        FlakyCardSkin.fail = False
-        assert probe.render_skin_card({}, (96, 32)) is card_img  # still enabled
-
-    def test_card_success_via_mode_renderer_also_resets_strikes(self):
-        """The fallthrough path (render_vegas_card None -> mode renderer
-        True) resets the counter as well."""
-        class ModeOnlySkin(ScoreboardSkin):
-            def render_live(self, ctx, game):
-                ctx.draw.rectangle([0, 0, 5, 5], fill=(255, 0, 0))
-                return True
-
-        probe = self._probe(ModeOnlySkin({}, {}))
-        probe._skin_failures = 2
-        assert probe.render_skin_card({}, (96, 32)) is not None
-        assert probe._skin_failures == 0
-
-    def test_render_game_success_also_resets_strikes(self):
-        """Same reset contract on the display path, for symmetry."""
-        class GoodSkin(ScoreboardSkin):
-            def render_live(self, ctx, game):
-                return True
-
-        probe = _FallbackProbe(GoodSkin({}, {}))
-        probe._skin_failures = 2
-        probe._render_game({"status_text": "Q1"})
-        assert probe._skin_failures == 0

test/test_sports_capabilities.py

@@ -878,200 +878,3 @@ class TestCapabilityExports:
     def test_rotation_strategy_base_requires_a_schedule(self):
         with pytest.raises(NotImplementedError):
             RotationStrategy().schedule([game("a")])
-
-
-# ---------------------------------------------------------------------------
-# Celebrations: rendering + previously untested edges
-# ---------------------------------------------------------------------------
-
-from PIL import Image, ImageDraw, ImageFont  # noqa: E402
-
-
-class _RenderableLive(_FakeLive):
-    """A _FakeLive that can actually execute _draw_celebration_layout:
-    real fonts, a display manager holding a real PIL image, and the two
-    SportsCore drawing seams the mixin calls."""
-
-    def __init__(self, mode_config=None, favorite_teams=None,
-                 width=128, height=32, with_matrix=True):
-        super().__init__(mode_config=mode_config, favorite_teams=favorite_teams)
-        font = ImageFont.load_default()
-        self.fonts = {"time": font, "status": font, "score": font}
-        self.display_width = width
-        self.display_height = height
-        dm = MagicMock()
-        if with_matrix:
-            dm.matrix.width = width
-            dm.matrix.height = height
-        else:
-            dm.matrix = None
-        dm.image = Image.new("RGB", (width, height))
-        self.display_manager = dm
-        self.logo_calls = []
-
-    def _load_and_resize_logo(self, team_id, abbr, path, url):
-        self.logo_calls.append(abbr)
-        logo = Image.new("RGBA", (10, 10), (0, 200, 0, 255))
-        return logo
-
-    def _draw_text_with_outline(self, draw, text, position, font, fill=(255, 255, 255)):
-        draw.text(position, str(text), font=font, fill=fill)
-
-
-class _RenderableCelebrating(CelebrationMixin, _RenderableLive):
-    pass
-
-
-def _armed(manager, *, kind="score", side="home", started_ago=0.0):
-    manager._start_celebration(
-        game("g1", home_score=7, away_score=3), kind,
-        scored_side=side, team_abbr="HOM", away_score=3, home_score=7,
-        points=7,
-    )
-    manager.active_celebration["started_at"] = time.time() - started_ago
-    return manager.active_celebration
-
-
-class TestDrawCelebrationLayout:
-    """The takeover render path, executed for real (previously always
-    mocked out)."""
-
-    def test_renders_and_hands_frame_to_display_manager(self):
-        manager = _RenderableCelebrating()
-        celebration = _armed(manager)
-        manager._draw_celebration_layout(celebration)
-        # The final frame was assigned and pushed.
-        assert isinstance(manager.display_manager.image, Image.Image)
-        assert manager.display_manager.image.mode == "RGB"
-        assert manager.display_manager.image.size == (128, 32)
-        manager.display_manager.update_display.assert_called_once()
-        assert manager.display_manager.image.convert("L").getbbox() is not None
-
-    def test_force_clear_clears_display_first(self):
-        manager = _RenderableCelebrating()
-        celebration = _armed(manager)
-        manager._draw_celebration_layout(celebration, force_clear=True)
-        manager.display_manager.clear.assert_called_once()
-
-    def test_flash_background_within_first_window(self):
-        # elapsed < 1.2 with int(elapsed/0.2) even -> flash color backdrop.
-        manager = _RenderableCelebrating()
-        celebration = _armed(manager, started_ago=0.05)
-        manager._draw_celebration_layout(celebration)
-        flash = manager.display_manager.image
-        # After the flash window: plain black backdrop.
-        celebration["started_at"] = time.time() - 5
-        manager._draw_celebration_layout(celebration)
-        steady = manager.display_manager.image
-        # Corner pixels (away from logos/text) show the two backgrounds.
-        assert flash.getpixel((64, 30)) != steady.getpixel((64, 30)) or \
-            flash.getpixel((3, 0)) != steady.getpixel((3, 0))
-
-    def test_matrix_dims_fallback_to_display_attrs(self):
-        manager = _RenderableCelebrating(width=96, height=48, with_matrix=False)
-        celebration = _armed(manager)
-        manager._draw_celebration_layout(celebration)
-        assert manager.display_manager.image.size == (96, 48)
-
-    def test_highlight_color_alternates_with_elapsed(self):
-        manager = _RenderableCelebrating()
-        celebration = _armed(manager)
-        # int(elapsed*4) % 2 == 0 -> yellow; == 1 -> orange. Force each phase
-        # and diff the frames.
-        celebration["started_at"] = time.time() - 2.0   # 8 -> even
-        manager._draw_celebration_layout(celebration)
-        even = manager.display_manager.image.tobytes()
-        celebration["started_at"] = time.time() - 2.25  # 9 -> odd
-        manager._draw_celebration_layout(celebration)
-        odd = manager.display_manager.image.tobytes()
-        assert even != odd
-
-    def test_logo_failure_still_renders_text(self):
-        manager = _RenderableCelebrating()
-
-        def boom(*a, **k):
-            raise RuntimeError("disk gone")
-
-        manager._load_and_resize_logo = boom
-        celebration = _armed(manager, started_ago=5)  # steady background
-        manager._draw_celebration_layout(celebration)  # must not raise
-        assert manager.display_manager.image.convert("L").getbbox() is not None
-        manager.display_manager.update_display.assert_called_once()
-
-
-class TestCelebrationEdges:
-    def test_should_celebrate_for_three_way_branch(self, celebrating):
-        g = game("g1", home="FAV", away="OPP")
-        favored = celebrating(favorites=["FAV"])
-        assert favored._should_celebrate_for(g, "home") is True   # favorite
-        assert favored._should_celebrate_for(g, "away") is False  # opponent
-        favored.celebrate_opponent_scores = True
-        assert favored._should_celebrate_for(g, "away") is True   # opted in
-        unconfigured = celebrating(favorites=[])
-        assert unconfigured._should_celebrate_for(g, "away") is True  # no favs
-
-    def test_active_celebration_boundary_is_strict(self, celebrating):
-        manager = celebrating(mode_config={"celebration_duration": 3})
-        manager.active_celebration = {"started_at": time.time() - 3.0}
-        # elapsed == duration -> strictly-less-than comparison says done.
-        assert manager.has_active_celebration() is False
-        manager.active_celebration = None
-        assert manager.has_active_celebration() is False
-
-    @pytest.mark.parametrize("value,expected", [
-        ({"value": None}, None),      # int(float(None)) TypeError -> caught
-        ({"value": "abc"}, None),
-        ({"other": 1}, 0),            # neither key -> default 0
-        ([3], None),                  # list -> TypeError -> caught
-        ("-4", None),                 # regex fallback finds digits -> 4? No:
-    ])
-    def test_score_to_int_edges(self, value, expected):
-        result = CelebrationMixin._score_to_int(value)
-        if value == "-4":
-            # int(float("-4")) parses directly: -4.
-            assert result == -4
-        else:
-            assert result == expected
-
-    def test_both_teams_scoring_prefers_away(self, celebrating):
-        manager = celebrating(favorites=[])
-        manager._check_for_score(game("g1", home_score=0, away_score=0))
-        manager._check_for_score(game("g1", home_score=7, away_score=3))
-        assert manager.active_celebration["scored_side"] == "away"
-
-    def test_away_not_celebratable_falls_through_to_home(self, celebrating):
-        manager = celebrating(favorites=["HOM"])  # away is the opponent
-        manager._check_for_score(game("g1", home_score=0, away_score=0))
-        manager._check_for_score(game("g1", home_score=7, away_score=3))
-        assert manager.active_celebration["scored_side"] == "home"
-
-    def test_coalesce_expired_celebration_fires_fresh(self, celebrating):
-        manager = celebrating(cls=_Coalescing,
-                              mode_config={"celebration_duration": 1})
-        manager._check_for_score(game("g1"))
-        manager._check_for_score(game("g1", home_score=6))
-        first = manager.active_celebration
-        assert first is not None
-        first["started_at"] = time.time() - 2  # expired
-        manager._check_for_score(game("g1", home_score=7))
-        # A new celebration replaced the expired one (coalescing only
-        # suppresses while one is actively on screen).
-        assert manager.active_celebration is not first
-        assert manager.active_celebration["home_score"] == 7
-
-    def test_disabled_win_check_preserves_baseline(self, celebrating):
-        manager = celebrating(favorites=["HOM"])
-        manager._check_for_score(game("g1"))
-        assert "g1" in manager._score_baselines
-        manager.celebration_enabled = False
-        manager._check_for_win(game("g1", home_score=7))
-        # Early return BEFORE consuming the baseline: re-enabling later can
-        # still fire for this game.
-        assert "g1" in manager._score_baselines
-
-    def test_prune_drops_baselines_for_idless_live_games(self, celebrating):
-        manager = celebrating()
-        manager._score_baselines = {"g1": {"away": 0, "home": 0}}
-        manager.prune_score_baselines([{"no_id_here": True}])
-        # live ids collapse to {None}; g1 is not live -> dropped.
-        assert manager._score_baselines == {}

test/test_startup_validator.py

@@ -1,293 +0,0 @@
-"""
-Tests for src/startup_validator.py — pins the StartupValidator contract.
-
-Covers: required-key/config error reporting (errors never propagate out of
-validate_all), the load_config/get_config accessor split, cache-directory
-error-vs-warning downgrade behavior, plugin discovery/manifest checks with
-reserved config keys skipped, idempotent validate_all (fresh error/warning
-lists each run — the pre-fix behavior duplicated messages), and the
-exception classification precedence in raise_on_errors (config > cache >
-plugin > fallback ConfigError).
-"""
-
-import copy
-import os
-from unittest.mock import MagicMock
-
-import pytest
-
-from src.exceptions import CacheError, ConfigError, PluginError
-from src.startup_validator import StartupValidator
-
-GOOD_CONFIG = {
-    'display': {'hardware': {'rows': 32, 'cols': 64}},
-    'timezone': 'UTC',
-}
-
-
-def make_config_manager(config):
-    """Config manager whose load_config() and get_config() return `config`."""
-    mgr = MagicMock()
-    mgr.load_config.return_value = copy.deepcopy(config)
-    mgr.get_config.return_value = copy.deepcopy(config)
-    return mgr
-
-
-@pytest.fixture
-def good_cache(monkeypatch, tmp_path):
-    """Patch CacheManager so cache validation sees an existing writable dir.
-
-    _validate_cache_directory does `from src.cache_manager import CacheManager`
-    at call time, so patching the attribute on the module is picked up.
-    """
-    mock_cls = MagicMock()
-    mock_cls.return_value.get_cache_dir.return_value = str(tmp_path)
-    monkeypatch.setattr("src.cache_manager.CacheManager", mock_cls)
-    return tmp_path
-
-
-class TestValidateConfig:
-    """Configuration validation via load_config()."""
-
-    def test_happy_path(self, good_cache):
-        validator = StartupValidator(make_config_manager(GOOD_CONFIG))
-        is_valid, errors, warnings = validator.validate_all()
-        assert is_valid is True
-        assert errors == []
-        assert warnings == []
-
-    def test_missing_required_keys(self, good_cache):
-        validator = StartupValidator(make_config_manager({}))
-        is_valid, errors, warnings = validator.validate_all()
-        assert is_valid is False
-        assert "Missing required configuration key: display" in errors
-        assert "Missing required configuration key: timezone" in errors
-
-    def test_config_error_does_not_propagate(self, good_cache):
-        mgr = make_config_manager(GOOD_CONFIG)
-        mgr.load_config.side_effect = ConfigError("bad json")
-        validator = StartupValidator(mgr)
-        is_valid, errors, warnings = validator.validate_all()
-        assert is_valid is False
-        config_errors = [e for e in errors if e.startswith("Configuration error:")]
-        assert len(config_errors) == 1
-        assert "bad json" in config_errors[0]
-
-    def test_unexpected_error_does_not_propagate(self, good_cache):
-        mgr = make_config_manager(GOOD_CONFIG)
-        mgr.load_config.side_effect = RuntimeError("kapow")
-        validator = StartupValidator(mgr)
-        is_valid, errors, warnings = validator.validate_all()
-        assert is_valid is False
-        unexpected = [e for e in errors
-                      if e.startswith("Unexpected error validating configuration:")]
-        assert len(unexpected) == 1
-        assert "kapow" in unexpected[0]
-
-    def test_accessor_split_get_config_failure_is_warning_only(self, good_cache):
-        # _validate_config uses load_config(); _validate_display_config uses
-        # get_config(). A broken get_config must degrade to a warning, not
-        # crash or produce a config error.
-        mgr = make_config_manager(GOOD_CONFIG)
-        mgr.get_config.side_effect = RuntimeError("accessor broken")
-        validator = StartupValidator(mgr)
-        is_valid, errors, warnings = validator.validate_all()
-        assert is_valid is True
-        assert errors == []
-        assert any(w.startswith("Could not validate display configuration:")
-                   for w in warnings)
-        assert mgr.load_config.called
-        assert mgr.get_config.called
-
-
-class TestDisplayConfig:
-    """Display hardware validation via get_config()."""
-
-    def test_missing_hardware_section_is_error(self, good_cache):
-        config = {'display': {'runtime': {'gpio_slowdown': 2}}, 'timezone': 'UTC'}
-        validator = StartupValidator(make_config_manager(config))
-        is_valid, errors, warnings = validator.validate_all()
-        assert is_valid is False
-        assert "Display hardware configuration is missing" in errors
-
-    def test_missing_rows_cols_are_warnings_not_errors(self, good_cache):
-        config = {'display': {'hardware': {'brightness': 90}}, 'timezone': 'UTC'}
-        validator = StartupValidator(make_config_manager(config))
-        is_valid, errors, warnings = validator.validate_all()
-        assert is_valid is True
-        assert errors == []
-        assert "Display hardware setting 'rows' not specified, using default" in warnings
-        assert "Display hardware setting 'cols' not specified, using default" in warnings
-
-
-class TestCacheDirectory:
-    """Cache directory validation error/warning split."""
-
-    def _patch_cache_dir(self, monkeypatch, cache_dir):
-        mock_cls = MagicMock()
-        mock_cls.return_value.get_cache_dir.return_value = cache_dir
-        monkeypatch.setattr("src.cache_manager.CacheManager", mock_cls)
-
-    def test_nonexistent_cache_dir_is_error(self, monkeypatch, tmp_path):
-        missing = str(tmp_path / "does_not_exist")
-        self._patch_cache_dir(monkeypatch, missing)
-        validator = StartupValidator(make_config_manager(GOOD_CONFIG))
-        is_valid, errors, warnings = validator.validate_all()
-        assert is_valid is False
-        assert any("does not exist" in e and missing in e for e in errors)
-
-    def test_writable_cache_dir_no_errors(self, monkeypatch, tmp_path):
-        self._patch_cache_dir(monkeypatch, str(tmp_path))
-        validator = StartupValidator(make_config_manager(GOOD_CONFIG))
-        is_valid, errors, warnings = validator.validate_all()
-        assert is_valid is True
-        assert not any('cache' in e.lower() for e in errors)
-
-    def test_unwritable_cache_dir_is_error(self, monkeypatch, tmp_path):
-        # Root (common in CI) can write anywhere, so chmod tricks don't
-        # work — force os.access to deny writes for the cache dir only.
-        cache_dir = str(tmp_path)
-        self._patch_cache_dir(monkeypatch, cache_dir)
-        real_access = os.access
-
-        def fake_access(path, mode):
-            if str(path) == cache_dir and mode == os.W_OK:
-                return False
-            return real_access(path, mode)
-
-        monkeypatch.setattr(os, "access", fake_access)
-        validator = StartupValidator(make_config_manager(GOOD_CONFIG))
-        is_valid, errors, warnings = validator.validate_all()
-        assert is_valid is False
-        assert any("is not writable" in e for e in errors)
-
-    def test_none_cache_dir_is_warning_not_error(self, monkeypatch):
-        self._patch_cache_dir(monkeypatch, None)
-        validator = StartupValidator(make_config_manager(GOOD_CONFIG))
-        is_valid, errors, warnings = validator.validate_all()
-        assert is_valid is True
-        assert errors == []
-        assert "Cache directory not available - caching will be disabled" in warnings
-
-    def test_cache_manager_constructor_failure_is_warning(self, monkeypatch):
-        mock_cls = MagicMock(side_effect=RuntimeError("no disk"))
-        monkeypatch.setattr("src.cache_manager.CacheManager", mock_cls)
-        validator = StartupValidator(make_config_manager(GOOD_CONFIG))
-        is_valid, errors, warnings = validator.validate_all()
-        assert is_valid is True
-        assert errors == []
-        assert any(w.startswith("Could not validate cache directory:") for w in warnings)
-
-
-class TestPlugins:
-    """Plugin validation with a plugin manager present."""
-
-    def _config_with_plugins(self):
-        return {
-            'display': {'hardware': {'rows': 32, 'cols': 64}},
-            'schedule': {'enabled': True},  # reserved key that LOOKS enabled
-            'timezone': 'UTC',
-            'plugin_system': {},
-            'known': {'enabled': True},
-            'ghost': {'enabled': True},
-        }
-
-    def test_ghost_plugin_warns_and_reserved_keys_skipped(self, good_cache, tmp_path):
-        pm = MagicMock()
-        pm.discover_plugins.return_value = ['known']
-        known_dir = tmp_path / "known"
-        known_dir.mkdir()
-        (known_dir / "manifest.json").write_text("{}")
-        pm.get_plugin_directory.return_value = str(known_dir)
-
-        validator = StartupValidator(make_config_manager(self._config_with_plugins()), pm)
-        is_valid, errors, warnings = validator.validate_all()
-        assert is_valid is True
-        assert "Plugin 'ghost' is enabled but not found in plugins directory" in warnings
-        # Reserved sections are never treated as plugins, even when they
-        # contain an 'enabled' flag (schedule above).
-        for reserved in ('display', 'schedule', 'timezone', 'plugin_system'):
-            assert not any(f"'{reserved}'" in w for w in warnings)
-
-    def test_enabled_plugin_missing_manifest_is_error(self, good_cache, tmp_path):
-        pm = MagicMock()
-        pm.discover_plugins.return_value = ['known']
-        plugin_dir = tmp_path / "known"
-        plugin_dir.mkdir()  # exists, but no manifest.json inside
-        pm.get_plugin_directory.return_value = str(plugin_dir)
-
-        config = dict(GOOD_CONFIG, known={'enabled': True})
-        validator = StartupValidator(make_config_manager(config), pm)
-        is_valid, errors, warnings = validator.validate_all()
-        assert is_valid is False
-        assert "Plugin 'known' manifest.json not found" in errors
-
-    def test_disabled_plugin_not_checked_for_manifest(self, good_cache, tmp_path):
-        pm = MagicMock()
-        pm.discover_plugins.return_value = ['known']
-        pm.get_plugin_directory.return_value = str(tmp_path / "nowhere")
-
-        config = dict(GOOD_CONFIG, known={'enabled': False})
-        validator = StartupValidator(make_config_manager(config), pm)
-        is_valid, errors, warnings = validator.validate_all()
-        assert is_valid is True
-        assert errors == []
-
-
-class TestIdempotence:
-    """validate_all() resets error/warning state each run (the fixed bug)."""
-
-    def test_repeated_runs_do_not_accumulate(self, good_cache):
-        validator = StartupValidator(make_config_manager({}))
-        first = validator.validate_all()
-        second = validator.validate_all()
-        assert first == second
-        assert len(second[1]) == len(first[1])
-        assert len(second[2]) == len(first[2])
-
-
-class TestRaiseOnErrors:
-    """Exception classification and precedence in raise_on_errors()."""
-
-    def _validator(self, errors):
-        validator = StartupValidator(make_config_manager(GOOD_CONFIG))
-        validator.errors = list(errors)
-        return validator
-
-    def test_no_errors_returns_none(self):
-        assert self._validator([]).raise_on_errors() is None
-
-    def test_config_error(self):
-        msg = "Missing required configuration key: display"
-        with pytest.raises(ConfigError) as excinfo:
-            self._validator([msg]).raise_on_errors()
-        assert excinfo.value.message == "Configuration validation failed"
-        assert msg in excinfo.value.context['errors']
-
-    def test_cache_error(self):
-        msg = "Cache directory does not exist: /nope"
-        with pytest.raises(CacheError) as excinfo:
-            self._validator([msg]).raise_on_errors()
-        assert msg in excinfo.value.context['errors']
-
-    def test_plugin_error(self):
-        msg = "Plugin 'known' manifest.json not found"
-        with pytest.raises(PluginError) as excinfo:
-            self._validator([msg]).raise_on_errors()
-        assert msg in excinfo.value.context['errors']
-
-    def test_unclassified_error_falls_back_to_config_error(self):
-        msg = "Something entirely else went wrong"
-        with pytest.raises(ConfigError) as excinfo:
-            self._validator([msg]).raise_on_errors()
-        assert excinfo.value.message == "Startup validation failed"
-        assert msg in excinfo.value.context['errors']
-
-    def test_precedence_config_beats_cache(self):
-        # A message matching both 'config' and 'cache' substrings raises
-        # ConfigError because config classification is checked first.
-        msg = "config problem touching the cache layer"
-        with pytest.raises(ConfigError) as excinfo:
-            self._validator([msg]).raise_on_errors()
-        assert excinfo.value.message == "Configuration validation failed"
-        assert msg in excinfo.value.context['errors']

test/test_version_comparison_consistency.py

@@ -1,175 +0,0 @@
-"""
-Drift guard for version comparison.
-
-There is now ONE shared "should this plugin update?" comparator —
-`src.plugin_system.compatibility.is_update_available` — used by both the web
-UI's update badge (`api_v3._is_plugin_update_available`) and the store's
-`update_plugin` reinstall decision, so the badge and the actual reinstall
-can never disagree. (Historically the store used raw string equality, which
-reinstalled over cosmetic differences like "v1.2.0" vs "1.2.0" and even
-DOWNGRADED locally-ahead plugins; this file's tests killed that.)
-
-Two other version parsers legitimately remain and are pinned here so they
-don't drift: `compatibility.parse_semver` (the install-compatibility gate,
-range-spec oriented) and `skin_runtime._major` (skin API major gate).
-"""
-
-import json
-from unittest.mock import patch
-
-import pytest
-from packaging.version import parse as pkg_parse
-
-from src.plugin_system.compatibility import is_update_available, parse_semver
-from src.skin_system.skin_runtime import _major
-from src.plugin_system.store_manager import PluginStoreManager
-from web_interface.blueprints.api_v3 import _is_plugin_update_available
-
-
-# (installed, registry) -> update available?
-CASES = [
-    (("1.2.0", "1.2.0"), False),   # identical
-    (("v1.2.0", "1.2.0"), False),  # cosmetic v-prefix, semantically equal
-    (("1.2", "1.2.0"), False),     # short form, semantically equal
-    (("1.2.0", "1.2.0-rc1"), False),  # rc of same release is not newer
-    (("1.2.0", "1.3.0"), True),    # registry genuinely newer
-    (("2.0.0", "1.9.0"), False),   # locally ahead — never downgrade
-    (("abc.def", "1.0.0"), True),  # unparseable — surface the mismatch
-    (("", "1.0.0"), False),        # missing either side — nothing to do
-    (("1.0.0", ""), False),
-]
-
-
-class TestSharedComparatorMalformedInputs:
-    def test_truthy_non_string_surfaces_mismatch(self):
-        # A malformed manifest can carry version as a number; packaging would
-        # raise TypeError on it. The comparator must not raise.
-        assert is_update_available(1.2, "1.2.0") is True
-        assert is_update_available("1.2.0", 1.3) is True
-
-    def test_falsy_non_string_means_nothing_to_do(self):
-        assert is_update_available(None, "1.0.0") is False
-        assert is_update_available("1.0.0", None) is False
-        assert is_update_available(0, "1.0.0") is False
-
-
-class TestSharedComparator:
-    @pytest.mark.parametrize("pair,expected", CASES)
-    def test_is_update_available(self, pair, expected):
-        installed, latest = pair
-        assert is_update_available(installed, latest) is expected
-
-    @pytest.mark.parametrize("pair,expected", CASES)
-    def test_api_v3_helper_agrees(self, pair, expected):
-        # The UI badge helper must be a pure alias of the shared comparator.
-        installed, latest = pair
-        assert _is_plugin_update_available(installed, latest) is expected
-
-
-class TestStoreManagerUsesSharedComparator:
-    """Drive update_plugin's real code path to its version check."""
-
-    def _store(self, tmp_path, local_version, registry_version):
-        plugin_dir = tmp_path / "plugins" / "demo-plugin"
-        plugin_dir.mkdir(parents=True)
-        (plugin_dir / "manifest.json").write_text(json.dumps({
-            "id": "demo-plugin", "version": local_version,
-        }))
-        store = PluginStoreManager(
-            plugins_dir=str(tmp_path / "plugins"),
-            uninstalled_registry_path=str(tmp_path / "uninstalled.json"),
-        )
-        registry_info = {
-            "id": "demo-plugin",
-            "repo": "https://github.com/example/ledmatrix-plugins",
-            "latest_version": registry_version,
-        }
-        return store, registry_info
-
-    def _run_update(self, store, registry_info):
-        with patch.object(store, "fetch_registry", return_value={"plugins": [registry_info]}), \
-             patch.object(store, "get_plugin_info", return_value=registry_info), \
-             patch.object(store, "_reinstall_with_rollback", return_value=True) as reinstall:
-            result = store.update_plugin("demo-plugin")
-        return result, reinstall
-
-    def test_equal_strings_skip_reinstall(self, tmp_path):
-        store, info = self._store(tmp_path, "1.2.0", "1.2.0")
-        result, reinstall = self._run_update(store, info)
-        assert result is True
-        reinstall.assert_not_called()
-
-    def test_v_prefix_equivalent_skips_reinstall(self, tmp_path):
-        # "v1.2.0" == "1.2.0" semantically — no pointless reinstall.
-        store, info = self._store(tmp_path, "v1.2.0", "1.2.0")
-        result, reinstall = self._run_update(store, info)
-        assert result is True
-        reinstall.assert_not_called()
-
-    def test_locally_ahead_version_is_never_downgraded(self, tmp_path):
-        # A plugin ahead of the registry (local dev build) must not be
-        # "updated" — that would be a downgrade.
-        store, info = self._store(tmp_path, "2.0.0", "1.9.0")
-        result, reinstall = self._run_update(store, info)
-        assert result is True
-        reinstall.assert_not_called()
-
-    def test_registry_newer_triggers_reinstall(self, tmp_path):
-        store, info = self._store(tmp_path, "1.2.0", "1.3.0")
-        result, reinstall = self._run_update(store, info)
-        reinstall.assert_called_once()
-        assert result is True
-
-    def test_unparseable_version_surfaces_via_reinstall(self, tmp_path):
-        # Direction unknowable → reconcile by reinstalling from the registry.
-        store, info = self._store(tmp_path, "abc.def", "1.0.0")
-        result, reinstall = self._run_update(store, info)
-        reinstall.assert_called_once()
-        assert result is True
-
-    def test_empty_local_version_follows_comparator_no_reinstall(self, tmp_path):
-        # The comparator says "nothing to do" for a missing version, and the
-        # store must agree with the UI badge — no reinstall.
-        store, info = self._store(tmp_path, "", "1.0.0")
-        result, reinstall = self._run_update(store, info)
-        assert result is True
-        reinstall.assert_not_called()
-
-    def test_empty_registry_version_follows_comparator_no_reinstall(self, tmp_path):
-        store, info = self._store(tmp_path, "1.0.0", "")
-        result, reinstall = self._run_update(store, info)
-        assert result is True
-        reinstall.assert_not_called()
-
-
-class TestSkinRuntimeMajor:
-    def test_plain_versions(self):
-        assert _major("1.0.0") == 1
-        assert _major("2.1") == 2
-
-    def test_int_input_tolerated(self):
-        assert _major(2) == 2
-
-    def test_garbage_returns_none(self):
-        assert _major("garbage") is None
-        assert _major(None) is None
-
-    def test_v_prefix_not_tolerated(self):
-        # Unlike parse_semver, _major does NOT strip a leading 'v' —
-        # a skin.json declaring "v1.0.0" fails the API gate. Characterized
-        # so a manifest-format loosening elsewhere doesn't silently diverge.
-        assert _major("v1.0.0") is None
-
-
-class TestParseSemverAgreesWithPackaging:
-    """parse_semver and packaging must agree on ordering for plain X.Y.Z —
-    the region where the two ecosystems overlap and must never diverge."""
-
-    PLAIN = ["0.1.0", "1.0.0", "1.2.0", "1.2.3", "1.10.0", "2.0.0", "10.0.1"]
-
-    def test_pairwise_ordering_matches(self):
-        for a in self.PLAIN:
-            for b in self.PLAIN:
-                ours = parse_semver(a) < parse_semver(b)
-                theirs = pkg_parse(a) < pkg_parse(b)
-                assert ours == theirs, f"ordering diverges on ({a}, {b})"

test/test_web_api.py

@@ -393,9 +393,6 @@ class TestSystemAPI:
     @patch('web_interface.blueprints.api_v3.subprocess')
     def test_get_system_status(self, mock_subprocess, client):
         """Test getting system status."""
-        # The endpoint returns 503 without psutil, which is an optional
-        # runtime dependency (requirements-test.txt installs it for CI).
-        pytest.importorskip("psutil")
         mock_result = MagicMock()
         mock_result.stdout = 'active\n'
         mock_result.returncode = 0

test/web_interface/test_api_v3_helpers.py

@@ -1,232 +0,0 @@
-"""
-Unit tests for the module-level helper functions in
-web_interface/blueprints/api_v3.py.
-
-These helpers back the plugin config save endpoint (the largest function in
-the repo) and the store's update-available detection, but were previously
-exercised only indirectly through full Flask route tests. Testing them
-directly pins behavior that the routes rely on — including a few
-characterized quirks marked below.
-"""
-
-import sys
-from pathlib import Path
-from typing import Any, ClassVar, Dict
-
-import pytest
-
-project_root = Path(__file__).parent.parent.parent
-sys.path.insert(0, str(project_root))
-
-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,
-)
-
-
-class TestIsPluginUpdateAvailable:
-    def test_equal_versions_no_update(self):
-        assert _is_plugin_update_available("1.2.0", "1.2.0") is False
-
-    def test_newer_registry_version_needs_update(self):
-        assert _is_plugin_update_available("1.2.0", "1.3.0") is True
-
-    def test_installed_ahead_of_registry_no_update(self):
-        # A locally modified plugin ahead of the registry must not be
-        # flagged — this is the whole point of semantic comparison here.
-        assert _is_plugin_update_available("2.0.0", "1.9.0") is False
-
-    def test_empty_versions_no_update(self):
-        assert _is_plugin_update_available("", "1.0.0") is False
-        assert _is_plugin_update_available("1.0.0", "") is False
-        assert _is_plugin_update_available("", "") is False
-
-    def test_v_prefix_parses_as_equal(self):
-        # packaging.version treats "v1.2.0" == "1.2.0" (PEP 440 tolerates the
-        # prefix), so no update is flagged. Contrast with store_manager's
-        # string-equality check — see test_version_comparison_consistency.py.
-        assert _is_plugin_update_available("v1.2.0", "1.2.0") is False
-
-    def test_two_part_version_parses_as_equal(self):
-        assert _is_plugin_update_available("1.2", "1.2.0") is False
-
-    def test_unparseable_version_surfaces_mismatch(self):
-        # Direction unknowable → surface the difference rather than hide a
-        # potential update.
-        assert _is_plugin_update_available("abc.def", "1.0.0") is True
-
-    def test_prerelease_below_release(self):
-        assert _is_plugin_update_available("1.2.0-rc1", "1.2.0") is True
-
-
-class TestCoerceToBool:
-    @pytest.mark.parametrize("value", ["true", "TRUE", "on", "1", "yes", "YES"])
-    def test_truthy_strings(self, value):
-        assert _coerce_to_bool(value) is True
-
-    @pytest.mark.parametrize("value", ["false", "off", "0", "no", "", "banana"])
-    def test_falsey_strings(self, value):
-        assert _coerce_to_bool(value) is False
-
-    def test_none_is_false(self):
-        assert _coerce_to_bool(None) is False
-
-    def test_bools_pass_through(self):
-        assert _coerce_to_bool(True) is True
-        assert _coerce_to_bool(False) is False
-
-    def test_int_only_one_is_true(self):
-        # Characterized quirk: ints coerce via `value == 1`, so 2 (truthy in
-        # Python) is False here.
-        assert _coerce_to_bool(1) is True
-        assert _coerce_to_bool(2) is False
-        assert _coerce_to_bool(0) is False
-
-    def test_other_types_false(self):
-        assert _coerce_to_bool([1]) is False
-        assert _coerce_to_bool({"a": 1}) is False
-
-
-class TestDeepMerge:
-    def test_nested_dicts_merge_recursively(self):
-        base = {"a": {"x": 1, "y": 2}, "b": 1}
-        update = {"a": {"y": 3, "z": 4}}
-        assert deep_merge(base, update) == {"a": {"x": 1, "y": 3, "z": 4}, "b": 1}
-
-    def test_scalar_over_dict_replaces(self):
-        assert deep_merge({"a": {"x": 1}}, {"a": 5}) == {"a": 5}
-
-    def test_dict_over_scalar_replaces(self):
-        assert deep_merge({"a": 5}, {"a": {"x": 1}}) == {"a": {"x": 1}}
-
-    def test_lists_replaced_wholesale(self):
-        assert deep_merge({"a": [1, 2]}, {"a": [3]}) == {"a": [3]}
-
-    def test_top_level_not_mutated_but_shallow_copy(self):
-        # Characterized: result = base.copy() protects base's top level, but
-        # nested dicts NOT touched by the update are shared by reference.
-        base = {"a": {"x": 1}, "keep": {"y": 2}}
-        result = deep_merge(base, {"a": {"x": 9}})
-        assert base == {"a": {"x": 1}, "keep": {"y": 2}}  # base unchanged
-        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": {
-            "brightness": {"type": "integer"},
-            "customization": {
-                "type": "object",
-                "properties": {
-                    "time_text": {
-                        "type": "object",
-                        "properties": {"font": {"type": "string"}},
-                    },
-                },
-            },
-            "fifa.world": {"type": "object",
-                           "properties": {"enabled": {"type": "boolean"}}},
-        }
-    }
-
-    def test_top_level_lookup(self):
-        assert _get_schema_property(self.SCHEMA, "brightness") == {"type": "integer"}
-
-    def test_nested_dot_path(self):
-        prop = _get_schema_property(self.SCHEMA, "customization.time_text.font")
-        assert prop == {"type": "string"}
-
-    def test_dotted_schema_key_matched_longest_first(self):
-        # League keys like "fifa.world" contain a literal dot and must match
-        # as a single key, not be split into nested fifa -> world lookups.
-        prop = _get_schema_property(self.SCHEMA, "fifa.world.enabled")
-        assert prop == {"type": "boolean"}
-
-    def test_missing_path_returns_none(self):
-        assert _get_schema_property(self.SCHEMA, "nope.nope") is None
-
-    def test_no_properties_returns_none(self):
-        assert _get_schema_property({}, "a") is None
-        assert _get_schema_property(None, "a") is None
-
-
-class TestSetNestedValue:
-    def test_sets_top_level(self):
-        config = {}
-        _set_nested_value(config, "brightness", 80)
-        assert config == {"brightness": 80}
-
-    def test_creates_intermediate_dicts(self):
-        config = {}
-        _set_nested_value(config, "customization.time_text.font", "5x7")
-        assert config == {"customization": {"time_text": {"font": "5x7"}}}
-
-    def test_merges_into_existing_nested_dict(self):
-        config = {"customization": {"color": "red"}}
-        _set_nested_value(config, "customization.font", "5x7")
-        assert config == {"customization": {"color": "red", "font": "5x7"}}
-
-    def test_scalar_intermediate_replaced_with_dict(self):
-        # Characterized: a non-dict intermediate is silently replaced.
-        config = {"customization": "oops"}
-        _set_nested_value(config, "customization.font", "5x7")
-        assert config == {"customization": {"font": "5x7"}}
-
-    def test_existing_dotted_key_preserved(self):
-        # An existing literal "fifa.world" key must be updated in place, not
-        # exploded into nested {"fifa": {"world": ...}}.
-        config = {"fifa.world": {"enabled": False}}
-        _set_nested_value(config, "fifa.world.enabled", True)
-        assert config == {"fifa.world": {"enabled": True}}
-
-    def test_none_does_not_overwrite_existing(self):
-        config = {"a": 1}
-        _set_nested_value(config, "a", None)
-        assert config == {"a": 1}
-
-    def test_none_sets_missing_key(self):
-        config = {}
-        _set_nested_value(config, "a", None)
-        assert config == {"a": None}

test/web_interface/test_api_v3_secret_roundtrip.py

@@ -1,259 +0,0 @@
-"""
-End-to-end secret round-trips through the three api_v3 endpoints that
-separate secrets from regular config (main-config save, plugin-config save,
-plugin-config reset) — now backed by the canonical
-src/web_interface/secret_helpers implementations.
-
-Unlike test_web_api.py (which mocks the config manager), these tests run a
-REAL ConfigManager and a REAL SchemaManager over tmp_path files, so they
-prove the whole chain: endpoint separation -> config_secrets.json write ->
-atomic config.json save (strip) -> load_config (merge back), including the
-array-item secret shape (accounts[].token) the inline copies never
-supported.
-"""
-
-import json
-import sys
-from pathlib import Path
-from unittest.mock import MagicMock
-
-import pytest
-from flask import Flask
-
-project_root = Path(__file__).parent.parent.parent
-sys.path.insert(0, str(project_root))
-
-from src.config_manager import ConfigManager  # noqa: E402
-from src.plugin_system.schema_manager import SchemaManager  # noqa: E402
-from web_interface.blueprints.api_v3 import api_v3  # noqa: E402
-
-
-PLUGIN_ID = "testplugin"
-
-SCHEMA = {
-    "$schema": "http://json-schema.org/draft-07/schema#",
-    "type": "object",
-    "properties": {
-        "enabled": {"type": "boolean", "default": True},
-        "display_duration": {"type": "number", "default": 15},
-        "api_key": {"type": "string", "x-secret": True, "default": ""},
-        "city": {"type": "string", "default": "Austin"},
-        "accounts": {
-            "type": "array",
-            "default": [],
-            "items": {
-                "type": "object",
-                "properties": {
-                    "name": {"type": "string"},
-                    "token": {"type": "string", "x-secret": True},
-                },
-            },
-        },
-    },
-}
-
-
-@pytest.fixture
-def env(tmp_path):
-    """Real ConfigManager + SchemaManager over tmp_path, wired onto the
-    api_v3 blueprint with the remaining managers mocked."""
-    config_file = tmp_path / "config.json"
-    config_file.write_text("{}")
-
-    plugins_dir = tmp_path / "plugins"
-    plugin_dir = plugins_dir / PLUGIN_ID
-    plugin_dir.mkdir(parents=True)
-    (plugin_dir / "config_schema.json").write_text(json.dumps(SCHEMA))
-    (plugin_dir / "manifest.json").write_text(json.dumps({
-        "id": PLUGIN_ID, "name": "Test Plugin", "version": "1.0.0",
-    }))
-
-    config_manager = ConfigManager(
-        config_path=str(config_file),
-        secrets_path=str(tmp_path / "config_secrets.json"))
-    config_manager.template_path = str(tmp_path / "no-template.json")
-
-    schema_manager = SchemaManager(plugins_dir=plugins_dir,
-                                   project_root=tmp_path)
-
-    plugin_manager = MagicMock()
-    plugin_manager.plugin_manifests = {PLUGIN_ID: {"id": PLUGIN_ID}}
-    plugin_manager.plugins_dir = plugins_dir
-    plugin_manager.get_plugin.return_value = None
-
-    api_v3.config_manager = config_manager
-    api_v3.schema_manager = schema_manager
-    api_v3.plugin_manager = plugin_manager
-    api_v3.plugin_store_manager = MagicMock()
-    api_v3.saved_repositories_manager = MagicMock()
-    api_v3.operation_queue = MagicMock()
-    api_v3.plugin_state_manager = MagicMock()
-    api_v3.operation_history = MagicMock()
-    api_v3.cache_manager = MagicMock()
-
-    app = Flask(__name__)
-    app.config["TESTING"] = True
-    app.register_blueprint(api_v3, url_prefix="/api/v3")
-
-    class Env:
-        pass
-
-    e = Env()
-    e.client = app.test_client()
-    e.config_manager = config_manager
-    e.config_file = config_file
-    e.secrets_file = tmp_path / "config_secrets.json"
-    e.tmp_path = tmp_path
-
-    def fresh_load():
-        """Load via a NEW ConfigManager, as the next request/process would.
-
-        The endpoint's manager serves its post-save in-memory config via the
-        mtime fast path, and that copy predates the secrets it just
-        separated out — a pre-existing quirk that applies to scalar secrets
-        too. On-disk truth is what these tests care about.
-        """
-        fresh = ConfigManager(config_path=str(config_file),
-                              secrets_path=str(e.secrets_file))
-        fresh.template_path = str(tmp_path / "no-template.json")
-        return fresh.load_config()
-
-    e.fresh_load = fresh_load
-    return e
-
-
-def _on_disk(path):
-    return json.loads(path.read_text())
-
-
-class TestSaveMainConfig:
-    """Site A: POST /config/main with a plugin-id key."""
-
-    def test_array_and_scalar_secrets_routed_to_secrets_file(self, env):
-        resp = env.client.post("/api/v3/config/main", json={
-            PLUGIN_ID: {
-                "city": "Dallas",
-                "api_key": "s3cret-key",
-                "accounts": [
-                    {"name": "a", "token": "s3cret-a"},
-                    {"name": "b"},
-                ],
-            },
-        })
-        assert resp.status_code == 200, resp.get_json()
-
-        on_disk = _on_disk(env.config_file)
-        assert on_disk[PLUGIN_ID]["city"] == "Dallas"
-        assert "api_key" not in on_disk[PLUGIN_ID]
-        assert on_disk[PLUGIN_ID]["accounts"] == [{"name": "a"}, {"name": "b"}]
-        assert "s3cret" not in env.config_file.read_text()
-
-        secrets = _on_disk(env.secrets_file)
-        assert secrets[PLUGIN_ID]["api_key"] == "s3cret-key"
-        assert secrets[PLUGIN_ID]["accounts"] == [{"token": "s3cret-a"}, {}]
-
-    def test_load_config_merges_secrets_back(self, env):
-        env.client.post("/api/v3/config/main", json={
-            PLUGIN_ID: {"accounts": [{"name": "a", "token": "s3cret-a"}]},
-        })
-        merged = env.fresh_load()
-        assert merged[PLUGIN_ID]["accounts"] == [
-            {"name": "a", "token": "s3cret-a"}]
-
-
-class TestSavePluginConfig:
-    """Site B: POST /plugins/config (JSON body)."""
-
-    def _save(self, env, config):
-        return env.client.post("/api/v3/plugins/config", json={
-            "plugin_id": PLUGIN_ID, "config": config,
-        })
-
-    def test_round_trip_with_array_secrets(self, env):
-        resp = self._save(env, {
-            "enabled": True,
-            "city": "Houston",
-            "api_key": "s3cret-key",
-            "accounts": [
-                {"name": "a", "token": "s3cret-a"},
-                {"name": "b", "token": "s3cret-b"},
-            ],
-        })
-        assert resp.status_code == 200, resp.get_json()
-
-        assert "s3cret" not in env.config_file.read_text()
-        on_disk = _on_disk(env.config_file)
-        assert on_disk[PLUGIN_ID]["accounts"] == [{"name": "a"}, {"name": "b"}]
-
-        secrets = _on_disk(env.secrets_file)
-        assert secrets[PLUGIN_ID]["accounts"] == [
-            {"token": "s3cret-a"}, {"token": "s3cret-b"}]
-
-        merged = env.fresh_load()
-        assert merged[PLUGIN_ID]["accounts"][1]["token"] == "s3cret-b"
-
-    def test_secret_count_message_counts_top_level_keys(self, env):
-        # Pinned: the "(N secret field(s))" message counts TOP-LEVEL keys of
-        # the separated secrets dict. Here that is 2: the posted accounts
-        # array (all its item tokens count as ONE key) plus the schema's
-        # api_key default ("") that merge_with_defaults adds before
-        # separation.
-        resp = self._save(env, {
-            "accounts": [{"name": "a", "token": "t"}],
-        })
-        message = resp.get_json()["message"]
-        assert "(2 secret field(s) saved to config_secrets.json)" in message
-
-    def test_resave_replaces_stored_secrets_list_wholesale(self, env):
-        # Characterized: api_v3's deep_merge intentionally replaces lists,
-        # so a re-save's parallel secrets list is authoritative.
-        self._save(env, {"accounts": [
-            {"name": "a", "token": "old-a"},
-            {"name": "b", "token": "old-b"},
-        ]})
-        self._save(env, {"accounts": [{"name": "only", "token": "new-only"}]})
-
-        secrets = _on_disk(env.secrets_file)
-        assert secrets[PLUGIN_ID]["accounts"] == [{"token": "new-only"}]
-        merged = env.fresh_load()
-        assert merged[PLUGIN_ID]["accounts"] == [
-            {"name": "only", "token": "new-only"}]
-
-
-class TestResetPluginConfig:
-    """Site C: POST /plugins/config/reset."""
-
-    def _seed(self, env):
-        env.client.post("/api/v3/plugins/config", json={
-            "plugin_id": PLUGIN_ID,
-            "config": {"city": "Houston", "api_key": "s3cret-key",
-                       "accounts": [{"name": "a", "token": "s3cret-a"}]},
-        })
-
-    def test_reset_preserving_secrets(self, env):
-        self._seed(env)
-        resp = env.client.post("/api/v3/plugins/config/reset", json={
-            "plugin_id": PLUGIN_ID, "preserve_secrets": True,
-        })
-        assert resp.status_code == 200, resp.get_json()
-
-        on_disk = _on_disk(env.config_file)
-        assert on_disk[PLUGIN_ID]["city"] == "Austin"  # schema default
-        assert on_disk[PLUGIN_ID]["accounts"] == []    # schema default
-
-        # Existing secrets survive (top-level-only preserve merge, pinned).
-        secrets = _on_disk(env.secrets_file)
-        assert secrets[PLUGIN_ID]["api_key"] == "s3cret-key"
-        assert secrets[PLUGIN_ID]["accounts"] == [{"token": "s3cret-a"}]
-
-    def test_reset_without_preserving_secrets(self, env):
-        self._seed(env)
-        resp = env.client.post("/api/v3/plugins/config/reset", json={
-            "plugin_id": PLUGIN_ID, "preserve_secrets": False,
-        })
-        assert resp.status_code == 200, resp.get_json()
-
-        secrets = _on_disk(env.secrets_file)
-        # Replaced with schema-default secrets — the schema declares no
-        # secret defaults, so the plugin's secrets are emptied.
-        assert secrets[PLUGIN_ID] in ({}, {"api_key": ""})

test/web_interface/test_secret_helpers.py

@@ -1,241 +0,0 @@
-"""
-Tests for src/web_interface/secret_helpers.py — the canonical secret
-identification / separation / masking helpers.
-
-This module is the extracted single source of truth for x-secret handling,
-but until now had zero test coverage (only ``mask_secret_fields`` is even
-imported by production code, from pages_v3). api_v3.py still carries three
-inline re-implementations of ``find_secret_fields``/``separate_secrets`` —
-see test_secret_separation_parity.py — so pinning the canonical behavior
-here is a precondition for ever migrating those copies.
-"""
-
-import copy
-
-from src.web_interface.secret_helpers import (
-    find_secret_fields,
-    separate_secrets,
-    mask_secret_fields,
-    mask_all_secret_values,
-    remove_empty_secrets,
-)
-
-
-SCHEMA_PROPS = {
-    "api_key": {"type": "string", "x-secret": True},
-    "city": {"type": "string"},
-    "auth": {
-        "type": "object",
-        "properties": {
-            "token": {"type": "string", "x-secret": True},
-            "username": {"type": "string"},
-        },
-    },
-    "accounts": {
-        "type": "array",
-        "items": {
-            "type": "object",
-            "properties": {
-                "name": {"type": "string"},
-                "token": {"type": "string", "x-secret": True},
-            },
-        },
-    },
-    "recovery_codes": {
-        "type": "array",
-        "items": {"type": "string", "x-secret": True},
-    },
-}
-
-
-class TestFindSecretFields:
-    def test_top_level_secret(self):
-        assert "api_key" in find_secret_fields(SCHEMA_PROPS)
-
-    def test_non_secret_not_included(self):
-        assert "city" not in find_secret_fields(SCHEMA_PROPS)
-
-    def test_nested_object_secret_uses_dot_path(self):
-        assert "auth.token" in find_secret_fields(SCHEMA_PROPS)
-        assert "auth.username" not in find_secret_fields(SCHEMA_PROPS)
-
-    def test_array_item_object_secret_uses_bracket_path(self):
-        assert "accounts[].token" in find_secret_fields(SCHEMA_PROPS)
-
-    def test_array_of_secrets_uses_bracket_path(self):
-        assert "recovery_codes[]" in find_secret_fields(SCHEMA_PROPS)
-
-    def test_full_set(self):
-        assert find_secret_fields(SCHEMA_PROPS) == {
-            "api_key", "auth.token", "accounts[].token", "recovery_codes[]",
-        }
-
-    def test_non_dict_properties_tolerated(self):
-        assert find_secret_fields({"weird": "not-a-dict"}) == set()
-
-    def test_non_dict_input_returns_empty(self):
-        assert find_secret_fields(None) == set()
-        assert find_secret_fields([]) == set()
-
-
-class TestSeparateSecrets:
-    def test_flat_partition(self):
-        regular, secrets = separate_secrets(
-            {"api_key": "s3cret", "city": "Austin"}, {"api_key"})
-        assert regular == {"city": "Austin"}
-        assert secrets == {"api_key": "s3cret"}
-
-    def test_nested_partition(self):
-        config = {"auth": {"token": "t0k", "username": "chuck"}}
-        regular, secrets = separate_secrets(config, {"auth.token"})
-        assert regular == {"auth": {"username": "chuck"}}
-        assert secrets == {"auth": {"token": "t0k"}}
-
-    def test_empty_nested_dicts_pruned_from_regular(self):
-        # A dict that is all secrets leaves nothing behind on the regular
-        # side — the key must be dropped, not kept as {}.
-        config = {"auth": {"token": "t0k"}}
-        regular, secrets = separate_secrets(config, {"auth.token"})
-        assert regular == {}
-        assert secrets == {"auth": {"token": "t0k"}}
-
-    def test_whole_array_secret(self):
-        config = {"recovery_codes": ["a", "b"], "city": "Austin"}
-        regular, secrets = separate_secrets(config, {"recovery_codes[]"})
-        assert regular == {"city": "Austin"}
-        assert secrets == {"recovery_codes": ["a", "b"]}
-
-    def test_array_item_secrets_produce_parallel_lists(self):
-        # Per-item secrets keep the arrays index-aligned so they can be
-        # recombined: regular gets the stripped items, secrets a parallel
-        # list of the extracted values.
-        config = {"accounts": [
-            {"name": "a", "token": "ta"},
-            {"name": "b", "token": "tb"},
-        ]}
-        regular, secrets = separate_secrets(config, {"accounts[].token"})
-        assert regular == {"accounts": [{"name": "a"}, {"name": "b"}]}
-        assert secrets == {"accounts": [{"token": "ta"}, {"token": "tb"}]}
-
-    def test_array_item_non_dict_items_get_placeholder(self):
-        config = {"accounts": [{"name": "a", "token": "ta"}, "oddball"]}
-        regular, secrets = separate_secrets(config, {"accounts[].token"})
-        assert regular == {"accounts": [{"name": "a"}, "oddball"]}
-        assert secrets == {"accounts": [{"token": "ta"}, {}]}
-
-    def test_array_without_secret_paths_stays_regular(self):
-        config = {"teams": ["DAL", "HOU"]}
-        regular, secrets = separate_secrets(config, {"api_key"})
-        assert regular == {"teams": ["DAL", "HOU"]}
-        assert secrets == {}
-
-    def test_round_trip_loses_nothing(self):
-        # separate + naive recombine must reconstruct the original config.
-        config = {
-            "api_key": "k",
-            "city": "Austin",
-            "auth": {"token": "t", "username": "chuck"},
-            "recovery_codes": ["a", "b"],
-        }
-        paths = find_secret_fields(SCHEMA_PROPS)
-        regular, secrets = separate_secrets(copy.deepcopy(config), paths)
-
-        def recombine(reg, sec):
-            out = copy.deepcopy(reg)
-            for k, v in sec.items():
-                if isinstance(v, dict) and isinstance(out.get(k), dict):
-                    out[k] = recombine(out[k], v)
-                else:
-                    out[k] = v
-            return out
-
-        assert recombine(regular, secrets) == config
-
-
-class TestMaskSecretFields:
-    def test_masks_present_secret_to_empty_string(self):
-        result = mask_secret_fields({"api_key": "s3cret"}, SCHEMA_PROPS)
-        assert result["api_key"] == ""
-
-    def test_leaves_non_secret_untouched(self):
-        result = mask_secret_fields({"city": "Austin"}, SCHEMA_PROPS)
-        assert result["city"] == "Austin"
-
-    def test_none_and_empty_left_alone(self):
-        result = mask_secret_fields({"api_key": None}, SCHEMA_PROPS)
-        assert result["api_key"] is None
-        result = mask_secret_fields({"api_key": ""}, SCHEMA_PROPS)
-        assert result["api_key"] == ""
-
-    def test_falsey_but_set_values_are_masked(self):
-        # 0 and False are real values; the check is `is not None and != ''`.
-        # Note False == '' is False in Python, so False IS masked; 0 == '' is
-        # also False, so 0 is masked too.
-        result = mask_secret_fields({"api_key": 0}, SCHEMA_PROPS)
-        assert result["api_key"] == ""
-        result = mask_secret_fields({"api_key": False}, SCHEMA_PROPS)
-        assert result["api_key"] == ""
-
-    def test_nested_object_masked_without_mutating_input(self):
-        config = {"auth": {"token": "t0k", "username": "chuck"}}
-        original = copy.deepcopy(config)
-        result = mask_secret_fields(config, SCHEMA_PROPS)
-        assert result["auth"]["token"] == ""
-        assert result["auth"]["username"] == "chuck"
-        assert config == original  # input not mutated
-
-    def test_array_of_secrets_masked_elementwise(self):
-        result = mask_secret_fields(
-            {"recovery_codes": ["a", "b"]}, SCHEMA_PROPS)
-        assert result["recovery_codes"] == ["", ""]
-
-    def test_array_of_objects_masked_per_item(self):
-        config = {"accounts": [{"name": "a", "token": "ta"}, "oddball"]}
-        result = mask_secret_fields(config, SCHEMA_PROPS)
-        assert result["accounts"][0] == {"name": "a", "token": ""}
-        assert result["accounts"][1] == "oddball"
-
-    def test_non_dict_schema_property_tolerated(self):
-        assert mask_secret_fields({"x": 1}, {"x": "bogus"}) == {"x": 1}
-
-
-class TestMaskAllSecretValues:
-    def test_real_values_replaced_with_bullets(self):
-        assert mask_all_secret_values({"key": "abc"}) == {"key": "••••••••"}
-
-    def test_placeholders_preserved(self):
-        # YOUR_* placeholders must survive so the UI can show "not set".
-        result = mask_all_secret_values({"key": "YOUR_API_KEY_HERE"})
-        assert result == {"key": "YOUR_API_KEY_HERE"}
-
-    def test_empty_and_none_preserved(self):
-        assert mask_all_secret_values({"a": "", "b": None}) == {"a": "", "b": None}
-
-    def test_recurses_into_nested_dicts(self):
-        result = mask_all_secret_values({"plugin": {"token": "t", "empty": ""}})
-        assert result == {"plugin": {"token": "••••••••", "empty": ""}}
-
-    def test_non_string_real_values_masked(self):
-        assert mask_all_secret_values({"port": 8080}) == {"port": "••••••••"}
-
-
-class TestRemoveEmptySecrets:
-    def test_strips_empty_string(self):
-        assert remove_empty_secrets({"a": "", "b": "real"}) == {"b": "real"}
-
-    def test_strips_whitespace_only(self):
-        assert remove_empty_secrets({"a": "   "}) == {}
-
-    def test_strips_none(self):
-        assert remove_empty_secrets({"a": None}) == {}
-
-    def test_prunes_empty_nested_dicts(self):
-        assert remove_empty_secrets({"plugin": {"token": ""}}) == {}
-
-    def test_keeps_nested_real_values(self):
-        result = remove_empty_secrets({"plugin": {"token": "t", "empty": ""}})
-        assert result == {"plugin": {"token": "t"}}
-
-    def test_keeps_falsey_non_string_values(self):
-        # 0 and False are neither None nor blank strings — they are kept.
-        assert remove_empty_secrets({"a": 0, "b": False}) == {"a": 0, "b": False}

test/web_interface/test_secret_separation_parity.py

@@ -1,80 +0,0 @@
-"""
-Drift guard: api_v3 must use the canonical secret helpers.
-
-Historically web_interface/blueprints/api_v3.py carried THREE inline
-nested-function copies of ``find_secret_fields``/``separate_secrets`` (in the
-main-config save, plugin-config save, and plugin-config reset endpoints).
-They lacked the canonical module's array-item secret support and drifted from
-each other. They have been migrated onto
-``src/web_interface/secret_helpers`` — this file now guards against copies
-REAPPEARING, and keeps the canonical array-item behavior executable.
-"""
-
-import re
-from pathlib import Path
-
-from src.web_interface.secret_helpers import find_secret_fields, separate_secrets
-
-API_V3_PATH = (Path(__file__).resolve().parents[2]
-               / "web_interface" / "blueprints" / "api_v3.py")
-
-# The migration is complete: any inline reimplementation is a regression.
-EXPECTED_INLINE_COPIES = 0
-
-
-class TestNoInlineCopies:
-    def _count(self, name: str) -> int:
-        source = API_V3_PATH.read_text(encoding="utf-8")
-        return len(re.findall(rf"^\s*def {name}\(", source, flags=re.MULTILINE))
-
-    def test_no_inline_find_secret_fields(self):
-        count = self._count("find_secret_fields")
-        assert count == EXPECTED_INLINE_COPIES, (
-            f"api_v3.py has {count} inline find_secret_fields definitions, "
-            f"expected {EXPECTED_INLINE_COPIES}. Import it from "
-            f"src/web_interface/secret_helpers instead of re-implementing it."
-        )
-
-    def test_no_inline_separate_secrets(self):
-        count = self._count("separate_secrets")
-        assert count == EXPECTED_INLINE_COPIES, (
-            f"api_v3.py has {count} inline separate_secrets definitions, "
-            f"expected {EXPECTED_INLINE_COPIES}. Import it from "
-            f"src/web_interface/secret_helpers instead of re-implementing it."
-        )
-
-    def test_canonical_import_present(self):
-        # Tripwire: the endpoints still need the helpers, so removing the
-        # import means either dead secret handling or a new local copy.
-        source = API_V3_PATH.read_text(encoding="utf-8")
-        assert re.search(
-            r"from src\.web_interface\.secret_helpers import .*find_secret_fields",
-            source,
-        ), "api_v3.py no longer imports the canonical secret helpers"
-
-
-class TestCanonicalArrayItemBehavior:
-    """Executable documentation of the array-item secret contract the
-    endpoints now inherit from the canonical module."""
-
-    SCHEMA = {
-        "accounts": {
-            "type": "array",
-            "items": {
-                "type": "object",
-                "properties": {
-                    "name": {"type": "string"},
-                    "token": {"type": "string", "x-secret": True},
-                },
-            },
-        },
-    }
-
-    def test_canonical_module_routes_array_item_secrets(self):
-        paths = find_secret_fields(self.SCHEMA)
-        assert "accounts[].token" in paths
-
-        config = {"accounts": [{"name": "a", "token": "s3cret"}]}
-        regular, secrets = separate_secrets(config, paths)
-        assert regular == {"accounts": [{"name": "a"}]}
-        assert secrets == {"accounts": [{"token": "s3cret"}]}

test/web_interface/test_state_reconciliation.py

@@ -367,10 +367,6 @@ class TestStateReconciliationUnrecoverable(unittest.TestCase):
         self.store_manager.fetch_registry.return_value = {"plugins": []}
         self.store_manager.install_plugin.return_value = False
         self.store_manager.was_recently_uninstalled.return_value = False
-        # A bare Mock() returns a truthy Mock for is_plugin_uninstalled(),
-        # which reads as "persistently uninstalled" and skips auto-repair
-        # entirely — these tests need the repair path to run.
-        self.store_manager.is_plugin_uninstalled.return_value = False
 
         self.reconciler = StateReconciliation(
             state_manager=self.state_manager,

web_interface/blueprints/api_v3.py

@@ -21,7 +21,6 @@ logger = logging.getLogger(__name__)
 # Import new infrastructure
 from src.web_interface.api_helpers import success_response, error_response, validate_request_json
 from src.web_interface.errors import ErrorCode
-from src.web_interface.secret_helpers import find_secret_fields, separate_secrets
 from src.plugin_system.operation_types import OperationType
 from src.web_interface.validators import (
     validate_file_upload
@@ -125,13 +124,28 @@ def _is_plugin_update_available(installed_version: str, latest_version: str) ->
     """Return True when the registry's ``latest_version`` is strictly newer
     than the installed version.
 
-    Thin alias for the shared comparator in
+    Uses PEP 440 / semver-aware comparison so a locally modified plugin whose
-    `src.plugin_system.compatibility.is_update_available` — the store's
+    version is *ahead* of the published registry is not flagged as needing an
-    `update_plugin` uses the same function, so the UI badge and the actual
+    update. If either version string can't be parsed, falls back to a plain
-    reinstall decision can never disagree.
+    inequality check (any difference is surfaced so the user can reconcile).
     """
-    from src.plugin_system.compatibility import is_update_available
+    if not installed_version or not latest_version:
-    return is_update_available(installed_version, latest_version)
+        return False
+    if installed_version == latest_version:
+        return False
+    try:
+        from packaging.version import parse as _parse_version, InvalidVersion
+    except ImportError:
+        # packaging is a core dependency, but if it's somehow unavailable we
+        # can't compare semantically — surface the mismatch we already know
+        # exists (the two strings differ).
+        return True
+    try:
+        return _parse_version(latest_version) > _parse_version(installed_version)
+    except InvalidVersion:
+        # Unparseable version string: we can't tell direction, so surface the
+        # mismatch rather than silently hiding a potential update.
+        return True
 
 def _ensure_cache_manager():
     """Ensure cache manager is initialized."""
@@ -1197,6 +1211,18 @@ def save_main_config():
                         plugins_dir = PROJECT_ROOT / plugins_dir_name
                 schema_path = plugins_dir / plugin_id / 'config_schema.json'
 
+                def find_secret_fields(properties, prefix=''):
+                    """Recursively find fields marked with x-secret: true"""
+                    fields = set()
+                    for field_name, field_props in properties.items():
+                        full_path = f"{prefix}.{field_name}" if prefix else field_name
+                        if field_props.get('x-secret', False):
+                            fields.add(full_path)
+                        # Check nested objects
+                        if field_props.get('type') == 'object' and 'properties' in field_props:
+                            fields.update(find_secret_fields(field_props['properties'], full_path))
+                    return fields
+
                 if schema_path.exists():
                     try:
                         with open(schema_path, 'r', encoding='utf-8') as f:
@@ -1207,6 +1233,24 @@ def save_main_config():
                         logger.debug("Error reading schema for secret detection: %s", e)
 
                 # Separate secrets from regular config (same logic as save_plugin_config)
+                def separate_secrets(config, secrets_set, prefix=''):
+                    """Recursively separate secret fields from regular config"""
+                    regular = {}
+                    secrets = {}
+                    for key, value in config.items():
+                        full_path = f"{prefix}.{key}" if prefix else key
+                        if isinstance(value, dict):
+                            nested_regular, nested_secrets = separate_secrets(value, secrets_set, full_path)
+                            if nested_regular:
+                                regular[key] = nested_regular
+                            if nested_secrets:
+                                secrets[key] = nested_secrets
+                        elif full_path in secrets_set:
+                            secrets[key] = value
+                        else:
+                            regular[key] = value
+                    return regular, secrets
+
                 regular_config, secrets_config = separate_secrets(plugin_config, secret_fields)
 
                 # PRE-PROCESSING: Preserve 'enabled' state if not in regular_config
@@ -4010,11 +4054,6 @@ def deep_merge(base_dict, update_dict):
     """
     Deep merge update_dict into base_dict.
     For nested dicts, recursively merge. For other types, update_dict takes precedence.
-
-    Lists are intentionally REPLACED wholesale, never index-merged: form posts
-    carry complete arrays, and index-merging would resurrect items the user
-    deleted. This also applies to the parallel secrets lists produced by
-    separate_secrets — a newly saved secrets list is authoritative.
     """
     result = base_dict.copy()
     for key, value in update_dict.items():
@@ -5029,9 +5068,23 @@ def save_plugin_config():
                 # Default to True on error to avoid disabling plugins
                 plugin_config['enabled'] = True
 
-        # Find secret fields (supports nested schemas and array-item secrets)
+        # Find secret fields (supports nested schemas)
         secret_fields = set()
 
+        def find_secret_fields(properties, prefix=''):
+            """Recursively find fields marked with x-secret: true"""
+            fields = set()
+            if not isinstance(properties, dict):
+                return fields
+            for field_name, field_props in properties.items():
+                full_path = f"{prefix}.{field_name}" if prefix else field_name
+                if isinstance(field_props, dict) and field_props.get('x-secret', False):
+                    fields.add(full_path)
+                # Check nested objects
+                if isinstance(field_props, dict) and field_props.get('type') == 'object' and 'properties' in field_props:
+                    fields.update(find_secret_fields(field_props['properties'], full_path))
+            return fields
+
         if schema and 'properties' in schema:
             secret_fields = find_secret_fields(schema['properties'])
 
@@ -5346,8 +5399,29 @@ def save_plugin_config():
                     status_code=400
                 )
 
-        # Separate secrets from regular config (handles nested configs and
+        # Separate secrets from regular config (handles nested configs)
-        # array-item secrets — see src/web_interface/secret_helpers.py)
+        def separate_secrets(config, secrets_set, prefix=''):
+            """Recursively separate secret fields from regular config"""
+            regular = {}
+            secrets = {}
+
+            for key, value in config.items():
+                full_path = f"{prefix}.{key}" if prefix else key
+
+                if isinstance(value, dict):
+                    # Recursively handle nested dicts
+                    nested_regular, nested_secrets = separate_secrets(value, secrets_set, full_path)
+                    if nested_regular:
+                        regular[key] = nested_regular
+                    if nested_secrets:
+                        secrets[key] = nested_secrets
+                elif full_path in secrets_set:
+                    secrets[key] = value
+                else:
+                    regular[key] = value
+
+            return regular, secrets
+
         regular_config, secrets_config = separate_secrets(plugin_config, secret_fields)
 
         # Get current configs
@@ -5595,10 +5669,41 @@ def reset_plugin_config():
         schema = schema_mgr.load_schema(plugin_id, use_cache=True)
         secret_fields = set()
 
+        def find_secret_fields(properties, prefix=''):
+            """Recursively find fields marked with x-secret: true"""
+            fields = set()
+            if not isinstance(properties, dict):
+                return fields
+            for field_name, field_props in properties.items():
+                full_path = f"{prefix}.{field_name}" if prefix else field_name
+                if isinstance(field_props, dict) and field_props.get('x-secret', False):
+                    fields.add(full_path)
+                if isinstance(field_props, dict) and field_props.get('type') == 'object' and 'properties' in field_props:
+                    fields.update(find_secret_fields(field_props['properties'], full_path))
+            return fields
+
         if schema and 'properties' in schema:
             secret_fields = find_secret_fields(schema['properties'])
 
         # Separate defaults into regular and secret configs
+        def separate_secrets(config, secrets_set, prefix=''):
+            """Recursively separate secret fields from regular config"""
+            regular = {}
+            secrets = {}
+            for key, value in config.items():
+                full_path = f"{prefix}.{key}" if prefix else key
+                if isinstance(value, dict):
+                    nested_regular, nested_secrets = separate_secrets(value, secrets_set, full_path)
+                    if nested_regular:
+                        regular[key] = nested_regular
+                    if nested_secrets:
+                        secrets[key] = nested_secrets
+                elif full_path in secrets_set:
+                    secrets[key] = value
+                else:
+                    regular[key] = value
+            return regular, secrets
+
         default_regular, default_secrets = separate_secrets(defaults, secret_fields)
 
         # Update main config with defaults

web_interface/static/v3/js/widgets/array-table.js

@@ -160,24 +160,6 @@
 
     // ─── Cell rendering ─────────────────────────────────────────────────────
 
-    /**
-     * Visible text for one enum option.
-     *
-     * Mirrors the server-rendered table in plugin_config.html: a schema may
-     * supply x-options.labels, and anything unlabelled falls back to the raw
-     * value. Rows added here must match rows rendered by the template, or the
-     * same column would read differently before and after a page reload.
-     *
-     * @param {Object} colDef  column (or property) schema
-     * @param {*}      opt     the enum value
-     * @returns {string} label to display
-     */
-    function enumOptionLabel(colDef, opt) {
-        const options = (colDef && (colDef['x-options'] || colDef['x_options'])) || {};
-        const labels = options.labels || {};
-        return Object.prototype.hasOwnProperty.call(labels, opt) ? labels[opt] : opt;
-    }
-
     /**
      * Create one <td> for a display column.
      */
@@ -237,7 +219,7 @@
                 if (opt === null) return;
                 const o = document.createElement('option');
                 o.value    = opt;
-                o.textContent = enumOptionLabel(colDef, opt);
+                o.textContent = opt;
                 if (String(colValue) === String(opt)) o.selected = true;
                 sel.appendChild(o);
             });
@@ -664,7 +646,7 @@
             enumVals.forEach(opt => {
                 if (opt === null) return;
                 const o = document.createElement('option');
-                o.value = opt; o.textContent = enumOptionLabel(schema, opt);
+                o.value = opt; o.textContent = opt;
                 if (String(currentVal) === String(opt)) o.selected = true;
                 sel.appendChild(o);
             });

web_interface/templates/v3/partials/plugin_config.html

@@ -121,21 +121,14 @@
             </label>
             {% endif %}
             
-            {# Enum dropdown. Option text comes from x-options.labels when the
+            {# Enum dropdown #}
-               schema supplies it -- the same convention the checkbox-group
-               widget already uses -- because humanising the raw value cannot
-               express every label: "vs" reads as "Vs", and "abbrev" says
-               nothing about the "Sep 19" it produces. Values without a label
-               fall back to the humanised form, so existing schemas render
-               exactly as before. #}
             {% elif prop.enum %}
-            {% set enum_labels = (prop.get('x-options') or prop.get('x_options') or {}).get('labels') or {} %}
             <select id="{{ field_id }}"
                     name="{{ full_key }}" 
                     class="form-select w-full rounded-md border-gray-300 shadow-sm focus:border-blue-500 focus:ring-blue-500 bg-white text-black">
                 {% for option in prop.enum %}
                 <option value="{{ option }}" {% if value == option %}selected{% endif %}>
-                    {{ enum_labels.get(option, option|replace('_', ' ')|title) }}
+                    {{ option|replace('_', ' ')|title }}
                 </option>
                 {% endfor %}
             </select>
@@ -576,14 +569,10 @@
                                                        class="block w-20 px-2 py-1 border border-gray-300 rounded text-sm text-center"
                                                        {% if col_def.get('description') %}title="{{ col_def.get('description') }}"{% endif %}>
                                             {% elif col_enum %}
-                                                {# Labels are opt-in here and the fallback stays the raw
-                                                   value: table columns hold things like ticker symbols,
-                                                   which must not be title-cased behind the user's back. #}
-                                                {% set col_labels = (col_def.get('x-options') or col_def.get('x_options') or {}).get('labels') or {} %}
                                                 <select name="{{ full_key }}.{{ item_index }}.{{ col_name }}"
                                                         class="block w-full px-2 py-1 border border-gray-300 rounded text-sm bg-white">
                                                     {% for opt in col_enum %}{% if opt is not none %}
-                                                    <option value="{{ opt }}" {% if col_value == opt or (col_value is none and col_def.get('default') == opt) %}selected{% endif %}>{{ col_labels.get(opt, opt) }}</option>
+                                                    <option value="{{ opt }}" {% if col_value == opt or (col_value is none and col_def.get('default') == opt) %}selected{% endif %}>{{ opt }}</option>
                                                     {% endif %}{% endfor %}
                                                 </select>
                                             {% elif col_xwidget == 'date-picker' %}