Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/browser/src/client/tester/tester.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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__', {
Expand Down Expand Up @@ -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,
Expand Down
66 changes: 41 additions & 25 deletions packages/coverage-istanbul/src/index.ts
Original file line number Diff line number Diff line change
@@ -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
15 changes: 11 additions & 4 deletions packages/coverage-v8/src/browser.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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)) {
Expand Down
18 changes: 15 additions & 3 deletions packages/coverage-v8/src/index.ts
Original file line number Diff line number Diff line change
@@ -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', {
Expand Down Expand Up @@ -34,7 +42,11 @@ export default {
})
},

stopCoverage(): void {
stopCoverage({ isolate }) {
if (isolate === false) {
return
}

session.post('Profiler.stopPreciseCoverage')
session.post('Profiler.disable')
session.disconnect()
Expand All @@ -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://')) {
Expand Down
6 changes: 4 additions & 2 deletions packages/vitest/src/integrations/coverage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions packages/vitest/src/node/types/coverage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 & {})
Expand Down
17 changes: 7 additions & 10 deletions packages/vitest/src/runtime/runBaseTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
Expand All @@ -77,7 +74,7 @@ export async function run(
vi.restoreAllMocks()
}

await stopCoverageInsideWorker(config.coverage, executor)
await stopCoverageInsideWorker(config.coverage, executor, { isolate })
},
)

Expand Down
4 changes: 2 additions & 2 deletions packages/vitest/src/runtime/runVmTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down
6 changes: 6 additions & 0 deletions test/coverage-test/fixtures/setup.isolation.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { beforeAll } from "vitest";
import { branch } from "./src/branch";

beforeAll(() => {
branch(1);
});
7 changes: 7 additions & 0 deletions test/coverage-test/fixtures/src/branch.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export const branch = async (a: number) => {
if (a === 15) {
return true;
}

return false;
};
9 changes: 9 additions & 0 deletions test/coverage-test/fixtures/test/isolation-1-fixture.test.ts
Original file line number Diff line number Diff line change
@@ -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)
});
10 changes: 10 additions & 0 deletions test/coverage-test/fixtures/test/isolation-2-fixture.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
70 changes: 70 additions & 0 deletions test/coverage-test/test/isolation.test.ts
Original file line number Diff line number Diff line change
@@ -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
}
}
1 change: 1 addition & 0 deletions test/coverage-test/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export async function runVitest(config: UserConfig, options = { throwOnError: tr
headless: true,
name: 'chromium',
provider: 'playwright',
...config.browser,
},
})

Expand Down
Loading
Loading