Repository navigation
test: run tests with node:test, keep Jest for old Node.js versions - #292
Conversation
Tests are written for `node:test` and `assert`, and run with `node --test` on Node.js 22+. Node.js 10-20, which lack a usable `node:test` (snapshots need 22), still run the same files with Jest: `jest.config.js` maps `node:test` to `test/helpers/jest-node-test.js`, which provides the parts of the API the tests use on top of Jest. - `expect` assertions are converted to `assert` (only APIs available on Node.js 10, e.g. no `assert.match`). - Snapshots use the `node:test` snapshot files. The Jest adapter reads the same files, so there is one set of snapshots; update them with `npm run test:update-snapshots`. The 1344 Fuzzy snapshots are identical to the Jest ones; inline snapshots moved to snapshot files. - `describe.each`/`it.each` become loops, `expect.assertions` a call counter, and the `Array.prototype.sort` spy a plain wrapper. - `test/__mocks__/createMappingsSerializer.js` is removed: it was never loaded (its `jest.requireActual` path doesn't exist), so the `jest.mock` calls had no effect. - `test/helpers.js` moves to `test/helpers/index.js`, so `test/*.js` only matches test files. - `prettier-2` (Jest inline snapshots) and `@types/jest` are removed. - Coverage on Node.js 22+ comes from `--experimental-test-coverage` written to `lcov.info`, picked up by the Codecov upload. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01ThsbPWvW5vnYWEaFs2Q22F
|
One or more co-authors of this pull request were not found. You must specify co-authors in commit message trailer via: Supported
Alternatively, if the co-author should not be included, remove the Please update your commit message(s) by doing |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #292 +/- ##
==========================================
+ Coverage 98.30% 98.91% +0.61%
==========================================
Files 25 26 +1
Lines 2295 5708 +3413
Branches 738 742 +4
==========================================
+ Hits 2256 5646 +3390
- Misses 37 60 +23
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Merging this PR will regress 7 benchmarks
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (26)
💤 Files with no reviewable changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review. WalkthroughThe default test scripts now use Node’s built-in test runner and assertions. Jest remains available through legacy scripts, and the workflow selects the coverage script based on Node.js version. The test suites and snapshots are updated for the new runner. The Jest configuration includes an adapter for Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to No concrete merge-blocking issue is established; the runner migration remains subject to normal checks on the supported Node.js versions. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The cache-data representation changes, but the reviewed code preserves the expected hash bytes and does not show a new route into production privileges. Some compatibility and security coverage remains unverified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
`main` runs the tests with `node:test` (#292); convert `expect` to `assert` so the file runs under `node --test` and the Jest adapter. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01ThsbPWvW5vnYWEaFs2Q22F
* feat: emit the source map scopes field
A bundler rewrites an imported binding into a namespace read, and a
source map only maps positions, so a debugger cannot resolve the name
the source used. The Scopes proposal answers that by naming, per scope,
the expression each variable evaluates to.
Add map({ scopes: true }). A source declares what its names read through
a new OriginalSource argument, the chunk stream carries it beside the
source content, and the field is built while the mappings are written
rather than decoded back out of them.
* perf: load the scopes encoder only when the option asks for it
Requiring it from the two map assembly helpers put the encoder and its
lookup table in the module graph of every caller, which the memory
benchmarks measured as a constant cost on sources that never emit the
field.
* test: cover the finished-map path of the scopes encoder
The streaming writer reaches none of it, so a map handed in from
elsewhere went untested: the early returns, a segment naming no source
or one the map lacks, a source resumed after another, a name the map
already carries, and a value too large for one base64 digit.
* test: reach the scopes helpers through the public export
The cases required the helper module directly, so the getter that
exposes it on `util` was never run.
* docs: document the scopes option and the bindings a source declares
Also choose the mapping callback once instead of testing for the scopes
writer per segment. Interleaved against main on one machine, that test
cost OriginalSource.map({ columns: true }) a reproducible 1-3%; with the
callback chosen up front the same measurement is neutral.
* chore: expose _scopeBindings without @Private and regenerate types
`tsc` emits `@private` as a TypeScript `private` member, which makes
classes from two installed copies incompatible; `main` dropped these tags
when it moved to generated types (#291). Regenerate `types/` for the
scopes changes.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01ThsbPWvW5vnYWEaFs2Q22F
* test: port the scopes tests to node:test
`main` runs the tests with `node:test` (#292); convert `expect` to
`assert` so the file runs under `node --test` and the Jest adapter.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01ThsbPWvW5vnYWEaFs2Q22F
* fix: close the last scope range where the generated code ends
The scopes writer closed the open range at column 0 of the last line.
When the output does not end with a newline, a range that starts on that
line began after that point, so its end encoded a negative column delta,
which the unsigned VLQ writer turned into 4294967292 and an invalid
`scopes` field. Pass the generated end column through `finish`.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01ThsbPWvW5vnYWEaFs2Q22F
---------
Co-authored-by: Claude <[email protected]>
`test/__mocks__/createMappingsSerializer.js` validated each mapping, but it was never loaded, and it wrapped the serializer's default export, which the library no longer uses: `getFromStreamChunks` and `streamAndGetSourceAndMap` write through `createMappingsWriter`. #292 removed it as dead code. `test/helpers/validate-mappings.js` brings the check back for the writer. It replaces the module's `createMappingsWriter` export before any library file loads, so every writer checks that mappings come in generated order and that a segment without a source has no original position or name. A stream that breaks that now fails the test that produced it instead of being encoded into a quietly wrong `mappings` string. `node --test` loads it with `--require`, Jest with `setupFiles`, so one file serves both. The whole suite passes with the check. The one exception was a unit test feeding the writer a synthetic stream whose source-less segments carried original positions; those segments now use -1 like the library does. Claude-Session: https://claude.ai/code/session_01ThsbPWvW5vnYWEaFs2Q22F Co-authored-by: Claude <[email protected]>
Summary
The tests now use Node's built-in
node:testrunner andassert. Node.js 22+ runs them withnode --test. Node.js 10–20 run the same files with Jest, because they lack a usablenode:test; snapshots need Node.js 22.How it runs
npm testrunsnode --test "test/*.js", andnpm run test:coveragewrites coverage tolcov.info.npm run test:legacyandtest:legacy:coveragerun Jest.jest.config.jsmapsnode:testtotest/helpers/jest-node-test.js, a small adapter that provides the parts of the API the tests use on top of Jest:describe,itwith the test context, hooks, andt.assert.snapshot. It is written for Node.js 10.node:test's format (test/*.snapshot). The Jest adapter reads the same files, using the same keys and serialization. Update them withnpm run test:update-snapshotson Node.js 22+.test:coverage; Node.js 10–20 runtest:legacy:coverage -- --ci. The Codecov step uploads either result.Test changes
expectis converted toassert, using only APIs that exist on Node.js 10. There's noassert.match, so regex checks useassert.ok(re.test(...)).toThrow("text")becomesassert.throws(fn, /text/).describe.eachandit.eachbecomeforloops.expect.assertions(3)becomes a call counter.jest.spyOn(Array.prototype, "sort")becomes a plain save, patch and restore.node:test'smock.methodrejectsArray.prototype..snapshotfiles, sincenode:testhas no inline snapshots.test/helpers.jsmoves totest/helpers/index.js, sotest/*.jsonly matches test files.test/__mocks__/createMappingsSerializer.jswas never loaded. Itsjest.requireActual("../createMappingsSerializer")resolves to a file that doesn't exist, and athrowadded at its top didn't fail any test, so thejest.mock(...)calls had no effect. Reviving its mapping-order validation under both runners would be a separate change.prettier-2, only needed for Jest inline snapshots.@types/jest. The tests are type-checked withnode:test's types only.Test plan
node --test: 89,909 tests pass. That's the Jest count plus the new test from fix: hand out a recorded hash update as buffers #289, which runs twice.npm cideps): 89,909 pass.npm install --save-dev jest@27): 89,909 pass.pretty-formatsettings equals the old.snapentry.node --testand Jest.npm run lint,npm run fmt:check, andnpm run build:typeswith no diff.npm run test:coveragewriteslcov.infocovering all 24lib/files: 98.96% of lines, versus 98.88% with Jest. The tools count lines differently, so Codecov's percentage may shift slightly once.Notes
undefinedfields. Two former inline snapshots had"sourcesContent": undefined; they still catch that field gaining a value..gitattributesforces LF, so snapshot files match on Windows too.🤖 Generated with Claude Code
https://claude.ai/code/session_01ThsbPWvW5vnYWEaFs2Q22F
Generated by Claude Code
Summary by CodeRabbit