test(NODE-7493): Enable source maps for accurate test debugging#4947
test(NODE-7493): Enable source maps for accurate test debugging#4947seanrmilligan wants to merge 1 commit into
Conversation
seanrmilligan
force-pushed
the
sean.milligan/source-maps
branch
from
0eac018 to
5e04b2e
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Enables Node.js native source map support across the test runners so that stack traces from compiled TypeScript test files report original .ts line numbers. Adds a demonstration/verification test for the behavior.
Changes:
- Adds
--enable-source-mapsto thenode-optionarray in all mocha config files. - Enables
sourceMap: trueintest/tsconfig.jsonso emitted JS has source maps. - Adds a new unit test that verifies V8 natively maps stack frames back to TS source lines.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| .mocharc.js | Adds enable-source-maps to node-options for default test runs. |
| test/mocha_mongodb.js | Adds enable-source-maps for integration test runs. |
| test/mocha_lambda.js | Adds enable-source-maps for lambda test runs. |
| test/manual/mocharc.js | Adds enable-source-maps for manual test runs. |
| test/tsconfig.json | Enables sourceMap emission for tests. |
| test/unit/sourcemap_demo.test.ts | New test verifying V8 reports TS line numbers in stack frames. |
| package.json | Appends build:runtime-barrel to bundled check scripts using ;. |
| console.log('\n ── raw V8 frame (prepareStackTrace bypassed) ──'); | ||
| console.log(` file : ${frame.file}`); | ||
| console.log(` line : ${frame.line} (TypeScript source line is ${TS_SOURCE_LINE})`); | ||
| console.log(` col : ${frame.col}`); | ||
| console.log( | ||
| ` ${frame.line === TS_SOURCE_LINE ? '✔ line matches TS source' : `✘ line ${frame.line} ≠ TS source line ${TS_SOURCE_LINE} — source maps not applied by V8`}` | ||
| ); |
| describe('Source map verification (NODE-7493)', function () { | ||
| it('V8 native stack frame reports correct TypeScript line number', function () { |
Pass --enable-source-maps to Node for all mocha configs so V8 resolves inline source maps emitted by ts-node, making the debugger show correct TypeScript file/line locations. Also add sourceMap: true to test/tsconfig.json and ensure bundled test runs reset test/mongodb.ts so stale VM-context state does not affect subsequent debug sessions.
seanrmilligan
force-pushed
the
sean.milligan/source-maps
branch
from
5e04b2e to
228897f
Compare
| "check:search-indexes": "npm run build:bundle && nyc mocha --config test/mocha_mongodb.js test/manual/search-index-management.prose.test.ts", | ||
| "check:test": "npm run build:bundle && nyc mocha --config test/mocha_mongodb.js test/integration", | ||
| "check:test-bundled": "MONGODB_BUNDLED=true npm run check:test", | ||
| "check:test-bundled": "MONGODB_BUNDLED=true npm run check:test; npm run build:runtime-barrel", |
There was a problem hiding this comment.
What is the purpose of building the barrel after running tests? I don't think we need these two changes.
PavelSafronov
left a comment
PavelSafronov
left a comment
There was a problem hiding this comment.
One of the Acceptance Criteria is to verify hostMatchesWildcards debugging. This can be done manually (test locally, add the repro steps to the PR description, I'll verify on my side), or maybe we can update sourcemap.test.ts test to import and use hostMatchesWildcards and verify that the debugger experience is improved. Up to you how we verify this.
There was a problem hiding this comment.
Love the idea of checking programmatically that the correct line is reported. But the current implementation is a bit flaky: we are hard-coding TS_SOURCE_LINE. What if instead we populate that value during test runtime by reading the test file?
tadjik1
added
the
Primary Review
Description
Summary of Changes
test/tsconfig.jsonis updated to include"sourceMap": truein thecompilerOptions, ensuring that TypeScript generates source maps when compiling test files..mocharc.js,test/manual/mocharc.js,test/mocha_lambda.js,test/mocha_mongodb.js) are modified to consistently pass the--enable-source-mapsNode.js option. This directs the V8 engine to use available source maps for more accurate stack traces. Thenode-optionnow always includesenable-source-maps, regardless of the Node.js major version.test/unit/sourcemap_demo.test.ts, is introduced to explicitly verify that V8's native stack traces correctly map back to the original TypeScript source line numbers.check:test-bundledandcheck:unit-bundledscripts inpackage.jsonare modified to includenpm run build:runtime-barrelafter their respective test commands.Notes for Reviewers
The
sourcemap_demo.test.tsutilizes a temporary bypass ofsource-map-support'sError.prepareStackTraceoverride. This allows the test to observe V8's native behavior with the--enable-source-mapsflag in isolation, providing a direct confirmation that the V8 engine itself is correctly resolving stack trace line numbers to their TypeScript origins.What is the motivation for this change?
This change improves the debugging experience for tests. By enabling source map generation and activating V8's native source map support, stack traces for errors occurring in tests will now correctly point to the original TypeScript source files and line numbers, rather than the transpiled JavaScript output.
Release Highlight
Release notes highlight
Double check the following
npm run check:lint)type(NODE-xxxx)[!]: descriptionfeat(NODE-1234)!: rewriting everything in coffeescript