fix(coverage): correct coverage when isolate: false is used (#6957)

AriPerkkio · web-flow · commit 426ce6d8f922 · 2024-12-09T13:41:40.000+02:00
diff --git a/packages/browser/src/client/tester/tester.ts b/packages/browser/src/client/tester/tester.ts
@@ -122,7 +122,7 @@ async function executeTests(method: 'run' | 'collect', files: string[]) {
   try {
     await Promise.all([
       setupCommonEnv(config),
-      startCoverageInsideWorker(config.coverage, executor),
+      startCoverageInsideWorker(config.coverage, executor, { isolate: config.browser.isolate }),
       (async () => {
         const VitestIndex = await import('vitest')
         Object.defineProperty(window, '__vitest_index__', {
@@ -160,7 +160,7 @@ async function executeTests(method: 'run' | 'collect', files: string[]) {
       }, 'Cleanup Error')
     }
     state.environmentTeardownRun = true
-    await stopCoverageInsideWorker(config.coverage, executor).catch((error) => {
+    await stopCoverageInsideWorker(config.coverage, executor, { isolate: config.browser.isolate }).catch((error) => {
       client.rpc.onUnhandledError({
         name: error.name,
         message: error.message,
diff --git a/packages/coverage-istanbul/src/index.ts b/packages/coverage-istanbul/src/index.ts
@@ -1,33 +1,49 @@
+import type { CoverageMapData } from 'istanbul-lib-coverage'
+import type { CoverageProviderModule } from 'vitest/node'
 import type { IstanbulCoverageProvider } from './provider'
 import { COVERAGE_STORE_KEY } from './constants'
 
-export async function getProvider(): Promise<IstanbulCoverageProvider> {
-  // to not bundle the provider
-  const providerPath = './provider.js'
-  const { IstanbulCoverageProvider } = (await import(
-    /* @vite-ignore */
-    providerPath
-  )) as typeof import('./provider')
-  return new IstanbulCoverageProvider()
-}
-
-export function takeCoverage(): any {
-  // @ts-expect-error -- untyped global
-  const coverage = globalThis[COVERAGE_STORE_KEY]
+export default {
+  takeCoverage() {
+    // @ts-expect-error -- untyped global
+    return globalThis[COVERAGE_STORE_KEY]
+  },
 
   // Reset coverage map to prevent duplicate results if this is called twice in row
-  // @ts-expect-error -- untyped global
-  globalThis[COVERAGE_STORE_KEY] = {}
+  startCoverage() {
+    // @ts-expect-error -- untyped global
+    const coverageMap = globalThis[COVERAGE_STORE_KEY] as CoverageMapData
+
+    // When isolated, there are no previous results
+    if (!coverageMap) {
+      return
+    }
+
+    for (const filename in coverageMap) {
+      const branches = coverageMap[filename].b
+
+      for (const key in branches) {
+        branches[key] = branches[key].map(() => 0)
+      }
+
+      for (const metric of ['f', 's'] as const) {
+        const entry = coverageMap[filename][metric]
 
-  return coverage
-}
+        for (const key in entry) {
+          entry[key] = 0
+        }
+      }
+    }
+  },
 
-const _default: {
-  getProvider: () => Promise<IstanbulCoverageProvider>
-  takeCoverage: () => any
-} = {
-  getProvider,
-  takeCoverage,
-}
+  async getProvider(): Promise<IstanbulCoverageProvider> {
+    // to not bundle the provider
+    const providerPath = './provider.js'
+    const { IstanbulCoverageProvider } = (await import(
+      /* @vite-ignore */
+      providerPath
+    )) as typeof import('./provider')
 
-export default _default
+    return new IstanbulCoverageProvider()
+  },
+} satisfies CoverageProviderModule
diff --git a/packages/coverage-v8/src/browser.ts b/packages/coverage-v8/src/browser.ts
@@ -1,13 +1,21 @@
+import type { CoverageProviderModule } from 'vitest/node'
 import type { V8CoverageProvider } from './provider'
 import { cdp } from '@vitest/browser/context'
 import { loadProvider } from './load-provider'
 
 const session = cdp()
+let enabled = false
 
 type ScriptCoverage = Awaited<ReturnType<typeof session.send<'Profiler.takePreciseCoverage'>>>
 
 export default {
   async startCoverage() {
+    if (enabled) {
+      return
+    }
+
+    enabled = true
+
     await session.send('Profiler.enable')
     await session.send('Profiler.startPreciseCoverage', {
       callCount: true,
@@ -32,15 +40,14 @@ export default {
     return { result }
   },
 
-  async stopCoverage() {
-    await session.send('Profiler.stopPreciseCoverage')
-    await session.send('Profiler.disable')
+  stopCoverage() {
+    // Browser mode should not stop coverage as same V8 instance is shared between tests
   },
 
   async getProvider(): Promise<V8CoverageProvider> {
     return loadProvider()
   },
-}
+} satisfies CoverageProviderModule
 
 function filterResult(coverage: ScriptCoverage['result'][number]): boolean {
   if (!coverage.url.startsWith(window.location.origin)) {
diff --git a/packages/coverage-v8/src/index.ts b/packages/coverage-v8/src/index.ts
@@ -1,12 +1,20 @@
+import type { CoverageProviderModule } from 'vitest/node'
 import type { V8CoverageProvider } from './provider'
 import inspector, { type Profiler } from 'node:inspector'
 import { provider } from 'std-env'
 import { loadProvider } from './load-provider'
 
 const session = new inspector.Session()
+let enabled = false
 
 export default {
-  startCoverage(): void {
+  startCoverage({ isolate }) {
+    if (isolate === false && enabled) {
+      return
+    }
+
+    enabled = true
+
     session.connect()
     session.post('Profiler.enable')
     session.post('Profiler.startPreciseCoverage', {
@@ -34,7 +42,11 @@ export default {
     })
   },
 
-  stopCoverage(): void {
+  stopCoverage({ isolate }) {
+    if (isolate === false) {
+      return
+    }
+
     session.post('Profiler.stopPreciseCoverage')
     session.post('Profiler.disable')
     session.disconnect()
@@ -43,7 +55,7 @@ export default {
   async getProvider(): Promise<V8CoverageProvider> {
     return loadProvider()
   },
-}
+} satisfies CoverageProviderModule
 
 function filterResult(coverage: Profiler.ScriptCoverage): boolean {
   if (!coverage.url.startsWith('file://')) {
diff --git a/packages/vitest/src/integrations/coverage.ts b/packages/vitest/src/integrations/coverage.ts
@@ -79,11 +79,12 @@ export async function getCoverageProvider(
 export async function startCoverageInsideWorker(
   options: SerializedCoverageConfig | undefined,
   loader: Loader,
+  runtimeOptions: { isolate: boolean },
 ) {
   const coverageModule = await resolveCoverageProviderModule(options, loader)
 
   if (coverageModule) {
-    return coverageModule.startCoverage?.()
+    return coverageModule.startCoverage?.(runtimeOptions)
   }
 
   return null
@@ -105,11 +106,12 @@ export async function takeCoverageInsideWorker(
 export async function stopCoverageInsideWorker(
   options: SerializedCoverageConfig | undefined,
   loader: Loader,
+  runtimeOptions: { isolate: boolean },
 ) {
   const coverageModule = await resolveCoverageProviderModule(options, loader)
 
   if (coverageModule) {
-    return coverageModule.stopCoverage?.()
+    return coverageModule.stopCoverage?.(runtimeOptions)
   }
 
   return null
diff --git a/packages/vitest/src/node/types/coverage.ts b/packages/vitest/src/node/types/coverage.ts
@@ -66,7 +66,7 @@ export interface CoverageProviderModule {
   /**
    * Executed before tests are run in the worker thread.
    */
-  startCoverage?: () => unknown | Promise<unknown>
+  startCoverage?: (runtimeOptions: { isolate: boolean }) => unknown | Promise<unknown>
 
   /**
    * Executed on after each run in the worker thread. Possible to return a payload passed to the provider
@@ -76,7 +76,7 @@ export interface CoverageProviderModule {
   /**
    * Executed after all tests have been run in the worker thread.
    */
-  stopCoverage?: () => unknown | Promise<unknown>
+  stopCoverage?: (runtimeOptions: { isolate: boolean }) => unknown | Promise<unknown>
 }
 
 export type CoverageReporter = keyof ReportOptions | (string & {})
diff --git a/packages/vitest/src/runtime/runBaseTests.ts b/packages/vitest/src/runtime/runBaseTests.ts
@@ -25,8 +25,12 @@ export async function run(
 ): Promise<void> {
   const workerState = getWorkerState()
 
+  const isIsolatedThreads = config.pool === 'threads' && (config.poolOptions?.threads?.isolate ?? true)
+  const isIsolatedForks = config.pool === 'forks' && (config.poolOptions?.forks?.isolate ?? true)
+  const isolate = isIsolatedThreads || isIsolatedForks
+
   await setupGlobalEnv(config, environment, executor)
-  await startCoverageInsideWorker(config.coverage, executor)
+  await startCoverageInsideWorker(config.coverage, executor, { isolate })
 
   if (config.chaiConfig) {
     setupChaiConfig(config.chaiConfig)
@@ -50,14 +54,7 @@ export async function run(
         = performance.now() - workerState.durations.environment
 
       for (const file of files) {
-        const isIsolatedThreads
-          = config.pool === 'threads'
-          && (config.poolOptions?.threads?.isolate ?? true)
-        const isIsolatedForks
-          = config.pool === 'forks'
-          && (config.poolOptions?.forks?.isolate ?? true)
-
-        if (isIsolatedThreads || isIsolatedForks) {
+        if (isolate) {
           executor.mocker.reset()
           resetModules(workerState.moduleCache, true)
         }
@@ -77,7 +74,7 @@ export async function run(
         vi.restoreAllMocks()
       }
 
-      await stopCoverageInsideWorker(config.coverage, executor)
+      await stopCoverageInsideWorker(config.coverage, executor, { isolate })
     },
   )
 
diff --git a/packages/vitest/src/runtime/runVmTests.ts b/packages/vitest/src/runtime/runVmTests.ts
@@ -62,7 +62,7 @@ export async function run(
     getSourceMap: source => workerState.moduleCache.getSourceMap(source),
   })
 
-  await startCoverageInsideWorker(config.coverage, executor)
+  await startCoverageInsideWorker(config.coverage, executor, { isolate: false })
 
   if (config.chaiConfig) {
     setupChaiConfig(config.chaiConfig)
@@ -101,7 +101,7 @@ export async function run(
     vi.restoreAllMocks()
   }
 
-  await stopCoverageInsideWorker(config.coverage, executor)
+  await stopCoverageInsideWorker(config.coverage, executor, { isolate: false })
 }
 
 function resolveCss(mod: NodeJS.Module) {
diff --git a/test/coverage-test/fixtures/setup.isolation.ts b/test/coverage-test/fixtures/setup.isolation.ts
@@ -0,0 +1,6 @@
+import { beforeAll } from "vitest";
+import { branch } from "./src/branch";
+
+beforeAll(() => {
+  branch(1);
+});
diff --git a/test/coverage-test/fixtures/src/branch.ts b/test/coverage-test/fixtures/src/branch.ts
@@ -0,0 +1,7 @@
+export const branch = async (a: number) => {
+  if (a === 15) {
+    return true;
+  }
+
+  return false;
+};
diff --git a/test/coverage-test/fixtures/test/isolation-1-fixture.test.ts b/test/coverage-test/fixtures/test/isolation-1-fixture.test.ts
@@ -0,0 +1,9 @@
+import { test } from "vitest";
+import { multiply, remainder, subtract, sum } from "../src/math";
+
+test("Should run function sucessfully", async () => {
+  sum(1, 1);
+  subtract(1,2)
+  multiply(3,4)
+  remainder(6,7)
+});
diff --git a/test/coverage-test/fixtures/test/isolation-2-fixture.test.ts b/test/coverage-test/fixtures/test/isolation-2-fixture.test.ts
@@ -0,0 +1,10 @@
+import { test } from "vitest";
+import { branch } from "../src/branch";
+
+test("cover some lines", async () => {
+  branch(15);
+});
+
+test("cover lines", async () => {
+  branch(2);
+});
diff --git a/test/coverage-test/test/isolation.test.ts b/test/coverage-test/test/isolation.test.ts
@@ -0,0 +1,70 @@
+import type { WorkspaceSpec } from 'vitest/node'
+import { expect, test } from 'vitest'
+import { readCoverageMap, runVitest } from '../utils'
+
+const pools = ['forks']
+
+if (!process.env.COVERAGE_BROWSER) {
+  pools.push('threads')
+
+  const [major] = process.version.slice(1).split('.').map(num => Number(num))
+
+  if (major < 22) {
+    pools.push('vmForks', 'vmThreads')
+  }
+}
+
+for (const isolate of [true, false]) {
+  for (const pool of pools) {
+    test(`{ isolate: ${isolate}, pool: "${pool}" }`, async () => {
+      await runVitest({
+        include: ['fixtures/test/isolation-*'],
+        setupFiles: ['fixtures/setup.isolation.ts'],
+        sequence: { sequencer: Sorter },
+
+        pool,
+        isolate,
+        fileParallelism: false,
+
+        coverage: {
+          all: false,
+          reporter: ['json', 'html'],
+        },
+
+        // @ts-expect-error -- merged in runVitest
+        browser: {
+          isolate,
+        },
+      })
+
+      const coverageMap = await readCoverageMap()
+
+      const branches = coverageMap.fileCoverageFor('<process-cwd>/fixtures/src/branch.ts')
+      expect(branches.toSummary().lines.pct).toBe(100)
+      expect(branches.toSummary().statements.pct).toBe(100)
+      expect(branches.toSummary().functions.pct).toBe(100)
+      expect(branches.toSummary().branches.pct).toBe(100)
+
+      const math = coverageMap.fileCoverageFor('<process-cwd>/fixtures/src/math.ts')
+      expect(math.toSummary().lines.pct).toBe(100)
+      expect(math.toSummary().statements.pct).toBe(100)
+      expect(math.toSummary().functions.pct).toBe(100)
+      expect(math.toSummary().branches.pct).toBe(100)
+    })
+  }
+}
+
+class Sorter {
+  sort(files: WorkspaceSpec[]) {
+    return files.sort((a) => {
+      if (a.moduleId.includes('isolation-1')) {
+        return -1
+      }
+      return 1
+    })
+  }
+
+  shard(files: WorkspaceSpec[]) {
+    return files
+  }
+}
diff --git a/test/coverage-test/utils.ts b/test/coverage-test/utils.ts
@@ -51,6 +51,7 @@ export async function runVitest(config: UserConfig, options = { throwOnError: tr
       headless: true,
       name: 'chromium',
       provider: 'playwright',
+      ...config.browser,
     },
   })
 
diff --git a/test/coverage-test/vitest.config.ts b/test/coverage-test/vitest.config.ts
diff --git a/test/coverage-test/vitest.workspace.custom.ts b/test/coverage-test/vitest.workspace.custom.ts

Original file line number	Diff line number	Diff line change
`@@ -62,7 +62,7 @@ export async function run(`
`62`	`62`	`getSourceMap: source => workerState.moduleCache.getSourceMap(source),`
`63`	`63`	`})`
`64`	`64`
`65`		`- await startCoverageInsideWorker(config.coverage, executor)`
	`65`	`+ await startCoverageInsideWorker(config.coverage, executor, { isolate: false })`
`66`	`66`
`67`	`67`	`if (config.chaiConfig) {`
`68`	`68`	`setupChaiConfig(config.chaiConfig)`
`@@ -101,7 +101,7 @@ export async function run(`
`101`	`101`	`vi.restoreAllMocks()`
`102`	`102`	`}`
`103`	`103`
`104`		`- await stopCoverageInsideWorker(config.coverage, executor)`
	`104`	`+ await stopCoverageInsideWorker(config.coverage, executor, { isolate: false })`
`105`	`105`	`}`
`106`	`106`
`107`	`107`	`function resolveCss(mod: NodeJS.Module) {`