test(#565): correct two stale widen specs (widen is one-dimensional by design) - #567
Merged
Merged
Conversation
These two specs were NOT catching a product bug. Widen behaves exactly as
`panel-widen.ts` specifies; the specs were left behind by the grafana-grid@2
style-keyed placement shape.
Measured live in Chromium against the real fixture:
initial h=208px w= 867px t-sales={"grid":{"span":8}}
press 1 h=208px w=1305px t-sales={"grid":{"span":12,"height":2}}
press 2 h=208px w= 101px t-sales={"grid":{"span":1,"height":2}}
Span doubles 8 -> 12 and wraps to 1 at the maximum, and the height never
moves. That is the documented contract, stated three times in
`panel-widen.ts`: the module "never [does] anything two-dimensional" (the
corner drag, #291, owns free-form resize), grid "doubles span on each press"
with "height ... always resent unchanged", and `nextPanelPlacement` says
"Height is preserved". `t-sales` authors no height, so it resolves to
DEFAULT_GRID_HEIGHT_UNITS (2) -> `32 + 88*2` = 208px, before and after.
So the assertions were wrong, not the code:
- `expect(wide.height).toBeGreaterThan(before.height)` demanded the one thing
widen must NOT do. Now asserts the height is unchanged, which is the real
invariant and would fail if widen ever became two-dimensional.
- `toEqual({ span: 12, height: 4 })` used the flat v1 placement and a height
that never existed. Now `{ grid: { span: 12, height: 2 } }`.
- the narrow-tile spec polled `items['t-sales']?.span`, a v1 path that
resolves to `undefined` under v2, so `?? null` turned a shape mismatch into
an opaque 5s timeout. Now reads `...?.grid?.span`. Its expected value (2)
was always correct: the two presses wrap span to 1 and the menu row it then
clicks steps it to 2.
Also removes the `test.fail()` quarantine added when this was misdiagnosed as
a regression. Nothing in `src/` changes.
Chromium 21 passed, WebKit 20 passed, quarantine removed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GiubaoqEuBzAyo5C4P8Vqr
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.
What & why
Closes #565 — and corrects its premise. I opened that issue calling this a user-visible regression ("widen no longer grows a grid tile's height"). That was wrong. Widen behaves exactly as specified; the two specs were stale.
Measured live in Chromium against the real fixture:
Span doubles 8 → 12 and wraps to 1 at the maximum; the height never moves.
src/dashboard/application/panel-widen.tssays so three times:grid: "doubles span on each press and wraps to 1 at the maximum. Height is always resent unchanged";nextPanelPlacement: "Height is preserved."t-salesauthors no height, so it resolves toDEFAULT_GRID_HEIGHT_UNITS(2) →32 + 88*2= 208px, before and after. The failing assertion demanded> 208and got exactly208.What was actually stale
Both defects come from the grafana-grid@2 style-keyed placement shape, not from the widen step:
toBeGreaterThan(before.height)— demanded the one thing widen must not dotoBe(before.height)— the real invariant; fails if widen ever becomes two-dimensional{ span: 12, height: 4 }— flat v1 shape, and a height that never existed{ grid: { span: 12, height: 2 } }items['t-sales']?.span— v1 path, resolves toundefined, so?? nullturned a shape mismatch into an opaque 5s timeoutitems['t-sales']?.grid?.spanThe narrow-tile spec's expected value (
2) was always correct and is unchanged: the two presses wrap span to 1, and the⋯row it then clicks steps it to 2. I initially misread that as wrong too — the menu click is what makes 2 right.Note the direction of the height fix: asserting equality is strictly stronger here than the old
toBeGreaterThan, because it now pins the documented one-dimensional contract rather than leaving it unconstrained.Quarantine removed
The
test.fail()markers I added in #566 were based on the misdiagnosis. They're gone — these specs pass on their own merits now, no expected-failure bookkeeping.Verification
npx playwright test tests/e2e/tile-open-workbench.spec.js→ chromium 21 passed, webkit 20 passed, quarantine removed. Nothing undersrc/changes; this is a test-only correction.Worth recording why this rotted unnoticed: the placement shape changed under these specs and the browser suite never ran on a pull request until #564. This is the first thing that gate caught.
Checklist
npm testpasses (the per-file coverage gate is non-negotiable) — unaffected, nosrc/or unit changenpm run buildsucceeds (single-filedist/sql.html) — unaffectedsrc/core/, network insrc/net/(injected fetch), DOM insrc/ui/CHANGELOG.md([Unreleased]) updated if behavior or the deployed surface changed — n/a, no behaviour change🤖 Generated with Claude Code
https://claude.ai/code/session_01GiubaoqEuBzAyo5C4P8Vqr