[Bug]: shard option and global coverageThreshold config

[tleunen]

Version

28.0.1

Steps to reproduce

Run tests with the shard option and have coverageThreshold defined in the config with some global values.

Expected behavior

The global values should only be tested against tests who ran in the given shard

Actual behavior

the global values are used for all tests, even those who don't run in the same shard

Additional context

I'm actually not 100% sure about the right behavior to have here. But if we set some global coverage threshold, those won't be met because only a subset of tests run on each shard. All others are considered 0

Environment

System:
    OS: macOS 11.6.4
    CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
  Binaries:
    Node: 16.14.1 - ~/.volta/tools/image/node/16.14.1/bin/node
    npm: 8.5.0 - ~/.volta/tools/image/node/16.14.1/bin/npm
  npmPackages:
    jest: ^28.0.1 => 28.0.1

[github-actions[bot]]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.

[Belco90]

Ran into the same issue. I had to stop running the tests with coverage for now.

[luke-lacroix-healthy]

I can confirm this issue.

I am using "--shard" to overcome the memory leak issue (#11956). However, I have to turn coverage off because the shard is not limiting the coverage. I'm not sure this can be fixed though without some changes to how Jest collects and reports coverage.

Notably: the "--shard" option would have to output coverage to a location and then some final Jest invocation would have to combine the sharded coverage reports into one final report.

At the very least, Jest should warn about using a coverage threshold with shards.

[luisrpfigueiredo]

We are facing a similar issue. It'd be great to have the possibility to run the checks of a previously-generated coverage report against the global threshold. This way we could handle the merging of all sharded coverage reports ourselves, and once we have a single file with the final results, check that it passes the global threshold.

This is still possible at the moment but we have to create a script for it ourselves.

I think this would be a simple solution.

[diogoredin]

We are also facing the same issue. Even if we were able to combine the multiple reports into one, the first shard run considers all the other files (which will be tested in the next shards) as not tested. Is there any way to fix this without turning off the coverageThreshold? 💭

We found this issue referenced in the initial feature implementation and the mention of the --includes flag but it's not clear how that fixes the coverageThreshold requirement. https://github.com/facebook/jest/pull/12546#issuecomment-1068380480

Any insight on this? @marionebl @SimenB 🙏

[liangskyli]

Ran into the same issue. I had to stop use this features(shard). Because coverage is very important for me.

[ZanMarolt]

We have hit the same "bug" as well. We can not split unit tests via shards and have the code coverage enabled as it will look for all the files within global option instead of just the ones that were ran in the shard job. 🤔

@luke-lacroix-healthy wouldn't the command fail in the first place while generating the coverage with a coverageThreshold.global config set? I already see a potential problem that test cases for a single file get sharded into multiple jobs, so the file would get overriden, making the coverage wrong for that specific file if the test cases are split for that specific file?

Would love to see a workaround or a solution from the jest team 🙏

[luke-lacroix-healthy]

We have hit the same "bug" as well. We can not split unit tests via shards and have the code coverage enabled as it will look for all the files within global option instead of just the ones that were ran in the shard job. 🤔

@luke-lacroix-healthy wouldn't the command fail in the first place while generating the coverage with a coverageThreshold.global config set? I already see a potential problem that test cases for a single file get sharded into multiple jobs, so the file would get overriden, making the coverage wrong for that specific file?

Would love to see a workaround or a solution from the jest team 🙏

My thought is that each shards output would go into a separate folder and then combined at the end. Should work, in theory.

[marionebl]

Thinking through this we'll have to create a 2-phase pattern to retain the original behaviour of coverageThreshold.

1. Execute shards collecting coverage information

yarn jest --shard 1/2 --coverage
yarn jest --shard 2/2 --coverage

2. Merge coverage information and enforce threshold

yarn jest --mergeCoverage --coverageThreshold

There are some quality of life improvement to make, e.g. fail for certain --shard and --coverage* flag combinations with very helpful error messages. cc @SimenB - what are your thoughts on intended developer experience for this case?

[SimenB]

Multiple runs is the only to make Jest's threshold check work.

However, that threshold can be moved out of Jest (to e.g. coveralls or your CI) and then that could assert that the total coverage data is whatever threshold you want.

I'm not sure if adding coverage merging as a separate "mode" is something we want in Jest. However, I don't feel strongly.

[luke-lacroix-healthy]

@SimenB I could be mistaken but, wouldn't that still require changes to Jest so that coverage was still collected but not reported?

[mrazauskas]

@jcw- Do I get right that jest-a-coverage-slip-detector is the solution to this problem?

[SimenB]

I could be mistaken but, wouldn't that still require changes to Jest so that coverage was still collected but not reported?

You'd remove coverageThreshold from Jest but still collect coverage like you do already. The responsibility of a failing status check based on coverage would be moved away from Jest itself to something that looks at all test runs collective coverage

[luke-lacroix-healthy]

I could be mistaken but, wouldn't that still require changes to Jest so that coverage was still collected but not reported?

You'd remove coverageThreshold from Jest but still collect coverage like you do already. The responsibility of a failing status check based on coverage would be moved away from Jest itself to something that looks at all test runs collective coverage

Ok. I can test that out.

[jcw-]

@jcw- Do I get right that jest-a-coverage-slip-detector is the solution to this problem?

Yes - or at least, the same strategy is. You have to collect the coverage from all shards and merge it together before you validate it against coverage targets.

https://github.com/GetJobber/jest-a-coverage-slip-detector#concurrency-and-parallelism

[kimyu92]

Aside from jest-a-coverage-slip-detector

What is the alternative or the official recommendation/guide to merge coverage files when using shard option?

[jcw-]

Aside from jest-a-coverage-slip-detector

What is the alternative or the official recommendation/guide to merge coverage files when using shard option?

I struggled finding an authoritative answer (but would love to see one!), but with a lot of source code reading and trial and error, landed on this approach, which uses the same underlying library as jest itself (istanbul):

https://github.com/GetJobber/jest-a-coverage-slip-detector/blob/main/src/mergeCoverage.js

Feel free to leverage it directly in your project, you'll just need to add three dependencies (istanbul-lib-coverage, istanbul-lib-report, istanbul-reports). You'll also need to generate and collect full coverage reports for each shard (not just summaries).

[kbrooks]

I was able to add coverage back into my sharded tests on Jenkins using istanbuljs/nyc with the following approach:

1. Run sharded tests on n different CI executors
   1. Run the sharded tests with the --coverage option using the json reporter to produce coverage/coverage-final.json
   2. Upload coverage from the current executor as coverage-final-{shard_number}.json (I used GCS)
2. Merge coverage in a single CI executor after all sharded executors have completed
   1. Download the coverage-final-{shard_number}.json files into final-coverage directory
   2. In a script, run sed[1] to change the path of the files in final-coverage to match the current executor
      • nyc needs the absolute path referenced in the coverage file to calculate coverage percentages
   3. execute yarn test:ci:mergeCoverage, yarn test:ci:reportCoverage, and finally test:ci:validateCoverage [2] in a script

[1] sed script:

find final-coverage-files -type f -exec sed -i 's|/path/to/project/root/on/the/sharded/test/executors|'$(pwd)'|g' {} \;

[2] new scripts in package.json:

"test:ci:mergeCoverage": "mkdir -p merged-coverage && nyc merge final-coverage-files merged-coverage/coverage-final.json",
"test:ci:reportCoverage": "nyc report --reporter=text-summary --reporter=json-summary -t merged-coverage",
"test:ci:validateCoverage": "nyc check-coverage --branches 80 --functions 80 --lines 80 --statements 80 -t merged-coverage",

[jcw-]

I was able to add coverage back into my sharded tests on Jenkins using istanbuljs/nyc

Thanks for posting your approach! I wish that repo was still active, I considered using the CLI but it hasn't had any commits or releases for over two years. :(

[kimyu92]

https://github.com/facebook/jest/issues/12751#issuecomment-1314236352 This makes sense to me

Since there is no activity istanbuljs/nyc, and jest has already provided shard option, for better dev experience, it makes sense to provide mergeCoverage, reportCoverage and validateCoverage OOB to make shared coverage much painless.

@SimenB thoughts?

[luke-lacroix-healthy]

I've got this mostly working locally using nyc to merge the coverage-final.json documents and then generate a report. However, coverage checks in nyc are only global and not based on globs. That is: I cannot specify a different set of coverage options for different sets of files.

Additionally, some of the better GitHub Actions that report Jest coverage REQUIRE the report.json output from Jest, which is not a format that Istanbul knows how to merge. So, aside from writing a brand new GitHub action that appropriately merges the reports, generates markdown, and generates annotations on the PR, this isn't a good solution.

Having these features directly in Jest sounds best, especially since nyc hasn't been updated in quite some time.

[ColeWalker-Drizly]

Are there any plans to bring coverageThreshold compatibility to shards?

[anandtiwary]

This is a big problem for us. We are running our tests in shards. Once the test execution completes, Jest gets stuck in "running coverage on untested files" error. This is an easily reproducible error. We need to make shard optional and automatically collect coverage from included test files.

[MorenoMdz]

Bumping this one.

[OBellon]

Any update on this?