Skip to content

test runner: Could not report code coverage (regression) #54240

Closed
#54444
Closed
test runner: Could not report code coverage (regression)#54240
#54444
Labels
coverageIssues and PRs related to Node.js code coverage support.test_runnerIssues and PRs related to the test runner subsystem.

Description

@yume-chan
yume-chan
opened on Aug 7, 2024

Version

v22.6.0

Platform

Linux f5d6c39aef47 5.15.153.1-microsoft-standard-WSL2 #1 SMP Fri Mar 29 23:14:13 UTC 2024 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

Because the cryptic error message, I can't narrow the repro down further.

npm i -g pnpm
git clone --depth 1 https://github.com/yume-chan/ya-webadb.git
cd ya-webadb/
pnpm i
pnpm build
cd libraries/struct
npx tsc -b tsconfig.test.json
node --enable-source-maps --experimental-test-coverage --test esm/struct.spec.js

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

100%

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

Reports code coverage. It worked in v20.15.0

What do you see instead?

Begin with v20.16.0, it outputs:

ℹ Warning: Could not report code coverage. TypeError: Cannot read properties of undefined (reading 'line')

Additional information

No response

Activity

  1. avivkeller
    closed this as not plannedon Aug 7, 2024

  2. avivkeller commented on Aug 7, 2024

    @avivkeller
    avivkeller
    on Aug 7, 2024
    Member

    This is an issue that is caused by the tsc compilation. If I recall correctly, a fix is not planned, but I might be wrong.

    CC @nodejs/test_runner to be sure

  3. avivkeller
    added
    wontfixIssues that will not be fixed.
    on Aug 7, 2024

  4. cjihrig commented on Aug 7, 2024

    @cjihrig
    cjihrig
    on Aug 7, 2024
    Contributor

    This doesn't seem like a good error for users to encounter. We should improve it.

  5. cjihrig
    reopened this on Aug 7, 2024
  6. avivkeller
    removed
    wontfixIssues that will not be fixed.
    on Aug 7, 2024

  7. avivkeller commented on Aug 7, 2024

    @avivkeller
    avivkeller
    on Aug 7, 2024
    Member

    Thanks for taking a look! Sorry for my misjudgment.

  8. avivkeller
    added
    coverageIssues and PRs related to Node.js code coverage support.
    test_runnerIssues and PRs related to the test runner subsystem.
    on Aug 7, 2024

  9. avivkeller commented on Aug 7, 2024

    @avivkeller
    avivkeller
    on Aug 7, 2024
    Member

    Related:
    #52775
    #53719 (comment)
    #51392 (comment)

    nodejs/help#4325
    #51552

  10. avivkeller commented on Aug 7, 2024

    @avivkeller
    avivkeller
    on Aug 7, 2024
    Member

    FWIW I couldn't reproduce using the following code:

    interface Person {
    firstName: string;
    lastName: string;
    age: number;
    greet(): string;
}

class Student implements Person {
    firstName: string;
    lastName: string;
    age: number;
    grade: string;

    constructor(firstName: string, lastName: string, age: number, grade: string) {
        this.firstName = firstName;
        this.lastName = lastName;
        this.age = age;
        this.grade = grade;
    }

    greet(): string {
        return `Hello, my name is ${this.firstName} ${this.lastName} and I am ${this.age} years old.`;
    }

    getGrade(): string {
        return `I am in grade ${this.grade}.`;
    }
}

let student1 = new Student('John', 'Doe', 20, 'Junior');

console.log(student1.greet());
console.log(student1.getGrade());
    npx tsc source.ts
node --enable-source-maps --experimental-test-coverage --test source.js

    But I'll keep trying to find a minimal reproduction

  11. avivkeller commented on Aug 7, 2024

    @avivkeller
    avivkeller
    on Aug 7, 2024
    Member

    @yume-chan could you provide the compiled file + sourcemap?

  12. yume-chan commented on Aug 8, 2024

    @yume-chan
    yume-chan
    on Aug 8, 2024
    ContributorAuthor

    I found a minimal repro. It just needs enough lines:

    index.ts 
    1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
function a() {
  console.log(1);
}
a();
    index.js 
    1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
1;
function a() {
    console.log(1);
}
a();
//# sourceMappingURL=index.js.map

    index.js.map

    {"version":3,"file":"index.js","sourceRoot":"","sources":["index.ts"],"names":[],"mappings":"AAAA,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,CAAC,CAAC;AACF,SAAS,CAAC;IACR,OAAO,CAAC,GAAG,CAAC,CAAC,CAAC,CAAC;AACjB,CAAC;AACD,CAAC,EAAE,CAAC"}

    Any source map visualizer will tell you the generated source map is definitely correct: link

    Output:

    > node --enable-source-maps --experimental-test-coverage --test "index.js"                              
1
✔ D:\dev\sandbox\node-source-map\index.js (90.0243ms)
ℹ Warning: Could not report code coverage. TypeError: Cannot read properties of undefined (reading 'line')
ℹ tests 1
ℹ suites 0
ℹ pass 1
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 95.1846

    Node.js source map module calculates line lengths excluding the line break character:

    node/lib/internal/source_map/source_map_cache.js

    Lines 228 to 249 in 90dea9e

    // Cache the length of each line in the file that a source map was extracted
    // from. This allows translation from byte offset V8 coverage reports,
    // to line/column offset Source Map V3.
    function lineLengths(content) {
    const contentLength = content.length;
    const output = [];
    let lineLength = 0;
    for (let i = 0; i < contentLength; i++, lineLength++) {
    const codePoint = StringPrototypeCodePointAt(content, i);
    // We purposefully keep \r as part of the line-length calculation, in
    // cases where there is a \r\n separator, so that this can be taken into
    // account in coverage calculations.
    // codepoints for \n (new line), \u2028 (line separator) and \u2029 (paragraph separator)
    if (codePoint === 10 || codePoint === 0x2028 || codePoint === 0x2029) {
    ArrayPrototypePush(output, lineLength);
    lineLength = -1; // To not count the matched codePoint such as \n character
    }
    }
    ArrayPrototypePush(output, lineLength);
    return output;
    }

    The line lengths are then converted to line offsets, but it's done incorrectly:

    node/lib/internal/test_runner/coverage.js

    Lines 342 to 346 in 90dea9e

    const executedLines = ArrayPrototypeMap(lineLengths, (length, i) => {
    const coverageLine = new CoverageLine(i + 1, offset, null, length);
    offset += length;
    return coverageLine;
    });

    It didn't add the line break characters back, so after each line the offset is short by 1.

    It then got matched with correct offsets:

    node/lib/internal/test_runner/coverage.js

    Line 363 in 90dea9e

    const { lines } = mapRangeToLines(ranges[k], executedLines);

    So after enough lines, mapRangeToLines returns an empty lines array, causing this line to throw error:

    node/lib/internal/test_runner/coverage.js

    Line 366 in 90dea9e

    .findEntry(lines[0].line - 1, MathMax(0, startOffset - lines[0].startOffset));

    Fix:

    --- a/lib/internal/test_runner/coverage.js
+++ b/lib/internal/test_runner/coverage.js
@@ -340,8 +340,8 @@ class TestCoverage {
       const { data, lineLengths } = sourceMapCache[url];
       let offset = 0;
       const executedLines = ArrayPrototypeMap(lineLengths, (length, i) => {
-        const coverageLine = new CoverageLine(i + 1, offset, null, length);
-        offset += length;
+        const coverageLine = new CoverageLine(i + 1, offset, null, length + 1);
+        offset += length + 1;
         return coverageLine;
       });
       if (data.sourcesContent != null) {

  13. martinssonj commented on Aug 8, 2024

    @martinssonj
    martinssonj
    on Aug 8, 2024

    Thanks for investigating this issue. I also experience the same problem.

  14. cjihrig commented on Aug 13, 2024

    @cjihrig
    cjihrig
    on Aug 13, 2024
    Contributor

    @yume-chan thanks for looking into this. Would you like to open a PR?

  15. yume-chan commented on Aug 16, 2024

    @yume-chan
    yume-chan
    on Aug 16, 2024
    ContributorAuthor

    @cjihrig I'm really busy recently. A Node.js contributor may fix this quicker than me. maybe @MoLow can you take a look as you authored this part?

    ref #53315

  16. nodejs-github-bot
    closed this as completedin #54444on Aug 21, 2024
  17. nodejs-github-bot
    closed this as completedin 821ffabon Aug 21, 2024
  18. Trott
    added a commit that references this issue on Aug 21, 2024

    test_runner: account for newline in source maps

    821ffab
  19. RafaelGSS
    added a commit that references this issue on Aug 25, 2024

    test_runner: account for newline in source maps

    45b0250
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    coverageIssues and PRs related to Node.js code coverage support.test_runnerIssues and PRs related to the test runner subsystem.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions