test(configurator): CI-gated unit + e2e regression suites, UX/a11y polish - #313
Merged
Merged
Conversation
The QA sweeps that found the recent configurator bugs ran on throwaway specs — none of those findings had lasting coverage. This adds a permanent chromium suite (configurator/tests-e2e, 33 tests) pinning: - shell: Home landing, per-mode sidebar, mode/domain guards, keyboard cycling, console-clean on every route at 1600/1000/480px - preview: desktop pane toggle, narrow-viewport slide-over (scrim, in-bar theme/motion/close controls), the matchMedia resize round-trip, live repaint on brand-color edits - generators: scalar-only writes (no baked clamp() steps), shared viewport seeding across ramps, resets, garbage-proof edge inputs - presets/knobs: default detection, no cross-preset leftovers, single-undo semantics, shadow-strength encode/decode round-trip - import/export: layer//root round-trip and the garbage-import guard (byte-exact no-op), hostile-input sanitisation, corrupt-storage boot - a11y: horizontal-overflow audit at 5 widths, accessible names, aria-pressed/aria-expanded state, search scoping affordance - history: 5-step mixed chain unwinds and replays byte-for-byte npm run test:e2e builds and lets Playwright manage the preview server (webServer). Also clears the svelte-check baseline to 0 errors / 0 warnings so it can become a CI gate: intentional state capture in ScaleGenerator annotated, diff cells rewrapped (role=cell on spans), standard line-clamp added alongside the -webkit- prefix. https://claude.ai/code/session_01DCCWK2EPSRdhBDZ7f25NxT
The configurator's 346 node:test sync-tripwire tests (and svelte-check) were only ever run locally — no workflow executed them, so a framework token rename could land green while breaking the configurator. New 'Configurator tests' job runs the unit suite, svelte-check (now at 0 errors / 0 warnings) and the chromium e2e regression suite on every push/PR. https://claude.ai/code/session_01DCCWK2EPSRdhBDZ7f25NxT
|
Warning Review limit reached
More reviews will be available in 32 minutes and 21 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (17)
📝 WalkthroughWalkthroughThis PR introduces comprehensive end-to-end test coverage for the configurator using Playwright. It adds test infrastructure (CI job, config, helpers), reusable test utilities, and seven test suites covering shell navigation, presets, generators, preview UI, undo/redo, import/export, and accessibility. Minor component improvements include CSS line-clamp updates and OutputPanel accessibility markup. ChangesConfigurator E2E Test Suite
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
configurator/tests-e2e/shell.spec.js (1)
61-67: ⚡ Quick winStrengthen the rapid-toggle assertion to validate the selected domain, not just uniqueness.
Checking only
toHaveCount(1)can miss domain corruption where a wrong item is still active. Assert the active label remains expected after toggling.Proposed fix
test('rapid mode toggling never corrupts the active domain', async ({ page }) => { const errors = watchErrors(page); await gotoClean(page); await sideItem(page, 'Borders').click(); for (let i = 0; i < 10; i++) await page.keyboard.press(i % 2 ? 'b' : 'a'); - await expect(page.locator('.side__item.side__item--on')).toHaveCount(1); + const active = page.locator('.side__item.side__item--on'); + await expect(active).toHaveCount(1); + await expect(active).toContainText('Borders'); expect(errors).toEqual([]); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@configurator/tests-e2e/shell.spec.js` around lines 61 - 67, After rapid toggling, the test currently only asserts a single active side item via the selector '.side__item.side__item--on' which can miss domain corruption; update the assertion to also verify that the active item's label remains the expected domain (the one clicked earlier, e.g. 'Borders'). Locate the active element (via page.locator('.side__item.side__item--on') or by reusing sideItem helper) and assert its text/content equals 'Borders' (or the expected label) in addition to ensuring count is 1, keeping the existing watchErrors, gotoClean, and toggle loop intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@configurator/tests-e2e/generator.spec.js`:
- Line 72: The current test assertion in generator.spec.js uses a regex
(/^$|NaN|undefined|null/) that can match substrings; replace it with an anchored
full-string alternation so it only matches entire garbage values. Locate the
expect(...) line in generator.spec.js and change the regex to an anchored
non-capturing group (e.g., /^(?:|NaN|undefined|null)$/) so the .not.toMatch
check rejects exactly empty, "NaN", "undefined", or "null" and not substrings of
valid values.
In `@configurator/tests-e2e/helpers.js`:
- Around line 33-37: The helper readOverrides throws on malformed JSON from
localStorage; update readOverrides to mirror the app's tolerant loader by
wrapping the JSON.parse of localStorage.getItem(STORAGE_KEY) in a try/catch (or
otherwise detect parse errors) and return an empty object {} when parsing fails
or the stored value is invalid, ensuring the function always resolves to a plain
object instead of propagating a SyntaxError.
In `@configurator/tests-e2e/undo-redo.spec.js`:
- Around line 11-53: The test currently uses JSON.stringify(await
readOverrides(page)) which is order-dependent; replace those raw
stringifications with a canonicalized snapshot (e.g., a helper like
canonicalize(obj) that recursively sorts object keys and then JSON.stringify)
and call it wherever you push snapshots and in the undo/redo expect checks
(references: snapshots array, readOverrides(page), the pushes after each action,
and the expect comparisons inside the undo/redo loops). Ensure canonicalize is
used both when building snapshots and when comparing after undo/redo so key
order differences cannot cause flakes.
---
Nitpick comments:
In `@configurator/tests-e2e/shell.spec.js`:
- Around line 61-67: After rapid toggling, the test currently only asserts a
single active side item via the selector '.side__item.side__item--on' which can
miss domain corruption; update the assertion to also verify that the active
item's label remains the expected domain (the one clicked earlier, e.g.
'Borders'). Locate the active element (via
page.locator('.side__item.side__item--on') or by reusing sideItem helper) and
assert its text/content equals 'Borders' (or the expected label) in addition to
ensuring count is 1, keeping the existing watchErrors, gotoClean, and toggle
loop intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2962ce09-9111-4938-b297-fca5010239fe
⛔ Files ignored due to path filters (1)
configurator/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
.github/workflows/ci.ymlconfigurator/package.jsonconfigurator/playwright.config.jsconfigurator/src/components/Cheatsheet.svelteconfigurator/src/components/Home.svelteconfigurator/src/components/OutputPanel.svelteconfigurator/src/components/ScaleGenerator.svelteconfigurator/tests-e2e/a11y.spec.jsconfigurator/tests-e2e/generator.spec.jsconfigurator/tests-e2e/helpers.jsconfigurator/tests-e2e/import-export.spec.jsconfigurator/tests-e2e/presets.spec.jsconfigurator/tests-e2e/preview.spec.jsconfigurator/tests-e2e/shell.spec.jsconfigurator/tests-e2e/undo-redo.spec.js
…meta - Escape dismisses the slide-over preview overlay (narrow viewports only; the desktop pane is a layout region, not a dialog); on open the overlay focuses its close button and returns focus to the header toggle when it unmounts - Navigation prefs (mode / domain / output format) persist across reloads via a new validated localStorage slot — lib/uiState.js keeps the validator pure and unit-tested (invalid domains for the saved mode are dropped before they can flash a redirect) - index.html gains theme-color, an inline SVG favicon and basic OG/Twitter meta for sharing the public tool - cleanups deferred from the QA review: shared lib/clipboard.js replaces three divergent copy implementations (unified 1400ms feedback), DomainPanel card headers collapse into one snippet (4 duplicates -> 1), Home rows always show the intended copy (intro for domains, blurb for tools) - e2e suite extended to 36 tests (escape/focus behavior, persisted prefs incl. corrupt-slot fallback); unit suite to 352 https://claude.ai/code/session_01DCCWK2EPSRdhBDZ7f25NxT
- helpers.readOverrides tolerates malformed persisted JSON (mirrors the app's loader) so corrupt-storage scenarios fail on app behavior, not helper parsing - undo/redo snapshots canonicalised via stableSnapshot (sorted keys) — byte-for-byte comparisons no longer depend on key insertion order - garbage-value regex anchored to full-string matches - rapid-toggle test also asserts the active domain stays Borders https://claude.ai/code/session_01DCCWK2EPSRdhBDZ7f25NxT
This was referenced Jun 25, 2026
Merged
5 tasks
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
2 participants
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
Hardening follow-up to #311/#312 (both merged): the QA sweeps that found the recent configurator bugs ran on throwaway Playwright specs, and — more importantly — the configurator's 346-test unit suite (the whole sync-tripwire net) was never executed by any CI workflow. This PR makes both permanent, then ships the UX/a11y polish batch.
CI + permanent e2e suite
configurator/tests-e2e/Playwright suite (33 tests, chromium) pinning every behavior verified during the QA sweeps: shell guards & keyboard cycling, preview pane/overlay/resize round-trip, scalar-only generator writes with shared-viewport seeding, preset semantics (no cross-preset leftovers, single-undo), the garbage-import byte-exact no-op guard, hostile-input sanitisation, corrupt-storage boot, horizontal-overflow audit at 5 widths, aria state, and a 5-step mixed undo/redo chain replayed byte-for-byte.npm run test:e2eprebuilds; Playwright manages the preview server viawebServer.Configurator tests: configurator unit suite (npm test),svelte-checkas a type gate, and the e2e suite — on every push/PR.role="cell", standardline-clampadded).UX / a11y polish (follow-up commits on this PR)
index.html: theme-color, SVG favicon, OG/Twitter meta?? d.blurbfallback removedTesting
cd configurator && npm test— 346/346npm run check— 0 errors / 0 warningsnpm run test:e2e— 33/33https://claude.ai/code/session_01DCCWK2EPSRdhBDZ7f25NxT
Generated by Claude Code
Summary by CodeRabbit
Bug Fixes
Tests
Chores