diff --git a/scripts/check_plugin.py b/scripts/check_plugin.py index 997bf6d8..3f111d4d 100644 --- a/scripts/check_plugin.py +++ b/scripts/check_plugin.py @@ -41,6 +41,7 @@ from src.plugin_system.testing.loading import ( # noqa: E402 ) from src.plugin_system.testing.harness import ( # noqa: E402 RenderResult, render_plugin_matrix, compare_to_goldens, write_goldens, + check_empty_claimed, check_scale_up, ) from src.plugin_system.testing.sizes import ( # noqa: E402 @@ -115,6 +116,11 @@ def check_one(plugin_id: str, search_dirs: List[str], sizes, mock_data: Dict, declared = load_manifest(plugin_dir).get("display", {}).get("design_size", {}) design_size = (int(declared.get("width", 128)), int(declared.get("height", 32))) fill_strict = spec.get("fill_check") == "strict" + # A mode that renders nothing without returning False is never skipped by + # the display controller, so it holds a blank panel for its whole duration. + # Warn-only by default: a scroll mode's first frame is legitimately its + # blank scroll-in buffer. + empty_strict = spec.get("empty_check") == "strict" # Every run: the base config, plus one per harness.json "variant" — # a config overlay with its own golden dir (e.g. adaptive layout mode @@ -142,6 +148,7 @@ def check_one(plugin_id: str, search_dirs: List[str], sizes, mock_data: Dict, compare_to_goldens(results, golden_dir) check_scale_up(results, design_size=design_size, strict=fill_strict) + check_empty_claimed(results, strict=empty_strict) # Tag variant runs so the report and PNG dumps stay distinguishable. if variant_name: @@ -178,6 +185,9 @@ def print_report(all_results: Dict[str, List[RenderResult]]) -> bool: # warn-only underfill: big panel left mostly empty ex, ey = r.fill_extent detail += f" (fill warn: extent {ex:.0%}x{ey:.0%})" + if r.empty_claimed and r.empty_ok is None: + detail += (f" (empty warn: drew nothing but display() returned" + f" {r.display_returned!r}, so the mode is not skipped)") else: everything_ok = False if r.error is not None: @@ -191,6 +201,11 @@ def print_report(all_results: Dict[str, List[RenderResult]]) -> bool: ex, ey = r.fill_extent or (0.0, 0.0) status = "FAIL" detail = f" fill: extent {ex:.0%}x{ey:.0%} below required coverage" + elif r.empty_ok is False: + status = "FAIL" + detail = (f" drew nothing but display() returned" + f" {r.display_returned!r}; return False so the" + f" controller skips the mode") else: status, detail = "FAIL", "" print(f" [{status}] {r.size_label:>7} {r.mode}{detail}") diff --git a/src/plugin_system/testing/harness.py b/src/plugin_system/testing/harness.py index ee4d6613..51c688a2 100644 --- a/src/plugin_system/testing/harness.py +++ b/src/plugin_system/testing/harness.py @@ -73,6 +73,11 @@ class RenderResult: golden_ok: Optional[bool] = None golden_diff_pixels: int = 0 golden_max_delta: int = 0 + # what display() handed back; the controller skips a mode only on False + display_returned: Any = None + # empty-frame check: rendered nothing while not reporting "no content" + empty_claimed: Optional[bool] = None # True when that happened + empty_ok: Optional[bool] = None # False only in strict mode # fill / scale-up check (populated only for sizes >= 2x the design size) fill_checked: bool = False fill_ok: Optional[bool] = None # False only in strict mode @@ -92,6 +97,8 @@ class RenderResult: return False if self.fill_ok is False: return False + if self.empty_ok is False: + return False return True @@ -132,21 +139,25 @@ def _instantiate(plugin_id: str, manifest: Dict[str, Any], plugin_dir: Path, return plugin_instance -def _render_mode(plugin_instance: Any, mode: str) -> None: +def _render_mode(plugin_instance: Any, mode: str) -> Any: """Render a specific screen. Prefer an explicit display_mode kwarg; otherwise drive the plugin's internal mode state machine (first display() call renders - modes[current_mode_index] when current_display_mode is None).""" + modes[current_mode_index] when current_display_mode is None). + + Returns whatever display() returned. The display controller skips a mode + whose display() returns False, so that value decides whether an empty mode + is rotated past or sat on -- which makes it worth reporting rather than + discarding.""" sig = inspect.signature(plugin_instance.display) if "display_mode" in sig.parameters: - plugin_instance.display(force_clear=True, display_mode=mode) - return + return plugin_instance.display(force_clear=True, display_mode=mode) modes = getattr(plugin_instance, "modes", None) if modes and mode in modes: plugin_instance.current_mode_index = list(modes).index(mode) if hasattr(plugin_instance, "current_display_mode"): plugin_instance.current_display_mode = None - plugin_instance.display(force_clear=False) + return plugin_instance.display(force_clear=False) def _freeze(freeze_time: Optional[str]): @@ -234,7 +245,7 @@ def _render_size(plugin_id, manifest, plugin_dir, config, mock_data, logger.warning("update() raised a non-connectivity error for %s [%s]: %s", plugin_id, mode, e) if result.error is None: - _render_mode(inst, mode) + result.display_returned = _render_mode(inst, mode) result.image = dm.get_image() result.overflow = dm.check_overflow() except Exception as e: # noqa: BLE001 — a display crash is a real failure @@ -341,6 +352,44 @@ def fill_metrics(image: Image.Image) -> Tuple[float, float, float]: return (extent_x, extent_y, ink) +def check_empty_claimed(results: List[RenderResult], + strict: bool = False) -> List[RenderResult]: + """Flag a mode that rendered nothing without reporting "no content". + + 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 and does not return False therefore holds whatever is on + the panel for its whole display duration. Since a mode switch clears first, + that is a blank screen. Two sports plugins shipped exactly this: their + ``display()`` returned None on every path, so an out-of-season league sat + blank for its full duration rather than being rotated past. + + Warn-only by default, because a blank frame is not automatically wrong: a + scroll mode whose first frame is its blank scroll-in buffer renders empty + and is behaving correctly. ``strict=True`` sets ``empty_claimed`` such that + ``RenderResult.ok`` fails -- opt in per plugin via harness.json + ``{"empty_check": "strict"}`` once its modes are known to draw on the + fixture data. + + Note this can only catch what the fixtures actually render. A plugin whose + harness fixture seeds content never exercises its empty path here; the + source-level gate in the plugins repo covers that case. + """ + for r in results: + if r.image is None or r.error is not None: + continue + # An explicit False is the plugin correctly saying "nothing to show". + if r.display_returned is False: + continue + if r.image.convert("L").point( + lambda p: 255 if p > _LIT_THRESHOLD else 0).getbbox() is not None: + continue + r.empty_claimed = True + if strict: + r.empty_ok = False + return results + + def check_scale_up(results: List[RenderResult], design_size: Tuple[int, int] = (128, 32), min_extent: float = _MIN_FILL_EXTENT, diff --git a/test/test_harness_empty_claimed.py b/test/test_harness_empty_claimed.py new file mode 100644 index 00000000..2791516a --- /dev/null +++ b/test/test_harness_empty_claimed.py @@ -0,0 +1,103 @@ +"""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