Add Playwright E2E testing framework #1836
Conversation
- Add Playwright configuration in spec/dummy for cross-browser testing - Create comprehensive test suites for React on Rails components - Add test helpers and utilities for React on Rails testing - Add rake tasks for running Playwright tests in dummy app - Update dummy app package.json with Playwright scripts - Document Playwright usage in CLAUDE.md - Configure ESLint to ignore Playwright test files Playwright tests are properly located in spec/dummy where the actual Rails application lives. This provides a robust E2E testing framework that complements the existing RSpec and Jest tests, ensuring components work correctly across different browsers and scenarios. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Rate limit exceeded@justin808 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 1 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughIntroduces Playwright end-to-end testing across the repo and dummy app: adds dependencies, configs, scripts, Rake tasks, test helpers, and multiple test suites; updates ESLint and .gitignore ignores; expands documentation (including a duplicated section in CLAUDE.md). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Rake as Rake Tasks
participant Yarn as Yarn/NPM
participant PW as Playwright CLI
participant Rails as Rails Web Server
participant Browser as Playwright Browser
participant App as App Under Test
participant Artifacts as Reports/Results
Dev->>Rake: rake playwright:test / e2e
Rake->>Yarn: run script (e.g., test:e2e)
Yarn->>PW: playwright test
PW->>Rails: start via webServer command
Rails-->>PW: server ready at baseURL
PW->>Browser: launch projects (chromium/firefox/webkit)
Browser->>App: navigate to baseURL
App-->>Browser: SSR HTML
Browser-->>App: execute JS, hydrate
Browser->>PW: test results, traces, screenshots
PW->>Artifacts: write report/results
PW-->>Dev: exit code and summary
note over PW,Artifacts: HTML report available via "playwright show-report"
sequenceDiagram
autonumber
participant Test as Test Code
participant Browser as Playwright Page
participant ROR as ReactOnRails
participant DOM as Document
Test->>Browser: page.goto("/")
Browser-->>DOM: render SSR
Test->>Browser: wait for networkidle
Test->>Browser: evaluate wait for ROR.mountedComponents
Browser->>ROR: check hydration state
ROR-->>Browser: hydrated
Test->>DOM: interact with inputs/selectors
DOM-->>Test: updated text/content assertions pass
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
🔍 Code Review: Playwright E2E Testing FrameworkThank you for adding Playwright E2E testing to React on Rails! This is a valuable addition. I've conducted a thorough review and identified several areas that need attention before merging. ✅ Strengths
🚨 Critical Issues (Must Fix)1. Type Safety Violations in test-helpers.ts
2. Version Mismatch
3. Incomplete Test Implementation
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (18)
.gitignore (1)
70-73: Consider also ignoring the blob reporter outputIf you enable Playwright’s blob reporter in CI, add:
+/blob-report/This complements
/playwright-report/and/test-results/.eslint.config.ts (1)
51-55: Don’t blanket-ignore Playwright tests; lint them with relaxed rulesIgnoring
spec/dummy/playwright/**/*.ts|jsskips linting altogether. Keep the config file ignored, but include tests via an override and relax problematic rules instead.Apply within this segment:
// Playwright test files - 'spec/dummy/playwright.config.ts', - 'spec/dummy/playwright/**/*.ts', - 'spec/dummy/playwright/**/*.js', + 'spec/dummy/playwright.config.ts',Then add an override to the config (near other overrides):
{ files: ['spec/dummy/playwright/**/*.{ts,tsx,js,jsx}'], rules: { // E2E specs often use console and Node-ish patterns 'import/no-unresolved': 'off', 'no-console': 'off', }, },This preserves lint value without noisy false-positives in the dummy app.
CLAUDE.md (1)
136-143: Clarify install location for PlaywrightMake it explicit to run installs in the dummy app to avoid adding deps at the repo root:
-# Install Playwright and its dependencies -yarn add -D @playwright/test -yarn playwright install --with-deps # Install browsers +# Install Playwright and its dependencies in the dummy app +cd spec/dummy +yarn add -D @playwright/test +yarn playwright install --with-deps # Install browsersspec/dummy/playwright/tests/turbolinks-integration.spec.ts (3)
1-1: Use the custom fixturestestto wait for hydration and reduce flake.Import the extended
testfrom fixtures so you can callwaitForReactOnRails(). Ifexpectisn’t re-exported from fixtures, keep it from@playwright/test.-import { test, expect } from '@playwright/test'; +// Prefer project fixtures for ReactOnRails helpers +import { test, expect } from '../fixtures/test-helpers';If
fixtures/test-helpers.tsdoes not re-exportexpect, change the import to:-import { test, expect } from '../fixtures/test-helpers'; +import { test } from '../fixtures/test-helpers'; +import { expect } from '@playwright/test';
7-13: Detect Turbo as well as Turbolinks and assert mounted components, not just presence.Right now the test silently does nothing if Turbolinks is absent. Support Turbo and fail fast or skip when neither is present; also assert mounted components via ReactOnRails rather than a boolean check.
- // Check if Turbolinks is loaded (if enabled) - const hasTurbolinks = await page.evaluate(() => { - return typeof (window as any).Turbolinks !== 'undefined'; - }); + // Detect Turbolinks or Turbo + const { hasTurbolinks, hasTurbo } = await page.evaluate(() => ({ + hasTurbolinks: typeof (window as any).Turbolinks !== 'undefined', + hasTurbo: typeof (window as any).Turbo !== 'undefined', + })); + + if (!hasTurbolinks && !hasTurbo) { + test.skip(true, 'Turbolinks/Turbo not enabled in this app'); + } @@ - // Verify React on Rails components are still mounted - const componentRegistry = await page.evaluate(() => { - return typeof (window as any).ReactOnRails !== 'undefined'; - }); - expect(componentRegistry).toBeTruthy(); + // Verify React on Rails components are mounted + await waitForReactOnRails?.(); // from fixtures + const mountedCount = await page.evaluate(() => { + const ROR = (window as any).ReactOnRails; + return ROR && ROR.mountedComponents ? ROR.mountedComponents.length : 0; + }); + expect(mountedCount).toBeGreaterThan(0);Also applies to: 18-23
36-41: Anchor selector only catches Turbo; include Turbolinks and wait for navigation.Broaden the selector and wait for URL change to stabilize assertions.
-const links = await page.locator('a[data-turbo]').count(); +const linkLocator = page.locator('a[data-turbo], a[data-turbolinks]'); +const links = await linkLocator.count(); if (links > 0) { - await page.locator('a[data-turbo]').first().click(); + const prevUrl = page.url(); + await linkLocator.first().click(); + await page.waitForURL((url) => url.toString() !== prevUrl);spec/dummy/playwright/README.md (1)
81-86: Clarify config location and CI artifacts (optional).Small clarity wins: call out the config path and suggest using the blob reporter in CI for easy artifact upload.
-Configuration is in `playwright.config.ts`: +Configuration is in `spec/dummy/playwright.config.ts`: @@ -- Generates HTML reports +- Generates HTML reports locally (consider Playwright "blob" reporter in CI for artifact upload)spec/dummy/playwright.config.ts (1)
17-17: Consider adding blob reporter on CI for downloadable artifacts.Keeps GitHub summary clean while preserving a single ZIP to upload.
- reporter: process.env.CI ? 'github' : 'html', + reporter: process.env.CI + ? [['github'], ['blob', { fileName: 'playwright-report.zip' }]] + : [['html', { open: 'never' }]],Based on learnings.
spec/dummy/playwright/tests/performance.spec.ts (3)
12-14: Use Navigation Timing for stability and relax brittle thresholds.
Date.now()+networkidlecan be flaky in CI. Preferperformance.getEntriesByType('navigation')when available, and avoid tight 3s hard caps.-// Page should load within 3 seconds -expect(loadTime).toBeLessThan(3000); +// Prefer Navigation Timing when available; fall back to coarse wall time +const nav = await page.evaluate(() => { + const [e] = performance.getEntriesByType('navigation') as PerformanceNavigationTiming[]; + return e ? e.loadEventEnd - e.startTime : null; +}); +const effectiveLoadMs = nav ?? loadTime; +expect(effectiveLoadMs).toBeLessThan(5000);
28-30: Time-to-interactive assertion is tight; make it resilient.Gate on input readiness with a timeout and allow a bit more headroom to reduce CI flake.
-// Should be interactive within 2 seconds -expect(interactiveTime).toBeLessThan(2000); +// Should be interactive within a reasonable threshold in CI +expect(interactiveTime).toBeLessThan(process.env.CI ? 4000 : 2000);
32-69: Scope memory test to Chromium and wait for the input before loops.
performance.memoryis Chromium-only; skipping on other projects keeps signal high.-test('should not have memory leaks during interactions', async ({ page }) => { +test('should not have memory leaks during interactions', async ({ page }) => { + if (!test.info().project.name.toLowerCase().includes('chromium')) { + test.skip(true, 'performance.memory not available outside Chromium'); + } @@ -const input = page.locator('#HelloWorld-react-component-1 input'); +const input = page.locator('#HelloWorld-react-component-1 input'); +await input.waitFor({ state: 'visible' });spec/dummy/playwright/tests/error-handling.spec.ts (3)
5-26: Centralize console error filtering or expand known benign patterns.Good pattern, but tests can fail on benign assets/issues (e.g., favicon 404). Either reuse a shared helper or extend filters here.
- if (message.type() === 'error') { + if (message.type() === 'error') { // Filter out known non-issues const text = message.text(); if ( !text.includes('Download the React DevTools') && - !text.includes('SharedArrayBuffer will require cross-origin isolation') + !text.includes('SharedArrayBuffer will require cross-origin isolation') && + !/favicon\.ico.*(404|Not Found)/i.test(text) ) { consoleErrors.push(text); } }If you add a helper like
expectNoConsoleErrors(page)in fixtures, both this and other suites can reuse it.
33-44: Wait for the input before filling to avoid race with hydration.Small stability win.
-const input = helloWorld.locator('input'); +const input = helloWorld.locator('input'); +await input.waitFor({ state: 'visible' });
60-70: Clean up routes after the network-failure test to avoid side effects.While
pageis per-test, usingunrouteAll()is a good hygiene and future-proof if scoping changes.-test('should handle network errors gracefully', async ({ page }) => { - // Intercept API calls and simulate failures - await page.route('**/api/**', (route) => { - route.abort('failed'); - }); - - await page.goto('/'); - - // Page should still load even if API calls fail - await expect(page.locator('#ReduxApp-react-component-0')).toBeVisible(); -}); +test('should handle network errors gracefully', async ({ page }) => { + // Intercept API calls and simulate failures + await page.route('**/api/**', (route) => route.abort('failed')); + try { + await page.goto('/'); + // Page should still load even if API calls fail + await expect(page.locator('#ReduxApp-react-component-0')).toBeVisible(); + } finally { + await page.unrouteAll({ waitForOngoing: true }); + } +});Based on learnings.
spec/dummy/lib/tasks/playwright.rake (4)
47-50: Makecleancross‑platform and target the dummy app paths.
rm -rfis not portable on Windows and currently runs relative to repo root.Apply this diff:
desc "Clean Playwright artifacts" task :clean do - sh "rm -rf playwright-report test-results" + FileUtils.rm_rf([ + File.join(DUMMY_DIR, "playwright-report"), + File.join(DUMMY_DIR, "test-results") + ]) end
39-41: Add missing descriptions for Firefox/WebKit tasks forrake -T.This improves discoverability when listing tasks.
- task :firefox do + desc "Run Playwright tests with specific project: Firefox" + task :firefox do yarn_in_dummy "playwright test --project=firefox" end - task :webkit do + desc "Run Playwright tests with specific project: WebKit" + task :webkit do yarn_in_dummy "playwright test --project=webkit" endAlso applies to: 43-45
5-7: Optional: provide a bootstrap task to install deps + browsers.Reduces friction vs. having to run
yarn installseparately in spec/dummy.desc "Install Playwright browsers" task :install do yarn_in_dummy "playwright install --with-deps" end + + desc "Install Node dependencies and Playwright browsers" + task :bootstrap do + yarn_in_dummy "install" + yarn_in_dummy "playwright install --with-deps" + end
3-3: BlockLength disable is fine; consider extraction to avoid the cop.If you prefer, extract helpers into
spec/dummy/lib/tasks/playwright_helpers.rband keep tasks concise to droprubocop:disable Metrics/BlockLength. Purely optional.As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (14)
.gitignore(1 hunks)CLAUDE.md(2 hunks)eslint.config.ts(1 hunks)package.json(1 hunks)spec/dummy/.gitignore(1 hunks)spec/dummy/lib/tasks/playwright.rake(1 hunks)spec/dummy/package.json(2 hunks)spec/dummy/playwright.config.ts(1 hunks)spec/dummy/playwright/README.md(1 hunks)spec/dummy/playwright/fixtures/test-helpers.ts(1 hunks)spec/dummy/playwright/tests/basic-react-components.spec.ts(1 hunks)spec/dummy/playwright/tests/error-handling.spec.ts(1 hunks)spec/dummy/playwright/tests/performance.spec.ts(1 hunks)spec/dummy/playwright/tests/turbolinks-integration.spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
{Gemfile,Rakefile,**/*.{rb,rake,gemspec,ru,js,jsx,ts,tsx,json,yml,yaml,md,css,scss}}
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure all committed files end with a trailing newline character
Files:
spec/dummy/playwright/tests/turbolinks-integration.spec.tseslint.config.tsspec/dummy/playwright.config.tsspec/dummy/playwright/tests/error-handling.spec.tsspec/dummy/lib/tasks/playwright.rakespec/dummy/package.jsonspec/dummy/playwright/tests/basic-react-components.spec.tsspec/dummy/playwright/fixtures/test-helpers.tsspec/dummy/playwright/README.mdpackage.jsonCLAUDE.mdspec/dummy/playwright/tests/performance.spec.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use ESLint for JavaScript/TypeScript and fix all linter offenses before committing
Files:
spec/dummy/playwright/tests/turbolinks-integration.spec.tseslint.config.tsspec/dummy/playwright.config.tsspec/dummy/playwright/tests/error-handling.spec.tsspec/dummy/playwright/tests/basic-react-components.spec.tsspec/dummy/playwright/fixtures/test-helpers.tsspec/dummy/playwright/tests/performance.spec.ts
**/*.{js,jsx,ts,tsx,json,yml,yaml,md,css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
Let Prettier handle all non-Ruby file formatting; never manually format these files
Files:
spec/dummy/playwright/tests/turbolinks-integration.spec.tseslint.config.tsspec/dummy/playwright.config.tsspec/dummy/playwright/tests/error-handling.spec.tsspec/dummy/package.jsonspec/dummy/playwright/tests/basic-react-components.spec.tsspec/dummy/playwright/fixtures/test-helpers.tsspec/dummy/playwright/README.mdpackage.jsonCLAUDE.mdspec/dummy/playwright/tests/performance.spec.ts
{Gemfile,Rakefile,**/*.{rb,rake,gemspec,ru}}
📄 CodeRabbit inference engine (CLAUDE.md)
{Gemfile,Rakefile,**/*.{rb,rake,gemspec,ru}}: Before every commit/push, runbundle exec rubocopand fix all violations in Ruby code
Let RuboCop handle all Ruby code formatting; never manually format Ruby files
Files:
spec/dummy/lib/tasks/playwright.rake
🧠 Learnings (5)
📚 Learning: 2024-10-08T20:53:47.076Z
Learnt from: theforestvn88
PR: shakacode/react_on_rails#1620
File: spec/dummy/client/app/startup/HelloTurboStream.jsx:3-3
Timestamp: 2024-10-08T20:53:47.076Z
Learning: The `RailsContext` import in `spec/dummy/client/app/startup/HelloTurboStream.jsx` is used later in the project, as clarified by the user theforestvn88.
Applied to files:
spec/dummy/playwright/tests/turbolinks-integration.spec.ts
📚 Learning: 2025-02-13T16:50:26.861Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: node_package/src/turbolinksUtils.ts:34-36
Timestamp: 2025-02-13T16:50:26.861Z
Learning: In React on Rails, when checking for Turbolinks version 5 using `turbolinksVersion5()`, always ensure `Turbolinks` exists first by checking `turbolinksInstalled()` to prevent TypeError when accessing properties.
Applied to files:
spec/dummy/playwright/tests/turbolinks-integration.spec.ts
📚 Learning: 2024-12-12T13:07:09.929Z
Learnt from: alexeyr-ci
PR: shakacode/react_on_rails#1644
File: node_package/src/ReactOnRailsRSC.ts:87-87
Timestamp: 2024-12-12T13:07:09.929Z
Learning: When handling errors in 'node_package/src/ReactOnRailsRSC.ts', include the error stack in error messages in development and test environments to aid debugging.
Applied to files:
spec/dummy/playwright/tests/error-handling.spec.ts
📚 Learning: 2025-09-22T07:21:12.910Z
Learnt from: CR
PR: shakacode/react_on_rails#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-22T07:21:12.910Z
Learning: Applies to {Gemfile,Rakefile,**/*.{rb,rake,gemspec,ru}} : Before every commit/push, run `bundle exec rubocop` and fix all violations in Ruby code
Applied to files:
CLAUDE.md
📚 Learning: 2025-09-22T07:21:12.910Z
Learnt from: CR
PR: shakacode/react_on_rails#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-22T07:21:12.910Z
Learning: Applies to {Gemfile,Rakefile,**/*.{rb,rake,gemspec,ru,js,jsx,ts,tsx,json,yml,yaml,md,css,scss}} : Ensure all committed files end with a trailing newline character
Applied to files:
CLAUDE.md
🧬 Code graph analysis (4)
spec/dummy/playwright/tests/turbolinks-integration.spec.ts (1)
spec/dummy/playwright/fixtures/test-helpers.ts (2)
test(9-39)expect(41-41)
spec/dummy/playwright/tests/error-handling.spec.ts (1)
spec/dummy/playwright/fixtures/test-helpers.ts (1)
test(9-39)
spec/dummy/playwright/tests/basic-react-components.spec.ts (1)
spec/dummy/playwright/fixtures/test-helpers.ts (1)
test(9-39)
spec/dummy/playwright/tests/performance.spec.ts (1)
spec/dummy/playwright/fixtures/test-helpers.ts (1)
test(9-39)
🪛 markdownlint-cli2 (0.18.1)
spec/dummy/playwright/README.md
16-16: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: rspec-package-tests (3.4, minimum)
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: rspec-package-tests (3.2, latest)
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: markdown-link-check
- GitHub Check: claude-review
- GitHub Check: build-and-test
🔇 Additional comments (3)
spec/dummy/.gitignore (1)
2-5: LGTM for Playwright artifacts in dummy appThe ignores match Playwright defaults for report and results at this path.
spec/dummy/lib/tasks/playwright.rake (2)
53-54: Alias task looks good.Nice DX touch. Works well with the namespaced tasks.
1-55: Formatting/RuboCop check.Please ensure a trailing newline and that
bundle exec rubocoppasses on Ruby files before merging.As per coding guidelines.
| "@babel/preset-react": "^7.26.3", | ||
| "@babel/preset-typescript": "^7.27.1", | ||
| "@eslint/compat": "^1.2.7", | ||
| "@playwright/test": "^1.55.1", |
There was a problem hiding this comment.
🧩 Analysis chain
Scope Playwright to the dummy app or align versions across the repo
If Playwright runs only under spec/dummy, keeping a second copy at the repo root adds install time and invites version drift. Prefer removing the root devDependency or ensure both places use the same version.
Apply if scoping to dummy app only:
- "@playwright/test": "^1.55.1",Run to compare versions (root vs dummy):
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in package.json spec/dummy/package.json; do
if [ -f "$f" ]; then
printf "%-28s %s\n" "$f" "$(jq -r '.devDependencies["@playwright/test"] // .dependencies["@playwright/test"] // "MISSING"' "$f")"
fi
doneLength of output: 73
Align @playwright/test versions across root and dummy
@playwright/test is ^1.55.1 in package.json but ^1.41.0 in spec/dummy/package.json; update the dummy app to ^1.55.1 or remove the root entry if you intend to scope Playwright solely under spec/dummy.
🤖 Prompt for AI Agents
In package.json around line 39, the root dependency "@playwright/test" is
^1.55.1 while spec/dummy/package.json pins ^1.41.0; update the dummy app to use
^1.55.1 so versions are aligned (or, if Playwright should only live under
spec/dummy, remove the root entry instead). Edit spec/dummy/package.json to bump
the "@playwright/test" version to ^1.55.1, then reinstall (npm/yarn/pnpm
install) and update the lockfile; alternatively delete the root-level
"@playwright/test" entry and reinstall to scope Playwright to spec/dummy only.
| task :install do | ||
| sh "yarn playwright install --with-deps" | ||
| end |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Fix CWD: tasks run from repo root, but scripts live under spec/dummy.
As written, yarn … calls will execute in the current directory (usually repo root), so they won’t find spec/dummy/package.json or the Playwright binary. Anchor all calls to the dummy app directory.
Apply this diff to centralize the working directory and update all tasks:
# frozen_string_literal: true
+require "fileutils"
-namespace :playwright do # rubocop:disable Metrics/BlockLength
+namespace :playwright do # rubocop:disable Metrics/BlockLength
+ DUMMY_DIR = File.expand_path("../..", __dir__)
+
+ def yarn_in_dummy(cmd)
+ sh "yarn --cwd #{DUMMY_DIR} #{cmd}"
+ end
desc "Install Playwright browsers"
task :install do
- sh "yarn playwright install --with-deps"
+ yarn_in_dummy "playwright install --with-deps"
end
desc "Run Playwright E2E tests"
task :test do
- sh "yarn test:e2e"
+ yarn_in_dummy "test:e2e"
end
desc "Run Playwright tests in UI mode"
task :ui do
- sh "yarn test:e2e:ui"
+ yarn_in_dummy "test:e2e:ui"
end
desc "Run Playwright tests in headed mode"
task :headed do
- sh "yarn test:e2e:headed"
+ yarn_in_dummy "test:e2e:headed"
end
desc "Debug Playwright tests"
task :debug do
- sh "yarn test:e2e:debug"
+ yarn_in_dummy "test:e2e:debug"
end
desc "Show Playwright test report"
task :report do
- sh "yarn test:e2e:report"
+ yarn_in_dummy "test:e2e:report"
end
desc "Run Playwright tests with specific project"
task :chromium do
- sh "yarn playwright test --project=chromium"
+ yarn_in_dummy "playwright test --project=chromium"
end
task :firefox do
- sh "yarn playwright test --project=firefox"
+ yarn_in_dummy "playwright test --project=firefox"
end
task :webkit do
- sh "yarn playwright test --project=webkit"
+ yarn_in_dummy "playwright test --project=webkit"
endRun from repo root to confirm:
As per coding guidelines.
Also applies to: 10-12, 15-17, 20-22, 25-27, 30-32, 35-37, 39-41, 43-45
🏁 Script executed:
#!/bin/bash
fd -a spec/dummy/package.json
jq -r '.scripts' spec/dummy/package.json
# Expected: file exists and scripts include test:e2e variantsLength of output: 1246
🏁 Script executed:
#!/bin/bash
echo "=== File Content ==="
sed -n '1,200p' spec/dummy/lib/tasks/playwright.rake
echo
echo "=== Line Count ==="
wc -l spec/dummy/lib/tasks/playwright.rake
echo
echo "=== Last Lines (showing line breaks) ==="
tail -n5 spec/dummy/lib/tasks/playwright.rake | sed -n 'l'Length of output: 1346
Refactor spec/dummy/lib/tasks/playwright.rake: anchor Yarn commands, swap shell rm for FileUtils, add missing descs, and ensure EOF newline
- Introduce
DUMMY_DIR(e.g.File.expand_path("../..", __dir__)), requirefileutils, and wrap allyarncalls with a helper (e.g.yarn_in_dummy "…"). - Replace
sh "rm -rf playwright-report test-results"withFileUtils.rm_rf("#{DUMMY_DIR}/playwright-report")andFileUtils.rm_rf("#{DUMMY_DIR}/test-results"). - Add
desc "Run Playwright tests with Firefox"beforetask :firefoxanddesc "Run Playwright tests with Webkit"beforetask :webkit. - Ensure a single trailing newline at end of file.
🤖 Prompt for AI Agents
In spec/dummy/lib/tasks/playwright.rake around lines 5 to 7, the Rake tasks
should be hardened: add a DUMMY_DIR constant (e.g. File.expand_path("../..",
__dir__)) and require "fileutils"; create a helper method yarn_in_dummy(cmd)
that runs yarn commands anchored to DUMMY_DIR (so replace direct sh "yarn …"
calls with that helper); replace shell cleanup sh "rm -rf playwright-report
test-results" with FileUtils.rm_rf(File.join(DUMMY_DIR, "playwright-report"))
and FileUtils.rm_rf(File.join(DUMMY_DIR, "test-results")); add desc "Run
Playwright tests with Firefox" immediately before task :firefox and desc "Run
Playwright tests with Webkit" immediately before task :webkit; ensure the file
ends with a single trailing newline.
| "@babel/plugin-transform-runtime": "7.17.0", | ||
| "@babel/preset-env": "7", | ||
| "@babel/preset-react": "^7.10.4", | ||
| "@playwright/test": "^1.41.0", |
There was a problem hiding this comment.
Upgrade Playwright devDependency to current release
Locking @playwright/test at ^1.41.0 (Jan 2024) leaves the dummy app on browsers and tooling that are over a year old. The current stable is 1.55.1, and the rest of the stack (including the new config and scripts) will expect the up-to-date CLI/browsers. Please bump this dependency so the dummy app installs the same modern bits as the repo root and avoids stale browser bundles.(classic.yarnpkg.com)
- "@playwright/test": "^1.41.0",
+ "@playwright/test": "^1.55.1",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "@playwright/test": "^1.41.0", | |
| "@playwright/test": "^1.55.1", |
🤖 Prompt for AI Agents
In spec/dummy/package.json around line 36, the devDependency "@playwright/test"
is pinned to ^1.41.0 which is outdated; update the version to the current stable
(e.g. ^1.55.1) in package.json, then run your package manager install
(yarn/npm/pnpm) and run the Playwright browser install step (npx playwright
install or playwright install) so the dummy app pulls the modern CLI and browser
bundles consistent with the repo root.
| /* Run your local dev server before starting the tests */ | ||
| webServer: { | ||
| command: 'bundle exec rails server -p 3000', | ||
| url: 'http://localhost:3000', | ||
| reuseExistingServer: !process.env.CI, | ||
| timeout: 120 * 1000, | ||
| }, |
There was a problem hiding this comment.
Run Rails in the test environment to isolate data and config.
Starting the server without RAILS_ENV=test risks hitting development DB/config on CI and locally.
- webServer: {
- command: 'bundle exec rails server -p 3000',
+ webServer: {
+ command: 'RAILS_ENV=test bundle exec rails server -p 3000',
url: 'http://localhost:3000',
reuseExistingServer: !process.env.CI,
timeout: 120 * 1000,
},Optionally set cwd: __dirname to be explicit if tests are run from repo root.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /* Run your local dev server before starting the tests */ | |
| webServer: { | |
| command: 'bundle exec rails server -p 3000', | |
| url: 'http://localhost:3000', | |
| reuseExistingServer: !process.env.CI, | |
| timeout: 120 * 1000, | |
| }, | |
| /* Run your local dev server before starting the tests */ | |
| webServer: { | |
| command: 'RAILS_ENV=test bundle exec rails server -p 3000', | |
| url: 'http://localhost:3000', | |
| reuseExistingServer: !process.env.CI, | |
| timeout: 120 * 1000, | |
| }, |
🤖 Prompt for AI Agents
In spec/dummy/playwright.config.ts around lines 56 to 62, the webServer command
starts Rails without setting RAILS_ENV which can hit development DB/config;
update the webServer.command to run Rails with RAILS_ENV=test (e.g. prefix the
command with RAILS_ENV=test or set the env appropriately) and add an explicit
cwd: __dirname property to the webServer config so the server starts from the
dummy app directory when tests run.
| return { | ||
| mountedComponents: ReactOnRails.mountedComponents, | ||
| registeredComponents: Object.keys(ReactOnRails.getComponent ? {} : ReactOnRails), | ||
| }; |
There was a problem hiding this comment.
Fix registeredComponents lookup
ReactOnRails.getComponent is defined on every page, so this ternary always resolves to {} and the helper hands tests an empty registeredComponents array—even when components are registered. Please inspect the real registry object (for example ReactOnRails.componentStore or whatever the library exposes in this version) so the fixture returns the actual component names instead of an empty list.
🤖 Prompt for AI Agents
In spec/dummy/playwright/fixtures/test-helpers.ts around lines 31 to 34, the
ternary uses ReactOnRails.getComponent which is always defined so
registeredComponents becomes Object.keys({}) and returns an empty array; replace
that logic to inspect the actual registry exposed by this ReactOnRails version
(e.g. ReactOnRails.componentStore or ReactOnRails.components or
ReactOnRails._components) and extract keys from that object (with a safe
fallback to {} if the property is undefined) so the fixture returns the real
registered component names.
| ## Custom Helpers | ||
|
|
||
| The `fixtures/test-helpers.ts` file provides utilities for React on Rails testing: | ||
|
|
||
| - `waitForHydration()` - Wait for client-side hydration to complete | ||
| - `getServerRenderedData()` - Extract server-rendered props/data | ||
| - `expectNoConsoleErrors()` - Assert no console errors occurred | ||
|
|
There was a problem hiding this comment.
Docs/helpers mismatch with actual exports; align helper names and descriptions.
README lists helpers not present in fixtures/test-helpers.ts (“waitForHydration”, “getServerRenderedData”, “expectNoConsoleErrors”). The code exports waitForReactOnRails and getComponentRegistry.
-## Custom Helpers
-
-The `fixtures/test-helpers.ts` file provides utilities for React on Rails testing:
-
-- `waitForHydration()` - Wait for client-side hydration to complete
-- `getServerRenderedData()` - Extract server-rendered props/data
-- `expectNoConsoleErrors()` - Assert no console errors occurred
+## Custom Helpers
+
+The `fixtures/test-helpers.ts` file provides utilities for React on Rails testing:
+
+- `waitForReactOnRails()` - Wait until ReactOnRails is available and components are mounted
+- `getComponentRegistry()` - Inspect mounted and registered components
+
+// If you add more helpers later (e.g., console error assertions, SSR data extraction),
+// document them here to keep README and code in sync.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## Custom Helpers | |
| The `fixtures/test-helpers.ts` file provides utilities for React on Rails testing: | |
| - `waitForHydration()` - Wait for client-side hydration to complete | |
| - `getServerRenderedData()` - Extract server-rendered props/data | |
| - `expectNoConsoleErrors()` - Assert no console errors occurred | |
| ## Custom Helpers | |
| The `fixtures/test-helpers.ts` file provides utilities for React on Rails testing: | |
| - `waitForReactOnRails()` - Wait until ReactOnRails is available and components are mounted | |
| - `getComponentRegistry()` - Inspect mounted and registered components | |
| // If you add more helpers later (e.g., console error assertions, SSR data extraction), | |
| // document them here to keep README and code in sync. |
🤖 Prompt for AI Agents
spec/dummy/playwright/README.md lines 69-76: the README documents helper names
that don't exist in fixtures/test-helpers.ts; update the README to match actual
exports (replace "waitForHydration" with "waitForReactOnRails",
"getServerRenderedData" with "getComponentRegistry" and remove or rename
"expectNoConsoleErrors" to whatever the file actually exports) and adjust
descriptions to accurately describe what each exported helper does, or
alternatively modify fixtures/test-helpers.ts to export the documented helper
names — choose one approach and make names and descriptions consistent across
README and code.
| test('should maintain React state across Turbo navigations', async ({ page }) => { | ||
| await page.goto('/'); | ||
|
|
||
| // Set some state in a component | ||
| const helloWorld = page.locator('#HelloWorld-react-component-1'); | ||
| const input = helloWorld.locator('input'); | ||
| await input.fill('Test State'); | ||
|
|
||
| // If there are navigation links, test them | ||
| // This is a placeholder - you'd need actual navigation in your app | ||
| const links = await page.locator('a[data-turbo]').count(); | ||
| if (links > 0) { | ||
| // Click first turbo link | ||
| await page.locator('a[data-turbo]').first().click(); | ||
|
|
||
| // Navigate back | ||
| await page.goBack(); | ||
|
|
||
| // Check if component remounts properly | ||
| await expect(page.locator('#HelloWorld-react-component-1')).toBeVisible(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Test name vs. assertion mismatch; verify state persistence or rename.
You fill “Test State” but only check visibility after navigation. Either assert the value persisted (if that’s the contract) or rename the test to reflect remount behavior.
- test('should maintain React state across Turbo navigations', async ({ page }) => {
+ test('should maintain React state across Turbo navigations', async ({ page }) => {
@@
- // Check if component remounts properly
- await expect(page.locator('#HelloWorld-react-component-1')).toBeVisible();
+ // Verify component is visible and the state persisted
+ const helloWorldAfter = page.locator('#HelloWorld-react-component-1');
+ await expect(helloWorldAfter).toBeVisible();
+ const inputAfter = helloWorldAfter.locator('input');
+ await expect(inputAfter).toHaveValue('Test State');If remount is expected to reset state, rename the test to “remounts and resets state on Turbo navigation” and assert empty value instead.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test('should maintain React state across Turbo navigations', async ({ page }) => { | |
| await page.goto('/'); | |
| // Set some state in a component | |
| const helloWorld = page.locator('#HelloWorld-react-component-1'); | |
| const input = helloWorld.locator('input'); | |
| await input.fill('Test State'); | |
| // If there are navigation links, test them | |
| // This is a placeholder - you'd need actual navigation in your app | |
| const links = await page.locator('a[data-turbo]').count(); | |
| if (links > 0) { | |
| // Click first turbo link | |
| await page.locator('a[data-turbo]').first().click(); | |
| // Navigate back | |
| await page.goBack(); | |
| // Check if component remounts properly | |
| await expect(page.locator('#HelloWorld-react-component-1')).toBeVisible(); | |
| } | |
| }); | |
| test('should maintain React state across Turbo navigations', async ({ page }) => { | |
| await page.goto('/'); | |
| // Set some state in a component | |
| const helloWorld = page.locator('#HelloWorld-react-component-1'); | |
| const input = helloWorld.locator('input'); | |
| await input.fill('Test State'); | |
| // If there are navigation links, test them | |
| // This is a placeholder - you'd need actual navigation in your app | |
| const links = await page.locator('a[data-turbo]').count(); | |
| if (links > 0) { | |
| // Click first turbo link | |
| await page.locator('a[data-turbo]').first().click(); | |
| // Navigate back | |
| await page.goBack(); | |
| // Verify component is visible and the state persisted | |
| const helloWorldAfter = page.locator('#HelloWorld-react-component-1'); | |
| await expect(helloWorldAfter).toBeVisible(); | |
| const inputAfter = helloWorldAfter.locator('input'); | |
| await expect(inputAfter).toHaveValue('Test State'); | |
| } | |
| }); |
🤖 Prompt for AI Agents
In spec/dummy/playwright/tests/turbolinks-integration.spec.ts around lines 26 to
47, the test title says it should maintain React state across Turbo navigations
but the only assertion checks visibility after navigation; either change the
test to assert the input still contains "Test State" after navigating back
(verify locator.input.value or use expect(input).toHaveValue('Test State')) if
persistence is the intended contract, or rename the test to indicate
remount/reset behavior (e.g., "remounts and resets state on Turbo navigation")
and change the assertion to expect the input to be empty (toHaveValue('')) —
update the test name and the assertion accordingly.
- Add comprehensive Playwright testing section to dummy app README - Include setup instructions for Playwright - Document all available test commands - Explain test structure and file organization - Provide example for writing new Playwright tests - Link to detailed Playwright README for more information This ensures developers working with the dummy app understand how to use Playwright for E2E testing alongside RSpec and Jest. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Code Review: Add Playwright E2E Testing FrameworkThank you for this excellent addition to the React on Rails testing suite! The Playwright implementation is well-structured and comprehensive. ✅ StrengthsCode Quality & Architecture
Test Coverage
Documentation
🔧 Suggestions for Improvement1. CI/CD Integration MissingAdd .github/workflows/playwright.yml for automated testing on PRs 2. Type SafetyReplace 'any' types in test helpers with proper Playwright types 3. Test StabilityMake timeouts configurable via environment variables for CI vs local 4. Port ConfigurationUse dynamic port allocation instead of hardcoded port 3000 5. Browser-Specific APIsAdd checks for Chrome-only features like performance.memory
|
Code Review: Playwright E2E Testing FrameworkThank you for adding Playwright E2E testing to React on Rails! This is a valuable addition. I've reviewed the implementation and have the following feedback: Strengths
Suggestions for Improvement1. Test Helper ImplementationThe test-helpers.ts file appears incomplete. The waitForReactOnRails fixture is defined but implementation is cut off. Please ensure all helper functions are fully implemented. 2. Version InconsistencyVersion mismatch between:
3. Error FilteringIn error-handling.spec.ts, consider moving hard-coded error filters to a configuration file for better maintainability. 4. Missing CI ConfigurationWhile PR mentions GitHub Actions integration, the workflow file is missing. Consider adding .github/workflows/playwright.yml with artifact uploading and parallel execution strategy. Potential Issues1. Rake Task Block LengthThe Playwright rake task disables RuboCop block length check. Consider refactoring into smaller, focused tasks. 2. Security: Webserver ConfigurationEnsure the Rails server only binds to localhost in test mode and consider using a random available port to avoid conflicts. 3. Test IsolationAdd proper setup/teardown including database cleaner strategies, localStorage clearing, and cookie cleanup between tests. Test Coverage RecommendationsConsider adding tests for:
Performance Considerations
Overall AssessmentThis is a solid implementation that adds significant value to the React on Rails testing suite. The structure is clean, documentation is comprehensive, and integration with existing tools is well thought out. With the suggested improvements above, particularly completing test helpers, aligning package versions, and adding the CI workflow, this will be an excellent addition to the project. Great work on this implementation! |
Summary
Key Changes
spec/dummy/playwright.config.tswith support for Chrome, Firefox, Safari, and mobile browsersbasic-react-components.spec.ts- Tests React component rendering and interactionsturbolinks-integration.spec.ts- Tests Turbolinks/Turbo integrationerror-handling.spec.ts- Console error monitoring and error handlingperformance.spec.ts- Performance testing for load times and memory usagefixtures/test-helpers.tsfor React on Rails specific testingImplementation Details
The Playwright tests are properly located in
spec/dummy/where the actual Rails test application lives. This ensures tests run against a real Rails environment with compiled assets.Features:
Test Plan
Install Playwright dependencies:
cd spec/dummy yarn install yarn playwright install --with-depsRun Playwright tests:
Run tests in UI mode for debugging:
Verify existing tests still pass:
Breaking Changes
None - this is an additive change that doesn't affect existing functionality.
Security Implications
None - Playwright tests run locally and in CI only. No production code changes.
🤖 Generated with Claude Code
This change is
Summary by CodeRabbit
Tests
Documentation
Chores