Add Playwright global setup for artifacts directory#165
Conversation
WalkthroughThis PR introduces a Playwright global setup script that runs before tests. A new Changes
Sequence DiagramsequenceDiagram
participant Playwright
participant GlobalSetup
participant FileSystem
Playwright->>GlobalSetup: Run globalSetup (_config)
GlobalSetup->>FileSystem: Resolve artifacts directory path
GlobalSetup->>FileSystem: Create directory (mkdirSync recursive)
FileSystem-->>GlobalSetup: Directory created/exists
GlobalSetup-->>Playwright: Setup complete
Playwright->>Playwright: Run tests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
playwright.config.ts(1 hunks)playwright.global-setup.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
playwright.config.ts (1)
frontend/tools/perf/run-perf-budget.js (1)
path(3-3)
playwright.global-setup.ts (1)
frontend/tools/perf/run-perf-budget.js (2)
path(3-3)fs(2-2)
🔇 Additional comments (3)
playwright.config.ts (1)
18-18: LGTM! Global setup integration looks correct.The path resolution correctly points to the new global setup script. Since both this config file and the setup script use
__dirnamefrom the same directory level, the artifacts directory created by the setup will align with the JUnit reporter's output path (line 5).playwright.global-setup.ts (2)
1-3: LGTM! Imports are correct and minimal.All necessary dependencies are imported:
fsfor directory creation,pathfor resolution, andFullConfigfor type safety.
10-10: LGTM! Default export is correct.The function is properly exported as the default export, which is the standard pattern for Playwright global setup scripts.
| async function globalSetup(_config: FullConfig) { | ||
| const artifactsDir = path.resolve(__dirname, 'artifacts'); | ||
| await fs.promises.mkdir(artifactsDir, { recursive: true }); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Implementation is correct and follows best practices.
The global setup function correctly:
- Uses
fs.promises.mkdirwith{ recursive: true }to safely create the directory structure - Properly awaits the async operation
- Resolves the path relative to the current file using
__dirname
The unused _config parameter is appropriately prefixed with an underscore.
Optionally, consider adding a console log for better observability during test runs:
async function globalSetup(_config: FullConfig) {
const artifactsDir = path.resolve(__dirname, 'artifacts');
await fs.promises.mkdir(artifactsDir, { recursive: true });
+ console.log(`✓ Artifacts directory ready: ${artifactsDir}`);
}📝 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.
| async function globalSetup(_config: FullConfig) { | |
| const artifactsDir = path.resolve(__dirname, 'artifacts'); | |
| await fs.promises.mkdir(artifactsDir, { recursive: true }); | |
| } | |
| async function globalSetup(_config: FullConfig) { | |
| const artifactsDir = path.resolve(__dirname, 'artifacts'); | |
| await fs.promises.mkdir(artifactsDir, { recursive: true }); | |
| console.log(`✓ Artifacts directory ready: ${artifactsDir}`); | |
| } |
🤖 Prompt for AI Agents
In playwright.global-setup.ts around lines 5 to 8, the globalSetup correctly
creates the artifacts directory but lacks runtime observability; add a concise
console.log (or other logger) after the directory is created to indicate success
and the artifactsDir path so test runs emit a clear message when setup
completes.
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @shayancoin. * #165 (comment) The following files were modified: * `playwright.global-setup.ts`
Docstrings generation was requested by @shayancoin. * #165 (comment) The following files were modified: * `playwright.global-setup.ts` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* Ensure Playwright artifacts directory exists * 📝 Add docstrings to `codex/add-setup-step-for-playwright-tests` (#297) Docstrings generation was requested by @shayancoin. * #165 (comment) The following files were modified: * `playwright.global-setup.ts` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68f28d94ed388330b33794418146e5df
Summary by CodeRabbit