coverage.thresholdAutoUpdate and coverage.perFile Conflict

[michaelfaith]

Describe the bug

We're pretty strict on our unit testing goals, and have our thresholds applied at the per file, level, but we'd also like to take advantage of the really nice auto updating feature for continuous improvement. However, when using coverage.thresholdAutoUpdate, the thresholds are bumped to corresponding with the global coverage achieved, and not the lowest file coverage achieved for each metric. If coverage.perFile is set, the auto update should use the lowest individual file coverage for each of the four metrics, rather than the global coverage for its update. With this conflict, there's no way to use both together. The first test run will raise the threshold higher than the lowest individual file coverage, which results in a failed test run the second time. We have to disable the auto update, when using perFile

e.g. in this example, the statement and branch thresholds should be set to 97.2 and 83.33, but instead are being set to 99.5, and 92.59

----------------------------------|---------|----------|---------|---------|-------------------
File                              | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
----------------------------------|---------|----------|---------|---------|-------------------
All files                         |   99.55 |    92.59 |     100 |   99.55 |                  
 findWorkspacesRoot               |     100 |      100 |     100 |     100 |                  
  findWorkspacesRoot.ts           |     100 |      100 |     100 |     100 |                  
  index.ts                        |     100 |      100 |     100 |     100 |                  
 hasConfigFile                    |     100 |      100 |     100 |     100 |                  
  hasConfigFile.ts                |     100 |      100 |     100 |     100 |                  
 hasDependency                    |     100 |    83.33 |     100 |     100 |                  
  hasDependency.ts                |     100 |    83.33 |     100 |     100 | 19,38            
 isInMonorepo                     |     100 |      100 |     100 |     100 |                  
  isInMonorepo.ts                 |     100 |      100 |     100 |     100 |                  
 loadConfig                       |    97.2 |    95.23 |     100 |    97.2 |                  
  loadConfig.ts                   |    97.2 |    95.23 |     100 |    97.2 | 89-92            
 loadConfig/__fixtures__/demo-lib |     100 |      100 |     100 |     100 |                  
  test.config.js                  |     100 |      100 |     100 |     100 |                  
 readNearestPackage               |     100 |       90 |     100 |     100 |                  
  PackageJson.ts                  |     100 |      100 |     100 |     100 |                  
  index.ts                        |     100 |      100 |     100 |     100 |                  
  readNearestPackage.ts           |     100 |    89.28 |     100 |     100 | 14,37,72    

I searched existing issues for this, and don't see anything related to it.

Reproduction

Generating coverage folder doesn't seem to work in stackblitz? Hopefully, the description is thorough enough.

System Info

System:
    OS: Windows 10 10.0.22000
    CPU: (12) x64 11th Gen Intel(R) Core(TM) i5-11500H @ 2.90GHz
    Memory: 7.56 GB / 31.67 GB
  Binaries:
    Node: 16.19.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 3.5.0 - C:\Program Files\nodejs\yarn.CMD
    npm: 8.19.3 - C:\Program Files\nodejs\npm.CMD

Used Package Manager

yarn

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[AriPerkkio]

@michaelfaith is this coverage.provider: c8 or coverage.provider: istanbul specific, or both?

[michaelfaith]

Ah, I should have specified. I've only tried this with c8. Here is our config:

import { defineConfig } from 'vitest/config'

export default defineConfig({
  test: {
    root: 'src',
    reporters: ['default', 'junit'],
    outputFile: {
      junit: 'junit.xml',
    },
    coverage: {
      provider: 'c8',
      enabled: true,
      thresholdAutoUpdate: true, 
      reporter: ['text', 'json-summary', 'html', 'cobertura'],
      statements: 97,
      branches: 83,
      perFile: true,
    },
    clearMocks: true,
  },
});

[AriPerkkio]

This looks like a bug, or a missing feature. Both providers are affected.

The updateThresholds here should accept a perFile option from coverage providers that is used to resolve the thresholds.

vitest/packages/vitest/src/utils/coverage.ts

Line 23 in baf902a

const summary = coverageMap.getCoverageSummary()

Similarly as done here in threshold checking:

vitest/packages/coverage-istanbul/src/provider.ts

Lines 172 to 183 in baf902a

	checkThresholds(coverageMap: CoverageMap, thresholds: Record<Threshold, number | undefined>) {
	// Construct list of coverage summaries where thresholds are compared against
	const summaries = this.options.perFile
	? coverageMap.files()
	.map((file: string) => ({
	file,
	summary: coverageMap.fileCoverageFor(file).toSummary(),
	}))
	: [{
	file: null,
	summary: coverageMap.getCoverageSummary(),
	}]

[michaelfaith]

Thanks for the analysis / confirmation

[michaelfaith]

Thanks for the quick turnaround on this!