test(chartgen): assert the headroom against the budget, not the render - #178
Merged
Merged
Conversation
`52e27b1` landed the caption growth allowance between the headroom test's branch point and its merge, so the two commits never conflicted textually while the later one dissolved the property the earlier one asserted. `the_tightest_slice_keeps_two_caption_lines_spare_at_six_backends` proved its margin by rendering the six-backend slice with two further notes and requiring the render to succeed. That was decisive against a hard budget. It is not against a soft one: a caption up to MAX_CAPTION_GROWTH_LINES (8) over budget now grows the canvas instead of refusing, so the render succeeds whether the headroom exists or not. Measured on merged `main`: planting one unconditional caption line in the `parallel-vs-sequenced` note set leaves the test green. The margin it reports is spent and it cannot tell. Assert the canvas height instead. Two further notes must seat on the base `HEIGHT` canvas, leaving the growth allowance unspent — spending it is the budget being gone, not headroom. Under the same planted line the test now fails (656 != 640) while all three wording tests still pass, which is the asymmetry the criterion asked for: it fails when the budget is approached, not when a sentence is reworded. Tests only; no production change and no SVG churn.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
shove-docs | 87adb81 | Commit Preview URL Branch Preview URL |
Sep 09 2026, 08:38 AM |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
1 participant
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.
Two commits that never conflicted, and the later one dissolved the earlier one's property
#174 added a test asserting the
parallel-vs-sequenced64 KiB caption's headroom rather than its wording, so that it fails when the budget is next approached rather than when a sentence is reworded.It does not do that on merged
main— because of a sibling, not because of anything wrong with the test as written.What happened
4b69b80(#165)52e27b1(#176)MAX_CAPTION_GROWTH_LINES(8) over budget grows the canvas instead of refusinga36b614(#174)#174 proved its margin by rendering the six-backend slice with two further notes added and requiring the render to succeed. Against a hard budget that is decisive. Against a soft one it is not: the render now succeeds whether the headroom exists or not.
The two never touched the same lines, so nothing conflicted and CI stayed green on both.
Measured, not argued
On merged
main(eb9a7fa), one unconditional caption line planted in theparallel-vs-sequencednote set:656 != 640#174's own control run reported this test failing with
frame()'s refusal message. That was measured on the branch, which predated #176 — a hard budget still refused. The soft budget absorbs it silently.The change
Assert the canvas height, not the
Ok. Both the six-backend redelivery fixture and its two-further-notes variant must seat on the baseHEIGHTcanvas, leaving the growth allowance unspent — spending it is the budget being gone rather than headroom.That restores the asymmetry intended: it fails when the budget is approached, whatever spends it, and while the margin still exists.
Also folds the growth test's inline
heightparsing into the sharedcanvas_heighthelper — same parse, previously duplicated.Gates
cargo nextest run --no-default-features— 343 passed, including the committed-SVG byte-comparecargo fmt -- --check,cargo clippy --no-default-features --all-targets -- -D warnings— cleanDepends-on: none — the implementation this asserts against (#165) and the growth allowance that weakened the assertion (#176) are both already on
main, as is the test this replaces (#174).