fix: ten defects found validating the whole plugin fleet on hardware (#534)

* fix(core): register tom_thumb, accept frame_hold in the test double, wire api_v3's managers

Three independent fixes found while validating every plugin on a 256x64 rig.

FontManager never registered tom_thumb even though assets/fonts/tom-thumb.bdf
ships with the core, so every plugin offering it logged "Font family
'tom_thumb' not found" (16 warnings per countdown render) and had to carry a
private loader to use a bundled font. Closes #524.

VisualTestDisplayManager.set_scrolling_state() lacked the frame_hold parameter
that DisplayManager gained, so any plugin passing it died with TypeError at
render time and failed every size. Nine plugins now make that call;
ledmatrix-stocks and ledmatrix-leaderboard were failing outright and the other
seven only passed because their scroll path was unreachable without data.
Closes #525.

api_v3 declared module-level config_manager/plugin_manager = None that nothing
ever assigned -- app.py sets the blueprint attributes, which the other 150+
call sites use. Three sites read the decoys, so /health reported the config
unreadable and the plugin system uninitialised (making "degraded" permanent and
unreachable-by-design) and /display/current fell back to a hardcoded 128x64 on
every rig. The decoys are removed rather than assigned, so a bare name is now a
NameError at test time instead of a silent None. The same function's first-call
uptime was computed from two separate clock reads and came out negative.
Closes #529.

Verified on the rig: both previously-failing plugins render, the tom_thumb
warnings are gone, /health reports "healthy" with all three checks passing, and
/display/current reports the real 256x64.

* fix(core): unique snapshot temp name, honour on-demand requests, skip empty starlark

The preview snapshot wrote through a fixed "<snapshot>.tmp". /tmp is
world-writable and sticky, and the display service runs as a different user
from the tooling, so a leftover temp owned by anyone else became unopenable
even by root -- fs.protected_regular refuses O_CREAT on a foreign file in a
sticky directory. The preview and the health check's liveness proxy then froze
until someone deleted the file by hand; on the test rig that meant 23 hours of
a healthy display reporting "hardware: stale". Now uses tempfile.mkstemp with
cleanup on failure, matching the hardware-status write a few hundred lines
above. Closes #528.

_poll_on_demand_requests read its mailbox with max_age=3600, and get() defaults
the in-memory TTL to max_age -- so the first request was pinned in memory for an
hour and every later poll returned that stale copy. No second on-demand request
was honoured until the service restarted, while the API kept returning 200.
get() already documents memory_ttl=0 for exactly this cross-process case.
The consumed request is also now deleted: leaving it on disk meant a restart
replayed the previous request, activated it, and ignored the one the caller had
just made. Closes #530.

starlark-apps returned None from display() when it has no app to show, which is
the state of every install without Pixlet and of a fresh one before any app is
added. The controller only skips on a boolean False, so that held a black panel
for the full display_duration instead of rotating on. Closes #456 (core side).

Verified on the rig: two consecutive on-demand requests with no restart between
them are both activated, where the second was previously dropped in silence.

* perf(harness): share one cache across a plugin's renders

_instantiate built a fresh MockCacheManager for every (size, mode), and that
mock is a per-instance in-memory dict, so each render was a cold start. A plugin
that fetches per game or per player re-fetched everything N times over --
baseball-scoreboard at one size took 840s for nine renders where the arithmetic
said ~72s, and at eight sizes it exceeded a 900s timeout.

The second and later renders also never exercised the cache-hit path, which is
what a running rig executes almost all of the time, so a caching regression
could not be caught here.

The cache is now built once per render_plugin_matrix call and threaded down.
The display manager stays per-render -- the bounds checking depends on that --
so only fetched data is shared.

Measured on the rig, same render counts and same goldens:
  tide-display         2s -> 1s   (32 renders)
  cricket-scoreboard  10s -> 3s   (24 renders)
No pass/fail change across tide-display, cricket-scoreboard, clock-simple,
geochron, christmas-countdown, of-the-day, web-ui-info and incoming-packages.

Closes #533.

* fix(scripts): run standalone plugin tests instead of collecting nothing

run_plugin_tests.py discovered every plugin test file and handed the lot to
pytest. Most plugin tests are standalone scripts -- module-level main() plus an
`if __name__ == "__main__"` guard, signalling through an exit code -- and pytest
collects zero items from those. The run printed how many files it had *found*,
then "no tests ran", and exited without executing any of them. On a rig with all
44 first-party plugins that is 151 of 248 files.

Files are now classified and each kind runs under the right runner: pytest for
real test modules, subprocess for scripts, honouring the 0 pass / 2 skip / 1
fail convention ledmatrix-plugins' own runner established (a script that wants a
tty or an LED matrix is a skip, not a regression).

Before:
    $ python3 scripts/run_plugin_tests.py -p countdown -d ~/LEDMatrix/plugin-repos
    Found 1 test file(s)
    collected 0 items
    no tests ran in 0.31s                      rc=0

After:
    Found 1 test file(s) -- 0 collectable, 1 standalone script(s)
    1 passed, 0 skipped, 0 failed (scripts)    rc=0

Verified across three shapes: countdown (1 script), jellyfin-now-playing and
pomodoro-timer (pytest only, 16 and 42 tests), and ledmatrix-flights (11 files
split 4 collectable / 7 scripts, all seven of which had never run).

Closes #532.

Running the flights scripts for the first time also surfaced four genuinely
failing tests there, hidden by the mirror-image bug in the plugins repo's own
runner -- filed as ChuckBuilds/ledmatrix-plugins#464 and #465.

* fix(harness): give an empty-looking mode a few frames before warning about it

check_plugin's "drew nothing but display() returned X" warning fired on a single
frame, rendered with force_clear=True, under a frozen clock. All three defeat a
scrolling plugin, whose first frame is legitimately its blank scroll-in buffer.
Across 44 first-party plugins, 60 of 76 warnings were false -- the rate at which
people stop reading a warning, which matters because the true positives are
real: a mode that draws nothing and does not return False holds a blank panel
for its whole display duration.

An apparently-empty frame is now re-driven for up to 48 more frames with
force_clear=False (force_clear means "reset the scroll", so repeating it would
redraw frame 1 for ever) and with the clock advancing -- freezegun's factory
where time is frozen, a real sleep where it is not, since scroll position is
usually a function of elapsed time. The first frame that draws content replaces
the result.

The clock is moved back afterwards. It is shared by every render in the matrix,
so time borrowed by the probe leaked into later modes and drifted their goldens
-- f1_upcoming picked up 5 spurious drifts before this was restored.

Measured on the rig:

                        empty warns          check
                        before  after
  f1-scoreboard            42      0    48 PASS / 0 FAIL, goldens intact
  ledmatrix-elections      16      0    16 PASS / 0 FAIL
  on-air                    8      8    true positive, kept
  nfl-draft                 8      8    true positive, kept
  clock-simple/geochron/    0      0    unchanged
  christmas-countdown

58 false positives gone, both true positives kept, no golden regressions. Cost
is confined to modes that really are blank: plugins that draw immediately are
unchanged (clock-simple and tide-display still 2s), while on-air -- eight
deliberately blank modes -- goes to 21s.

Closes #527.

* fix(harness): load nested schema defaults, and merge caller config at leaf level

load_config_defaults read only top-level properties. An object property carries
its defaults on its children, not on itself, so everything nested was dropped --
2,386 defaults across 37 of 44 plugins, soccer-scoreboard alone losing 539 of
565. render_plugin_matrix's comment says the plugin then "behaves like a real
install", which for most of the fleet it did not.

_defaults_from_properties now recurses. merge_config deep-merges the caller's
config onto the result so an override lands at the leaf: a shallow merge would
let -c '{"nhl": {"enabled": true}}' replace the whole nhl subtree and discard
every other nhl default, which is the same class of bug being fixed here.

Measured before/after across all 49 installed plugins on the rig: **no render
changed** -- identical PASS/FAIL counts, byte-identical output, goldens intact.
Plugins already fall back to the same values internally via config.get(key,
default), so supplying them explicitly agrees with what they were doing. The
defaults really are arriving now:

  ufc-scoreboard        9 -> 87 defaults
  ledmatrix-flights    51 -> 95
  masters-tournament   10 -> 51
  cricket-scoreboard   22 -> 50
  tide-display         12 -> 18

and hockey-scoreboard, which used to load nhl.enabled=None, now gets
nhl.enabled=True with its full display_modes block.

Caveat worth carrying: the eight plugins with the most nested config
(soccer, baseball, basketball, hockey, lacrosse, football, afl, nrl -- 1,634 of
the 2,386 dropped defaults, 68%) could not be measured. They import
src.common.sports_shared, which the test rig's core branch predates, so they
fail to load there identically before and after. Re-run this comparison against
a core that has that module before trusting the "nothing changed" result for
them; those are exactly the plugins whose renders should change most.

Closes #531.

* refactor: narrow the exception handlers this branch introduced

Codacy flagged the new code; it passes on other recent PRs, so the finding is
mine. Four of the five broad `except Exception` clauses I added were catching
far more than they needed to, which is the same shape as several bugs this
branch fixes -- hello-world's TypeError sat invisible for exactly this reason.

  freezer() / move_to() / tick()   -> (AttributeError, TypeError, ValueError)
  cache_manager.delete()           -> (OSError, AttributeError, KeyError)

The fifth stays broad and now says why: it wraps a call into a plugin's own
display(), which can raise anything, and the first frame has already rendered --
so a failure there must not turn a good result into an error.

Verified against a checkout of main: f1-scoreboard 48 PASS / 0 FAIL with 0 empty
warnings, on-air keeps its 8 true positives, clock-simple 8 PASS. geochron shows
7 golden drifts both before and after this branch, so it is not from these
changes -- its committed goldens predate #521's 1-bit text rendering.

* fix: resolve CodeRabbit review and Codacy findings on #534

CodeRabbit raised six; all six were real.

The test double had drifted ahead of production. VisualTestDisplayManager
accepted set_scrolling_state(frame_hold=...) while DisplayManager did not,
so such a call passed every harness run and would raise TypeError on the
panel -- the one failure a safety harness exists to prevent. frame_hold
belongs to the change that adds it to DisplayManager (#523), so it moves
there and the double matches main again.

The harness swallowed exceptions from re-rendered frames. _settle_loop
re-renders a mode that came back blank, to give a scroll time to draw;
returning silently on a crash meant a mode that renders one good frame
and then explodes was reported as passing. Recorded on result.error now,
keeping the captured frame so the failure stays inspectable.

starlark-apps display() returned True after _display_frame() failed, so
the controller held a dead frame for the whole display_duration instead
of rotating on. _display_frame now returns bool on all three paths.

run_plugin_tests.py used env.setdefault for PYTHONPATH and
LEDMATRIX_CORE, so an inherited value won and the subprocess imported a
different core than the one under test -- ledmatrix-plugins#467 exactly.
Prepends PROJECT_ROOT and sets LEDMATRIX_CORE unconditionally.

The on-demand mailbox is polled after every frame, ~125x/second on a
scrolling mode, and the read is deliberately uncached, so it was that
many disk reads per second to find nothing. Floored at 250ms, which is
imperceptible for a web-UI click. Consuming it also deleted whatever was
present rather than what had just been processed, so a request posted
while the previous one was in flight was thrown away and never ran; the
delete is now keyed by request_id. That narrows the window rather than
closing it -- a true atomic claim needs a primitive the cache layer does
not offer, and the code says so rather than implying otherwise.

Codacy's 2 criticals were bandit B404/B603 on the subprocess call added
to run_plugin_tests.py. Fixed interpreter, argument list, no shell;
annotated with the repo's existing nosec convention. Bandit is clean on
the file.

Adds test/test_on_demand_mailbox.py (8), test_starlark_display_contract.py
(4) and two settle cases in test_harness_empty_claimed.py. 4, 4 and 2 of
those fail against the pre-fix code. Full suite: 3961 passed.

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

* chore: satisfy Codacy's subprocess checks on the new test runner

Codacy runs Bandit and Opengrep (its Semgrep fork). The new
subprocess.run in scripts/run_plugin_tests.py trips three patterns, on
two different lines:

  Bandit   B404 on the import, B603 on the call
  Opengrep dangerous-subprocess-use-audit          on the run( line
           dangerous-subprocess-use-tainted-env-args on the argv line

A nosemgrep applies only to its own line, so the call line and the argv
line each need one; a single comment on the call covered neither rule
fully. Suppression is the right answer here rather than a rewrite: the
interpreter is sys.executable, the arguments are a list, and no shell is
involved, so there is nothing to word-split or expand.

Matches the pair the rest of the repo already uses for this shape --
permission_utils.py, plugin_loader.py, install_dependencies_apt.py.

Codacy: 0 new issues, up to standards.

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

* chore: leave visual_display_manager untouched so #523 can merge

The only change this branch made to that file was a docstring, and it
collided with #523's rewrite of the same method -- so #534 and #523 each
merged cleanly against main but conflicted with each other. Reverted to
main's text; #523 owns this method and adds frame_hold to it.

The note the docstring carried ('frame_hold arrives in #523') would have
been stale the moment #523 landed anyway. The parity test in #523 is
what actually keeps the two signatures honest.

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

* chore: add the Ruff suppression nosec/nosemgrep do not cover

Ruff reports S603 on the same call Bandit and Opengrep do, and none of
the three suppressions covers the others. Confirmed the precondition
first: path comes from discover_plugin_tests(), which globs test files
inside the repo, and the call is a fixed interpreter with a list argv
and no shell.

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

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Chuck
2026-09-07 13:37:38 -04:00
committed by GitHub
co-authored by Claude Opus 5
parent 696acdbc7b
commit a0d3e64099
11 changed files with 647 additions and 49 deletions
+24 -8
View File
@@ -681,12 +681,18 @@ class StarlarkAppsPlugin(BasePlugin):
if app.is_enabled() and app.should_render(current_time): if app.is_enabled() and app.should_render(current_time):
self._render_app(app, force=False) self._render_app(app, force=False)
def display(self, force_clear: bool = False) -> None: def display(self, force_clear: bool = False) -> bool:
""" """
Display current Starlark app. Display current Starlark app.
This method is called during the display rotation. This method is called during the display rotation.
Displays frames from the currently active app. Displays frames from the currently active app.
Returns False when there is no app to show -- which is the state of
every install without Pixlet, and of a fresh one before any app is
added. The display controller only skips a mode on a boolean False
(it checks isinstance(result, bool)), so returning None held a black
panel for the full display_duration instead of rotating on.
""" """
try: try:
if force_clear: if force_clear:
@@ -699,20 +705,24 @@ class StarlarkAppsPlugin(BasePlugin):
if not self.current_app: if not self.current_app:
# No apps available # No apps available
self.logger.debug("No Starlark apps to display") self.logger.debug("No Starlark apps to display")
return return False
# Render app if needed # Render app if needed
if not self.current_app.frames: if not self.current_app.frames:
success = self._render_app(self.current_app, force=True) success = self._render_app(self.current_app, force=True)
if not success: if not success:
self.logger.error(f"Failed to render app: {self.current_app.app_id}") self.logger.error(f"Failed to render app: {self.current_app.app_id}")
return return False
# Display current frame # Display current frame. The result is propagated: a failed frame
self._display_frame() # update is not a displayed frame, and returning True regardless
# told the controller the mode had rendered, so it held the dead
# frame for the whole display_duration instead of rotating on.
return self._display_frame()
except Exception as e: except Exception as e:
self.logger.error(f"Error displaying Starlark app: {e}") self.logger.error(f"Error displaying Starlark app: {e}")
return False
def _select_next_app(self) -> None: def _select_next_app(self) -> None:
"""Select the next enabled app for display.""" """Select the next enabled app for display."""
@@ -835,10 +845,13 @@ class StarlarkAppsPlugin(BasePlugin):
self.logger.error(f"Error loading frames for {app.app_id}: {e}") self.logger.error(f"Error loading frames for {app.app_id}: {e}")
return False return False
def _display_frame(self) -> None: def _display_frame(self) -> bool:
"""Display the current frame of the current app.""" """Display the current frame of the current app.
:returns: whether a frame actually reached the display manager.
"""
if not self.current_app or not self.current_app.frames: if not self.current_app or not self.current_app.frames:
return return False
try: try:
current_time = time.time() current_time = time.time()
@@ -856,8 +869,11 @@ class StarlarkAppsPlugin(BasePlugin):
) )
self.current_app.last_frame_time = current_time self.current_app.last_frame_time = current_time
return True
except Exception as e: except Exception as e:
self.logger.error(f"Error displaying frame: {e}") self.logger.error(f"Error displaying frame: {e}")
return False
def install_app(self, app_id: str, star_file_path: str, metadata: Optional[Dict[str, Any]] = None, assets_dir: Optional[str] = None) -> bool: def install_app(self, app_id: str, star_file_path: str, metadata: Optional[Dict[str, Any]] = None, assets_dir: Optional[str] = None) -> bool:
""" """
+98 -8
View File
@@ -6,6 +6,9 @@ Discovers and runs tests for LEDMatrix plugins.
Supports both unittest and pytest. Supports both unittest and pytest.
""" """
import os
import re
import subprocess # nosec B404 - list-form argv only, no shell # nosemgrep
import sys import sys
import argparse import argparse
from pathlib import Path from pathlib import Path
@@ -68,6 +71,79 @@ def _find_tests_in_dir(directory: Path) -> list:
return sorted(set(test_files)) return sorted(set(test_files))
def _is_script_style(path) -> bool:
"""True when a test file is a standalone script, not a pytest module.
Most plugin tests are written as `def main()` plus an `if __name__ ==
"__main__"` guard and signal through an exit code. pytest collects zero
items from those, so handing them to pytest printed "no tests ran" and this
runner reported success over work it had not done -- 151 of 248 files on a
fully populated rig.
"""
try:
src = Path(path).read_text(encoding="utf-8", errors="replace")
except OSError:
return False
has_pytest_items = re.search(r"^\s*(def test_|class Test|async def test_)", src, re.M)
has_main_guard = "__main__" in src and "__name__" in src
return bool(has_main_guard and not has_pytest_items)
def run_script_tests(test_files: list, verbose: bool = False) -> int:
"""Run standalone test scripts, honouring the 0 pass / 2 skip / 1 fail
convention that ledmatrix-plugins' own runner established.
Scripts opt into skipping by printing "SKIP: <reason>" and exiting 2 --
a script that needs a tty or an LED matrix is not a regression.
"""
env = dict(os.environ)
# Prepend rather than setdefault. An inherited PYTHONPATH -- a developer's
# shell, a tox run, another checkout -- otherwise wins outright, and the
# subprocess imports a different copy of the core than the one under test.
# That is exactly the failure ledmatrix-plugins#467 describes, and it is
# invisible: the tests pass or fail against a tree nobody meant to test.
inherited = env.get("PYTHONPATH")
env["PYTHONPATH"] = (f"{PROJECT_ROOT}{os.pathsep}{inherited}"
if inherited else str(PROJECT_ROOT))
env["LEDMATRIX_CORE"] = str(PROJECT_ROOT)
passed = skipped = failed = 0
failures = []
for path in test_files:
try:
# Fixed interpreter (sys.executable) plus a test path this script
# discovered by globbing the repo; argument list, no shell, so
# nothing is word-split or expanded. Same suppression pair the
# rest of the repo uses for this shape (see permission_utils.py).
proc = subprocess.run( # noqa: S603 # nosec B603 - no shell invoked (list-form argv) # nosemgrep
[sys.executable, str(path)], # nosemgrep
cwd=str(Path(path).parent),
capture_output=True, text=True, env=env,
stdin=subprocess.DEVNULL, timeout=300,
)
rc = proc.returncode
tail = " | ".join((proc.stdout or proc.stderr or "").strip().splitlines()[-2:])[:200]
except subprocess.TimeoutExpired:
rc, tail = 1, "timed out after 300s"
if rc == 0:
passed += 1
label = "pass"
elif rc == 2:
skipped += 1
label = "SKIP"
else:
failed += 1
label = "FAIL"
failures.append(f"{Path(path).name}: exit {rc} | {tail}")
if verbose or rc != 0:
print(f" [{label}] {Path(path).name}" + (f" -- {tail}" if rc != 0 else ""))
print(f"\n{passed} passed, {skipped} skipped, {failed} failed (scripts)")
for f in failures:
print(f" - {f}", file=sys.stderr)
return 1 if failed else 0
def run_unittest_tests(test_files: list, verbose: bool = False) -> int: def run_unittest_tests(test_files: list, verbose: bool = False) -> int:
""" """
Run tests using unittest. Run tests using unittest.
@@ -186,11 +262,16 @@ def main():
print("No test files found in plugins directory") print("No test files found in plugins directory")
return 0 return 0
print(f"Found {len(test_files)} test file(s)") scripts = [f for f in test_files if _is_script_style(f)]
modules = [f for f in test_files if f not in scripts]
print(f"Found {len(test_files)} test file(s)"
+ (f" -- {len(modules)} collectable, {len(scripts)} standalone script(s)"
if scripts else ""))
for test_file in test_files: for test_file in test_files:
print(f" - {test_file}") print(f" - {test_file}")
print() print()
# Determine runner # Determine runner
runner = args.runner runner = args.runner
if runner == 'auto': if runner == 'auto':
@@ -199,12 +280,21 @@ def main():
runner = 'pytest' runner = 'pytest'
except ImportError: except ImportError:
runner = 'unittest' runner = 'unittest'
# Run tests # Standalone scripts cannot be collected by pytest or unittest -- run them
if runner == 'pytest': # as the scripts they are. Doing this rather than silently collecting zero
return run_pytest_tests(test_files, args.verbose, args.coverage) # items is the whole point: this runner used to report success having
else: # executed nothing.
return run_unittest_tests(test_files, args.verbose) rc = 0
if scripts:
rc |= run_script_tests(scripts, args.verbose)
if modules:
if runner == 'pytest':
rc |= run_pytest_tests(modules, args.verbose, args.coverage)
else:
rc |= run_unittest_tests(modules, args.verbose)
return rc
if __name__ == '__main__': if __name__ == '__main__':
+58 -2
View File
@@ -198,6 +198,10 @@ class DisplayController:
# the main run loop reconciles (loads/unloads) on its own thread so # the main run loop reconciles (loads/unloads) on its own thread so
# mutating available_modes never races with rendering. # mutating available_modes never races with rendering.
self._pending_plugin_reconcile = False self._pending_plugin_reconcile = False
# Monotonic stamp of the last mailbox disk read; see
# _poll_on_demand_requests. None means "never polled", so the first
# call always goes through.
self._last_on_demand_poll: Optional[float] = None
self.on_demand_active = False self.on_demand_active = False
self.on_demand_mode: Optional[str] = None self.on_demand_mode: Optional[str] = None
self.on_demand_modes: List[str] = [] # All modes for the on-demand plugin self.on_demand_modes: List[str] = [] # All modes for the on-demand plugin
@@ -1267,12 +1271,34 @@ class DisplayController:
self.on_demand_schedule_override = False self.on_demand_schedule_override = False
self._publish_on_demand_state() self._publish_on_demand_state()
#: Shortest gap between mailbox disk reads. This is called after every
#: frame -- about 125 times a second on a scrolling mode -- and the read
#: below is deliberately uncached, so without a floor it was 125 disk reads
#: per second to find nothing. An on-demand request comes from a person
#: clicking in the web UI, so a quarter second of latency is not
#: perceptible, and it cuts the read rate by 30x.
ON_DEMAND_POLL_INTERVAL = 0.25
def _poll_on_demand_requests(self) -> None: def _poll_on_demand_requests(self) -> None:
"""Poll cache for new on-demand requests from external controllers.""" """Poll cache for new on-demand requests from external controllers."""
now = time.monotonic()
if (self._last_on_demand_poll is not None
and now - self._last_on_demand_poll < self.ON_DEMAND_POLL_INTERVAL):
return
self._last_on_demand_poll = now
try: try:
# Use a long max_age (1 hour) to ensure requests aren't expired before processing # Use a long max_age (1 hour) to ensure requests aren't expired before processing
# The request_id check prevents duplicate processing # The request_id check prevents duplicate processing.
request = self.cache_manager.get('display_on_demand_request', max_age=3600) #
# memory_ttl=0 is required, not optional: this key is a mailbox the
# web process writes and this process reads. get() defaults the
# in-memory TTL to max_age, so without it the first request read was
# pinned in memory for the full hour and every later poll returned
# that stale copy -- meaning no second on-demand request was honoured
# for an hour, while the API still reported success.
request = self.cache_manager.get('display_on_demand_request',
max_age=3600, memory_ttl=0)
except (OSError, RuntimeError, ValueError, TypeError) as err: except (OSError, RuntimeError, ValueError, TypeError) as err:
logger.error("Failed to read on-demand request: %s", err, exc_info=True) logger.error("Failed to read on-demand request: %s", err, exc_info=True)
return return
@@ -1318,6 +1344,36 @@ class DisplayController:
# Mark as processed BEFORE processing (to prevent duplicate processing) # Mark as processed BEFORE processing (to prevent duplicate processing)
self.cache_manager.set('display_on_demand_processed_id', request_id, ttl=3600) self.cache_manager.set('display_on_demand_processed_id', request_id, ttl=3600)
self.on_demand_request_id = request_id self.on_demand_request_id = request_id
# Consume the mailbox entry. Leaving it on disk meant a restart replayed
# the previous request: the fresh controller read it, activated it and
# cached it, so the request the caller had just made was ignored and the
# panel silently showed the earlier plugin. processed_id still guards
# against double-processing if this delete fails.
try:
# Compare before deleting. The web process can post a newer request
# between the read above and this delete; an unconditional delete
# threw that one away and it was never processed -- the user's
# second click did nothing. Re-reading uncached and only deleting
# our own request_id means a newer request is left in the mailbox
# for the next poll instead.
#
# This narrows the window rather than closing it: a request landing
# between this re-read and the delete is still lost. Closing it
# properly needs an atomic claim (a rename, or a compare-and-delete
# primitive) that the cache layer does not currently offer, so the
# honest fix is a smaller window plus this note, not a bigger lock.
current = self.cache_manager.get('display_on_demand_request',
max_age=3600, memory_ttl=0)
if not current or current.get('request_id') == request_id:
self.cache_manager.delete('display_on_demand_request')
else:
logger.debug("Newer on-demand request %s arrived while processing "
"%s; leaving it in the mailbox",
current.get('request_id'), request_id)
except (OSError, AttributeError, KeyError) as err:
# Best-effort: processed_id still guards against reprocessing if the
# mailbox cannot be cleared.
logger.debug("Could not clear the on-demand request mailbox: %s", err)
if action == 'start': if action == 'start':
logger.info("Processing on-demand start request for plugin: %s", request.get('plugin_id')) logger.info("Processing on-demand start request for plugin: %s", request.get('plugin_id'))
+21 -3
View File
@@ -1453,12 +1453,30 @@ class DisplayManager:
if parent_dir and str(parent_dir) != '/tmp': # nosec B108 - guard to skip /tmp for permission ops if parent_dir and str(parent_dir) != '/tmp': # nosec B108 - guard to skip /tmp for permission ops
ensure_directory_permissions(parent_dir, get_assets_dir_mode()) ensure_directory_permissions(parent_dir, get_assets_dir_mode())
self._snapshot_dir_prepared = True self._snapshot_dir_prepared = True
# Write atomically: temp then replace # Write atomically: temp then replace. The temp name must be
tmp_path = f"{self._snapshot_path}.tmp" # unique, not "<snapshot>.tmp": /tmp is world-writable and sticky,
self.image.save(tmp_path, format='PNG') # and this file is written by whichever user the display service
# runs as while tests and tooling run as someone else. A leftover
# fixed-name temp owned by another user is then unopenable even by
# root (fs.protected_regular refuses O_CREAT on a foreign file in a
# sticky dir), which froze the preview and the health check's
# liveness proxy until somebody deleted it by hand. Same pattern as
# the hardware-status write above.
_fd, tmp_path = tempfile.mkstemp(
dir=str(snapshot_path_obj.parent),
prefix=f".{snapshot_path_obj.name}.", suffix=".tmp")
try: try:
with os.fdopen(_fd, "wb") as _f:
self.image.save(_f, format='PNG')
os.chmod(tmp_path, 0o644)
os.replace(tmp_path, self._snapshot_path) os.replace(tmp_path, self._snapshot_path)
except Exception: except Exception:
# Never leave the temp behind -- that is what made the failure
# permanent rather than transient.
try:
os.unlink(tmp_path)
except OSError:
pass
# Fallback to direct save if replace not supported # Fallback to direct save if replace not supported
self.image.save(self._snapshot_path, format='PNG') self.image.save(self._snapshot_path, format='PNG')
# Set proper file permissions after saving # Set proper file permissions after saving
+2 -1
View File
@@ -96,7 +96,8 @@ class FontManager:
self.common_fonts = { self.common_fonts = {
"press_start": "assets/fonts/PressStart2P-Regular.ttf", "press_start": "assets/fonts/PressStart2P-Regular.ttf",
"four_by_six": "assets/fonts/4x6-font.ttf", "four_by_six": "assets/fonts/4x6-font.ttf",
"five_by_seven": "assets/fonts/5x7.bdf" "five_by_seven": "assets/fonts/5x7.bdf",
"tom_thumb": "assets/fonts/tom-thumb.bdf"
# Note: cozette_bdf removed - font file not available # Note: cozette_bdf removed - font file not available
# To re-enable: download cozette.bdf from https://github.com/the-moonwitch/Cozette # To re-enable: download cozette.bdf from https://github.com/the-moonwitch/Cozette
# and add: "cozette_bdf": "assets/fonts/cozette.bdf" # and add: "cozette_bdf": "assets/fonts/cozette.bdf"
+140 -12
View File
@@ -14,6 +14,8 @@ via BoundsCheckingDisplayManager, and golden-image comparison.
import contextlib import contextlib
import http.client import http.client
import inspect import inspect
import time
from datetime import timedelta
import socket import socket
import ssl import ssl
import urllib.error import urllib.error
@@ -25,7 +27,7 @@ from PIL import Image, ImageChops
from src.logging_config import get_logger from src.logging_config import get_logger
from .bounds_display_manager import BoundsCheckingDisplayManager from .bounds_display_manager import BoundsCheckingDisplayManager
from .loading import load_config_defaults, load_manifest from .loading import load_config_defaults, load_manifest, merge_config
from .sizes import DEFAULT_TEST_SIZES, safe_mode_filename, size_label from .sizes import DEFAULT_TEST_SIZES, safe_mode_filename, size_label
logger = get_logger("[Plugin Harness]") logger = get_logger("[Plugin Harness]")
@@ -116,14 +118,23 @@ def list_modes(plugin_instance: Any, manifest: Dict[str, Any], plugin_id: str) -
def _instantiate(plugin_id: str, manifest: Dict[str, Any], plugin_dir: Path, def _instantiate(plugin_id: str, manifest: Dict[str, Any], plugin_dir: Path,
config: Dict[str, Any], mock_data: Dict[str, Any], config: Dict[str, Any], mock_data: Dict[str, Any],
display_manager: Any) -> Any: display_manager: Any, cache_manager: Any = None) -> Any:
"""Load and construct a plugin instance with mocked managers.""" """Load and construct a plugin instance with mocked managers.
Pass ``cache_manager`` to share one cache across the renders of a plugin.
Building a fresh one per (size, mode) made every render a cold start, so a
plugin that fetches per game or per player re-fetched everything N times --
baseball-scoreboard took 840s for nine renders where ~72s was the arithmetic
-- and the cache-hit path, which is what a running rig executes almost
always, was never exercised.
"""
from src.plugin_system.plugin_loader import PluginLoader from src.plugin_system.plugin_loader import PluginLoader
from src.plugin_system.testing import MockCacheManager, MockPluginManager from src.plugin_system.testing import MockCacheManager, MockPluginManager
cache_manager = MockCacheManager() if cache_manager is None:
for key, value in (mock_data or {}).items(): cache_manager = MockCacheManager()
cache_manager.set(key, value) for key, value in (mock_data or {}).items():
cache_manager.set(key, value)
loader = PluginLoader() loader = PluginLoader()
plugin_instance, _module = loader.load_plugin( plugin_instance, _module = loader.load_plugin(
@@ -160,6 +171,107 @@ def _render_mode(plugin_instance: Any, mode: str) -> Any:
return plugin_instance.display(force_clear=False) return plugin_instance.display(force_clear=False)
# How many extra frames to drive before believing a mode really draws nothing.
# A scroll starts with its content off-panel, so frame 1 is legitimately blank;
# measured across the fleet, content appears by frame 2-4 (f1-scoreboard),
# frame 4 (ledmatrix-elections) and frame 38 at 64px (ledmatrix-leaderboard).
EMPTY_RECHECK_FRAMES = 48
# Seconds to advance the clock between those frames. Scroll position is usually
# driven by elapsed time, which a frozen clock never provides.
EMPTY_RECHECK_STEP = 0.05
def _has_content(image) -> bool:
"""True when any pixel is lit above the threshold."""
if image is None:
return False
return image.convert("L").point(
lambda p: 255 if p > _LIT_THRESHOLD else 0).getbbox() is not None
def _render_mode_again(plugin_instance: Any, mode: str) -> Any:
"""Draw one more frame WITHOUT force_clear.
_render_mode passes force_clear=True, which for a scrolling plugin means
"reset the scroll to the start" -- so repeating it would redraw frame 1 for
ever. The re-check needs the plugin to advance.
"""
sig = inspect.signature(plugin_instance.display)
if "display_mode" in sig.parameters:
return plugin_instance.display(force_clear=False, display_mode=mode)
return plugin_instance.display(force_clear=False)
def _settle_empty_frame(inst, mode, dm, result, freezer) -> None:
"""Give an apparently-empty mode a few frames to draw before believing it.
One frame is not evidence: a scroll's first frame is its blank scroll-in
buffer. Without this, every scrolling plugin was warned about -- 60 of 76
warnings on a 44-plugin rig were false, which is the rate at which people
stop reading a warning.
"""
if result.error is not None or result.display_returned is False:
return
if _has_content(result.image):
return
# The frozen clock is shared by every render in the matrix, so any time this
# probe borrows has to be given back -- otherwise a mode that scrolls in
# leaves the clock advanced and every later mode renders at the wrong
# instant, drifting its golden. Seen as 5 spurious f1_upcoming drifts.
resume_at = None
if freezer is not None:
try:
resume_at = freezer()
except (AttributeError, TypeError, ValueError):
# Not a freezegun factory, or a version whose factory is not
# callable. Only used to restore the clock, never load-bearing.
resume_at = None
try:
_settle_loop(inst, mode, dm, result, freezer)
finally:
if resume_at is not None:
try:
freezer.move_to(resume_at)
except (AttributeError, TypeError, ValueError):
pass
def _settle_loop(inst, mode, dm, result, freezer) -> None:
tick = getattr(freezer, "tick", None) if freezer is not None else None
for _ in range(EMPTY_RECHECK_FRAMES):
if tick is not None:
# timedelta rather than a bare float: freezegun has accepted a
# number only since 1.x, and a stale pin would raise here.
try:
tick(timedelta(seconds=EMPTY_RECHECK_STEP))
except (AttributeError, TypeError, ValueError):
# Pacing is best-effort; a freezegun that will not take a
# timedelta just means this probe runs without advancing time.
pass
else:
# No frozen clock, so the real one has to do the advancing. Without
# this the 48 frames run in microseconds, elapsed time stays ~0, and
# a scroll driven by elapsed time never moves -- which is exactly
# the plugin this check is trying not to slander.
time.sleep(EMPTY_RECHECK_STEP)
try:
result.display_returned = _render_mode_again(inst, mode)
except Exception as e: # noqa: BLE001
# Deliberately broad: this calls a plugin's display(), which can
# raise anything. Recorded rather than swallowed -- a mode that
# renders one good frame and then crashes on the next is broken,
# and returning silently here reported it as passing. The frame
# already captured stays on the result so the failure is still
# inspectable.
result.error = repr(e)
return
image = dm.get_image()
if _has_content(image):
result.image = image
result.overflow = dm.check_overflow()
return
def _freeze(freeze_time: Optional[str]): def _freeze(freeze_time: Optional[str]):
"""Context manager that freezes wall-clock time when freeze_time is given, """Context manager that freezes wall-clock time when freeze_time is given,
so time-dependent plugins (clocks, countdowns) render deterministic goldens.""" so time-dependent plugins (clocks, countdowns) render deterministic goldens."""
@@ -193,7 +305,9 @@ def render_plugin_matrix(
manifest = load_manifest(plugin_dir) manifest = load_manifest(plugin_dir)
# Start from config_schema.json defaults so the plugin behaves like a real # Start from config_schema.json defaults so the plugin behaves like a real
# install; explicit caller config still wins over a schema default. # install; explicit caller config still wins over a schema default.
config = {"enabled": True, **load_config_defaults(plugin_dir), **(config or {})} config = merge_config(
merge_config({"enabled": True}, load_config_defaults(plugin_dir)),
config or {})
sizes = sizes or DEFAULT_TEST_SIZES sizes = sizes or DEFAULT_TEST_SIZES
results: List[RenderResult] = [] results: List[RenderResult] = []
@@ -202,25 +316,35 @@ def render_plugin_matrix(
# rendering a smaller one, instead of being clipped into a false pass. # rendering a smaller one, instead of being clipped into a false pass.
extent = (max(w for w, _ in sizes), max(h for _, h in sizes)) extent = (max(w for w, _ in sizes), max(h for _, h in sizes))
with _freeze(freeze_time): # One cache for the whole matrix: see _instantiate. The display manager
# stays per-render (the bounds checking depends on that); only fetched data
# is shared.
from src.plugin_system.testing import MockCacheManager
cache_manager = MockCacheManager()
for key, value in (mock_data or {}).items():
cache_manager.set(key, value)
with _freeze(freeze_time) as freezer:
for width, height in sizes: for width, height in sizes:
results.extend(_render_size( results.extend(_render_size(
plugin_id, manifest, plugin_dir, config, mock_data or {}, plugin_id, manifest, plugin_dir, config, mock_data or {},
width, height, run_update, extent, width, height, run_update, extent, cache_manager, freezer,
)) ))
return results return results
def _render_size(plugin_id, manifest, plugin_dir, config, mock_data, def _render_size(plugin_id, manifest, plugin_dir, config, mock_data,
width, height, run_update, extent) -> List[RenderResult]: width, height, run_update, extent,
cache_manager=None, freezer=None) -> List[RenderResult]:
"""Render every mode at one size. A fresh instance per mode avoids state leaks.""" """Render every mode at one size. A fresh instance per mode avoids state leaks."""
results: List[RenderResult] = [] results: List[RenderResult] = []
# Discover modes once per size (instance build can depend on config). # Discover modes once per size (instance build can depend on config).
try: try:
probe_dm = BoundsCheckingDisplayManager(width=width, height=height, overflow_extent=extent) probe_dm = BoundsCheckingDisplayManager(width=width, height=height, overflow_extent=extent)
probe = _instantiate(plugin_id, manifest, plugin_dir, config, mock_data, probe_dm) probe = _instantiate(plugin_id, manifest, plugin_dir, config, mock_data, probe_dm,
cache_manager)
modes = list_modes(probe, manifest, plugin_id) modes = list_modes(probe, manifest, plugin_id)
except Exception as e: # noqa: BLE001 — surface any load failure as a result except Exception as e: # noqa: BLE001 — surface any load failure as a result
return [RenderResult(plugin_id, width, height, "<load>", error=repr(e))] return [RenderResult(plugin_id, width, height, "<load>", error=repr(e))]
@@ -229,7 +353,8 @@ def _render_size(plugin_id, manifest, plugin_dir, config, mock_data,
result = RenderResult(plugin_id, width, height, mode) result = RenderResult(plugin_id, width, height, mode)
dm = BoundsCheckingDisplayManager(width=width, height=height, overflow_extent=extent) dm = BoundsCheckingDisplayManager(width=width, height=height, overflow_extent=extent)
try: try:
inst = _instantiate(plugin_id, manifest, plugin_dir, config, mock_data, dm) inst = _instantiate(plugin_id, manifest, plugin_dir, config, mock_data, dm,
cache_manager)
if run_update: if run_update:
try: try:
inst.update() inst.update()
@@ -248,6 +373,9 @@ def _render_size(plugin_id, manifest, plugin_dir, config, mock_data,
result.display_returned = _render_mode(inst, mode) result.display_returned = _render_mode(inst, mode)
result.image = dm.get_image() result.image = dm.get_image()
result.overflow = dm.check_overflow() result.overflow = dm.check_overflow()
# A blank first frame is not proof of a blank mode; see
# _settle_empty_frame.
_settle_empty_frame(inst, mode, dm, result, freezer)
except Exception as e: # noqa: BLE001 — a display crash is a real failure except Exception as e: # noqa: BLE001 — a display crash is a real failure
result.error = repr(e) result.error = repr(e)
results.append(result) results.append(result)
+40 -5
View File
@@ -33,6 +33,45 @@ def load_manifest(plugin_dir: Union[str, Path]) -> Dict[str, Any]:
return json.load(f) return json.load(f)
def _defaults_from_properties(properties: Dict[str, Any]) -> Dict[str, Any]:
"""Defaults for one `properties` block, recursing into nested objects.
An object property carries its defaults on its children, not on itself, so
reading only the top level dropped everything nested. That is most of the
fleet: config organised by league, or under customization/display_options,
lost 2,386 defaults across 37 of 44 plugins -- soccer-scoreboard alone lost
539 of 565 -- and the harness rendered them with a config no install would
ever have.
"""
defaults: Dict[str, Any] = {}
for key, prop in (properties or {}).items():
if not isinstance(prop, dict):
continue
if prop.get('type') == 'object' and isinstance(prop.get('properties'), dict):
nested = _defaults_from_properties(prop['properties'])
if nested:
defaults[key] = nested
elif 'default' in prop:
defaults[key] = prop['default']
return defaults
def merge_config(base: Dict[str, Any], override: Dict[str, Any]) -> Dict[str, Any]:
"""Deep-merge override onto base, without dropping sibling defaults.
A shallow merge would let `-c '{"nhl": {"enabled": true}}'` replace the whole
nhl subtree and silently discard every other nhl default -- the same class of
bug this function exists to fix.
"""
merged = dict(base)
for key, value in (override or {}).items():
if isinstance(value, dict) and isinstance(merged.get(key), dict):
merged[key] = merge_config(merged[key], value)
else:
merged[key] = value
return merged
def load_config_defaults(plugin_dir: Union[str, Path]) -> Dict[str, Any]: def load_config_defaults(plugin_dir: Union[str, Path]) -> Dict[str, Any]:
"""Extract default values from a plugin's config_schema.json (empty if none).""" """Extract default values from a plugin's config_schema.json (empty if none)."""
schema_path = Path(plugin_dir) / 'config_schema.json' schema_path = Path(plugin_dir) / 'config_schema.json'
@@ -40,11 +79,7 @@ def load_config_defaults(plugin_dir: Union[str, Path]) -> Dict[str, Any]:
return {} return {}
with open(schema_path, 'r') as f: with open(schema_path, 'r') as f:
schema = json.load(f) schema = json.load(f)
defaults: Dict[str, Any] = {} return _defaults_from_properties(schema.get('properties', {}))
for key, prop in schema.get('properties', {}).items():
if isinstance(prop, dict) and 'default' in prop:
defaults[key] = prop['default']
return defaults
def load_harness_spec(plugin_dir: Union[str, Path]) -> Dict[str, Any]: def load_harness_spec(plugin_dir: Union[str, Path]) -> Dict[str, Any]:
+58
View File
@@ -101,3 +101,61 @@ class TestSkippedResults:
r = _result(None, returned=None) r = _result(None, returned=None)
check_empty_claimed([r], strict=True) check_empty_claimed([r], strict=True)
assert r.empty_claimed is None assert r.empty_claimed is None
class TestSettleRecordsLaterFailures:
"""A mode that renders one good frame and then crashes is broken.
_settle_loop re-renders a mode that came back blank, to give a scroll or an
animation time to put something on the panel. Swallowing an exception from
those later frames meant the harness reported a passing result for a mode
that crashes as soon as it is asked for a second frame -- exactly the kind
of defect the harness exists to catch.
"""
class Boom:
"""Renders once, then raises."""
def __init__(self):
self.calls = 0
def display(self, force_clear=False):
self.calls += 1
raise RuntimeError("second frame exploded")
def _settle(self, inst, dm, result):
from src.plugin_system.testing import harness
harness._settle_loop(inst, "mode", dm, result, None)
def test_the_exception_is_recorded_on_the_result(self, monkeypatch):
from src.plugin_system.testing import harness
# Keep the probe short; this test is about the error, not the pacing.
monkeypatch.setattr(harness, "EMPTY_RECHECK_FRAMES", 1)
monkeypatch.setattr(harness, "EMPTY_RECHECK_STEP", 0)
result = _result(_blank())
assert result.error is None
self._settle(self.Boom(), _FakeDM(), result)
assert result.error is not None, "a crash on a later frame was swallowed"
assert "second frame exploded" in result.error
def test_the_already_captured_frame_is_kept(self, monkeypatch):
from src.plugin_system.testing import harness
monkeypatch.setattr(harness, "EMPTY_RECHECK_FRAMES", 1)
monkeypatch.setattr(harness, "EMPTY_RECHECK_STEP", 0)
image = _blank()
result = _result(image)
self._settle(self.Boom(), _FakeDM(), result)
assert result.image is image, "the good frame was discarded along with the error"
class _FakeDM:
"""Minimal display-manager double for _settle_loop."""
def get_image(self):
return _blank()
def check_overflow(self):
return None
+106
View File
@@ -0,0 +1,106 @@
"""The on-demand request mailbox: how often it is read, and how it is consumed.
The mailbox is a cache key the web process writes and the display process
reads. Two properties matter and neither is obvious from the call site:
* it is polled after every rendered frame, so an uncached read here is a
disk read at frame rate;
* consuming it must not throw away a request that arrived while the previous
one was being processed.
"""
from unittest.mock import MagicMock
import pytest
class TestPollingIsBounded:
"""_poll_on_demand_requests runs ~125x/second on a scrolling mode.
The read is deliberately uncached (memory_ttl=0) because a cached one
pinned the first request for an hour. That makes the call a real disk read,
so it needs a floor -- without one it was ~125 reads per second to find
nothing at all.
"""
def test_first_call_always_reads(self, test_display_controller):
c = test_display_controller
c.cache_manager.get = MagicMock(return_value=None)
c._poll_on_demand_requests()
assert c.cache_manager.get.call_count == 1
def test_immediate_second_call_does_not_read(self, test_display_controller):
c = test_display_controller
c.cache_manager.get = MagicMock(return_value=None)
c._poll_on_demand_requests()
for _ in range(50):
c._poll_on_demand_requests()
assert c.cache_manager.get.call_count == 1, "polling was not bounded"
def test_reads_again_once_the_interval_has_passed(self, test_display_controller, monkeypatch):
c = test_display_controller
c.cache_manager.get = MagicMock(return_value=None)
clock = {"t": 1000.0}
monkeypatch.setattr("src.display_controller.time.monotonic", lambda: clock["t"])
c._poll_on_demand_requests()
clock["t"] += c.ON_DEMAND_POLL_INTERVAL / 2
c._poll_on_demand_requests()
assert c.cache_manager.get.call_count == 1, "read before the interval elapsed"
clock["t"] += c.ON_DEMAND_POLL_INTERVAL
c._poll_on_demand_requests()
assert c.cache_manager.get.call_count == 2
def test_the_interval_is_short_enough_to_feel_instant(self, test_display_controller):
# A person clicking in the web UI must not notice the floor.
assert test_display_controller.ON_DEMAND_POLL_INTERVAL <= 0.5
class TestMailboxIsConsumedByIdentity:
"""Deleting whatever is in the mailbox loses a request that raced in."""
def _arrange(self, controller, first, later):
"""Mailbox returns `first`, then `later` on the pre-delete re-read."""
controller.on_demand_active = False
controller.on_demand_request_id = None
controller._last_on_demand_poll = None
reads = iter([first, later])
def fake_get(key, *a, **kw):
if key == 'display_on_demand_request':
return next(reads, later)
return None # processed-id lookup
controller.cache_manager.get = MagicMock(side_effect=fake_get)
controller.cache_manager.set = MagicMock()
controller.cache_manager.delete = MagicMock()
controller._activate_on_demand = MagicMock()
REQ_A = {'request_id': 'A', 'action': 'start', 'plugin_id': 'p', 'mode': 'm'}
REQ_B = {'request_id': 'B', 'action': 'start', 'plugin_id': 'p', 'mode': 'm'}
def test_own_request_is_deleted(self, test_display_controller):
c = test_display_controller
self._arrange(c, self.REQ_A, self.REQ_A)
c._poll_on_demand_requests()
c.cache_manager.delete.assert_called_once_with('display_on_demand_request')
def test_a_newer_request_is_left_for_the_next_poll(self, test_display_controller):
c = test_display_controller
self._arrange(c, self.REQ_A, self.REQ_B)
c._poll_on_demand_requests()
assert c.cache_manager.delete.call_count == 0, \
"request B was deleted without ever being processed"
def test_an_already_empty_mailbox_is_still_cleared(self, test_display_controller):
c = test_display_controller
self._arrange(c, self.REQ_A, None)
c._poll_on_demand_requests()
c.cache_manager.delete.assert_called_once_with('display_on_demand_request')
def test_the_request_is_still_processed(self, test_display_controller):
c = test_display_controller
self._arrange(c, self.REQ_A, self.REQ_B)
c._poll_on_demand_requests()
c._activate_on_demand.assert_called_once()
+82
View File
@@ -0,0 +1,82 @@
"""starlark-apps: display() must report what actually reached the panel.
The display controller skips a mode only on a boolean False. Returning True
after the frame update failed told it the mode had rendered, so it held a dead
frame for the whole display_duration instead of rotating on -- the same class
of defect as a display() that returns None.
"""
import sys
from pathlib import Path
from unittest.mock import MagicMock
import pytest
PLUGIN_DIR = Path(__file__).resolve().parent.parent / "plugin-repos" / "starlark-apps"
@pytest.fixture(scope="module")
def manager_module():
if not PLUGIN_DIR.exists():
pytest.skip("starlark-apps plugin is not checked out")
sys.path.insert(0, str(PLUGIN_DIR))
try:
import importlib
spec = importlib.util.spec_from_file_location(
"starlark_manager_under_test", PLUGIN_DIR / "manager.py")
module = importlib.util.module_from_spec(spec)
spec.loader.exec_module(module)
return module
except Exception as e: # noqa: BLE001 - optional deps (pixlet, fcntl) may be absent
pytest.skip(f"starlark-apps manager is not importable here: {e}")
finally:
sys.path.remove(str(PLUGIN_DIR))
def _plugin(manager_module):
"""A manager with __init__ bypassed -- only display paths are under test."""
cls = manager_module.StarlarkAppsPlugin
inst = cls.__new__(cls)
inst.logger = MagicMock()
inst.display_manager = MagicMock()
inst.current_app = None
return inst
class _App:
def __init__(self, frames):
self.frames = frames
self.current_frame_index = 0
self.last_frame_time = 0.0
self.app_id = "app"
class TestDisplayFramePropagates:
def test_a_failed_update_returns_false(self, manager_module):
p = _plugin(manager_module)
p.current_app = _App([("frame", 100)])
p.display_manager.update_display.side_effect = RuntimeError("panel gone")
assert p._display_frame() is False
def test_a_good_update_returns_true(self, manager_module):
p = _plugin(manager_module)
p.current_app = _App([("frame", 100)])
assert p._display_frame() is True
def test_no_frames_returns_false(self, manager_module):
p = _plugin(manager_module)
p.current_app = _App([])
assert p._display_frame() is False
def test_display_reports_the_frame_failure(self, manager_module):
p = _plugin(manager_module)
p.current_app = _App([("frame", 100)])
p.display_manager.update_display.side_effect = RuntimeError("panel gone")
result = p.display()
assert result is False, "display() claimed success over a failed frame update"
assert isinstance(result, bool), "the controller only skips on a real bool"
+18 -10
View File
@@ -78,8 +78,11 @@ def _scrub_git_remote_url(url: str) -> str:
return url return url
# Will be initialized when blueprint is registered # Will be initialized when blueprint is registered
config_manager = None # NOTE: the managers live on the blueprint object (app.py sets
plugin_manager = None # api_v3.config_manager / api_v3.plugin_manager). Deliberately not
# mirrored as module globals: a bare `config_manager` used to resolve to
# a None that was never assigned, which silently disabled the /health
# checks and made /display/current fall back to a hardcoded 128x64.
plugin_store_manager = None plugin_store_manager = None
saved_repositories_manager = None saved_repositories_manager = None
cache_manager = None cache_manager = None
@@ -1598,11 +1601,16 @@ def get_health():
} }
# Check web interface service # Check web interface service
# Stamp the start time before measuring against it -- reading it with a
# fallback of time.time() and only assigning afterwards made the very
# first call subtract two separate clock reads, reporting a small
# negative uptime.
if not hasattr(get_health, '_start_time'):
get_health._start_time = time.time()
health_status['services']['web_interface'] = { health_status['services']['web_interface'] = {
'status': 'running', 'status': 'running',
'uptime_seconds': time.time() - (getattr(get_health, '_start_time', time.time())) 'uptime_seconds': time.time() - get_health._start_time
} }
get_health._start_time = getattr(get_health, '_start_time', time.time())
# Check display service # Check display service
display_service_status = _get_display_service_status() display_service_status = _get_display_service_status()
@@ -1613,8 +1621,8 @@ def get_health():
# Check config file accessibility # Check config file accessibility
try: try:
if config_manager: if api_v3.config_manager:
test_config = config_manager.load_config() test_config = api_v3.config_manager.load_config()
health_status['checks']['config_file'] = { health_status['checks']['config_file'] = {
'status': 'accessible', 'status': 'accessible',
'readable': True 'readable': True
@@ -1633,9 +1641,9 @@ def get_health():
# Check plugin system # Check plugin system
try: try:
if plugin_manager: if api_v3.plugin_manager:
# Try to discover plugins (lightweight check) # Try to discover plugins (lightweight check)
plugin_count = len(plugin_manager.get_available_plugins()) if hasattr(plugin_manager, 'get_available_plugins') else 0 plugin_count = len(api_v3.plugin_manager.get_available_plugins()) if hasattr(api_v3.plugin_manager, 'get_available_plugins') else 0
health_status['checks']['plugin_system'] = { health_status['checks']['plugin_system'] = {
'status': 'operational', 'status': 'operational',
'plugin_count': plugin_count 'plugin_count': plugin_count
@@ -2468,8 +2476,8 @@ def get_display_current():
# Get display dimensions from config # Get display dimensions from config
try: try:
if config_manager: if api_v3.config_manager:
main_config = config_manager.load_config() main_config = api_v3.config_manager.load_config()
hardware_config = main_config.get('display', {}).get('hardware', {}) hardware_config = main_config.get('display', {}).get('hardware', {})
cols = hardware_config.get('cols', 64) cols = hardware_config.get('cols', 64)
chain_length = hardware_config.get('chain_length', 2) chain_length = hardware_config.get('chain_length', 2)