Skip to content

test yarn.lock fix - #10343

Draft
hsubox76 wants to merge 3 commits into
mainfrom
ch-vitest-logger
Draft

test yarn.lock fix#10343
hsubox76 wants to merge 3 commits into
mainfrom
ch-vitest-logger

Conversation

@hsubox76

Copy link
Copy Markdown
Contributor

No description provided.

Manvi1203 and others added 3 commits August 31, 2026 18:05
- 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
@changeset-bot

changeset-bot Bot commented Aug 31, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: f346896

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +26 to +28
const headlessShellPath = execPath
.replace('/chromium-', '/chromium_headless_shell-')
.replace(/\/chrome$/, '/headless_shell');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
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');

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants