jest: [Bug]: shard option and global coverageThreshold config

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

About this issue

  • Original URL
  • State: open
  • Created 2 years ago
  • Reactions: 41
  • Comments: 25 (4 by maintainers)

Most upvoted comments

Bumping this one.

Are there any plans to bring coverageThreshold compatibility to shards?

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.

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
  1. 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?

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?

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- 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

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

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

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 🙏

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- Do I get right that jest-a-coverage-slip-detector is the solution to this problem?

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.

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