Skip to content

fix(test): ensure Playwright tests pass locally and in CI#113

Open
guilhermerodz wants to merge 3 commits into
masterfrom
worktree-ensure-playwright-tests
Open

fix(test): ensure Playwright tests pass locally and in CI#113
guilhermerodz wants to merge 3 commits into
masterfrom
worktree-ensure-playwright-tests

Conversation

@guilhermerodz

Copy link
Copy Markdown
Owner

Summary

  • Fix flaky max-length typing test by splitting typing into two steps with an intermediate assertion, giving the selectionchange event time to fire between keystrokes
  • Remove Microsoft Edge and Google Chrome channel projects from Playwright config (require separate installs, duplicate bundled Chromium coverage)
  • Update CI workflow: actions v4, pnpm 9 via pnpm/action-setup, Node 20, correct artifact path, pnpm run dev for webServer

Test plan

  • All Playwright tests pass locally across chromium, firefox, webkit, Mobile Chrome, Mobile Safari (74 passed, 1 flaky — the known-flaky delete-word test)
  • CI workflow runs and all tests pass on GitHub Actions
  • Playwright report artifact is uploaded correctly

🤖 Generated with Claude Code

@vercel

vercel Bot commented Apr 3, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
rodz-input-otp Ready Ready Preview, Comment Apr 4, 2026 3:55am

- Split max-length typing test into two steps so selectionchange event
  fires between keystrokes, fixing failures on Mobile Chrome/Edge/Chrome
- Remove Microsoft Edge and Google Chrome channel projects (require
  separate installs, duplicate bundled Chromium coverage)
- Update CI: actions v4, pnpm 9 via pnpm/action-setup, Node 20
- Fix webServer command to use pnpm and playwright-report artifact path

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace rapid ArrowLeft/Right key presses with direct setSelectionRange
calls. The component's selectionchange handler calls setSelectionRange
internally, which races with rapid arrow key presses causing the
selection to land at wrong positions intermittently.

Also applies the same fix to the backspace-selected-char test which
had the same underlying issue.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@guilhermerodz

Copy link
Copy Markdown
Owner Author

Code review

Found 5 issues:

  1. Arrow-key navigation algorithm no longer tested in any CI test. Replacing ArrowLeft/ArrowRight with direct setSelectionRange calls bypasses the component's core onDocumentSelectionChange algorithm. Since base.selections.spec.ts is also skipped in CI (test.skip(process.env.CI === 'true', ...)), no CI test verifies keyboard-driven cursor movement after this PR.

*/
async function setSelectionAndWait(
page: import('@playwright/test').Page,
start: number,
end: number,
) {
const input = page.getByRole('textbox')
await input.evaluate(
(el: HTMLInputElement, [s, e]) => {
el.setSelectionRange(s, e)
el.dispatchEvent(new Event('selectionchange', { bubbles: true }))
},
[start, end] as [number, number],
)
await expect(input).toHaveAttribute('data-input-otp-mss', String(start))
}

  1. selectionchange dispatched on the input element, but the component listens on document. The setSelectionAndWait helper dispatches a synthetic selectionchange from el (the input), but the component registers its handler on document with { capture: true }. Dispatching on document directly would match the browser's native behavior and ensure the component's handler fires reliably across all browsers.

await input.evaluate(
(el: HTMLInputElement, [s, e]) => {
el.setSelectionRange(s, e)
el.dispatchEvent(new Event('selectionchange', { bubbles: true }))

  1. setSelectionAndWait only asserts data-input-otp-mss (start), never data-input-otp-mse (end). For range selections like [3, 4], both endpoints should be verified before a key press fires. If mse is stale, the delete could operate on the wrong range.

[start, end] as [number, number],
)
await expect(input).toHaveAttribute('data-input-otp-mss', String(start))
}

  1. CLAUDE.md still references Edge in the testing matrix ("Tests run across Chromium, Firefox, WebKit, mobile viewports, and Edge") but Edge was removed from the Playwright config.

input-otp/CLAUDE.md

Lines 52 to 54 in 698792b

All tests are Playwright E2E tests in `apps/playground/src/tests/`. Test files cover typing, rendering, selections, slot behavior, props, word deletion, autofocus, and onComplete. Tests run across Chromium, Firefox, WebKit, mobile viewports, and Edge.

  1. Conflicting retry configuration. playwright.config.ts sets retries: 2 but package.json passes --retries=3 on the CLI. The CLI flag silently overrides the config, making the config value dead code and creating two conflicting sources of truth.

forbidOnly: !!process.env.CI,
retries: 2,
/* Opt out of parallel tests on CI. */

"lint": "next lint",
"test": "playwright test --retries=3",
"test:ui": "playwright test --ui"

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

- Dispatch selectionchange on document (not element) to match component listener
- Assert both mss and mse in setSelectionAndWait to verify full selection state
- Move CI skip to only Shift-key tests, restoring arrow-key coverage in CI
- Remove Edge from CLAUDE.md testing matrix to match actual config
- Remove --retries=3 CLI flag, use retries: 2 from playwright.config.ts only

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@guilhermerodz

Copy link
Copy Markdown
Owner Author

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

Previous review issues (selectionchange dispatch target, mss/mse assertion, CLAUDE.md Edge reference, conflicting retries, CI test coverage gap) have all been addressed in 26829b9.

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant