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

fix(jest-runtime): correctly report V8 coverage with resetModules: true #12912

Merged
merged 4 commits into from
Jun 5, 2022
Merged

fix(jest-runtime): correctly report V8 coverage with resetModules: true #12912

merged 4 commits into from
Jun 5, 2022

Conversation

mrazauskas
Copy link
Contributor

Fixes #12904

Summary

Bumped into the same issue yesterday in one of my projects. Using resetModules: true option or calling jest.resetModules() breaks V8 coverage. Jest reports zero coverage for modules which were reset.

Problem

Seems like the root of the issue is this line: https://github.com/facebook/jest/blob/9806c906f32b91868b4a5712f962a96484bcdf8c/packages/jest-runtime/src/index.ts#L1158

Calling resetModules() clears off this._fileTransforms map which is needed later in V8 coverage logic (to detect if a file was executed and to map coverage to sources as well):

https://github.com/facebook/jest/blob/9806c906f32b91868b4a5712f962a96484bcdf8c/packages/jest-runtime/src/index.ts#L1206-L1217

Solution

Perhaps keeping a copy of sources for V8 in this._v8CoverageSources is the most optimal?

It is also possible to collect coverage results with each resetModule call. Not sure if that is good idea, because resetModule does not interrupt V8 anyhow.

Test plan

An e2e test, which is failing on main is added.

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great catch!

@SimenB SimenB changed the title fix(jest-runtime): correctly report V8 coverage with resetModules: true fix(jest-runtime): correctly report V8 coverage with resetModules: true Jun 5, 2022
@SimenB SimenB merged commit 3a8696b into jestjs:main Jun 5, 2022
@mrazauskas mrazauskas deleted the fix-resetModules-v8Coverage branch June 5, 2022 09:59
@mrazauskas
Copy link
Contributor Author

Thanks! Just checked v28.1.1, now all works as expected.

@github-actions
Copy link

github-actions bot commented Jul 8, 2022

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Unable to generate v8 coverage with Jest v28 for a monorepo
3 participants