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

Incorrect coverage reported with Node 20.10.0 #236

Open
Snafuh opened this issue Dec 29, 2023 · 2 comments
Open

Incorrect coverage reported with Node 20.10.0 #236

Snafuh opened this issue Dec 29, 2023 · 2 comments

Comments

@Snafuh
Copy link

Snafuh commented Dec 29, 2023

Under Node >=20.10.0 Jest reports incorrect line coverage with the v8 coverage collector.

The issue was initially raised here jestjs/jest#14766 and here jestjs/jest#14764

The issue was further forwarded to the Node team here nodejs/node#51251 but it seems like Node does report the correct coverage.
There was a small change in the format of the V8 Coverage JSON, which might lead to the incorrect results in jest. Jest seems to use this package for v8 coverage, so I think the issue might originate here.

The difference in the coverage report for one of the reproductions was outline in this answer
nodejs/node#51251 (comment)

@AriPerkkio
Copy link
Contributor

This sounds a bit similar as https://bugs.chromium.org/p/v8/issues/detail?id=10029. Though it doesn't make sense why this comes up only when Node version is upgraded. Maybe branches are counted differently based on the order the files are executed?

Can you post minimal reproduction case without Jest or any dependencies? Include the source file and the code the test files are executing.

Ideally script below should be able to reproduce this issue:

import { writeFileSync } from "node:fs";
import inspector from "node:inspector";

// Import the source file here
import * as sourceFile from "./source-file"

const session = new inspector.Session();
const collectedCoverage = [];

session.connect();
session.post("Profiler.enable");
session.post("Runtime.enable");
session.post("Profiler.startPreciseCoverage", {
  callCount: true,
  detailed: true,
});

// Run the methods the tests are executing here
sourceFile.someFunction(); // Comment out this line to see how coverage changes
await collectCoverage();

sourceFile.anotherFunction(); // Comment out this line to see how coverage changes
await collectCoverage();

// Report coverage
writeFileSync(
  "./coverage.json",
  JSON.stringify(collectedCoverage, null, 2),
  "utf8"
);

async function collectCoverage() {
  await new Promise((resolve, reject) => {
    session.post("Profiler.takePreciseCoverage", async (error, coverage) => {
      if (error) return reject(error);

      const filtered = coverage.result.filter(
        (entry) => !entry.url.startsWith("node:") // Exclude NodeJS internals
      );

      collectedCoverage.push(...filtered);

      resolve();
    });
  });
}

@Vinnl
Copy link

Vinnl commented Feb 21, 2024

I think we're running into this too, but it's unpredictable, making it really hard to submit a minimal reproduction. After the upgrade to 20.10, suddenly some lines started to get marked as uncovered. Adding some ignore comments, then re-running the tests, only resulted in other lines getting marked as uncovered. And to make things even harder to reproduce, it's not always the same lines.

What might or might not be helpful, however, is this suspicious result, where I added an ignore comment, and then that comment itself got marked as uncovered. (Possibly related to #238? Though I only saw that for this comment.)

image

For a non-minimal reproduction, I've got:

git clone git@github.com:mozilla/blurts-server.git && cd blurts-server && git checkout 2acab6b && npm install && cp .env-dist .env

If you then run npm test, you should see the coverage thresholds not being met (see /coverage/lcov-report/index.html) - but adding /* c8 ignore next */ comments turns up new places that are marked as uncovered.

It does look like I can keep adding ignore comments until I eventually get to 100% again, but of course, that's not really satisfactory :)

Vinnl added a commit to mozilla/blurts-server that referenced this issue Feb 21, 2024
The Node 20.10 upgrade suddenly started marking them as uncovered.
It's unclear what exactly the cause is, but there's a bug on file
that might be related:
istanbuljs/v8-to-istanbul#236
Vinnl added a commit to mozilla/blurts-server that referenced this issue Feb 21, 2024
The Node 20.10 upgrade suddenly started marking them as uncovered.
It's unclear what exactly the cause is, but there's a bug on file
that might be related:
istanbuljs/v8-to-istanbul#236
Vinnl added a commit to mozilla/blurts-server that referenced this issue Feb 21, 2024
The Node 20.10 upgrade suddenly started marking them as uncovered.
It's unclear what exactly the cause is, but there's a bug on file
that might be related:
istanbuljs/v8-to-istanbul#236
Vinnl added a commit to mozilla/blurts-server that referenced this issue Feb 21, 2024
The Node 20.10 upgrade suddenly started marking them as uncovered.
It's unclear what exactly the cause is, but there's a bug on file
that might be related:
istanbuljs/v8-to-istanbul#236
Vinnl added a commit to mozilla/blurts-server that referenced this issue Feb 21, 2024
The Node 20.10 upgrade suddenly started marking them as uncovered.
It's unclear what exactly the cause is, but there's a bug on file
that might be related:
istanbuljs/v8-to-istanbul#236
Vinnl added a commit to mozilla/blurts-server that referenced this issue Feb 21, 2024
The Node 20.10 upgrade suddenly started marking them as uncovered.
It's unclear what exactly the cause is, but there's a bug on file
that might be related:
istanbuljs/v8-to-istanbul#236
Vinnl added a commit to mozilla/blurts-server that referenced this issue Feb 21, 2024
The Node 20.10 upgrade suddenly started marking them as uncovered.
It's unclear what exactly the cause is, but there's a bug on file
that might be related:
istanbuljs/v8-to-istanbul#236
Vinnl added a commit to mozilla/blurts-server that referenced this issue Mar 6, 2024
As we add code, more existing code suddenly gets marked as
uncovered by unit tests intermittently. This adds c8 ignore
comments for everything that turned up when running

   npm test -- --no-cache --runInBand

But I really hope this gets fixed upstream soon, because this isn't
sustainable.

For more context, see:
#4234

And the (hopefully correct) upstream issue:
istanbuljs/v8-to-istanbul#236
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants