test(e2e): retarget the editor specs at the v4 shell#166
Merged
Conversation
… click FloatingInspector floats its "Collapse inspector" button over the reused pane header (position: absolute, right: 12, 26px wide). That lands squarely on whatever the header puts at its right edge and eats its clicks. The Help button was the casualty: rendered on every facet, visible, and unreachable by mouse. CaptionsPane's status icon sits under it too. Fixed where all three headers share a definition rather than nudging the one button: .paneHead now reserves the floated control its own lane. Every pane wearing that class is rendered inside FloatingInspector, so there is no other caller to regress. Caught by the e2e rewrite in the following commit.
The three editor specs were written against the pre-v4 shell and could not pass: they waited on a `timeline-pane` testid that died with TimelinePane.tsx, read data-clip-count / data-zoom-multiplier / data-px-per-sec attributes the v4 timeline never had, and seeded the browser shim through the localStorage keys it stopped reading when it moved to `browser-shim-projects-v2`. Their fixtures were schemaVersion 3, so they exercised the v3→v4→v5 migration path rather than the shape the app actually stores. roadmap-coverage.spec.ts → v4-shell.spec.ts, renamed off the retired roadmap task ids, three behaviours kept and two dropped: - asset file size, now read off MediaStage's cards; the sized and unsized branches share one fixture instead of two page loads - the facet help popover, still live in RightPanes' Pane but reached from the inspector rail rather than a right-panel tab. Its "Settings for X" fallback assertion is gone with the fallback itself — every pane has real help text - ctrl+wheel zoom, which no longer has a px-per-sec ceiling; v4 zooms a nav window bounded at a 2% minimum span, so that is what the test measures Dropped: the ruler hover-guide, which does not exist in v4 (the only ruler overlay left is the pill-drag snap guide), and the chat "applied:" case, which asserted that the browser shim's canned reply carries no tool calls — the mock, not the app, and the positive half is unreachable without a real provider. seed.spec.ts → editor-smoke.spec.ts. Its timeline half was unsalvageable but its preview half still holds, and it seeds nothing, which makes it the only cover for the cold-boot branch (Preview → EditorEmptyState) and for a crash during mount against an empty shim. Now also listens for pageerror, not just console.error — an uncaught mount exception is the failure a boot smoke test exists to catch. diagnostic.spec.ts deleted. It dumped the DOM and a11y tree to find selectors in the old shell and ended in expect(true).toBe(true); the selectors it existed to discover are written down in v4-shell.spec.ts now. Both specs read E2E_BASE_URL, so tests/e2e no longer hardcodes a port. E2E_BASE_URL=http://localhost:5199 npx playwright test tests/e2e/v4-shell.spec.ts tests/e2e/editor-smoke.spec.ts → 4 passed
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
EtienneLescot
added a commit
that referenced
this pull request
Jul 26, 2026
ci.yml was scoped to main only, so every PR landing on the long-lived feat/ai-edition integration branch merged with no lint, no typecheck, no tests and no PR-title check. Adds a Docs job running check-docs.mjs. It skips the shared setup action on purpose: the script imports only node builtins, so npm ci would cost a minute for nothing. Drops this branch's comment edit to tests/e2e/roadmap-coverage.spec.ts — #166 deletes that file, and modify-vs-delete is a conflict for no gain.
EtienneLescot
added a commit
that referenced
this pull request
Jul 26, 2026
ci.yml was scoped to main only, so every PR landing on the long-lived feat/ai-edition integration branch merged with no lint, no typecheck, no tests and no PR-title check. Adds a Docs job running check-docs.mjs. It skips the shared setup action on purpose: the script imports only node builtins, so npm ci would cost a minute for nothing. Drops this branch's comment edit to tests/e2e/roadmap-coverage.spec.ts — #166 deletes that file, and modify-vs-delete is a conflict for no gain.
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.
Summary
The three Playwright editor specs were written against the pre-v4 shell and could not pass on this branch. They waited on a
timeline-panetestid that died withTimelinePane.tsx, readdata-clip-count/data-zoom-multiplier/data-px-per-secattributes the v4 timeline never had, and seeded the browser shim through thelocalStoragekeys it stopped reading when it moved tobrowser-shim-projects-v2. Their fixtures wereschemaVersion: 3, so they exercised the v3→v4→v5 migration path rather than the shape the app actually stores.Decided per file — rewrite against v4, or delete:
roadmap-coverage.spec.tsv4-shell.spec.ts(renamed off the retired roadmap task ids)seed.spec.tseditor-smoke.spec.tsdiagnostic.spec.tsv4-shell.spec.ts— three behaviours kept, two dropped:MediaStage's cards; the sized and unsized branches now share one fixture instead of two page loadsRightPanes'Panebut reached from the inspector rail rather than a right-panel tab. Its "Settings for X" fallback assertion is gone along with the fallback itself — every pane has real help text nowDropped: the ruler hover-guide, which does not exist in v4 (the only ruler overlay left is the pill-drag snap guide), and the chat "applied:" case, which asserted that the browser shim's canned reply carries no tool calls — the mock, not the app, and the positive half is unreachable without a real provider.
editor-smoke.spec.ts—seed.spec.ts's timeline half was unsalvageable, but its preview half still holds (data-testid="preview"and its transport attributes survive), and it seeds nothing. That makes it the only cover for the cold-boot branch (Preview→EditorEmptyState) and for a crash during mount against an empty shim. It now also listens forpageerror, not justconsole.error— an uncaught mount exception is exactly the failure a boot smoke test exists to catch.diagnostic.spec.ts— deleted. It dumped the DOM and a11y tree to find selectors in the old shell and ended inexpect(true).toBe(true), so it could never fail. The selectors it existed to discover are written down inv4-shell.spec.tsnow.Both specs read
E2E_BASE_URL, sotests/e2eno longer hardcodes a port.One real bug, caught by the rewrite
The help-popover test failed on a defect, not a selector.
FloatingInspectorfloats its "Collapse inspector" button over the reused pane header (position: absolute; right: 12, 26px wide), landing squarely on the Help button and eating its clicks — Help is visible on every facet and unreachable by mouse. Confirmed by measurement: Help atx=265,w=28, collapse atx=271,w=26.CaptionsPane's status icon sits under it too.Fixed in
1210c920where all three headers share a definition rather than nudging the one button:.paneHeadreserves the floated control its own lane. Every pane wearing that class renders insideFloatingInspector, so there is no other caller to regress.Related issue
Type of change
Release impact
Desktop impact
Screenshots / video
The CSS change is a 46px right padding on the inspector's pane header. Measured rather than screenshotted, since the point is the hit-testing, not the look: after the fix Help spans
235–263and the collapse button271–297, no overlap, and the pane title does not truncate at the 298px panel width.Testing
Verified the help popover fails before the CSS commit and passes after.
biome check tests/e2e/andtsc --noEmitboth clean. Run against a dev server on thevite-deventry in.claude/launch.json.