Files
LEDMatrix/test/test_harness_empty_claimed.py
T
ChuckBuildsandClaude Opus 5 80781dda46 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
2026-09-06 18:36:44 -04:00

162 lines
5.8 KiB
Python

"""Tests for the harness empty-frame check (src/plugin_system/testing/harness.py).
The display controller skips a mode whose display() returns False and treats
anything else -- including None -- as "content was shown". A mode that draws
nothing without returning False is therefore never skipped, and since a mode
switch clears the panel first, it sits on a blank screen for its whole display
duration.
Two sports plugins shipped exactly that: their display() returned None on every
path, so an out-of-season league held a blank panel instead of being rotated
past. The harness rendered those modes and passed them, because it discarded
the return value entirely.
"""
from PIL import Image
from src.plugin_system.testing.harness import RenderResult, check_empty_claimed
def _blank(w=64, h=32):
return Image.new("RGB", (w, h), (0, 0, 0))
def _drawn(w=64, h=32):
img = _blank(w, h)
img.paste(Image.new("RGB", (10, 10), (255, 255, 255)), (5, 5))
return img
def _result(image, returned=None, **kw):
return RenderResult("p", 64, 32, "mode", image=image,
display_returned=returned, **kw)
class TestCheckEmptyClaimed:
def test_blank_frame_returning_none_is_flagged(self):
# The shape that shipped: nothing drawn, nothing reported.
r = _result(_blank(), returned=None)
check_empty_claimed([r])
assert r.empty_claimed is True
def test_blank_frame_returning_true_is_flagged(self):
# Just as broken, and more explicit about it.
r = _result(_blank(), returned=True)
check_empty_claimed([r])
assert r.empty_claimed is True
def test_blank_frame_returning_false_is_fine(self):
# The plugin correctly said "no content"; the controller will skip it.
r = _result(_blank(), returned=False)
check_empty_claimed([r])
assert r.empty_claimed is None
def test_a_drawn_frame_is_fine_whatever_it_returns(self):
for returned in (None, True, False):
r = _result(_drawn(), returned=returned)
check_empty_claimed([r])
assert r.empty_claimed is None, returned
def test_near_black_still_counts_as_drawn(self):
# Guard the threshold: content dim enough to look black to the eye is
# still content, and flagging it would train people to ignore this.
img = _blank()
img.paste(Image.new("RGB", (4, 4), (60, 60, 60)), (2, 2))
r = _result(img, returned=None)
check_empty_claimed([r])
assert r.empty_claimed is None
class TestWarnVersusStrict:
def test_warn_only_by_default(self):
# A scroll mode's first frame is legitimately its blank scroll-in
# buffer, so this must not fail a run unless opted in.
r = _result(_blank(), returned=None)
check_empty_claimed([r])
assert r.empty_ok is None
assert r.ok is True
def test_strict_fails_the_result(self):
r = _result(_blank(), returned=None)
check_empty_claimed([r], strict=True)
assert r.empty_ok is False
assert r.ok is False
def test_strict_still_allows_an_honest_false(self):
r = _result(_blank(), returned=False)
check_empty_claimed([r], strict=True)
assert r.empty_ok is None
assert r.ok is True
class TestSkippedResults:
def test_a_crashed_render_is_left_alone(self):
# error already fails the result; adding a second reason just muddies
# the report.
r = _result(None, returned=None, error="boom")
check_empty_claimed([r], strict=True)
assert r.empty_claimed is None
def test_a_result_with_no_image_is_left_alone(self):
r = _result(None, returned=None)
check_empty_claimed([r], strict=True)
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