Switch testing from mocha/chai/karma/c8 to vitest. - #56
Draft
davidlehn wants to merge 4 commits into
Draft
Conversation
davidlehn
force-pushed
the
remove-cjs-support
branch
2 times, most recently
from
August 11, 2026 01:17
bb0518d to
cc1f63d
Compare
Karma is unmaintained. Vitest replaces the runner, the browser test harness, and the coverage tool with a single dependency and config. - Add `vitest.config.js` with `node` and `browser` projects. The browser project runs Chromium through `@vitest/browser-playwright`, replacing karma and karma-webpack. - Keep the existing `should`-style assertions unchanged. `tests/setup.js` installs the global `should` from vitest's re-exported chai, so `chai` is no longer a dependency. - Start the HTTP/HTTPS test servers in `tests/globalSetup.js` and pass their ephemeral hosts to tests with `inject()`. This replaces starting them in the karma config and injecting the hosts via webpack's `DefinePlugin`, and lets `tests/utils-browser.js` be removed. - Split the suite by environment into `10-client-api.spec.js` (shared), `20-node.spec.js`, and `30-browser.spec.js` instead of branching on `isNode` at runtime. This keeps node-only modules out of the browser project and drops the `detect-node` dependency. Test bodies are unchanged; both projects still run 17 tests. - Report coverage with `@vitest/coverage-v8` across both projects, so browser-only code paths are now covered. Reported totals shift slightly because vitest and c8 count executable lines differently. - Replace the `test-karma` CI job with `test-browser`, and install and cache Playwright's Chromium in the browser and coverage jobs. Add an `exports` field with a `browser` condition for `agentCompatibility` and import it via a self-reference. Vite does not apply the top-level `browser` field to package-internal relative imports, so this is also a fix for browser bundlers, which would otherwise pull `undici` into their builds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## update-deps #56 +/- ##
=================================================
+ Coverage 88.27% 100.00% +11.72%
=================================================
Files 3 4 +1
Lines 290 84 -206
=================================================
- Hits 256 84 -172
+ Misses 34 0 -34
... and 3 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Coverage was 79.51% of statements. Add tests for the paths that had none: - `httpClient.create()` and the proxied `stop` signal. The existing "can use create()" test never called `create()`. - A direct call with a method that is not proxied, which goes straight to `ky` and so skips the response and error handling. - `convertAgent` declining to override a custom `fetch` from another lib. - The `fetch` override used when the installed undici cannot drive the platform `fetch`. No supported node takes that path with `undici@7`, so the test forces it by reporting an incompatible platform undici major. - Importing when the version read fails, which must not throw at module load. Now at 100% of statements, lines, and functions, and 98.27% of branches. The one uncovered branch is the fallback for an installed undici major that is not in the compatibility table. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The `Possible CORS error` message was produced by matching the literal string `Failed to fetch`, which is Chromium's wording for a failed or blocked `fetch`. Firefox reports `NetworkError when attempting to fetch resource.` and WebKit reports `Load failed`, so neither matched and both fell through to `ky`'s generic network error. Match against a set of the three engines' messages instead. Node.js `fetch failed` is deliberately excluded: there is no CORS in Node.js, so the hint would be misleading there. All three strings were measured, not assumed, and each is exercised by the browser test matrix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This is what keeps the per-engine CORS messages honest, and running the shared suite in each engine surfaces any other behavior difference. Three changes were needed to make the other engines work: - Advertise the test servers as `127.0.0.1` rather than the `0.0.0.0` that `address()` reports. Only Chromium treats `0.0.0.0` as loopback when used as a request host; Firefox and WebKit refuse to connect, which failed every test that used a local server. - Replace the raw `--no-sandbox` launch args with `chromiumSandbox: false`. WebKit rejects unknown options and fails to launch; the `playwright` option is applied to Chromium only. - Use the `istanbul` coverage provider rather than `v8`. v8 coverage is gathered over CDP, which only Chromium supports, so `coverage` refused to start once the other engines were added. `istanbul` counts default parameters and some conditionals that `v8` did not, which surfaced an untested documented option, so also test `parseBody: false` and `create()`/`extend()` with no overrides. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Karma is unmaintained. Vitest replaces the runner, the browser test harness, and the coverage tool with a single dependency and config.
vitest.config.jswithnodeandbrowserprojects. The browser project runs Chromium through@vitest/browser-playwright, replacing karma and karma-webpack.should-style assertions unchanged.tests/setup.jsinstalls the globalshouldfrom vitest's re-exported chai, sochaiis no longer a dependency.tests/globalSetup.jsand pass their ephemeral hosts to tests withinject(). This replaces starting them in the karma config and injecting the hosts via webpack'sDefinePlugin, and letstests/utils-browser.jsbe removed.10-client-api.spec.js(shared),20-node.spec.js, and30-browser.spec.jsinstead of branching onisNodeat runtime. This keeps node-only modules out of the browser project and drops thedetect-nodedependency. Test bodies are unchanged; both projects still run 17 tests.@vitest/coverage-v8across both projects, so browser-only code paths are now covered. Reported totals shift slightly because vitest and c8 count executable lines differently.test-karmaCI job withtest-browser, and install and cache Playwright's Chromium in the browser and coverage jobs.Add an
exportsfield with abrowsercondition foragentCompatibilityand import it via a self-reference. Vite does not apply the top-levelbrowserfield to package-internal relative imports, so this is also a fix for browser bundlers, which would otherwise pullundiciinto their builds.