test yarn.lock fix - #10343
Conversation
- Replaced legacy Karma and Mocha runners with unified Vitest multi-project runner - Added test/polyfills.ts, test/setup.ts, and src/types/vitest-globals.d.ts - Added scripts/ensure_playwright.js guard to test:all and test:browser - Deleted deprecated karma.conf.js - Added resolutions in package.json and updated yarn.lock to preserve CommonJS string-width for CLI tools in Node 22 CI Impact: • Test Count: 58 passed across 2 projects (29 Node + 29 Browser Chromium) [100% parity, 0 regressions] • Before (Karma + Mocha): ~7.8s total (Mocha 1.98s + Karma/Webpack ~5.8s) • After (Vitest Unified): 1.42s total execution time
|
There was a problem hiding this comment.
Code Review
This pull request migrates the test suite for the logger package from Karma and Mocha to Vitest and Playwright. It introduces a new script to ensure Playwright Chromium is installed, configures Vitest for both Node and browser environments, and adds a test setup file. Feedback was provided regarding a cross-platform path resolution issue in the Playwright installation script that would cause failures on Windows environments.
| const headlessShellPath = execPath | ||
| .replace('/chromium-', '/chromium_headless_shell-') | ||
| .replace(/\/chrome$/, '/headless_shell'); |
There was a problem hiding this comment.
The path replacement logic for headlessShellPath is not cross-platform and will fail on Windows. On Windows, the path separator is \ instead of /, and the executable ends with .exe (e.g., chrome.exe). As a result, the .replace calls will do nothing, and headlessShellPath will incorrectly resolve to the standard chrome.exe path, bypassing the check for the headless shell.
Using a regular expression that handles both path separators and the optional .exe extension makes this robust across platforms.
| const headlessShellPath = execPath | |
| .replace('/chromium-', '/chromium_headless_shell-') | |
| .replace(/\/chrome$/, '/headless_shell'); | |
| const headlessShellPath = execPath | |
| .replace(/([\/\\\\])chromium-/, '$1chromium_headless_shell-') | |
| .replace(/([\/\\\\])chrome(\\.exe)?$/, '$1headless_shell$2'); |
No description provided.