Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bad error message in --experimental-test-coverage output when a source maps source module is missing #54756

Open
jaydenseric opened this issue Sep 4, 2024 · 5 comments · May be fixed by #55037
Assignees
Labels
confirmed-bug Issues with confirmed bugs. coverage Issues and PRs related to native coverage support. source maps Issues and PRs related to source map support. test_runner Issues and PRs related to the test runner subsystem.

Comments

@jaydenseric
Copy link
Contributor

jaydenseric commented Sep 4, 2024

Version

v22.8.0

Platform

Darwin TRI-N93DLJDY6Y 23.6.0 Darwin Kernel Version 23.6.0: Mon Jul 29 21:14:30 PDT 2024; root:xnu-10063.141.2~1/RELEASE_ARM64_T6000 arm64

Subsystem

No response

What steps will reproduce the bug?

In dist/a.mjs:

console.log("Hi");
export {};
//# sourceMappingURL=a.mjs.map

In dist/a.mjs.map:

{"version":3,"file":"a.mjs","sourceRoot":"","sources":["../src/a.mts"],"names":[],"mappings":"AAAA,OAAO,CAAC,GAAG,CAAC,IAAI,CAAC,CAAC"}

In test.mjs:

import "./dist/a.mjs";

Then, run:

node --experimental-test-coverage --test test.mjs

How often does it reproduce? Is there a required condition?

Every time.

What is the expected behavior? Why is that the expected behavior?

An error message reported after the tests output, explaining that a particular source module at a path is missing. If multiple are missing, then output the list of missing paths.

What do you see instead?

Hi
✔ test.mjs (46.715834ms)
ℹ Warning: Could not report code coverage. TypeError: Cannot read properties of undefined (reading '0')
ℹ tests 1
ℹ suites 0
ℹ pass 1
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 51.063542

Additional information

This scenario comes up very frequently when working in a TypeScript project with source maps enabled, that has src and dist directories. Often you have a build script watching the src directory, and as you work you rename or delete a source module. The result is that old artifacts don't get cleaned in the dist directory, but the next time the tests run on the dist directory the coverage explodes with the error this issue reports.

Note that if multiple test modules run, then the coverage report for everything is obscured by the error message:

ℹ Warning: Could not report code coverage. TypeError: Cannot read properties of undefined (reading '0')
@RedYetiDev RedYetiDev added coverage Issues and PRs related to native coverage support. source maps Issues and PRs related to source map support. test_runner Issues and PRs related to the test runner subsystem. labels Sep 4, 2024
@RedYetiDev
Copy link
Member

RedYetiDev commented Sep 4, 2024

See also #54444, which resolved a similar error, so maybe they are somewhat related?

@cjihrig
Copy link
Contributor

cjihrig commented Sep 14, 2024

If someone wants to pick this up, this diff plus the test case provided in the bug report should get you started:

diff --git a/lib/internal/test_runner/coverage.js b/lib/internal/test_runner/coverage.js
index 7ef5702072..4348ea06c3 100644
--- a/lib/internal/test_runner/coverage.js
+++ b/lib/internal/test_runner/coverage.js
@@ -388,8 +388,12 @@ class TestCoverage {
             continue;
           }
 
-          newUrl ??= startEntry?.originalSource;
-          const mappedLines = this.getLines(newUrl);
+          const mappedLines = this.getLines(startEntry.originalSource);
+          if (!mappedLines) {
+            throw new Error('insert better error message here');
+          }
+
+          newUrl ??= startEntry.originalSource;
           const mappedStartOffset = this.entryToOffset(startEntry, mappedLines);
           const mappedEndOffset = this.entryToOffset(endEntry, mappedLines) + 1;
           for (let l = startEntry.originalLine; l <= endEntry.originalLine; l++) {

@RedYetiDev
Copy link
Member

RedYetiDev commented Sep 14, 2024

throw new Error('insert better error message here');

What even is the issue? Is it:

a particular source module at a path is missing

@cjihrig
Copy link
Contributor

cjihrig commented Sep 14, 2024

In that snippet, startEntry.originalSource is the name of a file that does not exist for whatever reason. getLines() handles this case, but this code didn't properly check the return value.

@RedYetiDev RedYetiDev added the confirmed-bug Issues with confirmed bugs. label Sep 20, 2024
@RedYetiDev RedYetiDev self-assigned this Sep 20, 2024
@RedYetiDev
Copy link
Member

Working on this now.

RedYetiDev added a commit to RedYetiDev/node that referenced this issue Sep 21, 2024
Fixes nodejs#54756

Co-Authored-By: Colin Ihrig <cjihrig@gmail.com>
Co-Authored-By: Jayden Seric <me@jaydenseric.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. coverage Issues and PRs related to native coverage support. source maps Issues and PRs related to source map support. test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants