-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
fix(jest-config): allow collectCoverage
to be configured at project level
#15854
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
base: main
Are you sure you want to change the base?
Conversation
… level Signed-off-by: hainenber <[email protected]>
✅ Deploy Preview for jestjs ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
Signed-off-by: hainenber <[email protected]>
babel-jest
babel-plugin-jest-hoist
babel-preset-jest
create-jest
@jest/diff-sequences
expect
@jest/expect-utils
jest
jest-changed-files
jest-circus
jest-cli
jest-config
@jest/console
@jest/core
@jest/create-cache-key-function
jest-diff
jest-docblock
jest-each
@jest/environment
jest-environment-jsdom
@jest/environment-jsdom-abstract
jest-environment-node
@jest/expect
@jest/fake-timers
@jest/get-type
@jest/globals
jest-haste-map
jest-jasmine2
jest-leak-detector
jest-matcher-utils
jest-message-util
jest-mock
@jest/pattern
jest-phabricator
jest-regex-util
@jest/reporters
jest-resolve
jest-resolve-dependencies
jest-runner
jest-runtime
@jest/schemas
jest-snapshot
@jest/snapshot-utils
@jest/source-map
@jest/test-result
@jest/test-sequencer
@jest/transform
@jest/types
jest-util
jest-validate
jest-watcher
jest-worker
pretty-format
commit: |
…undefined` Signed-off-by: hainenber <[email protected]>
Signed-off-by: hainenber <[email protected]>
Signed-off-by: hainenber <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested the current behavior of 30.2.0 withcollectCoverage
option. Here are what I found.
Given a config
export default {
collectCoverage: true,
projects: [
{
displayName: 'Package 1',
rootDir: '<rootDir>/packages/package-1',
testMatch: ['<rootDir>/test-e2e/**/*.e2e.js'],
},
{
displayName: 'Package 2',
rootDir: '<rootDir>/packages/package-2',
testMatch: ['<rootDir>/test-e2e/**/*.e2e.js'],
},
{
displayName: 'Package 3',
rootDir: '<rootDir>/packages/package-3',
testMatch: ['<rootDir>/test-e2e/**/*.e2e.js'],
},
],
};
- When having global
collectCoverage: true
, thecollectCoverage
value perproject
will be always set tofalse
- When setting a
collectCoverage: false
for aproject
which means overriding the global level. This overriding value is correctly set forconfig
argument ofshouldInstrument
function.
In summary
- Solving the warning issue in #15588 only requires the change in
jest-types
,test-utils
andjest-config/src/index.ts
- To make the overriding value of
collectCoverage
works, it requires a change herejest/packages/jest-runner/src/runTest.ts
Line 316 in 3e87145
await runtime.collectV8Coverage();
The condition collectV8Coverage
currently takes value from
jest/packages/jest-runner/src/runTest.ts
Line 301 in 3e87145
const collectV8Coverage = |
Thanks @ahnpnl for the review! That would help resolving one of the pain point of changing |
The original issue #15588 uses provider Usually, I hope these information will help you further for your investigation 👍 |
Thank you for your effort. I'll go back to the lab for this one. |
I've re-examined both approach and unfortunately with default I suppose we have to compromise by letting |
did you make this change to make v8 working correctly first? I think this one we should have Have you checked where Babel coverage is called? |
Yes, I've made changes to Changes in
![]()
![]() Result in test repo with ![]()
This is where I think the Babel coverage gets called after transformed by Babel. |
I got this now. Indeed if we make |
I think we just need to adjust https://github.com/jestjs/jest/blob/main/e2e/__tests__/coverageReport.test.ts to reflect this change and all will be good 👍 |
Summary
Closes #15588
collectCoverage
to be toggled on-off at project level would be pretty handy for folks in monorepo. To implement this,collectCoverage
's default value for projects has to beundefined
unless specified otherwise.This only affects
--showConfig
CLI option as JSON spec doesn't allowundefined
value to be printed out so the result will misscollectCoverage
value.One caveat from this approach is the folks specifying
collectCoverage: undefined
for a project would not see desired result of filtering coverage statistics for that particular project but fortunately, the accepted value type in the doc is Boolean so I suspect people will realize the issue and usecollectCoverage: true | false
per doc.Test plan