test: add Playwright e2e for playgrounds (dev + built)#990
Merged
Conversation
Drives the Vite DevTools dock entry directly to verify the Nuxt DevTools iframe loads, the side nav renders, and main tabs navigate with expected content. Covers `empty`, `tab-pinia`, and `tab-seo` playgrounds in both dev and built modes; pre-builds in globalSetup, disables Vite DevTools client auth for headless runs, and skips the known `tab-seo:built` issue (`useNuxtDevTools` not defined in prod bundle). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Deploying nuxt-devtools with
|
| Latest commit: |
1d56ea9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1207fca9.nuxt-devtools.pages.dev |
| Branch Preview URL: | https://antfu-e2e-playgrounds.nuxt-devtools.pages.dev |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Playwright-based E2E test infrastructure: package scripts and dependency, pnpm catalog entry, Playwright config with per-playground projects and webServer setup, global setup to pre-build playgrounds, DevTools fixtures, multiple E2E spec files covering DevTools and playground behavior, a tsconfig for tests, a glob utility for project filtering, .gitignore entries for Playwright artifacts, and a GitHub Actions workflow to run the E2E suite and upload the Playwright report. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/e2e/global-setup.ts`:
- Around line 10-21: The Promise that wraps the spawn call in
tests/e2e/global-setup.ts doesn't listen for child-process 'error' events and
also omits the terminating signal when reporting failures; update the spawn
handling around the child variable so you add child.on('error', err =>
reject(new Error(`Failed to spawn pnpm/nuxt for ${name}: ${err.message ||
err}`))) and modify the existing child.on('exit', (code, signal) => ...) branch
in the Promise to reject with a message that includes either the non-zero exit
code or the terminating signal (e.g., `Build failed for ${name}: exit code
${code}` or `terminated by signal ${signal}`) while keeping resolve() when code
=== 0.
In `@tests/e2e/playwright.config.ts`:
- Around line 32-38: The PW_PROJECT filter is being turned into a RegExp without
escaping regex metacharacters, causing crashes or incorrect matches; update the
logic around filter/specs to first escape special regex characters (e.g.,
implement an escapeRegExp helper that replaces characters like .+?^${}()|[]\\
with escaped versions), then convert glob-style '*' to '.*' and build the RegExp
from that escaped pattern; extract this into a shared helper function (e.g.,
buildProjectFilterRegex or escapeAndGlobToRegex) and use it both where filter is
computed (the current const filter/specs block) and in the duplicated logic in
global-setup.ts so both places reuse the safe implementation.
In `@tests/e2e/specs/playground-loads.spec.ts`:
- Around line 27-29: The test currently builds `fatal` from `consoleErrors`
using a broad regex (/HMR|404|favicon|websocket|ws/i) that can hide real
regressions; update the filter used to compute `fatal` so it only ignores
specific benign messages (e.g., exact HMR warnings and known favicon 404 lines
and explicit websocket connection warnings) rather than any occurrence of "404"
or "ws". Locate the `consoleErrors.filter(...)` expression (variable `fatal`)
and replace the broad alternations with tightened patterns that match the full
benign message text or anchored phrases (keep `HMR` and `favicon` matches narrow
and change `404` and `ws|websocket` to their exact harmless messages) so genuine
errors still fail the smoke test. Ensure the new regexes remain case-insensitive
and include explanatory comments next to the `fatal` filter.
In `@tests/e2e/specs/playground-tab-pinia.spec.ts`:
- Around line 13-15: The negative loader check is using the wrong string
('Connecting....' with four dots) so it can false-positive; update the assertion
in the expect call that targets devtoolsFrame().locator('body') to use the exact
loader text 'Connecting...' (three dots) in the .not.toContainText(...) call and
keep the same timeout parameter.
🪄 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
Run ID: c6dc7d46-b70e-4315-90f4-82971d923d1b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
.github/workflows/ci.yml.gitignorepackage.jsonpnpm-workspace.yamltests/e2e/fixtures/devtools.tstests/e2e/global-setup.tstests/e2e/playwright.config.tstests/e2e/specs/iframe.spec.tstests/e2e/specs/playground-empty.spec.tstests/e2e/specs/playground-loads.spec.tstests/e2e/specs/playground-tab-pinia.spec.tstests/e2e/specs/playground-tab-seo.spec.tstests/e2e/specs/tabs.spec.tstests/e2e/tsconfig.json
Comment on lines
+13
to
+15
| await expect(devtoolsFrame().locator('body')) | ||
| .not | ||
| .toContainText('Connecting....', { timeout: 30_000 }) |
Contributor
There was a problem hiding this comment.
Negative loader check has a false-positive typo.
Line 15 checks 'Connecting....' (four dots), while the loader text is 'Connecting...' (three dots). That mismatch can let the test pass even when the app is still stuck connecting.
Suggested fix
- await expect(devtoolsFrame().locator('body'))
- .not
- .toContainText('Connecting....', { timeout: 30_000 })
+ await expect(devtoolsFrame().locator('body'))
+ .not
+ .toContainText('Connecting...', { timeout: 30_000 })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/e2e/specs/playground-tab-pinia.spec.ts` around lines 13 - 15, The
negative loader check is using the wrong string ('Connecting....' with four
dots) so it can false-positive; update the assertion in the expect call that
targets devtoolsFrame().locator('body') to use the exact loader text
'Connecting...' (three dots) in the .not.toContainText(...) call and keep the
same timeout parameter.
- Run e2e in autofix.yml (PR CI) — ci.yml only runs on push to main - Escape regex metacharacters in PW_PROJECT glob, share between playwright.config.ts and global-setup.ts via a small helper - Handle child-process spawn failures and propagate signal in build error - Narrow benign-error filter in playground-loads.spec.ts (no longer hides unrelated 404s or websocket errors) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Run e2e tests as their own job so the result is a separate signal from lint/build/typecheck (and so it parallelizes with the existing ci/autofix workflows). Triggers on push to main and on PRs (excluding docs-only changes), with browser cache + report artifact on failure. Removes the duplicated e2e step from ci.yml and autofix.yml. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previous run looped on dev tests for `../../local` playgrounds (tab-pinia, tab-seo) — every test hit the 90s test timeout and (with retries: 2) tried 3x, dragging the job past 30 minutes before manual cancellation. Adding: - `timeout-minutes: 15` on the e2e job so a stuck run is bounded - `list` reporter prepended to CI reporters so per-test results show in workflow logs (the previous github+html-only reporters were silent on intermediate progress, leaving only WebServer dumps to debug from) - `retries: 1` everywhere (was 2 in CI) — one retry covers the cold-boot flake without amplifying genuine timeouts Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Local Claude Code state directory; not project content. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The tab-pinia and tab-seo playgrounds load DevTools via `../../local`, which spawns a separate Nuxt dev subprocess for the devtools client. On CI cold-start runners that subprocess can't compile the inner client app within the test's 90s budget — both the original attempt and the retry hit the timeout (verified in run 25207464186), and the job runs the full 15-min cap before terminating. Run the dev-mode devtools coverage on the `empty` playground only (which uses the prebuilt `@nuxt/devtools` and doesn't need the subprocess), and keep the built-mode smoke check across all three playgrounds. Local `pnpm test:e2e` still exercises the full matrix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
globalSetup runs concurrently with webServer startup, so when the built project's webServer ran \`nuxt preview\` it failed before the build finished — Playwright then aborted the run before any test fired (last CI run, build step never logged \`Pre-building empty...\`). Move the prebuild to a \`test:e2e:prebuild\` script invoked before \`PW_PROJECT='*:built' playwright test\` (locally and in CI). The dev projects don't need a prebuild, so they go straight to playwright. This also drops globalSetup entirely — \`tests/e2e/shared/glob.ts\` is still used by playwright.config.ts to filter projects + webServer entries. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
\`nuxt build\` for tab-pinia/tab-seo hangs on CI: after the first such playground builds, the next one's build never starts (last run, the third build never logged \`Building Nuxt for production\` and the job ran the full 15-min cap). The hang is in \`../../local\` — it spawns a subprocess for the devtools client, and a stale subprocess from the previous build appears to block the next build's app:resolve hook from completing. That's the same module path which already prevented those playgrounds from running dev tests on CI. Drop them from built-mode CI too. Coverage for the full matrix stays in local \`pnpm test:e2e\`; CI verifies that \`empty\` (prebuilt \`@nuxt/devtools\`, no subprocess) works in both modes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
All bundled playgrounds switch their devtools module via a \`NUXT_DEVTOOLS_LOCAL\` env var: - unset (default): load \`@nuxt/devtools\` (the published/built package, resolved to the workspace dist via pnpm). No subprocess. Tests can exercise the same code path consumers ship. - set: load \`../../local\`, the dev-mode entry that proxies to a Nuxt dev subprocess for HMR on the devtools client. Devs working on the client itself opt in this way. Previously most playgrounds hard-coded \`../../local\`; that subprocess hangs nuxt build on cold CI runners and times out dev iframe tests. Defaulting to the built module unblocks CI for all three e2e playgrounds (empty, tab-pinia, tab-seo). Local \`pnpm test:e2e\` and \`pnpm dev\` still work with HMR by setting \`NUXT_DEVTOOLS_LOCAL=1\`. CI workflow simplified to a single \`pnpm test:e2e\` step now that all playgrounds work with the default module path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
tests/e2e/covering 3 playgrounds (empty,tab-pinia,tab-seo) in both dev and built modes — verifies the Nuxt DevTools iframe loads, the side nav renders, and 8 main tabs navigate with expected content.switchEntry) and bypasses the brokenhost.devtools.navigate()(which writes to a non-existentpanel.store.value) by calling thehost:action:navigatehook directly.pnpm test:e2e(sequential dev → built) intoci.ymlwith browser cache + report artifact on failure; pre-builds playgrounds inglobalSetupto keep webServer boot fast; disables Vite DevTools client auth (VITE_DEVTOOLS_DISABLE_CLIENT_AUTH=true) so headless runs are trusted; useslocalhostbaseURL withHOST=0.0.0.0to work around macOS IPv6/IPv4 split between the main app server and the Vite DevTools websocket.tab-seo:builtis skipped with a comment — its production bundle currently throwsuseNuxtDevTools is not defined(auto-import fromlocal.tsdoesn't bake into the prod build).Test plan
pnpm test:e2e:dev→ 34 passed, 8 skippedpnpm test:e2e:built→ 2 passed, 40 skippedpnpm test:e2e(full suite) → 36 passed, 48 skipped, 0 failed (~83s)pnpm exec eslint tests/e2eclean🤖 Generated with Claude Code