From 1f2e55522a65f0f576da6e9abdf75d34150bd73c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ari=20Perkki=C3=B6?= Date: Thu, 13 Feb 2025 17:13:22 +0200 Subject: [PATCH] fix(coverage): `vite-node` to pass correct execution wrapper offset (#7417) --- packages/coverage-v8/src/index.ts | 13 ++-- packages/coverage-v8/src/provider.ts | 29 ++++---- packages/vite-node/src/client.ts | 4 ++ packages/vite-node/src/types.ts | 5 +- packages/vitest/src/integrations/coverage.ts | 19 ++---- packages/vitest/src/node/types/coverage.ts | 9 ++- packages/vitest/src/runtime/execute.ts | 9 +++ packages/vitest/src/runtime/worker.ts | 1 + packages/vitest/src/types/worker.ts | 3 +- packages/web-worker/src/utils.ts | 3 +- pnpm-lock.yaml | 3 + .../fixtures/src/cjs-package/entry.js | 2 - .../fixtures/src/cjs-package/package.json | 5 -- .../fixtures/src/cjs-package/target.js | 9 --- .../fixtures/src/worker-wrapper.ts | 29 ++++++++ test/coverage-test/fixtures/src/worker.ts | 25 +++++++ .../fixtures/test/web-worker.test.ts | 8 +++ test/coverage-test/package.json | 1 + .../test/convert-failure.v8.test.ts | 40 ----------- test/coverage-test/test/web-worker.test.ts | 67 +++++++++++++++++++ test/coverage-test/vitest.config.ts | 3 - 21 files changed, 193 insertions(+), 94 deletions(-) delete mode 100644 test/coverage-test/fixtures/src/cjs-package/entry.js delete mode 100644 test/coverage-test/fixtures/src/cjs-package/package.json delete mode 100644 test/coverage-test/fixtures/src/cjs-package/target.js create mode 100644 test/coverage-test/fixtures/src/worker-wrapper.ts create mode 100644 test/coverage-test/fixtures/src/worker.ts create mode 100644 test/coverage-test/fixtures/test/web-worker.test.ts delete mode 100644 test/coverage-test/test/convert-failure.v8.test.ts create mode 100644 test/coverage-test/test/web-worker.test.ts diff --git a/packages/coverage-v8/src/index.ts b/packages/coverage-v8/src/index.ts index 9497648ea65b..bc19c503c89e 100644 --- a/packages/coverage-v8/src/index.ts +++ b/packages/coverage-v8/src/index.ts @@ -1,6 +1,7 @@ import type { CoverageProviderModule } from 'vitest/node' -import type { V8CoverageProvider } from './provider' +import type { ScriptCoverageWithOffset, V8CoverageProvider } from './provider' import inspector, { type Profiler } from 'node:inspector' +import { fileURLToPath } from 'node:url' import { provider } from 'std-env' import { loadProvider } from './load-provider' @@ -23,15 +24,19 @@ export default { }) }, - takeCoverage(): Promise<{ result: Profiler.ScriptCoverage[] }> { + takeCoverage(options): Promise<{ result: ScriptCoverageWithOffset[] }> { return new Promise((resolve, reject) => { session.post('Profiler.takePreciseCoverage', async (error, coverage) => { if (error) { return reject(error) } - // Reduce amount of data sent over rpc by doing some early result filtering - const result = coverage.result.filter(filterResult) + const result = coverage.result + .filter(filterResult) + .map(res => ({ + ...res, + startOffset: options?.moduleExecutionInfo?.get(fileURLToPath(res.url))?.startOffset || 0, + })) resolve({ result }) }) diff --git a/packages/coverage-v8/src/provider.ts b/packages/coverage-v8/src/provider.ts index 55b93d349793..9810d5b29be3 100644 --- a/packages/coverage-v8/src/provider.ts +++ b/packages/coverage-v8/src/provider.ts @@ -25,11 +25,12 @@ import { cleanUrl } from 'vite-node/utils' import { BaseCoverageProvider } from 'vitest/coverage' import { version } from '../package.json' with { type: 'json' } -type TransformResults = Map -type RawCoverage = Profiler.TakePreciseCoverageReturnType +export interface ScriptCoverageWithOffset extends Profiler.ScriptCoverage { + startOffset: number +} -// TODO: vite-node should export this -const WRAPPER_LENGTH = 185 +type TransformResults = Map +interface RawCoverage { result: ScriptCoverageWithOffset[] } // Note that this needs to match the line ending as well const VITE_EXPORTS_LINE_PATTERN @@ -69,6 +70,14 @@ export class V8CoverageProvider extends BaseCoverageProvider({ onFileRead(coverage) { merged = mergeProcessCovs([merged, coverage]) + + // mergeProcessCovs sometimes loses startOffset, e.g. in vue + merged.result.forEach((result) => { + if (!result.startOffset) { + const original = coverage.result.find(r => r.url === result.url) + result.startOffset = original?.startOffset || 0 + } + }) }, onFinished: async (project, transformMode) => { const converted = await this.convertCoverage( @@ -230,15 +239,12 @@ export class V8CoverageProvider extends BaseCoverageProvider { const filePath = normalize(fileURLToPath(url)) - let isExecuted = true let transformResult: FetchResult | TransformResult | undefined = transformResults.get(filePath) if (!transformResult) { - isExecuted = false transformResult = await onTransform(removeStartsWith(url, FILE_PROTOCOL)).catch(() => undefined) } @@ -258,7 +264,6 @@ export class V8CoverageProvider extends BaseCoverageProvider { + chunk.map(async ({ url, functions, startOffset }) => { const sources = await this.getSources( url, transformResults, @@ -346,12 +350,9 @@ export class V8CoverageProvider extends BaseCoverageProvider { } } +export type ModuleExecutionInfo = Map + export class ViteNodeRunner { root: string @@ -505,6 +507,8 @@ export class ViteNodeRunner { columnOffset: -codeDefinition.length, } + this.options.moduleExecutionInfo?.set(options.filename, { startOffset: codeDefinition.length }) + const fn = vm.runInThisContext(code, options) await fn(...Object.values(context)) } diff --git a/packages/vite-node/src/types.ts b/packages/vite-node/src/types.ts index 30c607fc742d..4b952bbf474f 100644 --- a/packages/vite-node/src/types.ts +++ b/packages/vite-node/src/types.ts @@ -1,6 +1,6 @@ import type { EncodedSourceMap } from '@jridgewell/trace-mapping' import type { ViteHotContext } from 'vite/types/hot.js' -import type { ModuleCacheMap, ViteNodeRunner } from './client' +import type { ModuleCacheMap, ModuleExecutionInfo, ViteNodeRunner } from './client' export type Nullable = T | null | undefined export type Arrayable = T | Array @@ -87,6 +87,7 @@ export interface ViteNodeRunnerOptions { createHotContext?: CreateHotContextFunction base?: string moduleCache?: ModuleCacheMap + moduleExecutionInfo?: ModuleExecutionInfo interopDefault?: boolean requestStubs?: Record debug?: boolean @@ -140,4 +141,4 @@ export interface DebuggerOptions { loadDumppedModules?: boolean } -export type { ModuleCacheMap } +export type { ModuleCacheMap, ModuleExecutionInfo } diff --git a/packages/vitest/src/integrations/coverage.ts b/packages/vitest/src/integrations/coverage.ts index a4244e21ecb1..0474449f6451 100644 --- a/packages/vitest/src/integrations/coverage.ts +++ b/packages/vitest/src/integrations/coverage.ts @@ -1,14 +1,9 @@ import type { + CoverageModuleLoader, CoverageProvider, - CoverageProviderModule, } from '../node/types/coverage' import type { SerializedCoverageConfig } from '../runtime/config' -interface Loader { - executeId: (id: string) => Promise<{ default: CoverageProviderModule }> - isBrowser?: boolean -} - export const CoverageProviderMap: Record = { v8: '@vitest/coverage-v8', istanbul: '@vitest/coverage-istanbul', @@ -16,7 +11,7 @@ export const CoverageProviderMap: Record = { async function resolveCoverageProviderModule( options: SerializedCoverageConfig | undefined, - loader: Loader, + loader: CoverageModuleLoader, ) { if (!options?.enabled || !options.provider) { return null @@ -65,7 +60,7 @@ async function resolveCoverageProviderModule( export async function getCoverageProvider( options: SerializedCoverageConfig | undefined, - loader: Loader, + loader: CoverageModuleLoader, ): Promise { const coverageModule = await resolveCoverageProviderModule(options, loader) @@ -78,7 +73,7 @@ export async function getCoverageProvider( export async function startCoverageInsideWorker( options: SerializedCoverageConfig | undefined, - loader: Loader, + loader: CoverageModuleLoader, runtimeOptions: { isolate: boolean }, ) { const coverageModule = await resolveCoverageProviderModule(options, loader) @@ -92,12 +87,12 @@ export async function startCoverageInsideWorker( export async function takeCoverageInsideWorker( options: SerializedCoverageConfig | undefined, - loader: Loader, + loader: CoverageModuleLoader, ) { const coverageModule = await resolveCoverageProviderModule(options, loader) if (coverageModule) { - return coverageModule.takeCoverage?.() + return coverageModule.takeCoverage?.({ moduleExecutionInfo: loader.moduleExecutionInfo }) } return null @@ -105,7 +100,7 @@ export async function takeCoverageInsideWorker( export async function stopCoverageInsideWorker( options: SerializedCoverageConfig | undefined, - loader: Loader, + loader: CoverageModuleLoader, runtimeOptions: { isolate: boolean }, ) { const coverageModule = await resolveCoverageProviderModule(options, loader) diff --git a/packages/vitest/src/node/types/coverage.ts b/packages/vitest/src/node/types/coverage.ts index c2f3f408bbeb..433a17ff88fb 100644 --- a/packages/vitest/src/node/types/coverage.ts +++ b/packages/vitest/src/node/types/coverage.ts @@ -1,5 +1,6 @@ import type { ReportOptions } from 'istanbul-reports' import type { TransformResult as ViteTransformResult } from 'vite' +import type { ModuleExecutionInfo } from 'vite-node' import type { AfterSuiteRunMeta, Arrayable } from '../../types/general' import type { Vitest } from '../core' @@ -57,6 +58,12 @@ export interface ReportContext { allTestsRun?: boolean } +export interface CoverageModuleLoader { + executeId: (id: string) => Promise<{ default: CoverageProviderModule }> + isBrowser?: boolean + moduleExecutionInfo?: ModuleExecutionInfo +} + export interface CoverageProviderModule { /** * Factory for creating a new coverage provider @@ -71,7 +78,7 @@ export interface CoverageProviderModule { /** * Executed on after each run in the worker thread. Possible to return a payload passed to the provider */ - takeCoverage?: () => unknown | Promise + takeCoverage?: (runtimeOptions?: { moduleExecutionInfo?: ModuleExecutionInfo }) => unknown | Promise /** * Executed after all tests have been run in the worker thread. diff --git a/packages/vitest/src/runtime/execute.ts b/packages/vitest/src/runtime/execute.ts index a5a6802cb5d5..82edfb4af4eb 100644 --- a/packages/vitest/src/runtime/execute.ts +++ b/packages/vitest/src/runtime/execute.ts @@ -138,6 +138,9 @@ export async function startVitestExecutor(options: ContextExecutorOptions) { get moduleCache() { return state().moduleCache }, + get moduleExecutionInfo() { + return state().moduleExecutionInfo + }, get interopDefault() { return state().config.deps.interopDefault }, @@ -255,6 +258,10 @@ export class VitestExecutor extends ViteNodeRunner { return globalThis.__vitest_worker__ || this.options.state } + get moduleExecutionInfo() { + return this.options.moduleExecutionInfo + } + shouldResolveId(id: string, _importee?: string | undefined): boolean { if (isInternalRequest(id) || id.startsWith('data:')) { return false @@ -313,6 +320,8 @@ export class VitestExecutor extends ViteNodeRunner { columnOffset: -codeDefinition.length, } + this.options.moduleExecutionInfo?.set(options.filename, { startOffset: codeDefinition.length }) + const fn = vm.runInContext(code, vmContext, { ...options, // if we encountered an import, it's not inlined diff --git a/packages/vitest/src/runtime/worker.ts b/packages/vitest/src/runtime/worker.ts index 4ccfa155e408..67cec6af85e8 100644 --- a/packages/vitest/src/runtime/worker.ts +++ b/packages/vitest/src/runtime/worker.ts @@ -82,6 +82,7 @@ async function execute(method: 'run' | 'collect', ctx: ContextRPC) { ctx, // here we create a new one, workers can reassign this if they need to keep it non-isolated moduleCache: new ModuleCacheMap(), + moduleExecutionInfo: new Map(), config: ctx.config, onCancel, environment, diff --git a/packages/vitest/src/types/worker.ts b/packages/vitest/src/types/worker.ts index 494bf7dac6db..f2681ea05ee9 100644 --- a/packages/vitest/src/types/worker.ts +++ b/packages/vitest/src/types/worker.ts @@ -1,6 +1,6 @@ import type { CancelReason, FileSpecification, Task } from '@vitest/runner' import type { BirpcReturn } from 'birpc' -import type { ModuleCacheMap, ViteNodeResolveId } from 'vite-node' +import type { ModuleCacheMap, ModuleExecutionInfo, ViteNodeResolveId } from 'vite-node' import type { SerializedConfig } from '../runtime/config' import type { Environment } from './environment' import type { TransformMode } from './general' @@ -42,6 +42,7 @@ export interface WorkerGlobalState { environmentTeardownRun?: boolean onCancel: Promise moduleCache: ModuleCacheMap + moduleExecutionInfo?: ModuleExecutionInfo providedContext: Record durations: { environment: number diff --git a/packages/web-worker/src/utils.ts b/packages/web-worker/src/utils.ts index ce9cd5785ef5..7c04478b6130 100644 --- a/packages/web-worker/src/utils.ts +++ b/packages/web-worker/src/utils.ts @@ -81,7 +81,7 @@ export function createMessageEvent( export function getRunnerOptions(): any { const state = getWorkerState() - const { config, rpc, moduleCache } = state + const { config, rpc, moduleCache, moduleExecutionInfo } = state return { async fetchModule(id: string) { @@ -96,6 +96,7 @@ export function getRunnerOptions(): any { return rpc.resolveId(id, importer, 'web') }, moduleCache, + moduleExecutionInfo, interopDefault: config.deps.interopDefault ?? true, moduleDirectories: config.deps.moduleDirectories, root: config.root, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5590b95c7631..4dabfdd4d0cd 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1237,6 +1237,9 @@ importers: '@vitest/coverage-v8': specifier: workspace:* version: link:../../packages/coverage-v8 + '@vitest/web-worker': + specifier: workspace:* + version: link:../../packages/web-worker '@vue/test-utils': specifier: latest version: 2.4.6 diff --git a/test/coverage-test/fixtures/src/cjs-package/entry.js b/test/coverage-test/fixtures/src/cjs-package/entry.js deleted file mode 100644 index 883382e4e68e..000000000000 --- a/test/coverage-test/fixtures/src/cjs-package/entry.js +++ /dev/null @@ -1,2 +0,0 @@ -require("./target"); -module.exports = "Entry here" \ No newline at end of file diff --git a/test/coverage-test/fixtures/src/cjs-package/package.json b/test/coverage-test/fixtures/src/cjs-package/package.json deleted file mode 100644 index d7243077cec0..000000000000 --- a/test/coverage-test/fixtures/src/cjs-package/package.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "name": "cjs-package", - "main": "./entry.js", - "type": "commonjs" -} \ No newline at end of file diff --git a/test/coverage-test/fixtures/src/cjs-package/target.js b/test/coverage-test/fixtures/src/cjs-package/target.js deleted file mode 100644 index 8a5e164e4564..000000000000 --- a/test/coverage-test/fixtures/src/cjs-package/target.js +++ /dev/null @@ -1,9 +0,0 @@ -"use strict"; - -module.exports = { - debug: 0, - info: 1, - warn: 2, - error: 3, - fatal: 4, -}; diff --git a/test/coverage-test/fixtures/src/worker-wrapper.ts b/test/coverage-test/fixtures/src/worker-wrapper.ts new file mode 100644 index 000000000000..565616ffa434 --- /dev/null +++ b/test/coverage-test/fixtures/src/worker-wrapper.ts @@ -0,0 +1,29 @@ +export async function sumInBackground(a: number, b: number) { + const worker = new Worker(new URL("./worker?with-some-query=123", import.meta.url), { + type: "module", + }); + + const promise = new Promise(resolve => { + worker.onmessage = resolve; + }); + + function uncovered() { + return "This is uncovered" + } + + worker.postMessage({ a, b }); + + const result = await promise; + covered(); + worker.terminate(); + + return result.data; +} + +function covered() { + return "This is covered" +} + +export function uncovered() { + return "This is uncovered" +} diff --git a/test/coverage-test/fixtures/src/worker.ts b/test/coverage-test/fixtures/src/worker.ts new file mode 100644 index 000000000000..5be33af372c9 --- /dev/null +++ b/test/coverage-test/fixtures/src/worker.ts @@ -0,0 +1,25 @@ +self.onmessage = (ev: MessageEvent) => { + const {a, b} = ev.data; + + if (a === 5) { + uncovered() + throw new Error("uncovered"); + } + + if(b === 6) { + uncovered() + throw new Error("uncovered"); + + } + + covered(); + postMessage(a + b); +}; + +export function uncovered() { + return "This is uncovered" +} + +function covered() { + return "This is covered" +} diff --git a/test/coverage-test/fixtures/test/web-worker.test.ts b/test/coverage-test/fixtures/test/web-worker.test.ts new file mode 100644 index 000000000000..d56c4f7f6954 --- /dev/null +++ b/test/coverage-test/fixtures/test/web-worker.test.ts @@ -0,0 +1,8 @@ +import { expect, test } from 'vitest'; +import { sumInBackground } from '../src/worker-wrapper'; + +test('run a Worker', async () => { + const result = await sumInBackground(15, 7); + + expect(result).toBe(22); +}); diff --git a/test/coverage-test/package.json b/test/coverage-test/package.json index ab9467e7af50..71426fb64d0a 100644 --- a/test/coverage-test/package.json +++ b/test/coverage-test/package.json @@ -14,6 +14,7 @@ "@vitest/browser": "workspace:*", "@vitest/coverage-istanbul": "workspace:*", "@vitest/coverage-v8": "workspace:*", + "@vitest/web-worker": "workspace:*", "@vue/test-utils": "latest", "happy-dom": "latest", "istanbul-lib-coverage": "^3.2.0", diff --git a/test/coverage-test/test/convert-failure.v8.test.ts b/test/coverage-test/test/convert-failure.v8.test.ts deleted file mode 100644 index 2776b8d026d9..000000000000 --- a/test/coverage-test/test/convert-failure.v8.test.ts +++ /dev/null @@ -1,40 +0,0 @@ -import { createRequire } from 'node:module' -import { expect } from 'vitest' -import { coverageTest, normalizeURL, readCoverageMap, runVitest, test } from '../utils' - -test('logs warning but doesn\'t crash when coverage conversion fails', async () => { - const { stderr, exitCode } = await runVitest({ - include: [normalizeURL(import.meta.url)], - coverage: { reporter: 'json', include: ['fixtures/src/**'], all: false }, - }, { throwOnError: false }) - - // Logged warning should not set erroneous exit code - expect(exitCode).toBe(0) - - expect(stderr).toMatch('Failed to convert coverage for file://') - expect(stderr).toMatch('/fixtures/src/cjs-package/target.js.') - expect(stderr).toMatch('TypeError: Cannot read properties of undefined (reading \'endCol\')') - - const coverageMap = await readCoverageMap() - - expect(coverageMap.files()).toMatchInlineSnapshot(` - [ - "/fixtures/src/cjs-package/entry.js", - "/fixtures/src/cjs-package/target.js", - ] - `) -}) - -coverageTest('load file both from Vite and outside it', async () => { - const entry = createRequire(import.meta.url)('../fixtures/src/cjs-package' as any) - const target = await import('../fixtures/src/cjs-package/target.js' as any) - - expect(entry).toBe('Entry here') - expect(target.default).toStrictEqual({ - debug: 0, - error: 3, - fatal: 4, - info: 1, - warn: 2, - }) -}) diff --git a/test/coverage-test/test/web-worker.test.ts b/test/coverage-test/test/web-worker.test.ts new file mode 100644 index 000000000000..35bd85613391 --- /dev/null +++ b/test/coverage-test/test/web-worker.test.ts @@ -0,0 +1,67 @@ +import { expect } from 'vitest' +import { formatSummary, isV8Provider, readCoverageMap, runVitest, test } from '../utils' + +test('web worker coverage is correct', async () => { + await runVitest({ + setupFiles: ['@vitest/web-worker'], + include: ['fixtures/test/web-worker.test.ts'], + environment: 'jsdom', + coverage: { + include: [ + // Runs in web-worker's runner with custom context -> execution wrapper ~430 chars + 'fixtures/src/worker.ts', + + // Runs in default runner -> execution wrapper ~185 chars + 'fixtures/src/worker-wrapper.ts', + ], + reporter: 'json', + }, + }) + + const coverageMap = await readCoverageMap() + const worker = coverageMap.fileCoverageFor('/fixtures/src/worker.ts') + const wrapper = coverageMap.fileCoverageFor('/fixtures/src/worker-wrapper.ts') + + const summary = { + [worker.path]: formatSummary(worker.toSummary()), + [wrapper.path]: formatSummary(wrapper.toSummary()), + } + + // Check HTML report if these change unexpectedly + if (isV8Provider()) { + expect(summary).toMatchInlineSnapshot(` + { + "/fixtures/src/worker-wrapper.ts": { + "branches": "3/3 (100%)", + "functions": "2/4 (50%)", + "lines": "18/22 (81.81%)", + "statements": "18/22 (81.81%)", + }, + "/fixtures/src/worker.ts": { + "branches": "2/4 (50%)", + "functions": "2/3 (66.66%)", + "lines": "11/19 (57.89%)", + "statements": "11/19 (57.89%)", + }, + } + `) + } + else { + expect(summary).toMatchInlineSnapshot(` + { + "/fixtures/src/worker-wrapper.ts": { + "branches": "0/0 (100%)", + "functions": "3/5 (60%)", + "lines": "9/11 (81.81%)", + "statements": "9/11 (81.81%)", + }, + "/fixtures/src/worker.ts": { + "branches": "2/4 (50%)", + "functions": "2/3 (66.66%)", + "lines": "7/12 (58.33%)", + "statements": "7/12 (58.33%)", + }, + } + `) + } +}) diff --git a/test/coverage-test/vitest.config.ts b/test/coverage-test/vitest.config.ts index f07d07c42a30..88e7a0e286ac 100644 --- a/test/coverage-test/vitest.config.ts +++ b/test/coverage-test/vitest.config.ts @@ -1,9 +1,6 @@ import { defineConfig } from 'vitest/config' export default defineConfig({ - server: { - watch: null, - }, test: { reporters: 'verbose', isolate: false,