Configure Playwright reporting for CI#122
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a root Playwright config with dynamic runtime import and test settings, updates Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant PlaywrightTest as Playwright Test Runner
participant RootConfig as playwright.config.ts
participant FrontendConfig as frontend/playwright.config.ts
Note over RootConfig: dynamic require/import of Playwright typings
Developer->>PlaywrightTest: run tests
PlaywrightTest->>FrontendConfig: import default config
FrontendConfig->>RootConfig: re-exported config reference
RootConfig->>PlaywrightTest: supplies config (baseURL, reporters, timeouts, use/launchOptions)
PlaywrightTest->>PlaywrightTest: execute tests with configured options
PlaywrightTest-->>Developer: emit results (JUnit -> artifacts/junit.xml)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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
| export default defineConfig({ | ||
| testDir: 'frontend/tests/e2e', | ||
| timeout: 60_000, |
There was a problem hiding this comment.
The new root Playwright config sets testDir: 'frontend/tests/e2e'. When the frontend’s playwright.config.ts re‑exports this config, Playwright resolves that relative path from the frontend directory, yielding frontend/frontend/tests/e2e and causing npm run test:e2e to fail with no tests found. Consider resolving the test directory from the repo root (e.g., resolve(__dirname, 'frontend/tests/e2e')) or overriding it when the config is consumed from the frontend so that existing frontend scripts continue to work.
Useful? React with 👍 / 👎.
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.gitignore(1 hunks)frontend/playwright.config.ts(1 hunks)playwright.config.ts(1 hunks)
🔇 Additional comments (4)
.gitignore (1)
39-39: LGTM!The
artifacts/directory is correctly placed under the Node/Next.js section and aligns with the junit output configuration inplaywright.config.ts.playwright.config.ts (3)
13-13: Verify thatfullyParallel: falseis intentional.Disabling parallel test execution will slow down your test suite significantly. While this might be necessary for tests with shared state or resource constraints, it's typically better to design tests to be independently executable.
Please confirm:
- Is this setting required for test stability in CI?
- Have you considered refactoring tests to support parallel execution?
- Are there specific tests that need to run serially (which could be handled with
test.describe.serial()instead)?If parallel execution is causing issues, consider:
- Using Playwright's
test.describe.serial()for specific test groups- Implementing proper test isolation with unique test data
- Using Playwright's built-in fixtures for resource management
7-8: Good configuration choices.The environment-based
baseURLand centralizedjunitOutputpath are well-designed for CI/CD flexibility.
10-22: Configuration structure looks solid.The timeout values, test directory path, and reporter configuration are appropriate for CI environments. The
trace: 'retain-on-failure'setting is particularly useful for debugging failed CI runs.
| import config from '../playwright.config'; | ||
|
|
||
| export default defineConfig({ | ||
| testDir: './tests/e2e', | ||
| timeout: 30_000, | ||
| expect: { timeout: 15_000 }, | ||
| use: { | ||
| baseURL: process.env.BASE_URL || 'http://localhost:3000', | ||
| headless: true, | ||
| trace: 'retain-on-failure', | ||
| launchOptions: { | ||
| args: [ | ||
| '--ignore-gpu-blocklist', | ||
| '--use-gl=swiftshader', | ||
| '--enable-webgl', | ||
| '--disable-gpu-sandbox', | ||
| '--disable-web-security', // allow wasm decoders to initialise in CI | ||
| ], | ||
| }, | ||
| }, | ||
| }); | ||
| export default config; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clean re-export pattern, but consider future override needs.
The re-export approach effectively shares configuration from the root, which is clean and DRY. However, this approach loses the ability to override or extend configuration at the frontend level.
If you anticipate needing frontend-specific overrides in the future, consider this alternative pattern:
import rootConfig from '../playwright.config';
import { defineConfig } from '@playwright/test';
export default defineConfig({
...rootConfig,
// Future frontend-specific overrides can go here
});This maintains the shared configuration while allowing future customization without breaking changes.
🤖 Prompt for AI Agents
In frontend/playwright.config.ts around lines 1 to 3, the file simply re-exports
the root config which prevents easily adding frontend-specific overrides later;
change the export to use defineConfig and spread the root config so you can add
or override fields in the future (import the root config, call defineConfig with
...rootConfig and put any frontend-specific options after the spread).
playwright.config.ts
Outdated
| const requireFromFrontend = createRequire(resolve(__dirname, 'frontend/package.json')); | ||
| const { defineConfig } = requireFromFrontend('@playwright/test') as typeof import('@playwright/test'); |
There was a problem hiding this comment.
🧩 Analysis chain
Verify dependency installation and consider fragility of dynamic imports.
The createRequire pattern to dynamically load @playwright/test from the frontend's node_modules is unusual and fragile. This approach can fail if:
@playwright/testis not installed infrontend/node_modules- The
frontend/package.jsonpath changes - There are version mismatches between root and frontend dependencies
Consider these alternatives:
- Install
@playwright/testas a dev dependency at the root level - Use a monorepo tool (like npm workspaces) to manage shared dependencies
- Add error handling to provide clear messages if the dynamic import fails
Run this script to verify the current setup:
🏁 Script executed:
#!/bin/bash
# Verify @playwright/test installation and potential issues
echo "=== Checking if @playwright/test exists in frontend ==="
if [ -d "frontend/node_modules/@playwright" ]; then
echo "✓ @playwright/test found in frontend/node_modules"
fd --type f package.json frontend/node_modules/@playwright/test --exec jq -r '.version'
else
echo "✗ @playwright/test NOT found in frontend/node_modules"
fi
echo ""
echo "=== Checking root package.json for @playwright/test ==="
if [ -f "package.json" ]; then
cat package.json | jq -r '.devDependencies["@playwright/test"] // "Not found in root"'
else
echo "No root package.json found"
fi
echo ""
echo "=== Testing if playwright.config.ts can be loaded ==="
node -e "import('./playwright.config.ts').then(() => console.log('✓ Config loads successfully')).catch(e => console.error('✗ Config failed to load:', e.message))"Length of output: 369
Fix missing ‘@playwright/test’ installation and remove fragile dynamic import.
Runtime loading of playwright.config.ts fails (“Cannot find module '@playwright/test'”). Add “@playwright/test” as a devDependency at the repo root (or configure npm/yarn workspaces) and replace the createRequire pattern with a direct import.
🤖 Prompt for AI Agents
In playwright.config.ts around lines 4 to 5, the config uses a fragile
createRequire(resolve(...)) to load '@playwright/test' from the frontend package
and fails at runtime because '@playwright/test' is not installed at the repo
root; fix by adding '@playwright/test' as a devDependency at the repo root (or
configure workspaces to expose it) and replace the dynamic createRequire pattern
with a direct static import from '@playwright/test' at the top of the file,
removing the createRequire/resolve code paths.
| args: [ | ||
| '--ignore-gpu-blocklist', | ||
| '--use-gl=swiftshader', | ||
| '--enable-webgl', | ||
| '--disable-gpu-sandbox', | ||
| '--disable-web-security', | ||
| ], |
There was a problem hiding this comment.
Security risk: Remove --disable-web-security flag.
The --disable-web-security flag disables Chrome's same-origin policy and other security features. This is a significant security risk because:
- Tests could inadvertently access external resources or sensitive data
- It masks real CORS issues that would occur in production
- It creates a false sense of security in test environments
If you're encountering CORS issues, address them properly by:
- Configuring your test server to allow the necessary origins
- Using Playwright's built-in request interception/mocking features
- Setting up proper CORS headers in your development environment
Apply this diff to remove the unsafe flag:
launchOptions: {
args: [
'--ignore-gpu-blocklist',
'--use-gl=swiftshader',
'--enable-webgl',
'--disable-gpu-sandbox',
- '--disable-web-security',
],
},📝 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.
| args: [ | |
| '--ignore-gpu-blocklist', | |
| '--use-gl=swiftshader', | |
| '--enable-webgl', | |
| '--disable-gpu-sandbox', | |
| '--disable-web-security', | |
| ], | |
| launchOptions: { | |
| args: [ | |
| '--ignore-gpu-blocklist', | |
| '--use-gl=swiftshader', | |
| '--enable-webgl', | |
| '--disable-gpu-sandbox', | |
| ], | |
| }, |
🤖 Prompt for AI Agents
In playwright.config.ts around lines 24 to 30, remove the
'--disable-web-security' entry from the browser launch args to eliminate the
unsafe bypass of same-origin protections; update the args array to omit that
flag and, if needed, handle CORS in tests by configuring the test server's CORS
headers or using Playwright's request interception/mocking utilities instead.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
playwright.config.ts (3)
4-5: Install @playwright/test at root and remove fragile dynamic import.This dynamic require pattern is fragile and will fail if
@playwright/testis not installed infrontend/node_modules. Past review already identified this critical issue with a clear fix: add@playwright/testas a devDependency at the repository root and replace thecreateRequirepattern with a standard static import.
10-12: Fix testDir path resolution for frontend re-export.Past review correctly identified that when
frontend/playwright.config.tsre-exports this config, the relative path'frontend/tests/e2e'resolves from the frontend directory, resulting infrontend/frontend/tests/e2e. This breaks existing frontend test scripts.
29-29: Remove --disable-web-security flag.Past review correctly identified that
--disable-web-securitydisables Chrome's same-origin policy and creates significant security risks. This flag masks real CORS issues that should be addressed through proper test server configuration or Playwright's request interception features.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
playwright.config.ts(1 hunks)
🔇 Additional comments (3)
playwright.config.ts (3)
7-8: LGTM!The baseURL configuration with environment variable fallback and absolute path for JUnit output are both appropriate for CI environments.
15-18: LGTM!The reporter configuration with
listfor console output andjunitfor CI integration aligns perfectly with the PR objective to configure Playwright reporting for CI environments.
19-22: LGTM!The use options are well-configured for CI: headless mode for resource efficiency and trace retention on failure for debugging without excessive storage overhead.
| export default defineConfig({ | ||
| testDir: resolve(__dirname, 'frontend/tests/e2e'), | ||
| timeout: 60_000, | ||
| fullyParallel: false, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider enabling fullyParallel for CI performance.
Setting fullyParallel: false may slow down CI test runs. In CI environments, parallel test execution is typically preferred to reduce pipeline duration. Verify whether there's a specific reason to disable parallel execution (e.g., shared state, database constraints).
🤖 Prompt for AI Agents
In playwright.config.ts around line 13, fullyParallel is set to false which can
slow CI; update configuration to enable parallelism by setting fullyParallel:
true for CI runs (either change the value directly if no shared-state
constraints exist, or conditionally set fullyParallel = process.env.CI ? true :
false) and ensure any tests that rely on shared global state are made isolated
or marked as serial to avoid flakiness.
| '--ignore-gpu-blocklist', | ||
| '--use-gl=swiftshader', | ||
| '--enable-webgl', | ||
| '--disable-gpu-sandbox', |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Document the purpose of GPU workaround flags.
The GPU-related flags (--ignore-gpu-blocklist, --use-gl=swiftshader, --enable-webgl, --disable-gpu-sandbox) are common workarounds for headless Chrome rendering issues in CI environments. Consider adding a comment explaining why these flags are necessary to help future maintainers understand their purpose.
🤖 Prompt for AI Agents
In playwright.config.ts around lines 25 to 28, the GPU-related Chromium flags
are listed without explanation; add a concise comment above or next to these
flags explaining they are CI/workaround flags to force software GL
(SwiftShader), enable WebGL, ignore GPU blacklists and disable the GPU sandbox
to avoid headless Chrome rendering failures in certain CI/container
environments, and mention any relevant platform/CI (e.g., Docker/GitHub Actions)
and link to an upstream doc/issue if available.
|
Caution Docstrings generation - FAILED No docstrings were generated. |
1 participant
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68f135b0bce483308c4a3ab2a42d4f4a
Summary by CodeRabbit
New Features
Chores