test: cover colour, hit testing and the stateless components - #142
Merged
Conversation
44 tests across three modules that had none: `style/color.zig`, `input/hitbox.zig`, and the paginator, progress bar, spinner and keybinding map. Writing them turned up a real bug. `MouseState.update` checked the boundary crossing before the wheel, so a scroll that also moved the pointer into the region returned `.enter` and the scroll was dropped. A trackpad reports movement and scrolling in the same event, so the first scroll into any region was being lost. The wheel is now checked first; the crossing is still visible to the caller as `state.hover`. The tests pin down behaviour that is easy to get wrong later: hit boxes excluding their far edge and saturating instead of wrapping at the coordinate limit, the 256-colour ramp staying monotonic, contrast ratio matching WCAG at both extremes, NO_COLOR overriding COLORTERM, a paginator's partial last page, and a progress bar's head marker disappearing exactly when the bar is full.
The new colour tests failed on Windows, which turned out to be the implementation rather than the tests. `ColorProfile.detect` returned `.true_color` for Windows before it looked at `no_color`, so a user who set NO_COLOR on Windows Terminal got colour anyway. NO_COLOR is a cross-platform convention; it is now checked first, and the Windows shortcut applies to everything after it. `hasDarkBackground` ignored COLORFGBG on Windows and always assumed dark. Windows terminals rarely set it, but when one does there is no reason to disbelieve it. The remaining Windows-specific behaviour -- true colour regardless of TERM -- is asserted per platform in the test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Item 4 from the review — 64 of 88 source files had no tests. This is a first pass over the ones where behaviour is well-defined and bugs are cheap to miss.
44 tests across three files, covering
style/color.zig,input/hitbox.zig, and the paginator, progress bar, spinner and keybinding map.It found a real bug
MouseState.updatechecked the boundary crossing before the wheel:A trackpad reports movement and scrolling in the same event, so a scroll that also moves the pointer into a region returned
.enterand the scroll was dropped. In practice the first scroll into any scrollable region was being lost.The wheel is now checked first. The crossing is not lost — the caller can still see it as
state.hover.What the tests pin down
Behaviour that is easy to break later, and that I checked against the implementation rather than assuming:
overlapsis symmetric and excludes merely-touching edgesNO_COLORoverridesCOLORTERM=truecolorendIndexmust not run past the item countTwo things worth knowing, found while writing these
Progress.percent()returns 0–100, not 0–1. The tests now say so, since the name reads either way.viewfunctions allocate scratch strings and never free them, by design — they expect the frame arena. That makes them leak undertesting.allocator, so these tests hand them an arena. Worth documenting on the component contract.zig build testandzig buildclean on 0.16.0. Verified the hitbox test fails against the current code.