test(harness): flag a mode that draws nothing without reporting it - #447
Conversation
The 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 is therefore never skipped, and
because a mode switch clears the panel first, it sits on a blank screen
for its whole display duration. Two sports plugins shipped exactly that.
The harness rendered those modes and passed them, because it called
display() and discarded the result. Capture it, and warn when a render
produced no lit pixels while claiming content.
Warn-only by default, and deliberately so: a scroll mode's first frame
is legitimately its blank scroll-in buffer, which is 42 of these on the
F1 scoreboard alone. Plugins whose modes are known to draw on their
fixture data can opt into failing via harness.json {"empty_check":
"strict"}, matching how the fill check is staged.
Worth being clear about the limit: this only sees what the fixtures
render. It would not have caught the sports bug, whose fixture seeds
games so the empty path never renders -- that needs the source-level
gate in the plugins repo. What it does catch is the same mistake in any
plugin whose empty state the harness does happen to reach, which is
coverage there was none of before.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
|
Warning Review limit reached
Next review available in: 32 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 9 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
1 participant
Follow-up to the hockey blank-panel bug (plugins#263), closing one of the reasons nothing caught it.
The gap
The controller skips a mode whose
display()returnsFalseand treats anything else — includingNone— as "content was shown". A mode that draws nothing and doesn't returnFalseis therefore never skipped, and because a mode switch clears the panel first, it sits on a blank screen for its whole display duration.The harness rendered exactly those modes and passed them, because
_render_modecalleddisplay()and threw the result away.The change
Capture the return value onto
RenderResult, and warn when a render produced no lit pixels while claiming content:Warn-only by default, and deliberately: a scroll mode's first frame is legitimately its blank scroll-in buffer — 42 of these on the F1 scoreboard alone, all correct behaviour. Plugins whose modes are known to draw on their fixture data can opt into failing with
harness.json {"empty_check": "strict"}, matching howfill_checkis staged.What this does and doesn't buy
Being straight about the limit: it would not have caught the sports bug. Hockey's fixture deliberately seeds games ("one final, one in-progress, and one scheduled game so recent/upcoming render real game cards") and disables live mode outright, so the empty path never renders. That case needs the source-level gate added in plugins#263, which needs no data at all.
What this catches is the same mistake in any plugin whose empty state the harness does reach — coverage there was none of before, since the return value was previously unobservable.
I also considered asserting
display()returns a bool fleet-wide and rejected it: 25 of 43 plugins returnNone, correctly. A clock always has content, so "nothing to show" never arises and the controller's default is right for them. Only plugins with a real no-content state need the contract, which is why the strict version is scoped to sports in the plugins repo rather than enforced here.Verification
test/test_harness_empty_claimed.pycovering blank/drawn ×None/True/False, warn vs strict, the near-black threshold (dim content is still content), and skipping crashed renders.test_install_lowmem.py::TestDiskBackedTmpdir, which fails identically on a clean tree whereverTMPDIRis set).🤖 Generated with Claude Code
https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5