Skip to content

Add playwright tests for diffs and edit mode#927

Open
mdo wants to merge 2 commits into
beta-1.3from
mdo/pw-diffs-edit
Open

Add playwright tests for diffs and edit mode#927
mdo wants to merge 2 commits into
beta-1.3from
mdo/pw-diffs-edit

Conversation

@mdo

@mdo mdo commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Add browser E2E tests for diffs & edit mode

Adds a Playwright E2E suite for @pierre/diffs, covering behavior the jsdom
unit tests can't reach: shadow-DOM rendering and real contentEditable editing.
Mirrors the existing trees E2E setup — a Vite server serves the built dist
and Playwright drives real Chromium against static HTML fixtures.

Coverage

  • Diff rendering: split/unified toggle, hunk expansion, accept/reject
  • Edit mode: typing, onChange, undo/redo, additions-editable vs
    deletions-read-only, full-file rewrite
  • Find & replace, multi-cursor + indent/outdent, markers (hover +
    bounds), line selection + gutter utility, annotations,
    theme switching (selection-stable), selection actions (in-bounds)
  • Merge conflicts and added/deleted file states

Notable fix

  • Editor now falls back to ShadowRoot.getSelection() when
    Selection.getComposedRanges is unavailable (older browsers, WebViews, and
    the pinned CI Chromium), so a click reliably seeds the caret and typing works.

Infra

  • diffs:test-e2e + diffs:test-e2e-server moon tasks; diffs:test-e2e wired
    into CI
  • Reserves worktree port 4175 (scripts/wt.ts, scripts/README.md)
  • Shared test/e2e/e2e-globals.d.ts for fixture window globals

Running the tests

Run the full suite: moon run diffs:test-e2e

View the fixtures in your own browser — start the server and open the index:

The index page lists every fixture with a description, its ready-flag, and the
spec that drives it.

@vercel

vercel Bot commented Jul 1, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
pierre-docs-diffs Ready Ready Preview Jul 1, 2026 6:53pm
pierre-docs-diffshub Ready Ready Preview Jul 1, 2026 6:53pm
pierre-docs-trees Ready Ready Preview Jul 1, 2026 6:53pm
pierrejs-diff-demo Ready Ready Preview Jul 1, 2026 6:53pm

Request Review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b170f67cf1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

loadWorktreeEnv();

const portOffset = Number(process.env.PIERRE_PORT_OFFSET ?? 0);
const e2ePort = 4175 + portOffset;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Choose a port that does not collide with path-store demo

In a worktree where path-store's demo server is running, this new diffs E2E port uses the same 4175 + PIERRE_PORT_OFFSET base as packages/path-store/moon.yml:25. Since both Vite servers use strictPort, diffs:test-e2e will fail to start instead of choosing another port, and the worktree port tools now also report/clean that shared port as diffsE2E; please pick an unused base and update the config/port map together.

Useful? React with 👍 / 👎.

@mdo

mdo commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

Updated summary

Added Playwright E2E coverage for the diffs and edit-mode core UX that wasn't previously exercised in the browser: playground controls/interactions/edit-mode/deep-links, the /edit demo page (Selection, Markers, Find, History, Shortcuts), and the homepage examples (Token Hover, Accept/Reject, Merge Conflict, Split/Unified). /edit/live (AgentUi) is intentionally out of scope for this pass.

Since apps/docs is a dual-flavor Next.js build (NEXT_PUBLIC_SITE=diffs vs trees), and /playground//edit/homepage examples only exist in the diffs flavor, Playwright now runs two webServers and two projects (trees-site / diffs-site), routed via testMatch/testIgnore. Ports, scripts/wt.ts, and apps/docs/moon.yml's test-e2e deps were updated accordingly.

New test files

  • playground-controls.pw.ts — layout/theme/indicators/hunk separators + URL sync
  • playground-interactions.pw.ts — line selection, gutter comments
  • playground-edit-mode.pw.ts — Review/Edit toggle, markers toggle, contenteditable
  • playground-deep-links.pw.ts — query param load, copy-link clipboard
  • edit-demos.pw.ts — Find/History/Marker/Selection/Shortcuts demos on /edit
  • home-examples.pw.ts — accept-hunk UI, token hover, merge conflict, split/unified

Bugs found along the way (in the demos/tests, not the library)

  • Platform-detection mismatch under Playwright's UA emulation: devices['Desktop Chrome'] spoofs navigator.userAgentData.platform while leaving legacy navigator.platform untouched. The Find/History demos resolve Cmd/Ctrl from userAgentData.platform first; the editor resolves it from navigator.platform first — under Playwright these disagree and the demos' synthetic shortcut dispatches silently no-op. Patched via page.addInitScript in edit-demos.pw.ts.
  • Sticky header covering a hover target: the docs site's sticky top header overlaps the first ~60px of viewport; scrollIntoViewIfNeeded() doesn't account for it, so a "hover the token" test was really hovering the header. Fixed by centering the token instead.
  • [data-search-panel] is intentionally zero-height (a position: sticky CSS trick) — assertions now target its [data-editor-widget] child.
  • Split-diff line selection marks two DOM nodes per side (numbered row + trailing "buffer" row) × two gutters — selectors tightened to avoid over/under-counting.
  • A deep-link test used edit=1 where the app expects the literal edit=edit.

Verification

  • moonx docs:test-e2e (both projects) green; diffs-site (27 tests) reconfirmed stable across multiple repeated runs.
  • moon run root:format root:lint clean (pre-existing warnings only, unrelated to this change).
  • Noted one pre-existing, unrelated flaky test (trees-dev-item-customization.pw.ts, tree expand/collapse) — untouched by this PR, out of scope.

@necolas

necolas commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

This is a pretty big change all at once. But shouldn't the pw tests enable us to remove the unit/integration tests that do a lot of browser emulation? I'd get Claude to review which existing jsdom tests are being replaced by these tests and remove them from the unit test suite.

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.

2 participants