fix(coverage): cleanOnRerun: false to invalidate previous results (#6592)

AriPerkkio · web-flow · commit 88bde99cda26 · 2024-10-02T09:54:43.000+02:00
diff --git a/docs/config/index.md b/docs/config/index.md
@@ -1229,7 +1229,7 @@ Clean coverage results before running tests
 - **Available for providers:** `'v8' | 'istanbul'`
 - **CLI:** `--coverage.cleanOnRerun`, `--coverage.cleanOnRerun=false`
 
-Clean coverage report on watch rerun
+Clean coverage report on watch rerun. Set to `false` to preserve coverage results from previous run in watch mode.
 
 #### coverage.reportsDirectory
 
diff --git a/packages/browser/src/client/tester/runner.ts b/packages/browser/src/client/tester/runner.ts
@@ -91,6 +91,7 @@ export function createBrowserRunner(
       if (coverage) {
         await rpc().onAfterSuiteRun({
           coverage,
+          testFiles: files.map(file => file.name),
           transformMode: 'browser',
           projectName: this.config.name,
         })
diff --git a/packages/coverage-istanbul/src/provider.ts b/packages/coverage-istanbul/src/provider.ts
@@ -35,14 +35,30 @@ import { version } from '../package.json' with { type: 'json' }
 import { COVERAGE_STORE_KEY } from './constants'
 
 type Options = ResolvedCoverageOptions<'istanbul'>
-type Filename = string
-type CoverageFilesByTransformMode = Record<
-  AfterSuiteRunMeta['transformMode'],
-  Filename[]
+
+/**
+ * Holds info about raw coverage results that are stored on file system:
+ *
+ * ```json
+ * "project-a": {
+ *   "web": {
+ *     "tests/math.test.ts": "coverage-1.json",
+ *     "tests/utils.test.ts": "coverage-2.json",
+ * //                          ^^^^^^^^^^^^^^^ Raw coverage on file system
+ *   },
+ *   "ssr": { ... },
+ *   "browser": { ... },
+ * },
+ * "project-b": ...
+ * ```
+ */
+type CoverageFiles = Map<
+  NonNullable<AfterSuiteRunMeta['projectName']> | typeof DEFAULT_PROJECT,
+  Record<
+    AfterSuiteRunMeta['transformMode'],
+    { [TestFilenames: string]: string }
+  >
 >
-type ProjectName =
-  | NonNullable<AfterSuiteRunMeta['projectName']>
-  | typeof DEFAULT_PROJECT
 
 interface TestExclude {
   new (opts: {
@@ -70,7 +86,7 @@ export class IstanbulCoverageProvider extends BaseCoverageProvider implements Co
   instrumenter!: Instrumenter
   testExclude!: InstanceType<TestExclude>
 
-  coverageFiles: Map<ProjectName, CoverageFilesByTransformMode> = new Map()
+  coverageFiles: CoverageFiles = new Map()
   coverageFilesDirectory!: string
   pendingPromises: Promise<void>[] = []
 
@@ -188,7 +204,7 @@ export class IstanbulCoverageProvider extends BaseCoverageProvider implements Co
    * Note that adding new entries here and requiring on those without
    * backwards compatibility is a breaking change.
    */
-  onAfterSuiteRun({ coverage, transformMode, projectName }: AfterSuiteRunMeta): void {
+  onAfterSuiteRun({ coverage, transformMode, projectName, testFiles }: AfterSuiteRunMeta): void {
     if (!coverage) {
       return
     }
@@ -200,15 +216,18 @@ export class IstanbulCoverageProvider extends BaseCoverageProvider implements Co
     let entry = this.coverageFiles.get(projectName || DEFAULT_PROJECT)
 
     if (!entry) {
-      entry = { web: [], ssr: [], browser: [] }
+      entry = { web: {}, ssr: {}, browser: {} }
       this.coverageFiles.set(projectName || DEFAULT_PROJECT, entry)
     }
 
+    const testFilenames = testFiles.join()
     const filename = resolve(
       this.coverageFilesDirectory,
       `coverage-${uniqueId++}.json`,
     )
-    entry[transformMode].push(filename)
+
+    // If there's a result from previous run, overwrite it
+    entry[transformMode][testFilenames] = filename
 
     const promise = fs.writeFile(filename, JSON.stringify(coverage), 'utf-8')
     this.pendingPromises.push(promise)
@@ -246,12 +265,13 @@ export class IstanbulCoverageProvider extends BaseCoverageProvider implements Co
     this.pendingPromises = []
 
     for (const coveragePerProject of this.coverageFiles.values()) {
-      for (const filenames of [
+      for (const coverageByTestfiles of [
         coveragePerProject.ssr,
         coveragePerProject.web,
         coveragePerProject.browser,
       ]) {
         const coverageMapByTransformMode = libCoverage.createCoverageMap({})
+        const filenames = Object.values(coverageByTestfiles)
 
         for (const chunk of this.toSlices(
           filenames,
@@ -281,7 +301,9 @@ export class IstanbulCoverageProvider extends BaseCoverageProvider implements Co
       }
     }
 
-    if (this.options.all && allTestsRun) {
+    // Include untested files when all tests were run (not a single file re-run)
+    // or if previous results are preserved by "cleanOnRerun: false"
+    if (this.options.all && (allTestsRun || !this.options.cleanOnRerun)) {
       const coveredFiles = coverageMap.files()
       const uncoveredCoverage = await this.getCoverageMapForUncoveredFiles(
         coveredFiles,
diff --git a/packages/coverage-v8/src/provider.ts b/packages/coverage-v8/src/provider.ts
@@ -53,15 +53,31 @@ interface TestExclude {
 
 type Options = ResolvedCoverageOptions<'v8'>
 type TransformResults = Map<string, FetchResult>
-type Filename = string
 type RawCoverage = Profiler.TakePreciseCoverageReturnType
-type CoverageFilesByTransformMode = Record<
-  AfterSuiteRunMeta['transformMode'],
-  Filename[]
+
+/**
+ * Holds info about raw coverage results that are stored on file system:
+ *
+ * ```json
+ * "project-a": {
+ *   "web": {
+ *     "tests/math.test.ts": "coverage-1.json",
+ *     "tests/utils.test.ts": "coverage-2.json",
+ * //                          ^^^^^^^^^^^^^^^ Raw coverage on file system
+ *   },
+ *   "ssr": { ... },
+ *   "browser": { ... },
+ * },
+ * "project-b": ...
+ * ```
+ */
+type CoverageFiles = Map<
+  NonNullable<AfterSuiteRunMeta['projectName']> | typeof DEFAULT_PROJECT,
+  Record<
+    AfterSuiteRunMeta['transformMode'],
+    { [TestFilenames: string]: string }
+  >
 >
-type ProjectName =
-  | NonNullable<AfterSuiteRunMeta['projectName']>
-  | typeof DEFAULT_PROJECT
 
 type Entries<T> = [keyof T, T[keyof T]][]
 
@@ -86,7 +102,7 @@ export class V8CoverageProvider extends BaseCoverageProvider implements Coverage
   options!: Options
   testExclude!: InstanceType<TestExclude>
 
-  coverageFiles: Map<ProjectName, CoverageFilesByTransformMode> = new Map()
+  coverageFiles: CoverageFiles = new Map()
   coverageFilesDirectory!: string
   pendingPromises: Promise<void>[] = []
 
@@ -181,23 +197,26 @@ export class V8CoverageProvider extends BaseCoverageProvider implements Coverage
    * Note that adding new entries here and requiring on those without
    * backwards compatibility is a breaking change.
    */
-  onAfterSuiteRun({ coverage, transformMode, projectName }: AfterSuiteRunMeta): void {
+  onAfterSuiteRun({ coverage, transformMode, projectName, testFiles }: AfterSuiteRunMeta): void {
     if (transformMode !== 'web' && transformMode !== 'ssr' && transformMode !== 'browser') {
       throw new Error(`Invalid transform mode: ${transformMode}`)
     }
 
     let entry = this.coverageFiles.get(projectName || DEFAULT_PROJECT)
 
     if (!entry) {
-      entry = { web: [], ssr: [], browser: [] }
+      entry = { web: { }, ssr: { }, browser: { } }
       this.coverageFiles.set(projectName || DEFAULT_PROJECT, entry)
     }
 
+    const testFilenames = testFiles.join()
     const filename = resolve(
       this.coverageFilesDirectory,
       `coverage-${uniqueId++}.json`,
     )
-    entry[transformMode].push(filename)
+
+    // If there's a result from previous run, overwrite it
+    entry[transformMode][testFilenames] = filename
 
     const promise = fs.writeFile(filename, JSON.stringify(coverage), 'utf-8')
     this.pendingPromises.push(promise)
@@ -212,9 +231,10 @@ export class V8CoverageProvider extends BaseCoverageProvider implements Coverage
     this.pendingPromises = []
 
     for (const [projectName, coveragePerProject] of this.coverageFiles.entries()) {
-      for (const [transformMode, filenames] of Object.entries(coveragePerProject) as Entries<CoverageFilesByTransformMode>) {
+      for (const [transformMode, coverageByTestfiles] of Object.entries(coveragePerProject) as Entries<typeof coveragePerProject>) {
         let merged: RawCoverage = { result: [] }
 
+        const filenames = Object.values(coverageByTestfiles)
         const project = this.ctx.projects.find(p => p.getName() === projectName) || this.ctx.getCoreWorkspaceProject()
 
         for (const chunk of this.toSlices(filenames, this.options.processingConcurrency)) {
@@ -245,7 +265,9 @@ export class V8CoverageProvider extends BaseCoverageProvider implements Coverage
       }
     }
 
-    if (this.options.all && allTestsRun) {
+    // Include untested files when all tests were run (not a single file re-run)
+    // or if previous results are preserved by "cleanOnRerun: false"
+    if (this.options.all && (allTestsRun || !this.options.cleanOnRerun)) {
       const coveredFiles = coverageMap.files()
       const untestedCoverage = await this.getUntestedFiles(coveredFiles)
 
@@ -519,7 +541,7 @@ export class V8CoverageProvider extends BaseCoverageProvider implements Coverage
   private async convertCoverage(
     coverage: RawCoverage,
     project: WorkspaceProject = this.ctx.getCoreWorkspaceProject(),
-    transformMode?: keyof CoverageFilesByTransformMode,
+    transformMode?: AfterSuiteRunMeta['transformMode'],
   ): Promise<CoverageMap> {
     let fetchCache = project.vitenode.fetchCache
 
diff --git a/packages/vitest/src/public/index.ts b/packages/vitest/src/public/index.ts
@@ -264,7 +264,7 @@ export type {
 } from '../integrations/spy'
 export type { BrowserUI } from '../types/ui'
 
-/** @deprecated import from `vitest/node` instead */
+/** @deprecated import from `vitest/reporter` instead */
 export type Reporter = Reporter_
 /** @deprecated import from `vitest/node` instead */
 export type Vitest = Vitest_
diff --git a/packages/vitest/src/runtime/runners/index.ts b/packages/vitest/src/runtime/runners/index.ts
@@ -90,6 +90,7 @@ export async function resolveTestRunner(
     if (coverage) {
       rpc().onAfterSuiteRun({
         coverage,
+        testFiles: files.map(file => file.name).sort(),
         transformMode: state.environment.transformMode,
         projectName: state.ctx.projectName,
       })
diff --git a/packages/vitest/src/types/general.ts b/packages/vitest/src/types/general.ts
@@ -24,6 +24,7 @@ export interface ModuleCache {
 
 export interface AfterSuiteRunMeta {
   coverage?: unknown
+  testFiles: string[]
   transformMode: TransformMode | 'browser'
   projectName?: string
 }
diff --git a/test/coverage-test/fixtures/test/clean-on-rerun-fixture.test.ts b/test/coverage-test/fixtures/test/clean-on-rerun-fixture.test.ts
@@ -0,0 +1,9 @@
+import { expect, test } from 'vitest'
+import * as math from '../src/math'
+
+// This line will be changed by clean-on-rerun.test.ts
+const methodToTest = 'sum'
+
+test(`run ${methodToTest}`, () => {
+  expect(() => math[methodToTest](1, 2)).not.toThrow()
+})
diff --git a/test/coverage-test/test/changed.test.ts b/test/coverage-test/test/changed.test.ts
@@ -1,6 +1,6 @@
 import { readFileSync, rmSync, writeFileSync } from 'node:fs'
 import { resolve } from 'node:path'
-import { afterAll, beforeAll, expect } from 'vitest'
+import { beforeAll, expect } from 'vitest'
 import { readCoverageMap, runVitest, test } from '../utils'
 
 // Note that this test may fail if you have new files in "vitest/test/coverage/src"
@@ -11,23 +11,21 @@ const FILE_TO_CHANGE = resolve('./fixtures/src/file-to-change.ts')
 const NEW_UNCOVERED_FILE = resolve('./fixtures/src/new-uncovered-file.ts')
 
 beforeAll(() => {
-  let content = readFileSync(FILE_TO_CHANGE, 'utf8')
-  content = content.replace('This file will be modified by test cases', 'Changed!')
-  writeFileSync(FILE_TO_CHANGE, content, 'utf8')
+  const original = readFileSync(FILE_TO_CHANGE, 'utf8')
+  const changed = original.replace('This file will be modified by test cases', 'Changed!')
+  writeFileSync(FILE_TO_CHANGE, changed, 'utf8')
 
   writeFileSync(NEW_UNCOVERED_FILE, `
   // This file is not covered by any tests but should be picked by --changed
   export default function helloworld() {
     return 'Hello world'
   }
   `.trim(), 'utf8')
-})
 
-afterAll(() => {
-  let content = readFileSync(FILE_TO_CHANGE, 'utf8')
-  content = content.replace('Changed!', 'This file will be modified by test cases')
-  writeFileSync(FILE_TO_CHANGE, content, 'utf8')
-  rmSync(NEW_UNCOVERED_FILE)
+  return function restore() {
+    writeFileSync(FILE_TO_CHANGE, original, 'utf8')
+    rmSync(NEW_UNCOVERED_FILE)
+  }
 })
 
 test('{ changed: "HEAD" }', async () => {
diff --git a/test/coverage-test/test/clean-on-rerun.test.ts b/test/coverage-test/test/clean-on-rerun.test.ts
diff --git a/test/coverage-test/test/custom-provider.custom.test.ts b/test/coverage-test/test/custom-provider.custom.test.ts

Original file line number	Diff line number	Diff line change
`@@ -24,6 +24,7 @@ export interface ModuleCache {`
`24`	`24`
`25`	`25`	`export interface AfterSuiteRunMeta {`
`26`	`26`	`coverage?: unknown`
	`27`	`+ testFiles: string[]`
`27`	`28`	`transformMode: TransformMode \| 'browser'`
`28`	`29`	`projectName?: string`
`29`	`30`	`}`