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

v8 test coverage in jest is incorrect when upgrading to 20.10.0 #51251

Open
TWiStErRob opened this issue Dec 21, 2023 · 10 comments
Open

v8 test coverage in jest is incorrect when upgrading to 20.10.0 #51251

TWiStErRob opened this issue Dec 21, 2023 · 10 comments
Labels
inspector Issues and PRs related to the V8 inspector protocol v8 engine Issues and PRs related to the V8 dependency.

Comments

@TWiStErRob
Copy link

TWiStErRob commented Dec 21, 2023

Version

20.10.0

Platform

Macbook Pro M1/M2 / Ubuntu 22.04 x64 (GitHub runner)

Subsystem

V8

What steps will reproduce the bug?

See jestjs/jest#14766

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

Found a consistent repro, but initially it was flaky because of the order jest was running tests.

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

20.9.0 just works
20.10.0 fails

So this is a regression.

What do you see instead?

Coverage information is wrong in jest, unclear why, but based what we know Node is the place where this will most likely be fixed.

Additional information

Based on https://nodejs.org/en/blog/release/v20.10.0, cc @joyeecheung as you might be interested/know what's wrong.

Based on jestjs/jest#14764, cc @kwmhp as you experienced the same problem as us, and figure out the version upgrade is what triggers it.

@joyeecheung
Copy link
Member

Can you provide a repro without third-party modules such as jest? Or maybe someone like @SimenB who's more familiar with Jest knows how to minimize the repro.

@TWiStErRob
Copy link
Author

I can't, sorry, we spent a long time minimizing this much, and have no clue how to go further.

@Snafuh
Copy link

Snafuh commented Dec 21, 2023

@kwmhp here (work/private account)
I am trying to reproduce this with my test case, but I can't so far
node --test --experimental-test-coverage
return 100% coverage.
I will do some more experiments, as the problem in jest occurred only when executing tests in certain order/in-line.
The issue only occurs with v20.10.0, but the problem might be further downstream with aggregation of test coverage.

@koshic
Copy link

koshic commented Dec 23, 2023

Data returned from session.post('Profiler.takePreciseCoverage') (see https://github.com/SimenB/collect-v8-coverage/blob/2dd5d7aad3a8a1a07e33f9bf331f675cb5b9b480/index.js#L25-L27) a bit different for 20.9.0 and 21.5.0:
image

You can find that data at line jest/v8-flaky-coverage/node_modules/jest-runtime/build/index.js:1165, in this._v8CoverageResult.

Because c8 jest --runInBand shows 100% coverage despite of Node version (== NODE_V8_COVERAGE produces proper data), it looks like the root cause is somewhere near inspector and/or how it used by jest-runtime / collect-v8-coverage packages.

@juanarbol juanarbol added v8 engine Issues and PRs related to the V8 dependency. inspector Issues and PRs related to the V8 inspector protocol labels Jan 3, 2024
@ritchieanesco
Copy link

ritchieanesco commented Jan 10, 2024

I am also getting this issue after updating to 20.10.0 from 20.9.0. Is there any workaround?

Our code coverage failure seem to be related to tests that assert error exceptions which are not being included in the code coverage.

@joyeecheung
Copy link
Member

joyeecheung commented Jan 11, 2024

#51251 (comment) mentioned c8 produces correct results, so you could try that if that works. Otherwise it's difficult to tell whether this is a Node.js issue without a repro that only uses Node.js and not third-party modules.

@ritchieanesco
Copy link

ritchieanesco commented Jan 29, 2024

@joyeecheung

@koshic seems to be right with his diagnosis. I can verify that the following fixes the flaky code coverage jestjs/jest#14766 (comment)

The code changes relate to this repo https://github.com/SimenB/collect-v8-coverage/blob/main/index.js

Are you able to provide any insight why this might be the case?

@joyeecheung
Copy link
Member

I don't know much about how the jest package implements coverage collection but if commenting out the stop command makes a difference it might be that the package was stopping coverage collection too early or the ordering of start and stop commands it uses is wrong somehow. C8 spawns child processes with NODE_V8_COVERAGE which is implemented by Node.js to start the collection very early and stop the collection when the process is about to exit, that could be why it is more reliable. Maybe the jest approach could be updated to use NODE_V8_COVERAGE instead of using the inspector API too.

@cenfun
Copy link

cenfun commented Mar 19, 2024

Seems it can be fixed with await this.postSession('Debugger.enable');
see https://github.com/SimenB/collect-v8-coverage/pull/235/files

@kmccammon
Copy link

kmccammon commented Apr 16, 2024

I have been facing the same issue: my coverage report was 100% with 20.9.0, but as of 20.10.0, it now reports less than 100% coverage and somewhat randomly flags lines not covered.

I noticed that I get the same coverage reporting flakiness with 18.20.2 (I initially saw this issue when my GitHub actions started to fail). Coverage works fine with 18.19.1.

So there seems to be a change between 18.19.1 and 18.20.0 that causes the issue as well as between 20.9.0 to 20.10.0.

This is now causing my GitHub CI/CD actions to fail since GitHub is using 18.20.2.

pachun added a commit to pachun/too-many-men that referenced this issue Jul 17, 2024
Apparently, there's a difference in jest's V8 test reporting coverage
depending on which version of node is being used.

nodejs/node#51251

According to the folks talking about that issue in their repo, here:

chapter-three/next-drupal#740 (comment)

They tested and found that the coverage discrepancy occurs in node 18
after upgrading from version 18.19.1 to version 18.20.0.

It also occurs in node 20 after upgrading from version 20.9.0 to version
20.10.0.

Locally, we have been using 18.18.2 where V8 coverage reporting still
appears to be working correctly for us, but CI is reporting something
else (since it's only pinned to version 18 prior to this commit).

Since 18.18.2 seems to work for us, and the folks commenting in the
linked issue seem to agree that the issue shouldn't appear for us in
that version, we'll keep using node version 18.18.2 locally and
explicitly instruct GH actions to also use that version of nodejs.
eventualbuddha added a commit to votingworks/vxsuite that referenced this issue Jul 25, 2024
c8/v8 coverage is buggy post v20.9.0: nodejs/node#51251
eventualbuddha added a commit to votingworks/vxsuite that referenced this issue Jul 25, 2024
c8/v8 coverage is buggy post v20.9.0: nodejs/node#51251
eventualbuddha added a commit to votingworks/vxsuite that referenced this issue Jul 26, 2024
c8/v8 coverage is buggy post v20.9.0: nodejs/node#51251
eventualbuddha added a commit to votingworks/vxsuite that referenced this issue Jul 29, 2024
c8/v8 coverage is buggy post v20.9.0: nodejs/node#51251
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inspector Issues and PRs related to the V8 inspector protocol v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

No branches or pull requests

8 participants