feat(logger): migrate tests to Vitest - #10328
Conversation
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request migrates the testing framework for the logger package from Karma and Mocha to Vitest, introducing new configuration files, setup scripts, and polyfills for both Node.js and browser environments. The feedback highlights several areas for improvement: removing a global 'process.env': {} definition in vitest.config.mjs that breaks Node.js environment variables, using queueMicrotask instead of setTimeout for a more accurate process.nextTick polyfill, applying optional chaining in getTestTitle to prevent potential runtime errors, and removing redundant global type declarations that are already provided by vitest/globals.
b61cb84 to
3d340c0
Compare
a57bd88 to
fb3ed13
Compare
| "integration/*", | ||
| "repo-scripts/*" | ||
| ], | ||
| "resolutions": { |
There was a problem hiding this comment.
Why are we adding resolutions? I'm worried this could get very long. Can we resolve this by setting specific versions in yarn.lock instead?
| } | ||
|
|
||
| // Polyfill process for browser | ||
| if (typeof (globalThis as any).process === 'undefined') { |
There was a problem hiding this comment.
Are these polyfills injected into the global test environment? I'm worried it could affect some code that uses process or process.env to check what environment it's in, and do different behaviors. Probably not in logger but in other packages.
2df385a to
f0b5868
Compare
- 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
f0b5868 to
67a6b54
Compare
|
Vitest 👀 👀 👀 👀 👀 |
3 participants
Summary
Migrates
@firebase/loggerfrom legacy Karma + Mocha + Webpack test runner to a unified Vitest multi-project runner (nodeandbrowserwith Playwright Chromium headless).Changes
vitest.config.mjs)packages/logger/test/polyfills.tsandpackages/logger/test/setup.tspackages/logger/src/types/vitest-globals.d.tsscripts/ensure_playwright.jspackages/logger/karma.conf.jspackages/logger/package.jsonfor isolated migrationCI Impact & Verification