From 32d57269f09eec69246a1b488bc61e15c396cff4 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Wed, 12 Mar 2025 15:47:51 +0100 Subject: [PATCH 01/24] perf(browser): improve browser parallelisation --- packages/browser/src/client/client.ts | 2 +- packages/browser/src/client/orchestrator.ts | 7 +- packages/browser/src/node/pool.ts | 253 +++++++++++------- .../browser/src/node/providers/playwright.ts | 3 + packages/vitest/src/node/browser/sessions.ts | 2 +- pnpm-lock.yaml | 3 + test/browser/vitest.config.mts | 2 +- 7 files changed, 171 insertions(+), 101 deletions(-) diff --git a/packages/browser/src/client/client.ts b/packages/browser/src/client/client.ts index 72834befda7d..71eddfdfbe52 100644 --- a/packages/browser/src/client/client.ts +++ b/packages/browser/src/client/client.ts @@ -56,7 +56,7 @@ function createClient() { if (PAGE_TYPE !== 'orchestrator') { return } - getBrowserState().createTesters?.(files) + return getBrowserState().createTesters?.(files) }, cdpEvent(event: string, payload: unknown) { const cdp = getBrowserState().cdp diff --git a/packages/browser/src/client/orchestrator.ts b/packages/browser/src/client/orchestrator.ts index 37054b21faaf..76b28327e73a 100644 --- a/packages/browser/src/client/orchestrator.ts +++ b/packages/browser/src/client/orchestrator.ts @@ -14,7 +14,7 @@ class IframeOrchestrator { private runningFiles = new Set() private iframes = new Map() - public async init(testFiles: string[]) { + public init(testFiles: string[]) { debug('test files', testFiles.join(', ')) this.runningFiles.clear() @@ -234,13 +234,16 @@ async function getContainer(config: SerializedConfig): Promise { client.waitForConnection().then(async () => { const testFiles = getBrowserState().files - await orchestrator.init(testFiles) + orchestrator.init(testFiles) // if page was refreshed, there will be no test files // createTesters will be called again when tests are running in the UI if (testFiles.length) { await orchestrator.createTesters(testFiles) } + else { + await done() + } }) function generateFileId(file: string) { diff --git a/packages/browser/src/node/pool.ts b/packages/browser/src/node/pool.ts index be0ad6e60993..54932e057e54 100644 --- a/packages/browser/src/node/pool.ts +++ b/packages/browser/src/node/pool.ts @@ -1,12 +1,20 @@ -import type { BrowserProvider, ProcessPool, TestProject, TestSpecification, Vitest } from 'vitest/node' +import type { DeferPromise } from '@vitest/utils' +import type { + BrowserProvider, + ProcessPool, + TestProject, + TestSpecification, + Vitest, +} from 'vitest/node' import crypto from 'node:crypto' import * as nodeos from 'node:os' -import { relative } from 'pathe' -import { createDebugger } from 'vitest/node' +import { createDefer } from '@vitest/utils' +// import { relative } from 'pathe' +// import { createDebugger } from 'vitest/node' -const debug = createDebugger('vitest:browser:pool') +// const debug = createDebugger('vitest:browser:pool') -async function waitForTests( +async function waitForOrchestrator( method: 'run' | 'collect', sessionId: string, project: TestProject, @@ -16,101 +24,75 @@ async function waitForTests( return await context } -export function createBrowserPool(vitest: Vitest): ProcessPool { - const providers = new Set() +// TODO: support breakpoints +// async function setBreakpoint(project: TestProject, sessionId: string, file: string) { +// if (!project.config.inspector.waitForDebugger) { +// return +// } - const executeTests = async (method: 'run' | 'collect', project: TestProject, files: string[]) => { - vitest.state.clearFiles(project, files) - const browser = project.browser! +// const provider = project.browser!.provider - const threadsCount = getThreadsCount(project) +// if (!provider.getCDPSession) { +// throw new Error('Unable to set breakpoint, CDP not supported') +// } - const provider = browser.provider - providers.add(provider) +// const session = await provider.getCDPSession(sessionId) +// await session.send('Debugger.enable', {}) +// await session.send('Debugger.setBreakpointByUrl', { +// lineNumber: 0, +// urlRegex: escapePathToRegexp(file), +// }) +// } - const resolvedUrls = browser.vite.resolvedUrls - const origin = resolvedUrls?.local[0] ?? resolvedUrls?.network[0] +export function createBrowserPool(vitest: Vitest): ProcessPool { + const providers = new Set() - if (!origin) { - throw new Error( - `Can't find browser origin URL for project "${project.name}" when running tests for files "${files.join('", "')}"`, - ) - } + const numCpus + = typeof nodeos.availableParallelism === 'function' + ? nodeos.availableParallelism() + : nodeos.cpus().length - async function setBreakpoint(sessionId: string, file: string) { - if (!project.config.inspector.waitForDebugger) { - return - } + const threadsCount = vitest.config.watch + ? Math.max(Math.floor(numCpus / 2), 1) + : Math.max(numCpus - 1, 1) - if (!provider.getCDPSession) { - throw new Error('Unable to set breakpoint, CDP not supported') - } + const executeTests = async ( + method: 'run' | 'collect', + pool: BrowserPool, + project: TestProject, + files: string[], + ) => { + vitest.state.clearFiles(project, files) + providers.add(project.browser!.provider) - const session = await provider.getCDPSession(sessionId) - await session.send('Debugger.enable', {}) - await session.send('Debugger.setBreakpointByUrl', { - lineNumber: 0, - urlRegex: escapePathToRegexp(file), - }) - } + await pool.runTests(files) + } - const filesPerThread = Math.ceil(files.length / threadsCount) + const projectPools = new WeakMap() - // TODO: make it smarter, - // Currently if we run 4/4/4/4 tests, and one of the chunks ends, - // but there are pending tests in another chunks, we can't redistribute them - const chunks: string[][] = [] - for (let i = 0; i < files.length; i += filesPerThread) { - const chunk = files.slice(i, i + filesPerThread) - chunks.push(chunk) + const ensurePool = (project: TestProject) => { + if (projectPools.has(project)) { + return projectPools.get(project)! } - debug?.( - `[%s] Running %s tests in %s chunks (%s threads)`, - project.name || 'core', - files.length, - chunks.length, - threadsCount, - ) - - const orchestrators = [...browser.state.orchestrators.entries()] + const resolvedUrls = project.browser!.vite.resolvedUrls + const origin = resolvedUrls?.local[0] ?? resolvedUrls?.network[0] - const promises: Promise[] = [] + if (!origin) { + throw new Error( + `Can't find browser origin URL for project "${project.name}"`, + ) + } - chunks.forEach((files, index) => { - if (orchestrators[index]) { - const [sessionId, orchestrator] = orchestrators[index] - debug?.( - 'Reusing orchestrator (session %s) for files: %s', - sessionId, - [...files.map(f => relative(project.config.root, f))].join(', '), - ) - const promise = waitForTests(method, sessionId, project, files) - const tester = orchestrator.createTesters(files).catch((error) => { - if (error instanceof Error && error.message.startsWith('[birpc] rpc is closed')) { - return - } - return Promise.reject(error) - }) - promises.push(promise, tester) - } - else { - const sessionId = crypto.randomUUID() - const waitPromise = waitForTests(method, sessionId, project, files) - debug?.( - 'Opening a new session %s for files: %s', - sessionId, - [...files.map(f => relative(project.config.root, f))].join(', '), - ) - const url = new URL('/', origin) - url.searchParams.set('sessionId', sessionId) - const page = provider - .openPage(sessionId, url.toString(), () => setBreakpoint(sessionId, files[0])) - promises.push(page, waitPromise) - } + const pool = new BrowserPool(project, { + maxWorkers: getThreadsCount(project), + origin, + // method doesn't matter here, we just need to create an orchestrator + worker: sessionId => waitForOrchestrator('run', sessionId, project, []), }) + projectPools.set(project, pool) - await Promise.all(promises) + return pool } const runWorkspaceTests = async (method: 'run' | 'collect', specs: TestSpecification[]) => { @@ -136,15 +118,12 @@ export function createBrowserPool(vitest: Vitest): ProcessPool { if (!project.browser) { throw new TypeError(`The browser server was not initialized${project.name ? ` for the "${project.name}" project` : ''}. This is a bug in Vitest. Please, open a new issue with reproduction.`) } - await executeTests(method, project, files) + + const pool = ensurePool(project) + await executeTests(method, pool, project, files) } } - const numCpus - = typeof nodeos.availableParallelism === 'function' - ? nodeos.availableParallelism() - : nodeos.cpus().length - function getThreadsCount(project: TestProject) { const config = project.config.browser if (!config.headless || !project.browser!.provider.supportsParallelism) { @@ -159,9 +138,7 @@ export function createBrowserPool(vitest: Vitest): ProcessPool { return project.config.maxWorkers } - return vitest.config.watch - ? Math.max(Math.floor(numCpus / 2), 1) - : Math.max(numCpus - 1, 1) + return threadsCount } return { @@ -169,7 +146,7 @@ export function createBrowserPool(vitest: Vitest): ProcessPool { async close() { await Promise.all([...providers].map(provider => provider.close())) providers.clear() - vitest.resolvedProjects.forEach((project) => { + vitest.projects.forEach((project) => { project.browser?.state.orchestrators.forEach((orchestrator) => { orchestrator.$close() }) @@ -180,6 +157,90 @@ export function createBrowserPool(vitest: Vitest): ProcessPool { } } -function escapePathToRegexp(path: string): string { - return path.replace(/[/\\.?*()^${}|[\]+]/g, '\\$&') +// function escapePathToRegexp(path: string): string { +// return path.replace(/[/\\.?*()^${}|[\]+]/g, '\\$&') +// } + +class BrowserPool { + private _queue: string[] = [] + private _promise: DeferPromise | undefined + + constructor( + private project: TestProject, + private options: { + maxWorkers: number + origin: string + worker: (sessionId: string) => Promise + }, + ) {} + + get orchestrators() { + return this.project.browser!.state.orchestrators + } + + // open 4 browser contexts + async runTests(files: string[]) { + this._promise ??= createDefer() + + if (!files.length) { + this._promise.resolve() + return this._promise + } + + this._queue.push(...files) + + if (this.orchestrators.size >= this.options.maxWorkers) { + // TODO: select non-busy orchestrator and run tests there(?) + return this._promise + } + const promises: Promise[] = [] + for (let i = this.orchestrators.size; i < this.options.maxWorkers; i++) { + const sessionId = crypto.randomUUID() + const promise = this.options.worker(sessionId) + const url = new URL('/', this.options.origin) + url.searchParams.set('sessionId', sessionId) + const page = this.project.browser!.provider.openPage(sessionId, url.toString()) + promises.push( + Promise.all([promise, page]).then(() => { + this.runNextTest(sessionId) + }), + ) + } + await Promise.all(promises) + return this._promise + } + + private runNextTest(sessionId: string) { + const file = this._queue.shift() + if (!file) { + this._promise?.resolve() + this._promise = undefined + return + } + if (!this._promise) { + throw new Error(`Unexpected empty queue`) + } + const orchestrator = this.orchestrators.get(sessionId) + if (!orchestrator) { + // console.log('[fail]', file) + // TODO: handle this error + this._promise.reject( + new Error(`Orchestrator not found for session ${sessionId}`), + ) + return + } + + orchestrator.createTesters([file]) + .then(() => { + // console.log('[finish]', file) + this.runNextTest(sessionId) + }) + .catch((error) => { + // console.log('error', error) + if (error instanceof Error && error.message.startsWith('[birpc] rpc is closed')) { + return + } + return Promise.reject(error) + }) + } } diff --git a/packages/browser/src/node/providers/playwright.ts b/packages/browser/src/node/providers/playwright.ts index 33960e7ea09c..fa41a4069884 100644 --- a/packages/browser/src/node/providers/playwright.ts +++ b/packages/browser/src/node/providers/playwright.ts @@ -176,6 +176,9 @@ export class PlaywrightBrowserProvider implements BrowserProvider { const context = await this.createContext(sessionId) const page = await context.newPage() + // page.on('console', (m) => { + // console.log('pw', m.text()) + // }) this.pages.set(sessionId, page) if (process.env.VITEST_PW_DEBUG) { diff --git a/packages/vitest/src/node/browser/sessions.ts b/packages/vitest/src/node/browser/sessions.ts index dfff4f8644df..c97e320ae983 100644 --- a/packages/vitest/src/node/browser/sessions.ts +++ b/packages/vitest/src/node/browser/sessions.ts @@ -28,7 +28,7 @@ export class BrowserSessions { resolve: () => { defer.resolve() clearTimeout(timeout) - this.sessions.delete(sessionId) + // this.sessions.delete(sessionId) }, reject: defer.reject, }) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index dc29d793eb32..2eda745cf78d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -495,6 +495,9 @@ importers: sirv: specifier: 'catalog:' version: 3.0.1 + tinypool: + specifier: link:../../../tinypool + version: link:../../../tinypool tinyrainbow: specifier: 'catalog:' version: 2.0.0 diff --git a/test/browser/vitest.config.mts b/test/browser/vitest.config.mts index 7f7e784d3e14..415e6663a7c1 100644 --- a/test/browser/vitest.config.mts +++ b/test/browser/vitest.config.mts @@ -58,7 +58,7 @@ export default defineConfig({ ? playwrightInstances : webdriverioInstances, provider, - isolate: false, + // isolate: false, testerScripts: [ { content: 'globalThis.__injected = []', From 1a47659d8fc08c730f3f04727ac3fa2285fbd40f Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Wed, 12 Mar 2025 16:10:32 +0100 Subject: [PATCH 02/24] chore: undo lockfile change --- pnpm-lock.yaml | 3 --- 1 file changed, 3 deletions(-) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2eda745cf78d..dc29d793eb32 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -495,9 +495,6 @@ importers: sirv: specifier: 'catalog:' version: 3.0.1 - tinypool: - specifier: link:../../../tinypool - version: link:../../../tinypool tinyrainbow: specifier: 'catalog:' version: 2.0.0 From bd165782ea4de751a4842fd131a00fb601e404e0 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Thu, 13 Mar 2025 15:32:15 +0100 Subject: [PATCH 03/24] refactor: a lot of stuff --- packages/browser/src/client/client.ts | 4 +- packages/browser/src/client/orchestrator.ts | 78 +++++++++---------- .../src/client/public/esm-client-injector.js | 1 - packages/browser/src/client/tester/state.ts | 8 +- packages/browser/src/client/tester/tester.ts | 44 ++++++++++- packages/browser/src/client/utils.ts | 3 +- .../src/node/middlewares/testerMiddleware.ts | 2 +- packages/browser/src/node/plugin.ts | 9 --- packages/browser/src/node/pool.ts | 57 ++++++++------ packages/browser/src/node/rpc.ts | 21 ++--- .../browser/src/node/serverOrchestrator.ts | 7 +- packages/browser/src/node/serverTester.ts | 31 +++----- packages/browser/src/node/types.ts | 4 +- packages/vitest/src/node/browser/sessions.ts | 16 ++-- packages/vitest/src/node/types/browser.ts | 5 +- packages/vitest/src/runtime/config.ts | 4 +- 16 files changed, 152 insertions(+), 142 deletions(-) diff --git a/packages/browser/src/client/client.ts b/packages/browser/src/client/client.ts index 71eddfdfbe52..c7ea789ff047 100644 --- a/packages/browser/src/client/client.ts +++ b/packages/browser/src/client/client.ts @@ -52,11 +52,11 @@ function createClient() { ctx.rpc = createBirpc( { onCancel: setCancel, - async createTesters(files: string[]) { + async createTesters(method: 'run' | 'collect', files: string[]) { if (PAGE_TYPE !== 'orchestrator') { return } - return getBrowserState().createTesters?.(files) + return getBrowserState().createTesters?.(method, files) }, cdpEvent(event: string, payload: unknown) { const cdp = getBrowserState().cdp diff --git a/packages/browser/src/client/orchestrator.ts b/packages/browser/src/client/orchestrator.ts index 76b28327e73a..46c2004879d0 100644 --- a/packages/browser/src/client/orchestrator.ts +++ b/packages/browser/src/client/orchestrator.ts @@ -14,11 +14,10 @@ class IframeOrchestrator { private runningFiles = new Set() private iframes = new Map() - public init(testFiles: string[]) { - debug('test files', testFiles.join(', ')) + public init() { + debug('init orchestrator', getBrowserState().sessionId) this.runningFiles.clear() - testFiles.forEach(file => this.runningFiles.add(file)) channel.addEventListener( 'message', @@ -30,7 +29,7 @@ class IframeOrchestrator { ) } - public async createTesters(testFiles: string[]) { + public async createTesters(method: 'run' | 'collect', testFiles: string[]) { this.cancelled = false this.runningFiles.clear() testFiles.forEach(file => this.runningFiles.add(file)) @@ -51,7 +50,7 @@ class IframeOrchestrator { if (config.browser.isolate === false) { debug('create iframe', ID_ALL) - const iframe = this.createIframe(container, ID_ALL) + const iframe = this.createIframe(container, method, ID_ALL, testFiles) await setIframeViewport(iframe, width, height) return @@ -59,12 +58,11 @@ class IframeOrchestrator { for (const file of testFiles) { if (this.cancelled) { - done() return } debug('create iframe', file) - const iframe = this.createIframe(container, file) + const iframe = this.createIframe(container, method, file, [file]) await setIframeViewport(iframe, width, height) @@ -83,21 +81,39 @@ class IframeOrchestrator { } } - private createIframe(container: HTMLDivElement, file: string) { - if (this.iframes.has(file)) { - this.iframes.get(file)!.remove() - this.iframes.delete(file) + private createIframe(container: HTMLDivElement, method: 'run' | 'collect', iframeId: string, files: string[]) { + if (this.iframes.has(iframeId)) { + this.iframes.get(iframeId)!.remove() + this.iframes.delete(iframeId) } const iframe = document.createElement('iframe') + const src = `${url.pathname}__vitest_test__/__test__/?sessionId=${getBrowserState().sessionId}` iframe.setAttribute('loading', 'eager') - iframe.setAttribute( - 'src', - `${url.pathname}__vitest_test__/__test__/${ - getBrowserState().sessionId - }/${encodeURIComponent(file)}`, - ) + iframe.setAttribute('src', src) iframe.setAttribute('data-vitest', 'true') + iframe.onerror = (e) => { + debug('iframe error', e.toString()) + } + iframe.onload = () => { + debug(`iframe for ${src} loaded`) + const iframeWindow = iframe.contentWindow + if (!iframeWindow) { + debug('no window available') + // TODO: what happened here? + return + } + iframeWindow.postMessage( + JSON.stringify({ + event: 'init', + method, + files, + iframeId, + context: getBrowserState().providedContext, + }), + '*', + ) + } iframe.style.border = 'none' iframe.style.width = '100%' @@ -106,7 +122,7 @@ class IframeOrchestrator { iframe.setAttribute('allow', 'clipboard-write;') iframe.setAttribute('name', 'vitest-iframe') - this.iframes.set(file, iframe) + this.iframes.set(iframeId, iframe) container.appendChild(iframe) return iframe } @@ -159,7 +175,6 @@ class IframeOrchestrator { const id = generateFileId(filenames[filenames.length - 1]) ui.setCurrentFileId(id) } - await done() } else { // keep the last iframe @@ -180,9 +195,6 @@ class IframeOrchestrator { else { this.runningFiles.delete(iframeId) } - if (!this.runningFiles.size) { - await done() - } break } default: { @@ -195,7 +207,6 @@ class IframeOrchestrator { }, 'Unexpected Event', ) - await done() } } } @@ -204,18 +215,14 @@ class IframeOrchestrator { const orchestrator = new IframeOrchestrator() let promiseTesters: Promise | undefined -getBrowserState().createTesters = async (files) => { +getBrowserState().createTesters = async (method, files) => { await promiseTesters - promiseTesters = orchestrator.createTesters(files).finally(() => { + promiseTesters = orchestrator.createTesters(method, files).finally(() => { promiseTesters = undefined }) await promiseTesters } -async function done() { - await client.rpc.finishBrowserTests(getBrowserState().sessionId) -} - async function getContainer(config: SerializedConfig): Promise { if (config.browser.ui) { const element = document.querySelector('#tester-ui') @@ -232,18 +239,7 @@ async function getContainer(config: SerializedConfig): Promise { } client.waitForConnection().then(async () => { - const testFiles = getBrowserState().files - - orchestrator.init(testFiles) - - // if page was refreshed, there will be no test files - // createTesters will be called again when tests are running in the UI - if (testFiles.length) { - await orchestrator.createTesters(testFiles) - } - else { - await done() - } + orchestrator.init() }) function generateFileId(file: string) { diff --git a/packages/browser/src/client/public/esm-client-injector.js b/packages/browser/src/client/public/esm-client-injector.js index 394baa032a4e..720d9e669c65 100644 --- a/packages/browser/src/client/public/esm-client-injector.js +++ b/packages/browser/src/client/public/esm-client-injector.js @@ -22,7 +22,6 @@ moduleCache, config: { __VITEST_CONFIG__ }, viteConfig: { __VITEST_VITE_CONFIG__ }, - files: { __VITEST_FILES__ }, type: { __VITEST_TYPE__ }, sessionId: { __VITEST_SESSION_ID__ }, testerId: { __VITEST_TESTER_ID__ }, diff --git a/packages/browser/src/client/tester/state.ts b/packages/browser/src/client/tester/state.ts index a1698446dbf8..2e740a9d8913 100644 --- a/packages/browser/src/client/tester/state.ts +++ b/packages/browser/src/client/tester/state.ts @@ -1,13 +1,10 @@ import type { BrowserRPC } from '@vitest/browser/client' import type { WorkerGlobalState } from 'vitest' -import { parse } from 'flatted' import { getBrowserState } from '../utils' const config = getBrowserState().config const sessionId = getBrowserState().sessionId -const providedContext = parse(getBrowserState().providedContext) - const state: WorkerGlobalState = { ctx: { pool: 'browser', @@ -20,7 +17,8 @@ const state: WorkerGlobalState = { name: 'browser', options: null, }, - providedContext, + // this is populated before tests run + providedContext: {}, invalidates: [], }, onCancel: null as any, @@ -38,7 +36,7 @@ const state: WorkerGlobalState = { environment: 0, prepare: performance.now(), }, - providedContext, + providedContext: {}, } // @ts-expect-error not typed global diff --git a/packages/browser/src/client/tester/tester.ts b/packages/browser/src/client/tester/tester.ts index df9b218ef789..675782493b15 100644 --- a/packages/browser/src/client/tester/tester.ts +++ b/packages/browser/src/client/tester/tester.ts @@ -1,5 +1,6 @@ import { channel, client, onCancel } from '@vitest/browser/client' import { page, server, userEvent } from '@vitest/browser/context' +import { parse } from 'flatted' import { collectTests, setupCommonEnv, SpyModule, startCoverageInsideWorker, startTests, stopCoverageInsideWorker } from 'vitest/browser' import { executor, getBrowserState, getConfig, getWorkerState } from '../utils' import { setupDialogsSpy } from './dialog' @@ -203,7 +204,42 @@ async function executeTests(method: 'run' | 'collect', files: string[]) { } } -// @ts-expect-error untyped global for internal use -window.__vitest_browser_runner__.runTests = files => executeTests('run', files) -// @ts-expect-error untyped global for internal use -window.__vitest_browser_runner__.collectTests = files => executeTests('collect', files) +// listen when orchestrator sends a message +window.addEventListener('message', (e) => { + const data = JSON.parse(e.data) + debug('event', e.data) + + if (data.event === 'init') { + const { method, files, context, iframeId } = data as { + method: 'run' | 'collect' + files: string[] + context: string + iframeId: string + } + const state = getWorkerState() + const parsedContext = parse(context) + + state.ctx.providedContext = parsedContext + state.providedContext = parsedContext + getBrowserState().iframeId = iframeId + + if (method === 'collect') { + executeTests('collect', files).catch(err => unhandledError(err, 'Collect Error')) + } + else { + executeTests('run', files).catch(err => unhandledError(err, 'Run Error')) + } + } + else { + const error = new Error(`Unknown event: ${data.event}`) + unhandledError(error, 'Uknown Event') + } +}) + +function unhandledError(e: Error, type: string) { + client.rpc.onUnhandledError({ + name: e.name, + message: e.message, + stack: e.stack, + }, type).catch(() => {}) +} diff --git a/packages/browser/src/client/utils.ts b/packages/browser/src/client/utils.ts index f46307e1c253..4d83ff5b977d 100644 --- a/packages/browser/src/client/utils.ts +++ b/packages/browser/src/client/utils.ts @@ -76,8 +76,7 @@ export interface BrowserRunnerState { sessionId: string testerId: string method: 'run' | 'collect' - runTests?: (tests: string[]) => Promise - createTesters?: (files: string[]) => Promise + createTesters?: (method: 'run' | 'collect', files: string[]) => Promise commands: CommandsManager cdp?: { on: (event: string, listener: (payload: any) => void) => void diff --git a/packages/browser/src/node/middlewares/testerMiddleware.ts b/packages/browser/src/node/middlewares/testerMiddleware.ts index 435ff7149ed1..10026d9c694d 100644 --- a/packages/browser/src/node/middlewares/testerMiddleware.ts +++ b/packages/browser/src/node/middlewares/testerMiddleware.ts @@ -9,7 +9,7 @@ export function createTesterMiddleware(browserServer: ParentBrowserProject): Con return next() } const url = new URL(req.url, 'http://localhost') - if (!url.pathname.startsWith(browserServer.prefixTesterUrl)) { + if (!url.pathname.startsWith(browserServer.prefixTesterUrl) || !url.searchParams.has('sessionId')) { return next() } diff --git a/packages/browser/src/node/plugin.ts b/packages/browser/src/node/plugin.ts index 7439c0aedb61..082175b9cf9d 100644 --- a/packages/browser/src/node/plugin.ts +++ b/packages/browser/src/node/plugin.ts @@ -517,15 +517,6 @@ body { : null, ...parentServer.testerScripts, ...testerTags, - { - tag: 'script', - attrs: { - 'type': 'module', - 'data-vitest-append': '', - }, - children: '{__VITEST_APPEND__}', - injectTo: 'body', - } as const, ].filter(s => s != null) }, }, diff --git a/packages/browser/src/node/pool.ts b/packages/browser/src/node/pool.ts index 54932e057e54..307c1c99f323 100644 --- a/packages/browser/src/node/pool.ts +++ b/packages/browser/src/node/pool.ts @@ -15,12 +15,10 @@ import { createDefer } from '@vitest/utils' // const debug = createDebugger('vitest:browser:pool') async function waitForOrchestrator( - method: 'run' | 'collect', sessionId: string, project: TestProject, - files: string[], ) { - const context = project.vitest._browserSessions.createAsyncSession(method, sessionId, files, project) + const context = project.vitest._browserSessions.createSession(sessionId, project) return await context } @@ -65,7 +63,7 @@ export function createBrowserPool(vitest: Vitest): ProcessPool { vitest.state.clearFiles(project, files) providers.add(project.browser!.provider) - await pool.runTests(files) + await pool.runTests(method, files) } const projectPools = new WeakMap() @@ -87,8 +85,7 @@ export function createBrowserPool(vitest: Vitest): ProcessPool { const pool = new BrowserPool(project, { maxWorkers: getThreadsCount(project), origin, - // method doesn't matter here, we just need to create an orchestrator - worker: sessionId => waitForOrchestrator('run', sessionId, project, []), + worker: sessionId => waitForOrchestrator(sessionId, project), }) projectPools.set(project, pool) @@ -165,6 +162,8 @@ class BrowserPool { private _queue: string[] = [] private _promise: DeferPromise | undefined + private readySessions = new Set() + constructor( private project: TestProject, private options: { @@ -178,8 +177,8 @@ class BrowserPool { return this.project.browser!.state.orchestrators } - // open 4 browser contexts - async runTests(files: string[]) { + // open "maxWothers" browser contexts + async runTests(method: 'run' | 'collect', files: string[]) { this._promise ??= createDefer() if (!files.length) { @@ -189,32 +188,42 @@ class BrowserPool { this._queue.push(...files) + this.readySessions.forEach((sessionId) => { + this.readySessions.delete(sessionId) + this.runNextTest(method, sessionId) + }) + if (this.orchestrators.size >= this.options.maxWorkers) { - // TODO: select non-busy orchestrator and run tests there(?) return this._promise } + const promises: Promise[] = [] for (let i = this.orchestrators.size; i < this.options.maxWorkers; i++) { const sessionId = crypto.randomUUID() - const promise = this.options.worker(sessionId) - const url = new URL('/', this.options.origin) - url.searchParams.set('sessionId', sessionId) - const page = this.project.browser!.provider.openPage(sessionId, url.toString()) - promises.push( - Promise.all([promise, page]).then(() => { - this.runNextTest(sessionId) - }), - ) + const page = this.openPage(sessionId).then(() => { + // start running tests on the page when it's ready + this.runNextTest(method, sessionId) + }) + promises.push(page) } await Promise.all(promises) return this._promise } - private runNextTest(sessionId: string) { + private async openPage(sessionId: string) { + const promise = this.options.worker(sessionId) + const url = new URL('/', this.options.origin) + url.searchParams.set('sessionId', sessionId) + const page = this.project.browser!.provider.openPage(sessionId, url.toString()) + await Promise.all([promise, page]) + } + + private runNextTest(method: 'run' | 'collect', sessionId: string) { const file = this._queue.shift() if (!file) { this._promise?.resolve() this._promise = undefined + this.readySessions.add(sessionId) return } if (!this._promise) { @@ -222,7 +231,6 @@ class BrowserPool { } const orchestrator = this.orchestrators.get(sessionId) if (!orchestrator) { - // console.log('[fail]', file) // TODO: handle this error this._promise.reject( new Error(`Orchestrator not found for session ${sessionId}`), @@ -230,17 +238,18 @@ class BrowserPool { return } - orchestrator.createTesters([file]) + orchestrator.createTesters(method, [file]) .then(() => { - // console.log('[finish]', file) - this.runNextTest(sessionId) + this.runNextTest(method, sessionId) }) .catch((error) => { - // console.log('error', error) if (error instanceof Error && error.message.startsWith('[birpc] rpc is closed')) { return } return Promise.reject(error) }) } + + // TODO: onCancel + // TODO: isolate: false } diff --git a/packages/browser/src/node/rpc.ts b/packages/browser/src/node/rpc.ts index aa631930bf1e..697accb67e3d 100644 --- a/packages/browser/src/node/rpc.ts +++ b/packages/browser/src/node/rpc.ts @@ -57,11 +57,11 @@ export function setupBrowserRpc(globalServer: ParentBrowserProject): void { } const method = searchParams.get('method') as 'run' | 'collect' - if (method !== 'run' && method !== 'collect') { - return error( - new Error(`[vitest] Method query in ${request.url} is invalid. Method should be either "run" or "collect".`), - ) - } + // if (method !== 'run' && method !== 'collect') { + // return error( + // new Error(`[vitest] Method query in ${request.url} is invalid. Method should be either "run" or "collect".`), + // ) + // } if (type === 'orchestrator') { const session = vitest._browserSessions.getSession(sessionId) @@ -91,6 +91,9 @@ export function setupBrowserRpc(globalServer: ParentBrowserProject): void { debug?.('[%s] Browser API disconnected from %s', rpcId, type) clients.delete(rpcId) globalServer.removeCDPHandler(rpcId) + if (type === 'orchestrator') { + vitest._browserSessions.forgetSession(sessionId) + } }) }) }) @@ -241,10 +244,10 @@ export function setupBrowserRpc(globalServer: ParentBrowserProject): void { ) as any as BrowserCommandContext return await commands[command](context, ...payload) }, - finishBrowserTests(sessionId: string) { - debug?.('[%s] Finishing browser tests for session', sessionId) - return vitest._browserSessions.getSession(sessionId)?.resolve() - }, + // finishBrowserTests(sessionId: string) { + // debug?.('[%s] Finishing browser tests for session', sessionId) + // return vitest._browserSessions.getSession(sessionId)?.resolve() + // }, resolveMock(rawId, importer, options) { return mockResolver.resolveMock(rawId, importer, options) }, diff --git a/packages/browser/src/node/serverOrchestrator.ts b/packages/browser/src/node/serverOrchestrator.ts index 7dcf60920125..3290f0c85d12 100644 --- a/packages/browser/src/node/serverOrchestrator.ts +++ b/packages/browser/src/node/serverOrchestrator.ts @@ -1,6 +1,7 @@ import type { IncomingMessage, ServerResponse } from 'node:http' import type { ProjectBrowser } from './project' import type { ParentBrowserProject } from './projectParent' +import { stringify } from 'flatted' import { replacer } from './utils' export async function resolveOrchestrator( @@ -19,7 +20,6 @@ export async function resolveOrchestrator( // because the user could refresh the page which would remove the session id from the url const session = globalServer.vitest._browserSessions.getSession(sessionId!) - const files = session?.files ?? [] const browserProject = (session?.project.browser as ProjectBrowser | undefined) || [...globalServer.children][0] if (!browserProject) { @@ -36,12 +36,11 @@ export async function resolveOrchestrator( __VITEST_VITE_CONFIG__: JSON.stringify({ root: browserProject.vite.config.root, }), - __VITEST_METHOD__: JSON.stringify(session?.method || 'run'), - __VITEST_FILES__: JSON.stringify(files), + __VITEST_METHOD__: JSON.stringify('orchestrate'), __VITEST_TYPE__: '"orchestrator"', __VITEST_SESSION_ID__: JSON.stringify(sessionId), __VITEST_TESTER_ID__: '"none"', - __VITEST_PROVIDED_CONTEXT__: '{}', + __VITEST_PROVIDED_CONTEXT__: JSON.stringify(stringify(browserProject.project.getProvidedContext())), __VITEST_API_TOKEN__: JSON.stringify(globalServer.vitest.config.api.token), }) diff --git a/packages/browser/src/node/serverTester.ts b/packages/browser/src/node/serverTester.ts index 70e3f7bf9604..cbbe8e42993f 100644 --- a/packages/browser/src/node/serverTester.ts +++ b/packages/browser/src/node/serverTester.ts @@ -3,7 +3,6 @@ import type { Connect } from 'vite' import type { ProjectBrowser } from './project' import type { ParentBrowserProject } from './projectParent' import crypto from 'node:crypto' -import { stringify } from 'flatted' import { join } from 'pathe' import { replacer } from './utils' @@ -23,7 +22,7 @@ export async function resolveTester( ) } - const { sessionId, testFile } = globalServer.resolveTesterUrl(url.pathname) + const sessionId = url.searchParams.get('sessionId') || 'none' const session = globalServer.vitest._browserSessions.getSession(sessionId) if (!session) { @@ -33,17 +32,6 @@ export async function resolveTester( } const project = globalServer.vitest.getProjectByName(session.project.name || '') - const { testFiles } = await project.globTestFiles() - // if decoded test file is "__vitest_all__" or not in the list of known files, run all tests - const tests - = testFile === '__vitest_all__' - || !testFiles.includes(testFile) - ? '__vitest_browser_runner__.files' - : JSON.stringify([testFile]) - const iframeId = JSON.stringify(testFile) - const files = session.files ?? [] - const method = session.method ?? 'run' - const browserProject = (project.browser as ProjectBrowser | undefined) || [...globalServer.children][0] if (!browserProject) { @@ -59,15 +47,14 @@ export async function resolveTester( const injector = replacer(injectorJs, { __VITEST_PROVIDER__: JSON.stringify(project.browser!.provider.name), __VITEST_CONFIG__: JSON.stringify(browserProject.wrapSerializedConfig()), - __VITEST_FILES__: JSON.stringify(files), __VITEST_VITE_CONFIG__: JSON.stringify({ root: browserProject.vite.config.root, }), __VITEST_TYPE__: '"tester"', - __VITEST_METHOD__: JSON.stringify(method), + __VITEST_METHOD__: JSON.stringify('none'), __VITEST_SESSION_ID__: JSON.stringify(sessionId), __VITEST_TESTER_ID__: JSON.stringify(crypto.randomUUID()), - __VITEST_PROVIDED_CONTEXT__: JSON.stringify(stringify(project.getProvidedContext())), + __VITEST_PROVIDED_CONTEXT__: '{}', __VITEST_API_TOKEN__: JSON.stringify(globalServer.vitest.config.api.token), }) @@ -81,12 +68,12 @@ export async function resolveTester( const html = replacer(indexhtml, { __VITEST_FAVICON__: globalServer.faviconUrl, __VITEST_INJECTOR__: injector, - __VITEST_APPEND__: ` - __vitest_browser_runner__.runningFiles = ${tests} - __vitest_browser_runner__.iframeId = ${iframeId} - __vitest_browser_runner__.${method === 'run' ? 'runTests' : 'collectTests'}(__vitest_browser_runner__.runningFiles) - document.querySelector('script[data-vitest-append]').remove() - `, + // __VITEST_APPEND__: ` + // __vitest_browser_runner__.runningFiles = ${test} + // __vitest_browser_runner__.iframeId = ${iframeId} + // __vitest_browser_runner__.${method === 'run' ? 'runTests' : 'collectTests'}(__vitest_browser_runner__.runningFiles) + // document.querySelector('script[data-vitest-append]').remove() + // `, }) return html } diff --git a/packages/browser/src/node/types.ts b/packages/browser/src/node/types.ts index 53637d71429f..33bb529f45b8 100644 --- a/packages/browser/src/node/types.ts +++ b/packages/browser/src/node/types.ts @@ -17,7 +17,7 @@ export interface WebSocketBrowserHandlers { saveSnapshotFile: (id: string, content: string) => Promise removeSnapshotFile: (id: string) => Promise sendLog: (log: UserConsoleLog) => void - finishBrowserTests: (sessionId: string) => void + // finishBrowserTests: (sessionId: string) => void snapshotSaved: (snapshot: SnapshotResult) => void debug: (...args: string[]) => void resolveId: ( @@ -61,7 +61,7 @@ export interface WebSocketEvents export interface WebSocketBrowserEvents { onCancel: (reason: CancelReason) => void - createTesters: (files: string[]) => Promise + createTesters: (method: 'run' | 'collect', files: string[]) => Promise cdpEvent: (event: string, payload: unknown) => void } diff --git a/packages/vitest/src/node/browser/sessions.ts b/packages/vitest/src/node/browser/sessions.ts index c97e320ae983..15497c4e8e47 100644 --- a/packages/vitest/src/node/browser/sessions.ts +++ b/packages/vitest/src/node/browser/sessions.ts @@ -1,7 +1,6 @@ import type { TestProject } from '../project' import type { BrowserServerStateSession } from '../types/browser' import { createDefer } from '@vitest/utils' -import { relative } from 'pathe' export class BrowserSessions { private sessions = new Map() @@ -10,25 +9,22 @@ export class BrowserSessions { return this.sessions.get(sessionId) } - createAsyncSession(method: 'run' | 'collect', sessionId: string, files: string[], project: TestProject): Promise { + forgetSession(sessionId: string): void { + this.sessions.delete(sessionId) + } + + createSession(sessionId: string, project: TestProject): Promise { const defer = createDefer() const timeout = setTimeout(() => { - const tests = files.map(file => relative(project.config.root, file)).join('", "') - defer.reject(new Error(`Failed to connect to the browser session "${sessionId}" [${project.name}] for "${tests}" within the timeout.`)) + defer.reject(new Error(`Failed to connect to the browser session "${sessionId}" [${project.name}] within the timeout.`)) }, project.vitest.config.browser.connectTimeout ?? 60_000).unref() this.sessions.set(sessionId, { - files, - method, project, connected: () => { - clearTimeout(timeout) - }, - resolve: () => { defer.resolve() clearTimeout(timeout) - // this.sessions.delete(sessionId) }, reject: defer.reject, }) diff --git a/packages/vitest/src/node/types/browser.ts b/packages/vitest/src/node/types/browser.ts index 8b3fc22d9668..e29c45c930fc 100644 --- a/packages/vitest/src/node/types/browser.ts +++ b/packages/vitest/src/node/types/browser.ts @@ -236,16 +236,13 @@ export interface BrowserCommandContext { } export interface BrowserServerStateSession { - files: string[] - method: 'run' | 'collect' project: TestProject connected: () => void - resolve: () => void reject: (v: unknown) => void } export interface BrowserOrchestrator { - createTesters: (files: string[]) => Promise + createTesters: (method: 'run' | 'collect', files: string[]) => Promise onCancel: (reason: CancelReason) => Promise $close: () => void } diff --git a/packages/vitest/src/runtime/config.ts b/packages/vitest/src/runtime/config.ts index f9249182edcd..0383f144e237 100644 --- a/packages/vitest/src/runtime/config.ts +++ b/packages/vitest/src/runtime/config.ts @@ -134,9 +134,9 @@ export interface SerializedConfig { standalone: boolean logHeapUsage: boolean | undefined coverage: SerializedCoverageConfig - benchmark?: { + benchmark: { includeSamples: boolean - } + } | undefined } export interface SerializedCoverageConfig { From 8253d5d1ce8d582351f7efc05cc4afec49542127 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Thu, 13 Mar 2025 16:06:36 +0100 Subject: [PATCH 04/24] chore: cleanup --- packages/browser/src/client/tester/runner.ts | 4 +- packages/browser/src/node/pool.ts | 62 ++++++++++++++------ 2 files changed, 46 insertions(+), 20 deletions(-) diff --git a/packages/browser/src/client/tester/runner.ts b/packages/browser/src/client/tester/runner.ts index 50ac5636e15f..ef9376accaf0 100644 --- a/packages/browser/src/client/tester/runner.ts +++ b/packages/browser/src/client/tester/runner.ts @@ -59,7 +59,9 @@ export function createBrowserRunner( onTaskFinished = async (task: Task) => { if (this.config.browser.screenshotFailures && document.body.clientHeight > 0 && task.result?.state === 'fail') { - const screenshot = await page.screenshot().catch((err) => { + const screenshot = await page.screenshot({ + timeout: this.config.browser.providerOptions?.actionTimeout ?? 5_000, + }).catch((err) => { console.error('[vitest] Failed to take a screenshot', err) }) if (screenshot) { diff --git a/packages/browser/src/node/pool.ts b/packages/browser/src/node/pool.ts index 307c1c99f323..9c72b1d842c3 100644 --- a/packages/browser/src/node/pool.ts +++ b/packages/browser/src/node/pool.ts @@ -88,6 +88,9 @@ export function createBrowserPool(vitest: Vitest): ProcessPool { worker: sessionId => waitForOrchestrator(sessionId, project), }) projectPools.set(project, pool) + vitest.onCancel(() => { + pool.cancel() + }) return pool } @@ -105,20 +108,26 @@ export function createBrowserPool(vitest: Vitest): ProcessPool { isCancelled = true }) - // TODO: parallelize tests instead of running them sequentially (based on CPU?) - for (const [project, files] of groupedFiles.entries()) { - if (isCancelled) { - break - } - await project._initBrowserProvider() + // TODO: this might now be a good idea... should we run these in chunks? + await Promise.all( + [...groupedFiles.entries()].map(async ([project, files]) => { + if (isCancelled) { + return + } + await project._initBrowserProvider() - if (!project.browser) { - throw new TypeError(`The browser server was not initialized${project.name ? ` for the "${project.name}" project` : ''}. This is a bug in Vitest. Please, open a new issue with reproduction.`) - } + if (!project.browser) { + throw new TypeError(`The browser server was not initialized${project.name ? ` for the "${project.name}" project` : ''}. This is a bug in Vitest. Please, open a new issue with reproduction.`) + } - const pool = ensurePool(project) - await executeTests(method, pool, project, files) - } + if (isCancelled) { + return + } + + const pool = ensurePool(project) + await executeTests(method, pool, project, files) + }), + ) } function getThreadsCount(project: TestProject) { @@ -173,6 +182,10 @@ class BrowserPool { }, ) {} + cancel() { + this._queue = [] + } + get orchestrators() { return this.project.browser!.state.orchestrators } @@ -189,16 +202,25 @@ class BrowserPool { this._queue.push(...files) this.readySessions.forEach((sessionId) => { - this.readySessions.delete(sessionId) - this.runNextTest(method, sessionId) + if (this._queue.length) { + this.readySessions.delete(sessionId) + this.runNextTest(method, sessionId) + } }) if (this.orchestrators.size >= this.options.maxWorkers) { return this._promise } + // open the minimum amount of tabs + // if there is only 1 file running, we don't need 8 tabs running + const workerCount = Math.min( + this.options.maxWorkers - this.orchestrators.size, + files.length, + ) + const promises: Promise[] = [] - for (let i = this.orchestrators.size; i < this.options.maxWorkers; i++) { + for (let i = 0; i < workerCount; i++) { const sessionId = crypto.randomUUID() const page = this.openPage(sessionId).then(() => { // start running tests on the page when it's ready @@ -221,9 +243,12 @@ class BrowserPool { private runNextTest(method: 'run' | 'collect', sessionId: string) { const file = this._queue.shift() if (!file) { - this._promise?.resolve() - this._promise = undefined this.readySessions.add(sessionId) + // the last worker finished running tests + if (this.readySessions.size === this.orchestrators.size) { + this._promise?.resolve() + this._promise = undefined + } return } if (!this._promise) { @@ -246,10 +271,9 @@ class BrowserPool { if (error instanceof Error && error.message.startsWith('[birpc] rpc is closed')) { return } - return Promise.reject(error) + this._promise?.reject(error) }) } - // TODO: onCancel // TODO: isolate: false } From 8c64611681f2d27edbcc002e845be38583ddde6d Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Thu, 13 Mar 2025 16:13:16 +0100 Subject: [PATCH 05/24] chore: restrict isolated parallelism for now --- packages/browser/src/client/orchestrator.ts | 29 ++++++++++++--------- packages/browser/src/node/pool.ts | 22 +++++++++++++--- 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/packages/browser/src/client/orchestrator.ts b/packages/browser/src/client/orchestrator.ts index 46c2004879d0..afde616fcb0e 100644 --- a/packages/browser/src/client/orchestrator.ts +++ b/packages/browser/src/client/orchestrator.ts @@ -53,6 +53,7 @@ class IframeOrchestrator { const iframe = this.createIframe(container, method, ID_ALL, testFiles) await setIframeViewport(iframe, width, height) + await this.waitForIframeDoneEvent() return } @@ -66,18 +67,7 @@ class IframeOrchestrator { await setIframeViewport(iframe, width, height) - await new Promise((resolve) => { - channel.addEventListener( - 'message', - function handler(e: MessageEvent) { - // done and error can only be triggered by the previous iframe - if (e.data.type === 'done' || e.data.type === 'error') { - channel.removeEventListener('message', handler) - resolve() - } - }, - ) - }) + await this.waitForIframeDoneEvent() } } @@ -127,6 +117,21 @@ class IframeOrchestrator { return iframe } + private waitForIframeDoneEvent() { + return new Promise((resolve) => { + channel.addEventListener( + 'message', + function handler(e: MessageEvent) { + // done and error can only be triggered by the previous iframe + if (e.data.type === 'done' || e.data.type === 'error') { + channel.removeEventListener('message', handler) + resolve() + } + }, + ) + }) + } + private async onGlobalChannelEvent(e: MessageEvent) { debug('global channel event', JSON.stringify(e.data)) switch (e.data.type) { diff --git a/packages/browser/src/node/pool.ts b/packages/browser/src/node/pool.ts index 9c72b1d842c3..bc7bdd60d083 100644 --- a/packages/browser/src/node/pool.ts +++ b/packages/browser/src/node/pool.ts @@ -136,7 +136,8 @@ export function createBrowserPool(vitest: Vitest): ProcessPool { return 1 } - if (!config.fileParallelism) { + // FIXME: allow isolate with parallelism + if (!config.fileParallelism || project.config.isolate === false) { return 1 } @@ -240,9 +241,22 @@ class BrowserPool { await Promise.all([promise, page]) } - private runNextTest(method: 'run' | 'collect', sessionId: string) { + private getNextFiles() { + if (this.project.config.browser.isolate) { + const files = [...this._queue] + this._queue = [] + return files + } const file = this._queue.shift() - if (!file) { + if (file) { + return [file] + } + return [] + } + + private runNextTest(method: 'run' | 'collect', sessionId: string) { + const files = this.getNextFiles() + if (!files.length) { this.readySessions.add(sessionId) // the last worker finished running tests if (this.readySessions.size === this.orchestrators.size) { @@ -263,7 +277,7 @@ class BrowserPool { return } - orchestrator.createTesters(method, [file]) + orchestrator.createTesters(method, files) .then(() => { this.runNextTest(method, sessionId) }) From 18fe4458da92ffba96bc8dc4e7df3a648449a595 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Thu, 13 Mar 2025 17:26:53 +0100 Subject: [PATCH 06/24] chore: cleanup --- packages/browser/src/client/orchestrator.ts | 43 ++++++++++++++------ packages/browser/src/client/tester/runner.ts | 6 +-- packages/browser/src/client/tester/tester.ts | 10 ++--- packages/browser/src/client/types.ts | 7 ++++ packages/browser/src/node/pool.ts | 3 +- 5 files changed, 46 insertions(+), 23 deletions(-) create mode 100644 packages/browser/src/client/types.ts diff --git a/packages/browser/src/client/orchestrator.ts b/packages/browser/src/client/orchestrator.ts index afde616fcb0e..8ebd30224675 100644 --- a/packages/browser/src/client/orchestrator.ts +++ b/packages/browser/src/client/orchestrator.ts @@ -1,5 +1,6 @@ import type { GlobalChannelIncomingEvent, IframeChannelEvent, IframeChannelIncomingEvent } from '@vitest/browser/client' import type { SerializedConfig } from 'vitest' +import type { IframeInitEvent } from './types' import { channel, client, globalChannel } from '@vitest/browser/client' import { generateHash } from '@vitest/runner/utils' import { relative } from 'pathe' @@ -43,17 +44,18 @@ class IframeOrchestrator { container.parentElement!.setAttribute('data-ready', 'true') container.textContent = '' } - const { width, height } = config.browser.viewport this.iframes.forEach(iframe => iframe.remove()) this.iframes.clear() if (config.browser.isolate === false) { debug('create iframe', ID_ALL) - const iframe = this.createIframe(container, method, ID_ALL, testFiles) - - await setIframeViewport(iframe, width, height) - await this.waitForIframeDoneEvent() + await this.runTestsInIframe( + container, + method, + ID_ALL, + testFiles, + ) return } @@ -63,14 +65,31 @@ class IframeOrchestrator { } debug('create iframe', file) - const iframe = this.createIframe(container, method, file, [file]) - await setIframeViewport(iframe, width, height) - - await this.waitForIframeDoneEvent() + await this.runTestsInIframe( + container, + method, + file, + [file], + ) } } + private async runTestsInIframe( + container: HTMLDivElement, + method: 'run' | 'collect', + id: string, + files: string[], + ) { + const config = getConfig() + const { width, height } = config.browser.viewport + + const iframe = this.createIframe(container, method, id, files) + + await setIframeViewport(iframe, width, height) + await this.waitForIframeDoneEvent() + } + private createIframe(container: HTMLDivElement, method: 'run' | 'collect', iframeId: string, files: string[]) { if (this.iframes.has(iframeId)) { this.iframes.get(iframeId)!.remove() @@ -100,7 +119,7 @@ class IframeOrchestrator { files, iframeId, context: getBrowserState().providedContext, - }), + } satisfies IframeInitEvent), '*', ) } @@ -233,9 +252,9 @@ async function getContainer(config: SerializedConfig): Promise { const element = document.querySelector('#tester-ui') if (!element) { return new Promise((resolve) => { - setTimeout(() => { + queueMicrotask(() => { resolve(getContainer(config)) - }, 30) + }) }) } return element as HTMLDivElement diff --git a/packages/browser/src/client/tester/runner.ts b/packages/browser/src/client/tester/runner.ts index ef9376accaf0..2440b6afb8b3 100644 --- a/packages/browser/src/client/tester/runner.ts +++ b/packages/browser/src/client/tester/runner.ts @@ -123,7 +123,7 @@ export function createBrowserRunner( if (this.config.includeTaskLocation) { try { - await updateFilesLocations(files, this.sourceMapCache) + await updateTestFilesLocations(files, this.sourceMapCache) } catch {} } @@ -173,13 +173,13 @@ export async function initiateRunner( const runner = new BrowserRunner({ config, }) + cachedRunner = runner const [diffOptions] = await Promise.all([ loadDiffConfig(config, executor as unknown as VitestExecutor), loadSnapshotSerializers(config, executor as unknown as VitestExecutor), ]) runner.config.diffOptions = diffOptions - cachedRunner = runner getWorkerState().onFilterStackTrace = (stack: string) => { const stacks = parseStacktrace(stack, { getSourceMap(file) { @@ -191,7 +191,7 @@ export async function initiateRunner( return runner } -async function updateFilesLocations(files: File[], sourceMaps: Map) { +async function updateTestFilesLocations(files: File[], sourceMaps: Map) { const promises = files.map(async (file) => { const result = sourceMaps.get(file.filepath) || await rpc().getBrowserFileSourceMap(file.filepath) if (!result) { diff --git a/packages/browser/src/client/tester/tester.ts b/packages/browser/src/client/tester/tester.ts index 675782493b15..3eade85bcf22 100644 --- a/packages/browser/src/client/tester/tester.ts +++ b/packages/browser/src/client/tester/tester.ts @@ -1,3 +1,4 @@ +import type { IframeInitEvent } from '../types' import { channel, client, onCancel } from '@vitest/browser/client' import { page, server, userEvent } from '@vitest/browser/context' import { parse } from 'flatted' @@ -209,13 +210,8 @@ window.addEventListener('message', (e) => { const data = JSON.parse(e.data) debug('event', e.data) - if (data.event === 'init') { - const { method, files, context, iframeId } = data as { - method: 'run' | 'collect' - files: string[] - context: string - iframeId: string - } + if (typeof data === 'object' && data?.event === 'init') { + const { method, files, context, iframeId } = data as IframeInitEvent const state = getWorkerState() const parsedContext = parse(context) diff --git a/packages/browser/src/client/types.ts b/packages/browser/src/client/types.ts new file mode 100644 index 000000000000..dfe4b50182a8 --- /dev/null +++ b/packages/browser/src/client/types.ts @@ -0,0 +1,7 @@ +export interface IframeInitEvent { + event: 'init' + method: 'run' | 'collect' + files: string[] + iframeId: string + context: string +} diff --git a/packages/browser/src/node/pool.ts b/packages/browser/src/node/pool.ts index bc7bdd60d083..ec7d9239616d 100644 --- a/packages/browser/src/node/pool.ts +++ b/packages/browser/src/node/pool.ts @@ -242,7 +242,8 @@ class BrowserPool { } private getNextFiles() { - if (this.project.config.browser.isolate) { + // TODO: special handling for isolate: false -- how is it supposed to work with parallelisation? + if (this.project.config.browser.isolate === false) { const files = [...this._queue] this._queue = [] return files From 6a2ae8d32d0b43a47c1320763d63858b9eea6532 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Thu, 13 Mar 2025 18:25:12 +0100 Subject: [PATCH 07/24] chore: cleanup --- packages/browser/src/client/orchestrator.ts | 104 +++++++++++------- packages/browser/src/client/tester/runner.ts | 6 +- packages/browser/src/client/tester/tester.ts | 108 +++++++++---------- packages/browser/src/node/pool.ts | 24 +---- 4 files changed, 123 insertions(+), 119 deletions(-) diff --git a/packages/browser/src/client/orchestrator.ts b/packages/browser/src/client/orchestrator.ts index 8ebd30224675..cd8d9a39449d 100644 --- a/packages/browser/src/client/orchestrator.ts +++ b/packages/browser/src/client/orchestrator.ts @@ -48,16 +48,7 @@ class IframeOrchestrator { this.iframes.forEach(iframe => iframe.remove()) this.iframes.clear() - if (config.browser.isolate === false) { - debug('create iframe', ID_ALL) - await this.runTestsInIframe( - container, - method, - ID_ALL, - testFiles, - ) - return - } + const isolate = config.browser.isolate for (const file of testFiles) { if (this.cancelled) { @@ -66,63 +57,63 @@ class IframeOrchestrator { debug('create iframe', file) - await this.runTestsInIframe( + await this.runTestInIframe( container, method, + isolate === false ? ID_ALL : file, file, - [file], ) } } - private async runTestsInIframe( + private async runTestInIframe( container: HTMLDivElement, method: 'run' | 'collect', id: string, - files: string[], + file: string, ) { const config = getConfig() const { width, height } = config.browser.viewport - const iframe = this.createIframe(container, method, id, files) + const iframe = config.browser.isolate === false + ? this.startInIsolatedIframe(container, method, file) + : this.startInNewIframe(container, method, id, file) await setIframeViewport(iframe, width, height) await this.waitForIframeDoneEvent() } - private createIframe(container: HTMLDivElement, method: 'run' | 'collect', iframeId: string, files: string[]) { - if (this.iframes.has(iframeId)) { - this.iframes.get(iframeId)!.remove() - this.iframes.delete(iframeId) + private startIframeTest( + iframe: HTMLIFrameElement, + method: 'run' | 'collect', + iframeId: string, + file: string, + ) { + const iframeWindow = iframe.contentWindow + if (!iframeWindow) { + debug('no window available') + // TODO: what happened here? + return } + iframeWindow.postMessage( + JSON.stringify({ + event: 'init', + method, + files: [file], + iframeId, + context: getBrowserState().providedContext, + } satisfies IframeInitEvent), + '*', + ) + } + + private createTestIframe() { const iframe = document.createElement('iframe') const src = `${url.pathname}__vitest_test__/__test__/?sessionId=${getBrowserState().sessionId}` iframe.setAttribute('loading', 'eager') iframe.setAttribute('src', src) iframe.setAttribute('data-vitest', 'true') - iframe.onerror = (e) => { - debug('iframe error', e.toString()) - } - iframe.onload = () => { - debug(`iframe for ${src} loaded`) - const iframeWindow = iframe.contentWindow - if (!iframeWindow) { - debug('no window available') - // TODO: what happened here? - return - } - iframeWindow.postMessage( - JSON.stringify({ - event: 'init', - method, - files, - iframeId, - context: getBrowserState().providedContext, - } satisfies IframeInitEvent), - '*', - ) - } iframe.style.border = 'none' iframe.style.width = '100%' @@ -130,6 +121,37 @@ class IframeOrchestrator { iframe.setAttribute('allowfullscreen', 'true') iframe.setAttribute('allow', 'clipboard-write;') iframe.setAttribute('name', 'vitest-iframe') + return iframe + } + + // TODO: a lot of tests on how this actually works + private startInIsolatedIframe( + container: HTMLDivElement, + method: 'run' | 'collect', + file: string, + ) { + const cachedIframe = this.iframes.get(ID_ALL) + if (cachedIframe) { + this.startIframeTest(cachedIframe, method, ID_ALL, file) + return cachedIframe + } + return this.startInNewIframe(container, method, ID_ALL, file) + } + + private startInNewIframe(container: HTMLDivElement, method: 'run' | 'collect', iframeId: string, file: string) { + if (this.iframes.has(iframeId)) { + this.iframes.get(iframeId)!.remove() + this.iframes.delete(iframeId) + } + + const iframe = this.createTestIframe() + iframe.onerror = (e) => { + debug('iframe error', e.toString()) + } + iframe.onload = () => { + debug(`iframe for ${file} loaded`) + this.startIframeTest(iframe, method, iframeId, file) + } this.iframes.set(iframeId, iframe) container.appendChild(iframe) diff --git a/packages/browser/src/client/tester/runner.ts b/packages/browser/src/client/tester/runner.ts index 2440b6afb8b3..ad27ed738b2a 100644 --- a/packages/browser/src/client/tester/runner.ts +++ b/packages/browser/src/client/tester/runner.ts @@ -2,7 +2,7 @@ import type { CancelReason, File, Suite, Task, TaskEventPack, TaskResultPack, Vi import type { SerializedConfig, WorkerGlobalState } from 'vitest' import type { VitestExecutor } from 'vitest/execute' import type { VitestBrowserClientMocker } from './mocker' -import { globalChannel } from '@vitest/browser/client' +import { globalChannel, onCancel } from '@vitest/browser/client' import { page, userEvent } from '@vitest/browser/context' import { loadDiffConfig, loadSnapshotSerializers, takeCoverageInsideWorker } from 'vitest/browser' import { NodeBenchmarkRunner, VitestTestRunner } from 'vitest/runners' @@ -175,6 +175,10 @@ export async function initiateRunner( }) cachedRunner = runner + onCancel.then((reason) => { + runner.onCancel?.(reason) + }) + const [diffOptions] = await Promise.all([ loadDiffConfig(config, executor as unknown as VitestExecutor), loadSnapshotSerializers(config, executor as unknown as VitestExecutor), diff --git a/packages/browser/src/client/tester/tester.ts b/packages/browser/src/client/tester/tester.ts index 3eade85bcf22..4a1f7dbba3e4 100644 --- a/packages/browser/src/client/tester/tester.ts +++ b/packages/browser/src/client/tester/tester.ts @@ -1,3 +1,4 @@ +import type { BrowserRPC } from '@vitest/browser/client' import type { IframeInitEvent } from '../types' import { channel, client, onCancel } from '@vitest/browser/client' import { page, server, userEvent } from '@vitest/browser/context' @@ -18,11 +19,30 @@ const cleanupSymbol = Symbol.for('vitest:component-cleanup') const url = new URL(location.href) const reloadStart = url.searchParams.get('__reloadStart') -function debug(...args: unknown[]) { - const debug = getConfig().env.VITEST_BROWSER_DEBUG - if (debug && debug !== 'false') { - client.rpc.debug(...args.map(String)) - } +const commands = new CommandsManager() +getBrowserState().commands = commands + +let contextSwitched = false + +// webdiverio context depends on the iframe state, so we need to switch the context, +// we delay this in case the user doesn't use any userEvent commands to avoid the overhead +if (server.provider === 'webdriverio') { + let switchPromise: Promise | null = null + + commands.onCommand(async () => { + if (switchPromise) { + await switchPromise + } + // if this is the first command, make sure we switched the command context to an iframe + if (!contextSwitched) { + const rpc = getWorkerState().rpc as any as BrowserRPC + switchPromise = rpc.wdioSwitchContext('iframe').finally(() => { + switchPromise = null + contextSwitched = true + }) + await switchPromise + } + }) } async function prepareTestEnvironment(files: string[]) { @@ -37,8 +57,6 @@ async function prepareTestEnvironment(files: string[]) { state.onCancel = onCancel state.rpc = rpc as any - getBrowserState().commands = new CommandsManager() - // TODO: expose `worker` const interceptor = createModuleMockerInterceptor() const mocker = new VitestBrowserClientMocker( @@ -66,16 +84,11 @@ async function prepareTestEnvironment(files: string[]) { } }) - onCancel.then((reason) => { - runner.onCancel?.(reason) - }) - return { runner, config, state, rpc, - commands: getBrowserState().commands, } } @@ -97,7 +110,6 @@ async function executeTests(method: 'run' | 'collect', files: string[]) { | undefined | false - // if importing /@id/ failed, we reload the page waiting until Vite prebundles it try { preparedData = await prepareTestEnvironment(files) } @@ -120,34 +132,12 @@ async function executeTests(method: 'run' | 'collect', files: string[]) { debug('runner resolved successfully') - const { config, runner, state, commands, rpc } = preparedData + const { config, runner, state, rpc } = preparedData state.durations.prepare = performance.now() - state.durations.prepare debug('prepare time', state.durations.prepare, 'ms') - let contextSwitched = false - - // webdiverio context depends on the iframe state, so we need to switch the context, - // we delay this in case the user doesn't use any userEvent commands to avoid the overhead - if (server.provider === 'webdriverio') { - let switchPromise: Promise | null = null - - commands.onCommand(async () => { - if (switchPromise) { - await switchPromise - } - // if this is the first command, make sure we switched the command context to an iframe - if (!contextSwitched) { - switchPromise = rpc.wdioSwitchContext('iframe').finally(() => { - switchPromise = null - contextSwitched = true - }) - await switchPromise - } - }) - } - try { await Promise.all([ setupCommonEnv(config), @@ -173,31 +163,28 @@ async function executeTests(method: 'run' | 'collect', files: string[]) { } } finally { - try { - if (cleanupSymbol in page) { - (page[cleanupSymbol] as any)() + if (cleanupSymbol in page) { + try { + await (page[cleanupSymbol] as any)() } - // need to cleanup for each tester - // since playwright keyboard API is stateful on page instance level - await userEvent.cleanup() - if (contextSwitched) { - await rpc.wdioSwitchContext('parent') + catch (error: any) { + await unhandledError(error, 'Cleanup Error') } } - catch (error: any) { - await client.rpc.onUnhandledError({ - name: error.name, - message: error.message, - stack: String(error.stack), - }, 'Cleanup Error') + // need to cleanup for each tester + // since playwright keyboard API is stateful on page instance level + await userEvent.cleanup() + .catch(error => unhandledError(error, 'Cleanup Error')) + + // if isolation is disabled, Vitest reuses the same iframe and we + // don't need to switch the context back at all + if (server.config.browser.isolate !== false && contextSwitched) { + await rpc.wdioSwitchContext('parent') + .catch(error => unhandledError(error, 'Cleanup Error')) } state.environmentTeardownRun = true await stopCoverageInsideWorker(config.coverage, executor, { isolate: config.browser.isolate }).catch((error) => { - client.rpc.onUnhandledError({ - name: error.name, - message: error.message, - stack: String(error.stack), - }, 'Coverage Error').catch(() => {}) + return unhandledError(error, 'Coverage Error') }) debug('finished running tests') @@ -208,7 +195,7 @@ async function executeTests(method: 'run' | 'collect', files: string[]) { // listen when orchestrator sends a message window.addEventListener('message', (e) => { const data = JSON.parse(e.data) - debug('event', e.data) + debug('event from orchestrator', e.data) if (typeof data === 'object' && data?.event === 'init') { const { method, files, context, iframeId } = data as IframeInitEvent @@ -233,9 +220,16 @@ window.addEventListener('message', (e) => { }) function unhandledError(e: Error, type: string) { - client.rpc.onUnhandledError({ + return client.rpc.onUnhandledError({ name: e.name, message: e.message, stack: e.stack, }, type).catch(() => {}) } + +function debug(...args: unknown[]) { + const debug = getConfig().env.VITEST_BROWSER_DEBUG + if (debug && debug !== 'false') { + client.rpc.debug(...args.map(String)) + } +} diff --git a/packages/browser/src/node/pool.ts b/packages/browser/src/node/pool.ts index ec7d9239616d..eeae3ad81631 100644 --- a/packages/browser/src/node/pool.ts +++ b/packages/browser/src/node/pool.ts @@ -111,9 +111,6 @@ export function createBrowserPool(vitest: Vitest): ProcessPool { // TODO: this might now be a good idea... should we run these in chunks? await Promise.all( [...groupedFiles.entries()].map(async ([project, files]) => { - if (isCancelled) { - return - } await project._initBrowserProvider() if (!project.browser) { @@ -241,23 +238,9 @@ class BrowserPool { await Promise.all([promise, page]) } - private getNextFiles() { - // TODO: special handling for isolate: false -- how is it supposed to work with parallelisation? - if (this.project.config.browser.isolate === false) { - const files = [...this._queue] - this._queue = [] - return files - } - const file = this._queue.shift() - if (file) { - return [file] - } - return [] - } - private runNextTest(method: 'run' | 'collect', sessionId: string) { - const files = this.getNextFiles() - if (!files.length) { + const file = this._queue.shift() + if (!file) { this.readySessions.add(sessionId) // the last worker finished running tests if (this.readySessions.size === this.orchestrators.size) { @@ -278,7 +261,8 @@ class BrowserPool { return } - orchestrator.createTesters(method, files) + // TODO: createTesters has a 60s timeout, it should wait indefinetly, especially for isolate: false + orchestrator.createTesters(method, [file]) .then(() => { this.runNextTest(method, sessionId) }) From 3e4fe4b80a0692ac83f29cec7649e43b0f8d5e59 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Fri, 14 Mar 2025 12:47:24 +0100 Subject: [PATCH 08/24] fix: support dynamic context --- packages/browser/src/client/client.ts | 6 +-- packages/browser/src/client/orchestrator.ts | 43 +++++++++++--------- packages/browser/src/client/tester/logger.ts | 4 +- packages/browser/src/client/tester/runner.ts | 29 +++++++++---- packages/browser/src/client/tester/tester.ts | 14 +++++-- packages/browser/src/client/utils.ts | 4 +- packages/browser/src/node/pool.ts | 9 +++- packages/browser/src/node/projectParent.ts | 3 +- packages/browser/src/node/rpc.ts | 25 ++++-------- packages/browser/src/node/types.ts | 21 +++++++--- packages/vitest/src/node/types/browser.ts | 3 +- packages/vitest/src/public/index.ts | 2 + packages/vitest/src/public/node.ts | 5 ++- packages/vitest/src/types/browser.ts | 10 +++++ packages/vitest/src/types/worker.ts | 2 + 15 files changed, 115 insertions(+), 65 deletions(-) create mode 100644 packages/vitest/src/types/browser.ts diff --git a/packages/browser/src/client/client.ts b/packages/browser/src/client/client.ts index c7ea789ff047..d27668c1aff4 100644 --- a/packages/browser/src/client/client.ts +++ b/packages/browser/src/client/client.ts @@ -52,11 +52,11 @@ function createClient() { ctx.rpc = createBirpc( { onCancel: setCancel, - async createTesters(method: 'run' | 'collect', files: string[]) { + async createTesters(options) { if (PAGE_TYPE !== 'orchestrator') { - return + throw new TypeError('Only orchestrator can create testers.') } - return getBrowserState().createTesters?.(method, files) + return getBrowserState().createTesters?.(options) }, cdpEvent(event: string, payload: unknown) { const cdp = getBrowserState().cdp diff --git a/packages/browser/src/client/orchestrator.ts b/packages/browser/src/client/orchestrator.ts index cd8d9a39449d..4af330710e5d 100644 --- a/packages/browser/src/client/orchestrator.ts +++ b/packages/browser/src/client/orchestrator.ts @@ -1,5 +1,5 @@ import type { GlobalChannelIncomingEvent, IframeChannelEvent, IframeChannelIncomingEvent } from '@vitest/browser/client' -import type { SerializedConfig } from 'vitest' +import type { BrowserTesterOptions, SerializedConfig } from 'vitest' import type { IframeInitEvent } from './types' import { channel, client, globalChannel } from '@vitest/browser/client' import { generateHash } from '@vitest/runner/utils' @@ -30,13 +30,13 @@ class IframeOrchestrator { ) } - public async createTesters(method: 'run' | 'collect', testFiles: string[]) { + public async createTesters(options: BrowserTesterOptions) { this.cancelled = false this.runningFiles.clear() - testFiles.forEach(file => this.runningFiles.add(file)) + options.files.forEach(file => this.runningFiles.add(file)) const config = getConfig() - debug('create testers', testFiles.join(', ')) + debug('create testers', options.files.join(', ')) const container = await getContainer(config) if (config.browser.ui) { @@ -50,7 +50,7 @@ class IframeOrchestrator { const isolate = config.browser.isolate - for (const file of testFiles) { + for (const file of options.files) { if (this.cancelled) { return } @@ -59,25 +59,25 @@ class IframeOrchestrator { await this.runTestInIframe( container, - method, isolate === false ? ID_ALL : file, file, + options, ) } } private async runTestInIframe( container: HTMLDivElement, - method: 'run' | 'collect', id: string, file: string, + options: BrowserTesterOptions, ) { const config = getConfig() const { width, height } = config.browser.viewport const iframe = config.browser.isolate === false - ? this.startInIsolatedIframe(container, method, file) - : this.startInNewIframe(container, method, id, file) + ? this.startInIsolatedIframe(container, file, options) + : this.startInNewIframe(container, id, file, options) await setIframeViewport(iframe, width, height) await this.waitForIframeDoneEvent() @@ -85,9 +85,9 @@ class IframeOrchestrator { private startIframeTest( iframe: HTMLIFrameElement, - method: 'run' | 'collect', iframeId: string, file: string, + options: BrowserTesterOptions, ) { const iframeWindow = iframe.contentWindow if (!iframeWindow) { @@ -99,10 +99,10 @@ class IframeOrchestrator { iframeWindow.postMessage( JSON.stringify({ event: 'init', - method, + method: options.method, files: [file], iframeId, - context: getBrowserState().providedContext, + context: options.providedContext, } satisfies IframeInitEvent), '*', ) @@ -127,18 +127,23 @@ class IframeOrchestrator { // TODO: a lot of tests on how this actually works private startInIsolatedIframe( container: HTMLDivElement, - method: 'run' | 'collect', file: string, + options: BrowserTesterOptions, ) { const cachedIframe = this.iframes.get(ID_ALL) if (cachedIframe) { - this.startIframeTest(cachedIframe, method, ID_ALL, file) + this.startIframeTest(cachedIframe, ID_ALL, file, options) return cachedIframe } - return this.startInNewIframe(container, method, ID_ALL, file) + return this.startInNewIframe(container, ID_ALL, file, options) } - private startInNewIframe(container: HTMLDivElement, method: 'run' | 'collect', iframeId: string, file: string) { + private startInNewIframe( + container: HTMLDivElement, + iframeId: string, + file: string, + options: BrowserTesterOptions, + ) { if (this.iframes.has(iframeId)) { this.iframes.get(iframeId)!.remove() this.iframes.delete(iframeId) @@ -150,7 +155,7 @@ class IframeOrchestrator { } iframe.onload = () => { debug(`iframe for ${file} loaded`) - this.startIframeTest(iframe, method, iframeId, file) + this.startIframeTest(iframe, iframeId, file, options) } this.iframes.set(iframeId, iframe) @@ -261,9 +266,9 @@ class IframeOrchestrator { const orchestrator = new IframeOrchestrator() let promiseTesters: Promise | undefined -getBrowserState().createTesters = async (method, files) => { +getBrowserState().createTesters = async (options: BrowserTesterOptions) => { await promiseTesters - promiseTesters = orchestrator.createTesters(method, files).finally(() => { + promiseTesters = orchestrator.createTesters(options).finally(() => { promiseTesters = undefined }) await promiseTesters diff --git a/packages/browser/src/client/tester/logger.ts b/packages/browser/src/client/tester/logger.ts index c9fb2428c7c5..51d7602228e9 100644 --- a/packages/browser/src/client/tester/logger.ts +++ b/packages/browser/src/client/tester/logger.ts @@ -1,6 +1,7 @@ import { format, stringify } from 'vitest/utils' import { getConfig } from '../utils' import { rpc } from './rpc' +import { getBrowserRunner } from './runner' const { Date, console, performance } = globalThis @@ -141,7 +142,8 @@ function sendLog( = getConfig().printConsoleTrace && !disableStack ? new Error('STACK_TRACE').stack?.split('\n').slice(1).join('\n') : undefined - rpc().sendLog({ + const runner = getBrowserRunner() + rpc().sendLog(runner?.method || 'run', { origin, content, browser: true, diff --git a/packages/browser/src/client/tester/runner.ts b/packages/browser/src/client/tester/runner.ts index ad27ed738b2a..58a043505c9a 100644 --- a/packages/browser/src/client/tester/runner.ts +++ b/packages/browser/src/client/tester/runner.ts @@ -1,5 +1,5 @@ import type { CancelReason, File, Suite, Task, TaskEventPack, TaskResultPack, VitestRunner } from '@vitest/runner' -import type { SerializedConfig, WorkerGlobalState } from 'vitest' +import type { SerializedConfig, TestExecutionType, WorkerGlobalState } from 'vitest' import type { VitestExecutor } from 'vitest/execute' import type { VitestBrowserClientMocker } from './mocker' import { globalChannel, onCancel } from '@vitest/browser/client' @@ -22,22 +22,33 @@ interface CoverageHandler { takeCoverage: () => Promise } +interface BrowserVitestRunner extends VitestRunner { + sourceMapCache: Map + method: TestExecutionType + setMethod: (method: TestExecutionType) => void +} + export function createBrowserRunner( runnerClass: { new (config: SerializedConfig): VitestRunner }, mocker: VitestBrowserClientMocker, state: WorkerGlobalState, coverageModule: CoverageHandler | null, -): { new (options: BrowserRunnerOptions): VitestRunner & { sourceMapCache: Map } } { +): { new (options: BrowserRunnerOptions): BrowserVitestRunner } { return class BrowserTestRunner extends runnerClass implements VitestRunner { public config: SerializedConfig hashMap = browserHashMap public sourceMapCache = new Map() + public method = 'run' as TestExecutionType constructor(options: BrowserRunnerOptions) { super(options.config) this.config = options.config } + setMethod(method: 'run' | 'collect') { + this.method = method + } + onBeforeTryTask: VitestRunner['onBeforeTryTask'] = async (...args) => { await userEvent.cleanup() await super.onBeforeTryTask?.(...args) @@ -109,7 +120,7 @@ export function createBrowserRunner( } onCollectStart = (file: File) => { - return rpc().onQueued(file) + return rpc().onQueued(this.method, file) } onCollected = async (files: File[]): Promise => { @@ -127,11 +138,11 @@ export function createBrowserRunner( } catch {} } - return rpc().onCollected(files) + return rpc().onCollected(this.method, files) } onTaskUpdate = (task: TaskResultPack[], events: TaskEventPack[]): Promise => { - return rpc().onTaskUpdate(task, events) + return rpc().onTaskUpdate(this.method, task, events) } importFile = async (filepath: string) => { @@ -150,13 +161,17 @@ export function createBrowserRunner( } } -let cachedRunner: VitestRunner | null = null +let cachedRunner: BrowserVitestRunner | null = null + +export function getBrowserRunner(): BrowserVitestRunner | null { + return cachedRunner +} export async function initiateRunner( state: WorkerGlobalState, mocker: VitestBrowserClientMocker, config: SerializedConfig, -): Promise { +): Promise { if (cachedRunner) { return cachedRunner } diff --git a/packages/browser/src/client/tester/tester.ts b/packages/browser/src/client/tester/tester.ts index 4a1f7dbba3e4..997b6ecba39e 100644 --- a/packages/browser/src/client/tester/tester.ts +++ b/packages/browser/src/client/tester/tester.ts @@ -3,7 +3,14 @@ import type { IframeInitEvent } from '../types' import { channel, client, onCancel } from '@vitest/browser/client' import { page, server, userEvent } from '@vitest/browser/context' import { parse } from 'flatted' -import { collectTests, setupCommonEnv, SpyModule, startCoverageInsideWorker, startTests, stopCoverageInsideWorker } from 'vitest/browser' +import { + collectTests, + setupCommonEnv, + SpyModule, + startCoverageInsideWorker, + startTests, + stopCoverageInsideWorker, +} from 'vitest/browser' import { executor, getBrowserState, getConfig, getWorkerState } from '../utils' import { setupDialogsSpy } from './dialog' import { setupExpectDom } from './expect-element' @@ -45,7 +52,7 @@ if (server.provider === 'webdriverio') { }) } -async function prepareTestEnvironment(files: string[]) { +async function prepareTestEnvironment(method: 'run' | 'collect', files: string[]) { // TODO: all of this should be done only once except setMethod debug('trying to resolve runner', `${reloadStart}`) const config = getConfig() @@ -75,6 +82,7 @@ async function prepareTestEnvironment(files: string[]) { setupExpectDom() const runner = await initiateRunner(state, mocker, config) + runner.setMethod(method) const version = url.searchParams.get('browserv') || '' files.forEach((filename) => { @@ -111,7 +119,7 @@ async function executeTests(method: 'run' | 'collect', files: string[]) { | false try { - preparedData = await prepareTestEnvironment(files) + preparedData = await prepareTestEnvironment(method, files) } catch (error: any) { debug('runner cannot be loaded because it threw an error', error.stack || error.message) diff --git a/packages/browser/src/client/utils.ts b/packages/browser/src/client/utils.ts index 4d83ff5b977d..1a1317f9e6d5 100644 --- a/packages/browser/src/client/utils.ts +++ b/packages/browser/src/client/utils.ts @@ -1,4 +1,4 @@ -import type { SerializedConfig, WorkerGlobalState } from 'vitest' +import type { BrowserTesterOptions, SerializedConfig, WorkerGlobalState } from 'vitest' import type { CommandsManager } from './tester/utils' export async function importId(id: string): Promise { @@ -76,7 +76,7 @@ export interface BrowserRunnerState { sessionId: string testerId: string method: 'run' | 'collect' - createTesters?: (method: 'run' | 'collect', files: string[]) => Promise + createTesters?: (options: BrowserTesterOptions) => Promise commands: CommandsManager cdp?: { on: (event: string, listener: (payload: any) => void) => void diff --git a/packages/browser/src/node/pool.ts b/packages/browser/src/node/pool.ts index eeae3ad81631..2d1db6ce05ac 100644 --- a/packages/browser/src/node/pool.ts +++ b/packages/browser/src/node/pool.ts @@ -9,6 +9,7 @@ import type { import crypto from 'node:crypto' import * as nodeos from 'node:os' import { createDefer } from '@vitest/utils' +import { stringify } from 'flatted' // import { relative } from 'pathe' // import { createDebugger } from 'vitest/node' @@ -262,7 +263,13 @@ class BrowserPool { } // TODO: createTesters has a 60s timeout, it should wait indefinetly, especially for isolate: false - orchestrator.createTesters(method, [file]) + orchestrator.createTesters( + { + method, + files: [file], + providedContext: stringify(this.project.getProvidedContext()), + }, + ) .then(() => { this.runNextTest(method, sessionId) }) diff --git a/packages/browser/src/node/projectParent.ts b/packages/browser/src/node/projectParent.ts index a3e1986d695e..a8b5a6f547be 100644 --- a/packages/browser/src/node/projectParent.ts +++ b/packages/browser/src/node/projectParent.ts @@ -196,7 +196,7 @@ export class ParentBrowserProject { throw new Error(`CDP is not supported by the provider "${provider.name}".`) } - const promise = this.cdpSessionsPromises.get(rpcId) ?? await (async () => { + const session = await this.cdpSessionsPromises.get(rpcId) ?? await (async () => { const promise = provider.getCDPSession!(sessionId).finally(() => { this.cdpSessionsPromises.delete(rpcId) }) @@ -204,7 +204,6 @@ export class ParentBrowserProject { return promise })() - const session = await promise const rpc = (browser.state as BrowserServerState).testers.get(rpcId) if (!rpc) { throw new Error(`Tester RPC "${rpcId}" was not established.`) diff --git a/packages/browser/src/node/rpc.ts b/packages/browser/src/node/rpc.ts index 697accb67e3d..dc7bd821b4df 100644 --- a/packages/browser/src/node/rpc.ts +++ b/packages/browser/src/node/rpc.ts @@ -56,13 +56,6 @@ export function setupBrowserRpc(globalServer: ParentBrowserProject): void { ) } - const method = searchParams.get('method') as 'run' | 'collect' - // if (method !== 'run' && method !== 'collect') { - // return error( - // new Error(`[vitest] Method query in ${request.url} is invalid. Method should be either "run" or "collect".`), - // ) - // } - if (type === 'orchestrator') { const session = vitest._browserSessions.getSession(sessionId) // it's possible the session was already resolved by the preview provider @@ -80,7 +73,7 @@ export function setupBrowserRpc(globalServer: ParentBrowserProject): void { wss.handleUpgrade(request, socket, head, (ws) => { wss.emit('connection', ws, request) - const rpc = setupClient(project, rpcId, ws, method) + const rpc = setupClient(project, rpcId, ws) const state = project.browser!.state as BrowserServerState const clients = type === 'tester' ? state.testers : state.orchestrators clients.set(rpcId, rpc) @@ -112,7 +105,7 @@ export function setupBrowserRpc(globalServer: ParentBrowserProject): void { } } - function setupClient(project: TestProject, rpcId: string, ws: WebSocket, method: 'run' | 'collect') { + function setupClient(project: TestProject, rpcId: string, ws: WebSocket) { const mockResolver = new ServerMockResolver(globalServer.vite, { moduleDirectories: project.config.server?.deps?.moduleDirectories, }) @@ -126,7 +119,7 @@ export function setupBrowserRpc(globalServer: ParentBrowserProject): void { } vitest.state.catchError(error, type) }, - async onQueued(file) { + async onQueued(method, file) { if (method === 'collect') { vitest.state.collectFiles(project, [file]) } @@ -134,7 +127,7 @@ export function setupBrowserRpc(globalServer: ParentBrowserProject): void { await vitest._testRun.enqueued(project, file) } }, - async onCollected(files) { + async onCollected(method, files) { if (method === 'collect') { vitest.state.collectFiles(project, files) } @@ -142,7 +135,7 @@ export function setupBrowserRpc(globalServer: ParentBrowserProject): void { await vitest._testRun.collected(project, files) } }, - async onTaskUpdate(packs, events) { + async onTaskUpdate(method, packs, events) { if (method === 'collect') { vitest.state.updateTasks(packs) } @@ -153,7 +146,7 @@ export function setupBrowserRpc(globalServer: ParentBrowserProject): void { onAfterSuiteRun(meta) { vitest.coverageProvider?.onAfterSuiteRun(meta) }, - async sendLog(log) { + async sendLog(method, log) { if (method === 'collect') { vitest.state.updateUserLog(log) } @@ -244,10 +237,6 @@ export function setupBrowserRpc(globalServer: ParentBrowserProject): void { ) as any as BrowserCommandContext return await commands[command](context, ...payload) }, - // finishBrowserTests(sessionId: string) { - // debug?.('[%s] Finishing browser tests for session', sessionId) - // return vitest._browserSessions.getSession(sessionId)?.resolve() - // }, resolveMock(rawId, importer, options) { return mockResolver.resolveMock(rawId, importer, options) }, @@ -268,7 +257,7 @@ export function setupBrowserRpc(globalServer: ParentBrowserProject): void { { post: msg => ws.send(msg), on: fn => ws.on('message', fn), - eventNames: ['onCancel', 'cdpEvent'], + eventNames: ['onCancel', 'cdpEvent', 'createTesters'], serialize: (data: any) => stringify(data, stringifyReplace), deserialize: parse, onTimeoutError(functionName) { diff --git a/packages/browser/src/node/types.ts b/packages/browser/src/node/types.ts index 33bb529f45b8..a5bef386c0b5 100644 --- a/packages/browser/src/node/types.ts +++ b/packages/browser/src/node/types.ts @@ -1,22 +1,31 @@ import type { ServerIdResolution, ServerMockResolution } from '@vitest/mocker/node' import type { TaskEventPack, TaskResultPack } from '@vitest/runner' import type { BirpcReturn } from 'birpc' -import type { AfterSuiteRunMeta, CancelReason, Reporter, RunnerTestFile, SnapshotResult, UserConsoleLog } from 'vitest' +import type { + AfterSuiteRunMeta, + BrowserTesterOptions, + CancelReason, + Reporter, + RunnerTestFile, + SnapshotResult, + TestExecutionType, + UserConsoleLog, +} from 'vitest' export interface WebSocketBrowserHandlers { resolveSnapshotPath: (testPath: string) => string resolveSnapshotRawPath: (testPath: string, rawPath: string) => string onUnhandledError: (error: unknown, type: string) => Promise - onQueued: (file: RunnerTestFile) => void - onCollected: (files: RunnerTestFile[]) => Promise - onTaskUpdate: (packs: TaskResultPack[], events: TaskEventPack[]) => void + onQueued: (method: TestExecutionType, file: RunnerTestFile) => void + onCollected: (method: TestExecutionType, files: RunnerTestFile[]) => Promise + onTaskUpdate: (method: TestExecutionType, packs: TaskResultPack[], events: TaskEventPack[]) => void onAfterSuiteRun: (meta: AfterSuiteRunMeta) => void onCancel: (reason: CancelReason) => void getCountOfFailedTests: () => number readSnapshotFile: (id: string) => Promise saveSnapshotFile: (id: string, content: string) => Promise removeSnapshotFile: (id: string) => Promise - sendLog: (log: UserConsoleLog) => void + sendLog: (method: TestExecutionType, log: UserConsoleLog) => void // finishBrowserTests: (sessionId: string) => void snapshotSaved: (snapshot: SnapshotResult) => void debug: (...args: string[]) => void @@ -61,7 +70,7 @@ export interface WebSocketEvents export interface WebSocketBrowserEvents { onCancel: (reason: CancelReason) => void - createTesters: (method: 'run' | 'collect', files: string[]) => Promise + createTesters: (options: BrowserTesterOptions) => Promise cdpEvent: (event: string, payload: unknown) => void } diff --git a/packages/vitest/src/node/types/browser.ts b/packages/vitest/src/node/types/browser.ts index e29c45c930fc..7a88549af198 100644 --- a/packages/vitest/src/node/types/browser.ts +++ b/packages/vitest/src/node/types/browser.ts @@ -2,6 +2,7 @@ import type { CancelReason } from '@vitest/runner' import type { Awaitable, ErrorWithDiff, ParsedStack } from '@vitest/utils' import type { StackTraceParserOptions } from '@vitest/utils/source-map' import type { ViteDevServer } from 'vite' +import type { BrowserTesterOptions } from '../../types/browser' import type { TestProject } from '../project' import type { ApiConfig, ProjectConfig } from './config' @@ -242,7 +243,7 @@ export interface BrowserServerStateSession { } export interface BrowserOrchestrator { - createTesters: (method: 'run' | 'collect', files: string[]) => Promise + createTesters: (options: BrowserTesterOptions) => Promise onCancel: (reason: CancelReason) => Promise $close: () => void } diff --git a/packages/vitest/src/public/index.ts b/packages/vitest/src/public/index.ts index 56d2d302f354..09dc1531dab1 100644 --- a/packages/vitest/src/public/index.ts +++ b/packages/vitest/src/public/index.ts @@ -179,6 +179,7 @@ export type WorkerContext = WorkerContext_ /** @deprecated import from `vitest/node` instead */ export type WorkerRPC = WorkerRPC_ +export type { BrowserTesterOptions } from '../types/browser' export type { AfterSuiteRunMeta, ErrorWithDiff, @@ -243,6 +244,7 @@ export type { ContextRPC, ContextTestEnvironment, ResolveIdFunction, + TestExecutionMethod as TestExecutionType, WorkerGlobalState, } from '../types/worker' export type { diff --git a/packages/vitest/src/public/node.ts b/packages/vitest/src/public/node.ts index 716250bb3093..4f42d870d4c2 100644 --- a/packages/vitest/src/public/node.ts +++ b/packages/vitest/src/public/node.ts @@ -128,14 +128,13 @@ export type { TestRunResult } from '../node/types/tests' export const TestFile: typeof _TestFile = _TestFile export type { WorkerContext } from '../node/types/worker' export { createViteLogger } from '../node/viteLogger' +export { distDir, rootDir } from '../paths' /** * @deprecated Use `ModuleDiagnostic` instead */ export type FileDiagnostic = _FileDiagnostic -export { distDir, rootDir } from '../paths' - export type { CollectLineNumbers as TypeCheckCollectLineNumbers, CollectLines as TypeCheckCollectLines, @@ -145,6 +144,8 @@ export type { RootAndTarget as TypeCheckRootAndTarget, } from '../typecheck/types' +export type { TestExecutionMethod as TestExecutionType } from '../types/worker' + export { createDebugger } from '../utils/debugger' export type { diff --git a/packages/vitest/src/types/browser.ts b/packages/vitest/src/types/browser.ts new file mode 100644 index 000000000000..625fde1b11e1 --- /dev/null +++ b/packages/vitest/src/types/browser.ts @@ -0,0 +1,10 @@ +import type { TestExecutionMethod } from './worker' + +/** + * @internal + */ +export interface BrowserTesterOptions { + method: TestExecutionMethod + files: string[] + providedContext: string +} diff --git a/packages/vitest/src/types/worker.ts b/packages/vitest/src/types/worker.ts index f2681ea05ee9..05b511c4a742 100644 --- a/packages/vitest/src/types/worker.ts +++ b/packages/vitest/src/types/worker.ts @@ -20,6 +20,8 @@ export interface ContextTestEnvironment { options: Record | null } +export type TestExecutionMethod = 'run' | 'collect' + export interface ContextRPC { pool: string worker: string From 6e8c1db941deb7e685c0bccf91e6006768ca82a0 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Fri, 14 Mar 2025 12:59:51 +0100 Subject: [PATCH 09/24] fix: remove timeout from createTesters --- packages/browser/src/client/client.ts | 1 + packages/browser/src/node/pool.ts | 95 +++++++++++++-------------- packages/browser/src/node/rpc.ts | 1 + packages/vitest/src/types/browser.ts | 3 - 4 files changed, 46 insertions(+), 54 deletions(-) diff --git a/packages/browser/src/client/client.ts b/packages/browser/src/client/client.ts index d27668c1aff4..c3f2653271ef 100644 --- a/packages/browser/src/client/client.ts +++ b/packages/browser/src/client/client.ts @@ -69,6 +69,7 @@ function createClient() { { post: msg => ctx.ws.send(msg), on: fn => (onMessage = fn), + timeout: -1, // createTesters can take a while serialize: e => stringify(e, (_, v) => { if (v instanceof Error) { diff --git a/packages/browser/src/node/pool.ts b/packages/browser/src/node/pool.ts index 2d1db6ce05ac..72576242f984 100644 --- a/packages/browser/src/node/pool.ts +++ b/packages/browser/src/node/pool.ts @@ -15,34 +15,6 @@ import { stringify } from 'flatted' // const debug = createDebugger('vitest:browser:pool') -async function waitForOrchestrator( - sessionId: string, - project: TestProject, -) { - const context = project.vitest._browserSessions.createSession(sessionId, project) - return await context -} - -// TODO: support breakpoints -// async function setBreakpoint(project: TestProject, sessionId: string, file: string) { -// if (!project.config.inspector.waitForDebugger) { -// return -// } - -// const provider = project.browser!.provider - -// if (!provider.getCDPSession) { -// throw new Error('Unable to set breakpoint, CDP not supported') -// } - -// const session = await provider.getCDPSession(sessionId) -// await session.send('Debugger.enable', {}) -// await session.send('Debugger.setBreakpointByUrl', { -// lineNumber: 0, -// urlRegex: escapePathToRegexp(file), -// }) -// } - export function createBrowserPool(vitest: Vitest): ProcessPool { const providers = new Set() @@ -86,7 +58,9 @@ export function createBrowserPool(vitest: Vitest): ProcessPool { const pool = new BrowserPool(project, { maxWorkers: getThreadsCount(project), origin, - worker: sessionId => waitForOrchestrator(sessionId, project), + worker: (sessionId) => { + return project.vitest._browserSessions.createSession(sessionId, project) + }, }) projectPools.set(project, pool) vitest.onCancel(() => { @@ -162,9 +136,9 @@ export function createBrowserPool(vitest: Vitest): ProcessPool { } } -// function escapePathToRegexp(path: string): string { -// return path.replace(/[/\\.?*()^${}|[\]+]/g, '\\$&') -// } +function escapePathToRegexp(path: string): string { + return path.replace(/[/\\.?*()^${}|[\]+]/g, '\\$&') +} class BrowserPool { private _queue: string[] = [] @@ -189,7 +163,6 @@ class BrowserPool { return this.project.browser!.state.orchestrators } - // open "maxWothers" browser contexts async runTests(method: 'run' | 'collect', files: string[]) { this._promise ??= createDefer() @@ -262,24 +235,44 @@ class BrowserPool { return } - // TODO: createTesters has a 60s timeout, it should wait indefinetly, especially for isolate: false - orchestrator.createTesters( - { - method, - files: [file], - providedContext: stringify(this.project.getProvidedContext()), - }, - ) - .then(() => { - this.runNextTest(method, sessionId) - }) - .catch((error) => { - if (error instanceof Error && error.message.startsWith('[birpc] rpc is closed')) { - return - } - this._promise?.reject(error) - }) + this.setBreakpoint(sessionId, file).then(() => { + orchestrator.createTesters( + { + method, + files: [file], + // this will be parsed by the test iframe, not the orchestrator + // so we need to stringify it first to avoid double serialization + providedContext: stringify(this.project.getProvidedContext()), + }, + ) + .then(() => { + this.runNextTest(method, sessionId) + }) + .catch((error) => { + if (error instanceof Error && error.message.startsWith('[birpc] rpc is closed')) { + return + } + this._promise?.reject(error) + }) + }).catch(err => this._promise?.reject(err)) } - // TODO: isolate: false + async setBreakpoint(sessionId: string, file: string) { + if (!this.project.config.inspector.waitForDebugger) { + return + } + + const provider = this.project.browser!.provider + + if (!provider.getCDPSession) { + throw new Error('Unable to set breakpoint, CDP not supported') + } + + const session = await provider.getCDPSession(sessionId) + await session.send('Debugger.enable', {}) + await session.send('Debugger.setBreakpointByUrl', { + lineNumber: 0, + urlRegex: escapePathToRegexp(file), + }) + } } diff --git a/packages/browser/src/node/rpc.ts b/packages/browser/src/node/rpc.ts index dc7bd821b4df..663717f14a4f 100644 --- a/packages/browser/src/node/rpc.ts +++ b/packages/browser/src/node/rpc.ts @@ -259,6 +259,7 @@ export function setupBrowserRpc(globalServer: ParentBrowserProject): void { on: fn => ws.on('message', fn), eventNames: ['onCancel', 'cdpEvent', 'createTesters'], serialize: (data: any) => stringify(data, stringifyReplace), + timeout: -1, // createTesters can take a long time deserialize: parse, onTimeoutError(functionName) { throw new Error(`[vitest-api]: Timeout calling "${functionName}"`) diff --git a/packages/vitest/src/types/browser.ts b/packages/vitest/src/types/browser.ts index 625fde1b11e1..11d7c2122425 100644 --- a/packages/vitest/src/types/browser.ts +++ b/packages/vitest/src/types/browser.ts @@ -1,8 +1,5 @@ import type { TestExecutionMethod } from './worker' -/** - * @internal - */ export interface BrowserTesterOptions { method: TestExecutionMethod files: string[] From 16fc7aceb36a9264ee8635389845f4fc9acc5fae Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Fri, 14 Mar 2025 13:45:34 +0100 Subject: [PATCH 10/24] chore: cleanup --- packages/browser/src/client/tester/runner.ts | 10 ++-- packages/browser/src/node/pool.ts | 1 + .../browser/src/node/providers/playwright.ts | 3 -- packages/browser/src/node/rpc.ts | 2 +- packages/browser/src/node/types.ts | 11 ++--- packages/expect/src/jest-expect.ts | 48 ++++++++++--------- packages/vitest/src/public/index.ts | 2 +- 7 files changed, 38 insertions(+), 39 deletions(-) diff --git a/packages/browser/src/client/tester/runner.ts b/packages/browser/src/client/tester/runner.ts index 58a043505c9a..358708147453 100644 --- a/packages/browser/src/client/tester/runner.ts +++ b/packages/browser/src/client/tester/runner.ts @@ -1,5 +1,5 @@ import type { CancelReason, File, Suite, Task, TaskEventPack, TaskResultPack, VitestRunner } from '@vitest/runner' -import type { SerializedConfig, TestExecutionType, WorkerGlobalState } from 'vitest' +import type { SerializedConfig, TestExecutionMethod, WorkerGlobalState } from 'vitest' import type { VitestExecutor } from 'vitest/execute' import type { VitestBrowserClientMocker } from './mocker' import { globalChannel, onCancel } from '@vitest/browser/client' @@ -24,8 +24,8 @@ interface CoverageHandler { interface BrowserVitestRunner extends VitestRunner { sourceMapCache: Map - method: TestExecutionType - setMethod: (method: TestExecutionType) => void + method: TestExecutionMethod + setMethod: (method: TestExecutionMethod) => void } export function createBrowserRunner( @@ -38,14 +38,14 @@ export function createBrowserRunner( public config: SerializedConfig hashMap = browserHashMap public sourceMapCache = new Map() - public method = 'run' as TestExecutionType + public method = 'run' as TestExecutionMethod constructor(options: BrowserRunnerOptions) { super(options.config) this.config = options.config } - setMethod(method: 'run' | 'collect') { + setMethod(method: TestExecutionMethod) { this.method = method } diff --git a/packages/browser/src/node/pool.ts b/packages/browser/src/node/pool.ts index 72576242f984..256e6ed9c10b 100644 --- a/packages/browser/src/node/pool.ts +++ b/packages/browser/src/node/pool.ts @@ -236,6 +236,7 @@ class BrowserPool { } this.setBreakpoint(sessionId, file).then(() => { + // this starts running tests inside the orchestrator orchestrator.createTesters( { method, diff --git a/packages/browser/src/node/providers/playwright.ts b/packages/browser/src/node/providers/playwright.ts index fa41a4069884..33960e7ea09c 100644 --- a/packages/browser/src/node/providers/playwright.ts +++ b/packages/browser/src/node/providers/playwright.ts @@ -176,9 +176,6 @@ export class PlaywrightBrowserProvider implements BrowserProvider { const context = await this.createContext(sessionId) const page = await context.newPage() - // page.on('console', (m) => { - // console.log('pw', m.text()) - // }) this.pages.set(sessionId, page) if (process.env.VITEST_PW_DEBUG) { diff --git a/packages/browser/src/node/rpc.ts b/packages/browser/src/node/rpc.ts index 663717f14a4f..6557dd4e756c 100644 --- a/packages/browser/src/node/rpc.ts +++ b/packages/browser/src/node/rpc.ts @@ -257,7 +257,7 @@ export function setupBrowserRpc(globalServer: ParentBrowserProject): void { { post: msg => ws.send(msg), on: fn => ws.on('message', fn), - eventNames: ['onCancel', 'cdpEvent', 'createTesters'], + eventNames: ['onCancel', 'cdpEvent'], serialize: (data: any) => stringify(data, stringifyReplace), timeout: -1, // createTesters can take a long time deserialize: parse, diff --git a/packages/browser/src/node/types.ts b/packages/browser/src/node/types.ts index a5bef386c0b5..16a46c091cf7 100644 --- a/packages/browser/src/node/types.ts +++ b/packages/browser/src/node/types.ts @@ -8,7 +8,7 @@ import type { Reporter, RunnerTestFile, SnapshotResult, - TestExecutionType, + TestExecutionMethod, UserConsoleLog, } from 'vitest' @@ -16,17 +16,16 @@ export interface WebSocketBrowserHandlers { resolveSnapshotPath: (testPath: string) => string resolveSnapshotRawPath: (testPath: string, rawPath: string) => string onUnhandledError: (error: unknown, type: string) => Promise - onQueued: (method: TestExecutionType, file: RunnerTestFile) => void - onCollected: (method: TestExecutionType, files: RunnerTestFile[]) => Promise - onTaskUpdate: (method: TestExecutionType, packs: TaskResultPack[], events: TaskEventPack[]) => void + onQueued: (method: TestExecutionMethod, file: RunnerTestFile) => void + onCollected: (method: TestExecutionMethod, files: RunnerTestFile[]) => Promise + onTaskUpdate: (method: TestExecutionMethod, packs: TaskResultPack[], events: TaskEventPack[]) => void onAfterSuiteRun: (meta: AfterSuiteRunMeta) => void onCancel: (reason: CancelReason) => void getCountOfFailedTests: () => number readSnapshotFile: (id: string) => Promise saveSnapshotFile: (id: string, content: string) => Promise removeSnapshotFile: (id: string) => Promise - sendLog: (method: TestExecutionType, log: UserConsoleLog) => void - // finishBrowserTests: (sessionId: string) => void + sendLog: (method: TestExecutionMethod, log: UserConsoleLog) => void snapshotSaved: (snapshot: SnapshotResult) => void debug: (...args: string[]) => void resolveId: ( diff --git a/packages/expect/src/jest-expect.ts b/packages/expect/src/jest-expect.ts index 25cad3178892..06ccdff227dd 100644 --- a/packages/expect/src/jest-expect.ts +++ b/packages/expect/src/jest-expect.ts @@ -1206,7 +1206,7 @@ function ordinalOf(i: number) { } function formatCalls(spy: MockInstance, msg: string, showActualCall?: any) { - if (spy.mock.calls) { + if (spy.mock.calls.length) { msg += c.gray( `\n\nReceived: \n\n${spy.mock.calls .map((callArg, i) => { @@ -1243,29 +1243,31 @@ function formatReturns( msg: string, showActualReturn?: any, ) { - msg += c.gray( - `\n\nReceived: \n\n${results - .map((callReturn, i) => { - let methodCall = c.bold( - ` ${ordinalOf(i + 1)} ${spy.getMockName()} call return:\n\n`, - ) - if (showActualReturn) { - methodCall += diff(showActualReturn, callReturn.value, { - omitAnnotationLines: true, - }) - } - else { - methodCall += stringify(callReturn) - .split('\n') - .map(line => ` ${line}`) - .join('\n') - } + if (results.length) { + msg += c.gray( + `\n\nReceived: \n\n${results + .map((callReturn, i) => { + let methodCall = c.bold( + ` ${ordinalOf(i + 1)} ${spy.getMockName()} call return:\n\n`, + ) + if (showActualReturn) { + methodCall += diff(showActualReturn, callReturn.value, { + omitAnnotationLines: true, + }) + } + else { + methodCall += stringify(callReturn) + .split('\n') + .map(line => ` ${line}`) + .join('\n') + } - methodCall += '\n' - return methodCall - }) - .join('\n')}`, - ) + methodCall += '\n' + return methodCall + }) + .join('\n')}`, + ) + } msg += c.gray( `\n\nNumber of calls: ${c.bold(spy.mock.calls.length)}\n`, ) diff --git a/packages/vitest/src/public/index.ts b/packages/vitest/src/public/index.ts index 09dc1531dab1..0da2ee3e7b71 100644 --- a/packages/vitest/src/public/index.ts +++ b/packages/vitest/src/public/index.ts @@ -244,7 +244,7 @@ export type { ContextRPC, ContextTestEnvironment, ResolveIdFunction, - TestExecutionMethod as TestExecutionType, + TestExecutionMethod, WorkerGlobalState, } from '../types/worker' export type { From 1b8b1fbdad5efcc4e47c7e9abbf75a60d7718225 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Fri, 14 Mar 2025 17:45:55 +0100 Subject: [PATCH 11/24] fix: run non-isolated tests correctly --- packages/browser/src/client/channel.ts | 68 ++-- packages/browser/src/client/client.ts | 12 +- packages/browser/src/client/orchestrator.ts | 317 +++++++++--------- packages/browser/src/client/tester/context.ts | 14 +- packages/browser/src/client/tester/runner.ts | 7 +- packages/browser/src/client/tester/tester.ts | 292 ++++++++-------- packages/browser/src/client/types.ts | 7 - packages/browser/src/client/utils.ts | 5 +- packages/browser/src/node/pool.ts | 33 +- packages/browser/src/node/types.ts | 1 + packages/vitest/src/node/types/browser.ts | 1 + 11 files changed, 395 insertions(+), 362 deletions(-) delete mode 100644 packages/browser/src/client/types.ts diff --git a/packages/browser/src/client/channel.ts b/packages/browser/src/client/channel.ts index 39f2f2dc237e..242f4aba02a6 100644 --- a/packages/browser/src/client/channel.ts +++ b/packages/browser/src/client/channel.ts @@ -1,25 +1,22 @@ import type { CancelReason } from '@vitest/runner' import { getBrowserState } from './utils' -export interface IframeDoneEvent { - type: 'done' - filenames: string[] - id: string +export interface IframeViewportEvent { + event: 'viewport' + width: number + height: number + iframeId: string } -export interface IframeErrorEvent { - type: 'error' - error: any - errorType: string - files: string[] - id: string +export interface IframeViewportFailEvent { + event: 'viewport:fail' + iframeId: string + error: string } -export interface IframeViewportEvent { - type: 'viewport' - width: number - height: number - id: string +export interface IframeViewportDoneEvent { + event: 'viewport:done' + iframeId: string } export interface GlobalChannelTestRunCanceledEvent { @@ -27,14 +24,35 @@ export interface GlobalChannelTestRunCanceledEvent { reason: CancelReason } +export interface IframeExecuteEvent { + event: 'execute' + method: 'run' | 'collect' + files: string[] + iframeId: string + context: string +} + +export interface IframeCleanupEvent { + event: 'cleanup' + iframeId: string +} + +export interface IframePrepareEvent { + event: 'prepare' + iframeId: string +} + export type GlobalChannelIncomingEvent = GlobalChannelTestRunCanceledEvent export type IframeChannelIncomingEvent = | IframeViewportEvent - | IframeErrorEvent - | IframeDoneEvent -export type IframeChannelOutgoingEvent = never +export type IframeChannelOutgoingEvent = + | IframeExecuteEvent + | IframeCleanupEvent + | IframePrepareEvent + | IframeViewportFailEvent + | IframeViewportDoneEvent export type IframeChannelEvent = | IframeChannelIncomingEvent @@ -44,17 +62,3 @@ export const channel: BroadcastChannel = new BroadcastChannel( `vitest:${getBrowserState().sessionId}`, ) export const globalChannel: BroadcastChannel = new BroadcastChannel('vitest:global') - -export function waitForChannel(event: IframeChannelOutgoingEvent['type']): Promise { - return new Promise((resolve) => { - channel.addEventListener( - 'message', - (e) => { - if (e.data?.type === event) { - resolve() - } - }, - { once: true }, - ) - }) -} diff --git a/packages/browser/src/client/client.ts b/packages/browser/src/client/client.ts index c3f2653271ef..e6f79edeb816 100644 --- a/packages/browser/src/client/client.ts +++ b/packages/browser/src/client/client.ts @@ -53,10 +53,18 @@ function createClient() { { onCancel: setCancel, async createTesters(options) { - if (PAGE_TYPE !== 'orchestrator') { + const orchestrator = getBrowserState().orchestrator + if (!orchestrator) { throw new TypeError('Only orchestrator can create testers.') } - return getBrowserState().createTesters?.(options) + return orchestrator.createTesters(options) + }, + async cleanupTesters() { + const orchestrator = getBrowserState().orchestrator + if (!orchestrator) { + throw new TypeError('Only orchestrator can cleanup testers.') + } + return orchestrator.cleanupTesters() }, cdpEvent(event: string, payload: unknown) { const cdp = getBrowserState().cdp diff --git a/packages/browser/src/client/orchestrator.ts b/packages/browser/src/client/orchestrator.ts index 4af330710e5d..5f88ca048465 100644 --- a/packages/browser/src/client/orchestrator.ts +++ b/packages/browser/src/client/orchestrator.ts @@ -1,6 +1,5 @@ -import type { GlobalChannelIncomingEvent, IframeChannelEvent, IframeChannelIncomingEvent } from '@vitest/browser/client' +import type { GlobalChannelIncomingEvent, IframeChannelIncomingEvent, IframeChannelOutgoingEvent, IframeViewportDoneEvent, IframeViewportFailEvent } from '@vitest/browser/client' import type { BrowserTesterOptions, SerializedConfig } from 'vitest' -import type { IframeInitEvent } from './types' import { channel, client, globalChannel } from '@vitest/browser/client' import { generateHash } from '@vitest/runner/utils' import { relative } from 'pathe' @@ -10,16 +9,14 @@ import { getBrowserState, getConfig } from './utils' const url = new URL(location.href) const ID_ALL = '__vitest_all__' -class IframeOrchestrator { +export class IframeOrchestrator { private cancelled = false - private runningFiles = new Set() + private recreateNonIsolatedIframe = false private iframes = new Map() - public init() { + constructor() { debug('init orchestrator', getBrowserState().sessionId) - this.runningFiles.clear() - channel.addEventListener( 'message', e => this.onIframeEvent(e), @@ -30,10 +27,8 @@ class IframeOrchestrator { ) } - public async createTesters(options: BrowserTesterOptions) { + public async createTesters(options: BrowserTesterOptions): Promise { this.cancelled = false - this.runningFiles.clear() - options.files.forEach(file => this.runningFiles.add(file)) const config = getConfig() debug('create testers', options.files.join(', ')) @@ -45,72 +40,146 @@ class IframeOrchestrator { container.textContent = '' } + if (config.browser.isolate === false) { + await this.runNonIsolatedTests(container, options) + return + } + this.iframes.forEach(iframe => iframe.remove()) this.iframes.clear() - const isolate = config.browser.isolate - - for (const file of options.files) { + for (let i = 0; i < options.files.length; i++) { if (this.cancelled) { return } + const file = options.files[i] debug('create iframe', file) - await this.runTestInIframe( + await this.runIsolatedTestInIframe( container, - isolate === false ? ID_ALL : file, file, options, ) } } - private async runTestInIframe( + public async cleanupTesters(): Promise { + const config = getConfig() + if (config.browser.isolate) { + // isolated mode assignes filepaths as ids + const files = Array.from(this.iframes.keys()) + // when the run is completed, show the last file in the UI + const ui = getUiAPI() + if (ui && files[0]) { + const id = generateFileId(files[0]) + ui.setCurrentFileId(id) + } + return + } + // we only cleanup non-isolated iframe because + // in isolated mode every iframe is cleaned up after the test + const iframe = this.iframes.get(ID_ALL) + if (!iframe) { + return + } + await sendEventToIframe({ + event: 'cleanup', + iframeId: ID_ALL, + }) + this.recreateNonIsolatedIframe = true + } + + private async runNonIsolatedTests(container: HTMLDivElement, options: BrowserTesterOptions) { + if (this.recreateNonIsolatedIframe) { + // recreate a new non-isolated iframe during watcher reruns + // because we called "cleanup" in the previous run + // the iframe is not removed immediately to let the user see the last test + this.recreateNonIsolatedIframe = false + this.iframes.get(ID_ALL)!.remove() + this.iframes.delete(ID_ALL) + } + + if (!this.iframes.has(ID_ALL)) { + await this.prepareIframe(container, ID_ALL) + } + + const config = getConfig() + const { width, height } = config.browser.viewport + const iframe = this.iframes.get(ID_ALL)! + + await setIframeViewport(iframe, width, height) + await sendEventToIframe({ + event: 'execute', + iframeId: ID_ALL, + files: options.files, + method: options.method, + context: options.providedContext, + }) + // we don't cleanup here because in non-isolated mode + // it is done after all tests finished running + } + + private async runIsolatedTestInIframe( container: HTMLDivElement, - id: string, file: string, options: BrowserTesterOptions, ) { const config = getConfig() const { width, height } = config.browser.viewport - const iframe = config.browser.isolate === false - ? this.startInIsolatedIframe(container, file, options) - : this.startInNewIframe(container, id, file, options) + if (this.iframes.has(file)) { + this.iframes.get(file)!.remove() + this.iframes.delete(file) + } + const iframe = await this.prepareIframe(container, file) await setIframeViewport(iframe, width, height) - await this.waitForIframeDoneEvent() + // running tests after the "prepare" event + await sendEventToIframe({ + event: 'execute', + files: [file], + method: options.method, + iframeId: file, + context: options.providedContext, + }) + // perform "cleanup" to cleanup resources and calculate the coverage + await sendEventToIframe({ + event: 'cleanup', + iframeId: file, + }) } - private startIframeTest( - iframe: HTMLIFrameElement, - iframeId: string, - file: string, - options: BrowserTesterOptions, - ) { - const iframeWindow = iframe.contentWindow - if (!iframeWindow) { - debug('no window available') - // TODO: what happened here? - return - } + private async prepareIframe(container: HTMLDivElement, iframeId: string) { + const iframe = this.createTestIframe(iframeId) + container.appendChild(iframe) - iframeWindow.postMessage( - JSON.stringify({ - event: 'init', - method: options.method, - files: [file], - iframeId, - context: options.providedContext, - } satisfies IframeInitEvent), - '*', - ) + await new Promise((resolve, reject) => { + iframe.onload = () => { + this.iframes.set(iframeId, iframe) + sendEventToIframe({ + event: 'prepare', + iframeId, + }).then(resolve, reject) + } + iframe.onerror = (e) => { + if (typeof e === 'string') { + reject(new Error(e)) + } + else if (e instanceof ErrorEvent) { + reject(e.error) + } + else { + reject(new Error(`Cannot load the iframe ${iframeId}.`)) + } + } + }) + return iframe } - private createTestIframe() { + private createTestIframe(iframeId: string) { const iframe = document.createElement('iframe') - const src = `${url.pathname}__vitest_test__/__test__/?sessionId=${getBrowserState().sessionId}` + const src = `${url.pathname}__vitest_test__/__test__/?sessionId=${getBrowserState().sessionId}&iframeId=${iframeId}` iframe.setAttribute('loading', 'eager') iframe.setAttribute('src', src) iframe.setAttribute('data-vitest', 'true') @@ -124,60 +193,6 @@ class IframeOrchestrator { return iframe } - // TODO: a lot of tests on how this actually works - private startInIsolatedIframe( - container: HTMLDivElement, - file: string, - options: BrowserTesterOptions, - ) { - const cachedIframe = this.iframes.get(ID_ALL) - if (cachedIframe) { - this.startIframeTest(cachedIframe, ID_ALL, file, options) - return cachedIframe - } - return this.startInNewIframe(container, ID_ALL, file, options) - } - - private startInNewIframe( - container: HTMLDivElement, - iframeId: string, - file: string, - options: BrowserTesterOptions, - ) { - if (this.iframes.has(iframeId)) { - this.iframes.get(iframeId)!.remove() - this.iframes.delete(iframeId) - } - - const iframe = this.createTestIframe() - iframe.onerror = (e) => { - debug('iframe error', e.toString()) - } - iframe.onload = () => { - debug(`iframe for ${file} loaded`) - this.startIframeTest(iframe, iframeId, file, options) - } - - this.iframes.set(iframeId, iframe) - container.appendChild(iframe) - return iframe - } - - private waitForIframeDoneEvent() { - return new Promise((resolve) => { - channel.addEventListener( - 'message', - function handler(e: MessageEvent) { - // done and error can only be triggered by the previous iframe - if (e.data.type === 'done' || e.data.type === 'error') { - channel.removeEventListener('message', handler) - resolve() - } - }, - ) - }) - } - private async onGlobalChannelEvent(e: MessageEvent) { debug('global channel event', JSON.stringify(e.data)) switch (e.data.type) { @@ -190,89 +205,52 @@ class IframeOrchestrator { private async onIframeEvent(e: MessageEvent) { debug('iframe event', JSON.stringify(e.data)) - switch (e.data.type) { + switch (e.data.event) { case 'viewport': { - const { width, height, id } = e.data + const { width, height, iframeId: id } = e.data const iframe = this.iframes.get(id) if (!iframe) { - const error = new Error(`Cannot find iframe with id ${id}`) + const error = `Cannot find iframe with id ${id}` channel.postMessage({ - type: 'viewport:fail', - id, - error: error.message, - }) + event: 'viewport:fail', + iframeId: id, + error, + } satisfies IframeViewportFailEvent) await client.rpc.onUnhandledError( { name: 'Teardown Error', - message: error.message, + message: error, }, 'Teardown Error', ) - return + break } await setIframeViewport(iframe, width, height) - channel.postMessage({ type: 'viewport:done', id }) - break - } - case 'done': { - const filenames = e.data.filenames - filenames.forEach(filename => this.runningFiles.delete(filename)) - - if (!this.runningFiles.size) { - const ui = getUiAPI() - // in isolated mode we don't change UI because it will slow down tests, - // so we only select it when the run is done - if (ui && filenames.length > 1) { - const id = generateFileId(filenames[filenames.length - 1]) - ui.setCurrentFileId(id) - } - } - else { - // keep the last iframe - const iframeId = e.data.id - this.iframes.get(iframeId)?.remove() - this.iframes.delete(iframeId) - } - break - } - // error happened at the top level, this should never happen in user code, but it can trigger during development - case 'error': { - const iframeId = e.data.id - this.iframes.delete(iframeId) - await client.rpc.onUnhandledError(e.data.error, e.data.errorType) - if (iframeId === ID_ALL) { - this.runningFiles.clear() - } - else { - this.runningFiles.delete(iframeId) - } + channel.postMessage({ event: 'viewport:done', iframeId: id } satisfies IframeViewportDoneEvent) break } default: { - e.data satisfies never + // ignore responses + if ( + typeof e.data.event === 'string' + && (e.data.event as string).startsWith('response:') + ) { + break + } - await client.rpc.onUnhandledError( - { - name: 'Unexpected Event', - message: `Unexpected event: ${(e.data as any).type}`, - }, - 'Unexpected Event', - ) + await client.rpc.onUnhandledError( + { + name: 'Unexpected Event', + message: `Unexpected event: ${(e.data as any).event}`, + }, + 'Unexpected Event', + ) } } } } -const orchestrator = new IframeOrchestrator() - -let promiseTesters: Promise | undefined -getBrowserState().createTesters = async (options: BrowserTesterOptions) => { - await promiseTesters - promiseTesters = orchestrator.createTesters(options).finally(() => { - promiseTesters = undefined - }) - await promiseTesters -} +getBrowserState().orchestrator = new IframeOrchestrator() async function getContainer(config: SerializedConfig): Promise { if (config.browser.ui) { @@ -289,9 +267,20 @@ async function getContainer(config: SerializedConfig): Promise { return document.querySelector('#vitest-tester') as HTMLDivElement } -client.waitForConnection().then(async () => { - orchestrator.init() -}) +async function sendEventToIframe(event: IframeChannelOutgoingEvent) { + channel.postMessage(event) + return new Promise((resolve) => { + channel.addEventListener( + 'message', + function handler(e) { + if (e.data.iframeId === event.iframeId && e.data.event === `response:${event.event}`) { + resolve() + channel.removeEventListener('message', handler) + } + }, + ) + }) +} function generateFileId(file: string) { const config = getConfig() diff --git a/packages/browser/src/client/tester/context.ts b/packages/browser/src/client/tester/context.ts index 463dcd45d709..888a5a172fdb 100644 --- a/packages/browser/src/client/tester/context.ts +++ b/packages/browser/src/client/tester/context.ts @@ -5,6 +5,7 @@ import type { Locator, UserEvent, } from '../../../context' +import type { IframeViewportEvent } from '../client' import type { BrowserRunnerState } from '../utils' import { ensureAwaited, getBrowserState, getWorkerState } from '../utils' import { convertElementToCssSelector, processTimeoutOptions } from './utils' @@ -241,15 +242,20 @@ export function cdp(): BrowserRunnerState['cdp'] { const screenshotIds: Record> = {} export const page: BrowserPage = { viewport(width, height) { - const id = getBrowserState().iframeId - channel.postMessage({ type: 'viewport', width, height, id }) + const id = getBrowserState().iframeId! + channel.postMessage({ + event: 'viewport', + width, + height, + iframeId: id, + } satisfies IframeViewportEvent) return new Promise((resolve, reject) => { channel.addEventListener('message', function handler(e) { - if (e.data.type === 'viewport:done' && e.data.id === id) { + if (e.data.event === 'viewport:done' && e.data.iframeId === id) { channel.removeEventListener('message', handler) resolve() } - if (e.data.type === 'viewport:fail' && e.data.id === id) { + if (e.data.event === 'viewport:fail' && e.data.iframeId === id) { channel.removeEventListener('message', handler) reject(new Error(e.data.error)) } diff --git a/packages/browser/src/client/tester/runner.ts b/packages/browser/src/client/tester/runner.ts index 358708147453..bc42eeecf00b 100644 --- a/packages/browser/src/client/tester/runner.ts +++ b/packages/browser/src/client/tester/runner.ts @@ -156,7 +156,12 @@ export function createBrowserRunner( const prefix = `/${/^\w:/.test(filepath) ? '@fs/' : ''}` const query = `browserv=${hash}` const importpath = `${prefix}${filepath}?${query}`.replace(/\/+/g, '/') - await import(/* @vite-ignore */ importpath) + try { + await import(/* @vite-ignore */ importpath) + } + catch (err) { + throw new Error(`Failed to import test file ${filepath}`, { cause: err }) + } } } } diff --git a/packages/browser/src/client/tester/tester.ts b/packages/browser/src/client/tester/tester.ts index 997b6ecba39e..4acfd3bc231c 100644 --- a/packages/browser/src/client/tester/tester.ts +++ b/packages/browser/src/client/tester/tester.ts @@ -1,5 +1,4 @@ -import type { BrowserRPC } from '@vitest/browser/client' -import type { IframeInitEvent } from '../types' +import type { BrowserRPC, IframeChannelEvent } from '@vitest/browser/client' import { channel, client, onCancel } from '@vitest/browser/client' import { page, server, userEvent } from '@vitest/browser/context' import { parse } from 'flatted' @@ -21,38 +20,76 @@ import { createSafeRpc } from './rpc' import { browserHashMap, initiateRunner } from './runner' import { CommandsManager } from './utils' -const cleanupSymbol = Symbol.for('vitest:component-cleanup') +channel.addEventListener('message', async (e) => { + await client.waitForConnection() -const url = new URL(location.href) -const reloadStart = url.searchParams.get('__reloadStart') + const data = e.data + debug('event from orchestrator', e.data) -const commands = new CommandsManager() -getBrowserState().commands = commands + if (!isEvent(data)) { + const error = new Error(`Unknown message: ${JSON.stringify(e.data)}`) + unhandledError(error, 'Uknown Iframe Message') + return + } -let contextSwitched = false + // ignore events to other iframes + if (!('iframeId' in data) || data.iframeId !== getBrowserState().iframeId) { + return + } -// webdiverio context depends on the iframe state, so we need to switch the context, -// we delay this in case the user doesn't use any userEvent commands to avoid the overhead -if (server.provider === 'webdriverio') { - let switchPromise: Promise | null = null + switch (data.event) { + case 'execute': { + const { method, files, context } = data + const state = getWorkerState() + const parsedContext = parse(context) - commands.onCommand(async () => { - if (switchPromise) { - await switchPromise + state.ctx.providedContext = parsedContext + state.providedContext = parsedContext + + if (method === 'collect') { + await executeTests('collect', files).catch(err => unhandledError(err, 'Collect Error')) + } + else { + await executeTests('run', files).catch(err => unhandledError(err, 'Run Error')) + } + break } - // if this is the first command, make sure we switched the command context to an iframe - if (!contextSwitched) { - const rpc = getWorkerState().rpc as any as BrowserRPC - switchPromise = rpc.wdioSwitchContext('iframe').finally(() => { - switchPromise = null - contextSwitched = true - }) - await switchPromise + case 'cleanup': { + await cleanup().catch(err => unhandledError(err, 'Cleanup Error')) + break + } + case 'prepare': { + await prepare().catch(err => unhandledError(err, 'Prepare Error')) + break + } + case 'viewport:done': + case 'viewport:fail': + case 'viewport': { + break } + default: { + const error = new Error(`Unknown event: ${(data as any).event}`) + unhandledError(error, 'Uknown Event') + } + } + + channel.postMessage({ + event: `response:${data.event}`, + iframeId: getBrowserState().iframeId!, }) -} +}) + +const url = new URL(location.href) +const reloadStart = url.searchParams.get('__reloadStart') +const iframeId = url.searchParams.get('iframeId')! -async function prepareTestEnvironment(method: 'run' | 'collect', files: string[]) { // TODO: all of this should be done only once except setMethod +const commands = new CommandsManager() +getBrowserState().commands = commands +getBrowserState().iframeId = iframeId + +let contextSwitched = false + +async function prepareTestEnvironment() { debug('trying to resolve runner', `${reloadStart}`) const config = getConfig() @@ -60,7 +97,6 @@ async function prepareTestEnvironment(method: 'run' | 'collect', files: string[] const state = getWorkerState() - state.ctx.files = files state.onCancel = onCancel state.rpc = rpc as any @@ -82,150 +118,130 @@ async function prepareTestEnvironment(method: 'run' | 'collect', files: string[] setupExpectDom() const runner = await initiateRunner(state, mocker, config) - runner.setMethod(method) - const version = url.searchParams.get('browserv') || '' - files.forEach((filename) => { - const currentVersion = browserHashMap.get(filename) - if (!currentVersion || currentVersion[1] !== version) { - browserHashMap.set(filename, version) - } - }) + // webdiverio context depends on the iframe state, so we need to switch the context, + // we delay this in case the user doesn't use any userEvent commands to avoid the overhead + if (server.provider === 'webdriverio') { + let switchPromise: Promise | null = null + + commands.onCommand(async () => { + if (switchPromise) { + await switchPromise + } + // if this is the first command, make sure we switched the command context to an iframe + if (!contextSwitched) { + switchPromise = rpc.wdioSwitchContext('iframe').finally(() => { + switchPromise = null + contextSwitched = true + }) + await switchPromise + } + }) + } return { runner, config, state, - rpc, } } -function done(files: string[]) { - channel.postMessage({ - type: 'done', - filenames: files, - id: getBrowserState().iframeId!, - }) -} +let preparedData: + | Awaited> + | undefined async function executeTests(method: 'run' | 'collect', files: string[]) { - await client.waitForConnection() - - debug('client is connected to ws server') - - let preparedData: - | Awaited> - | undefined - | false - - try { - preparedData = await prepareTestEnvironment(method, files) - } - catch (error: any) { - debug('runner cannot be loaded because it threw an error', error.stack || error.message) - await client.rpc.onUnhandledError({ - name: error.name, - message: error.message, - stack: String(error.stack), - }, 'Preload Error') - done(files) - return - } - - // page is reloading if (!preparedData) { - debug('page is reloading, waiting for the next run') - return + throw new Error(`Data was not properly initialized. This is a bug in Vitest. Please, open a new issue with reproduction.`) } debug('runner resolved successfully') - const { config, runner, state, rpc } = preparedData + const { runner, state } = preparedData + + state.ctx.files = files + runner.setMethod(method) + + const version = url.searchParams.get('browserv') || '' + files.forEach((filename) => { + const currentVersion = browserHashMap.get(filename) + if (!currentVersion || currentVersion[1] !== version) { + browserHashMap.set(filename, version) + } + }) state.durations.prepare = performance.now() - state.durations.prepare debug('prepare time', state.durations.prepare, 'ms') - try { - await Promise.all([ - setupCommonEnv(config), - startCoverageInsideWorker(config.coverage, executor, { isolate: config.browser.isolate }), - (async () => { - const VitestIndex = await import('vitest') - Object.defineProperty(window, '__vitest_index__', { - value: VitestIndex, - enumerable: false, - }) - })(), - ]) - - for (const file of files) { - state.filepath = file + for (const file of files) { + state.filepath = file - if (method === 'run') { - await startTests([file], runner) - } - else { - await collectTests([file], runner) - } - } - } - finally { - if (cleanupSymbol in page) { - try { - await (page[cleanupSymbol] as any)() - } - catch (error: any) { - await unhandledError(error, 'Cleanup Error') - } + if (method === 'run') { + await startTests([file], runner) } - // need to cleanup for each tester - // since playwright keyboard API is stateful on page instance level - await userEvent.cleanup() - .catch(error => unhandledError(error, 'Cleanup Error')) - - // if isolation is disabled, Vitest reuses the same iframe and we - // don't need to switch the context back at all - if (server.config.browser.isolate !== false && contextSwitched) { - await rpc.wdioSwitchContext('parent') - .catch(error => unhandledError(error, 'Cleanup Error')) + else { + await collectTests([file], runner) } - state.environmentTeardownRun = true - await stopCoverageInsideWorker(config.coverage, executor, { isolate: config.browser.isolate }).catch((error) => { - return unhandledError(error, 'Coverage Error') - }) - - debug('finished running tests') - done(files) } } -// listen when orchestrator sends a message -window.addEventListener('message', (e) => { - const data = JSON.parse(e.data) - debug('event from orchestrator', e.data) +async function prepare() { + preparedData = await prepareTestEnvironment() + + // page is reloading + debug('runner resolved successfully') + + const { config, state } = preparedData + + state.durations.prepare = performance.now() - state.durations.prepare + + debug('prepare time', state.durations.prepare, 'ms') - if (typeof data === 'object' && data?.event === 'init') { - const { method, files, context, iframeId } = data as IframeInitEvent - const state = getWorkerState() - const parsedContext = parse(context) + await Promise.all([ + setupCommonEnv(config), + startCoverageInsideWorker(config.coverage, executor, { isolate: config.browser.isolate }), + (async () => { + const VitestIndex = await import('vitest') + Object.defineProperty(window, '__vitest_index__', { + value: VitestIndex, + enumerable: false, + }) + })(), + ]) +} - state.ctx.providedContext = parsedContext - state.providedContext = parsedContext - getBrowserState().iframeId = iframeId +async function cleanup() { + const state = getWorkerState() + const config = getConfig() + const rpc = state.rpc as any as BrowserRPC + + const cleanupSymbol = Symbol.for('vitest:component-cleanup') - if (method === 'collect') { - executeTests('collect', files).catch(err => unhandledError(err, 'Collect Error')) + if (cleanupSymbol in page) { + try { + await (page[cleanupSymbol] as any)() } - else { - executeTests('run', files).catch(err => unhandledError(err, 'Run Error')) + catch (error: any) { + await unhandledError(error, 'Cleanup Error') } } - else { - const error = new Error(`Unknown event: ${data.event}`) - unhandledError(error, 'Uknown Event') + // need to cleanup for each tester + // since playwright keyboard API is stateful on page instance level + await userEvent.cleanup() + .catch(error => unhandledError(error, 'Cleanup Error')) + + // if isolation is disabled, Vitest reuses the same iframe and we + // don't need to switch the context back at all + if (contextSwitched) { + await rpc.wdioSwitchContext('parent') + .catch(error => unhandledError(error, 'Cleanup Error')) } -}) + state.environmentTeardownRun = true + await stopCoverageInsideWorker(config.coverage, executor, { isolate: config.browser.isolate }).catch((error) => { + return unhandledError(error, 'Coverage Error') + }) +} function unhandledError(e: Error, type: string) { return client.rpc.onUnhandledError({ @@ -241,3 +257,7 @@ function debug(...args: unknown[]) { client.rpc.debug(...args.map(String)) } } + +function isEvent(data: unknown): data is IframeChannelEvent { + return typeof data === 'object' && !!data && 'event' in data +} diff --git a/packages/browser/src/client/types.ts b/packages/browser/src/client/types.ts deleted file mode 100644 index dfe4b50182a8..000000000000 --- a/packages/browser/src/client/types.ts +++ /dev/null @@ -1,7 +0,0 @@ -export interface IframeInitEvent { - event: 'init' - method: 'run' | 'collect' - files: string[] - iframeId: string - context: string -} diff --git a/packages/browser/src/client/utils.ts b/packages/browser/src/client/utils.ts index 1a1317f9e6d5..1e974da8e0c0 100644 --- a/packages/browser/src/client/utils.ts +++ b/packages/browser/src/client/utils.ts @@ -1,4 +1,5 @@ -import type { BrowserTesterOptions, SerializedConfig, WorkerGlobalState } from 'vitest' +import type { SerializedConfig, WorkerGlobalState } from 'vitest' +import type { IframeOrchestrator } from './orchestrator' import type { CommandsManager } from './tester/utils' export async function importId(id: string): Promise { @@ -76,7 +77,7 @@ export interface BrowserRunnerState { sessionId: string testerId: string method: 'run' | 'collect' - createTesters?: (options: BrowserTesterOptions) => Promise + orchestrator?: IframeOrchestrator commands: CommandsManager cdp?: { on: (event: string, listener: (payload: any) => void) => void diff --git a/packages/browser/src/node/pool.ts b/packages/browser/src/node/pool.ts index 256e6ed9c10b..6d86cb185c04 100644 --- a/packages/browser/src/node/pool.ts +++ b/packages/browser/src/node/pool.ts @@ -212,28 +212,33 @@ class BrowserPool { await Promise.all([promise, page]) } + private getOrchestrator(sessionId: string) { + const orchestrator = this.orchestrators.get(sessionId) + if (!orchestrator) { + // TODO: handle this error + throw new Error(`Orchestrator not found for session ${sessionId}`) + } + return orchestrator + } + private runNextTest(method: 'run' | 'collect', sessionId: string) { const file = this._queue.shift() if (!file) { - this.readySessions.add(sessionId) - // the last worker finished running tests - if (this.readySessions.size === this.orchestrators.size) { - this._promise?.resolve() - this._promise = undefined - } + const orchestrator = this.getOrchestrator(sessionId) + orchestrator.cleanupTesters().finally(() => { + this.readySessions.add(sessionId) + // the last worker finished running tests + if (this.readySessions.size === this.orchestrators.size) { + this._promise?.resolve() + this._promise = undefined + } + }) return } if (!this._promise) { throw new Error(`Unexpected empty queue`) } - const orchestrator = this.orchestrators.get(sessionId) - if (!orchestrator) { - // TODO: handle this error - this._promise.reject( - new Error(`Orchestrator not found for session ${sessionId}`), - ) - return - } + const orchestrator = this.getOrchestrator(sessionId) this.setBreakpoint(sessionId, file).then(() => { // this starts running tests inside the orchestrator diff --git a/packages/browser/src/node/types.ts b/packages/browser/src/node/types.ts index 16a46c091cf7..d19620f86342 100644 --- a/packages/browser/src/node/types.ts +++ b/packages/browser/src/node/types.ts @@ -70,6 +70,7 @@ export interface WebSocketEvents export interface WebSocketBrowserEvents { onCancel: (reason: CancelReason) => void createTesters: (options: BrowserTesterOptions) => Promise + cleanupTesters: () => Promise cdpEvent: (event: string, payload: unknown) => void } diff --git a/packages/vitest/src/node/types/browser.ts b/packages/vitest/src/node/types/browser.ts index 7a88549af198..152d482c06a5 100644 --- a/packages/vitest/src/node/types/browser.ts +++ b/packages/vitest/src/node/types/browser.ts @@ -243,6 +243,7 @@ export interface BrowserServerStateSession { } export interface BrowserOrchestrator { + cleanupTesters: () => Promise createTesters: (options: BrowserTesterOptions) => Promise onCancel: (reason: CancelReason) => Promise $close: () => void From bbf0087493c4aea471f0831c8c997705f2b59527 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Fri, 14 Mar 2025 19:18:34 +0100 Subject: [PATCH 12/24] fix: correctly fail the test run if html threw an error --- .../browser/src/client/tester/unhandled.ts | 72 ------------------- packages/browser/src/node/pool.ts | 42 ++++++----- .../browser/src/node/providers/playwright.ts | 2 +- packages/browser/src/node/serverTester.ts | 10 +-- packages/vitest/src/node/browser/sessions.ts | 10 ++- packages/vitest/src/node/types/browser.ts | 2 +- 6 files changed, 37 insertions(+), 101 deletions(-) delete mode 100644 packages/browser/src/client/tester/unhandled.ts diff --git a/packages/browser/src/client/tester/unhandled.ts b/packages/browser/src/client/tester/unhandled.ts deleted file mode 100644 index 3ddd5cd57896..000000000000 --- a/packages/browser/src/client/tester/unhandled.ts +++ /dev/null @@ -1,72 +0,0 @@ -import { client } from '@vitest/browser/client' - -function on(event: string, listener: (...args: any[]) => void) { - window.addEventListener(event, listener) - return () => window.removeEventListener(event, listener) -} - -function serializeError(unhandledError: any) { - if (typeof unhandledError !== 'object' || !unhandledError) { - return { - message: String(unhandledError), - } - } - - return { - name: unhandledError.name, - message: unhandledError.message, - stack: String(unhandledError.stack), - } -} - -function catchWindowErrors(cb: (e: ErrorEvent) => void) { - let userErrorListenerCount = 0 - function throwUnhandlerError(e: ErrorEvent) { - if (userErrorListenerCount === 0 && e.error != null) { - cb(e) - } - else { - console.error(e.error) - } - } - const addEventListener = window.addEventListener.bind(window) - const removeEventListener = window.removeEventListener.bind(window) - window.addEventListener('error', throwUnhandlerError) - window.addEventListener = function ( - ...args: [any, any, any] - ) { - if (args[0] === 'error') { - userErrorListenerCount++ - } - return addEventListener.apply(this, args) - } - window.removeEventListener = function ( - ...args: [any, any, any] - ) { - if (args[0] === 'error' && userErrorListenerCount) { - userErrorListenerCount-- - } - return removeEventListener.apply(this, args) - } - return function clearErrorHandlers() { - window.removeEventListener('error', throwUnhandlerError) - } -} - -function registerUnexpectedErrors() { - catchWindowErrors(event => - reportUnexpectedError('Error', event.error), - ) - on('unhandledrejection', event => - reportUnexpectedError('Unhandled Rejection', event.reason)) -} - -async function reportUnexpectedError( - type: string, - error: any, -) { - const processedError = serializeError(error) - await client.rpc.onUnhandledError(processedError, type) -} - -registerUnexpectedErrors() diff --git a/packages/browser/src/node/pool.ts b/packages/browser/src/node/pool.ts index 6d86cb185c04..92ec0d33762b 100644 --- a/packages/browser/src/node/pool.ts +++ b/packages/browser/src/node/pool.ts @@ -55,11 +55,11 @@ export function createBrowserPool(vitest: Vitest): ProcessPool { ) } - const pool = new BrowserPool(project, { + const pool: BrowserPool = new BrowserPool(project, { maxWorkers: getThreadsCount(project), origin, - worker: (sessionId) => { - return project.vitest._browserSessions.createSession(sessionId, project) + session(sessionId) { + return project.vitest._browserSessions.createSession(sessionId, project, pool) }, }) projectPools.set(project, pool) @@ -104,12 +104,11 @@ export function createBrowserPool(vitest: Vitest): ProcessPool { function getThreadsCount(project: TestProject) { const config = project.config.browser - if (!config.headless || !project.browser!.provider.supportsParallelism) { - return 1 - } - - // FIXME: allow isolate with parallelism - if (!config.fileParallelism || project.config.isolate === false) { + if ( + !config.headless + || !config.fileParallelism + || !project.browser!.provider.supportsParallelism + ) { return 1 } @@ -151,19 +150,25 @@ class BrowserPool { private options: { maxWorkers: number origin: string - worker: (sessionId: string) => Promise + session: (sessionId: string) => Promise }, ) {} - cancel() { + public cancel(): void { this._queue = [] } + public reject(error: Error): void { + this._promise?.reject(error) + this._promise = undefined + this.cancel() + } + get orchestrators() { return this.project.browser!.state.orchestrators } - async runTests(method: 'run' | 'collect', files: string[]) { + async runTests(method: 'run' | 'collect', files: string[]): Promise { this._promise ??= createDefer() if (!files.length) { @@ -205,11 +210,14 @@ class BrowserPool { } private async openPage(sessionId: string) { - const promise = this.options.worker(sessionId) + const sessionPromise = this.options.session(sessionId) const url = new URL('/', this.options.origin) url.searchParams.set('sessionId', sessionId) - const page = this.project.browser!.provider.openPage(sessionId, url.toString()) - await Promise.all([promise, page]) + const pagePromise = this.project.browser!.provider.openPage( + sessionId, + url.toString(), + ) + await Promise.all([sessionPromise, pagePromise]) } private getOrchestrator(sessionId: string) { @@ -258,9 +266,9 @@ class BrowserPool { if (error instanceof Error && error.message.startsWith('[birpc] rpc is closed')) { return } - this._promise?.reject(error) + this.reject(error) }) - }).catch(err => this._promise?.reject(err)) + }).catch(err => this.reject(err)) } async setBreakpoint(sessionId: string, file: string) { diff --git a/packages/browser/src/node/providers/playwright.ts b/packages/browser/src/node/providers/playwright.ts index 33960e7ea09c..97b157e077d3 100644 --- a/packages/browser/src/node/providers/playwright.ts +++ b/packages/browser/src/node/providers/playwright.ts @@ -194,7 +194,7 @@ export class PlaywrightBrowserProvider implements BrowserProvider { // unhandled page crashes will hang vitest process page.on('crash', () => { const session = this.project.vitest._browserSessions.getSession(sessionId) - session?.reject(new Error('Page crashed when executing tests')) + session?.fail(new Error('Page crashed when executing tests')) }) return page diff --git a/packages/browser/src/node/serverTester.ts b/packages/browser/src/node/serverTester.ts index cbbe8e42993f..92603616b07d 100644 --- a/packages/browser/src/node/serverTester.ts +++ b/packages/browser/src/node/serverTester.ts @@ -68,17 +68,11 @@ export async function resolveTester( const html = replacer(indexhtml, { __VITEST_FAVICON__: globalServer.faviconUrl, __VITEST_INJECTOR__: injector, - // __VITEST_APPEND__: ` - // __vitest_browser_runner__.runningFiles = ${test} - // __vitest_browser_runner__.iframeId = ${iframeId} - // __vitest_browser_runner__.${method === 'run' ? 'runTests' : 'collectTests'}(__vitest_browser_runner__.runningFiles) - // document.querySelector('script[data-vitest-append]').remove() - // `, }) return html } - catch (err) { - session.reject(err) + catch (err: any) { + session.fail(err) next(err) } } diff --git a/packages/vitest/src/node/browser/sessions.ts b/packages/vitest/src/node/browser/sessions.ts index 15497c4e8e47..ece30ff3e783 100644 --- a/packages/vitest/src/node/browser/sessions.ts +++ b/packages/vitest/src/node/browser/sessions.ts @@ -13,7 +13,8 @@ export class BrowserSessions { this.sessions.delete(sessionId) } - createSession(sessionId: string, project: TestProject): Promise { + createSession(sessionId: string, project: TestProject, pool: { reject: (error: Error) => void }): Promise { + // this promise only waits for the WS connection with the orhcestrator to be established const defer = createDefer() const timeout = setTimeout(() => { @@ -26,7 +27,12 @@ export class BrowserSessions { defer.resolve() clearTimeout(timeout) }, - reject: defer.reject, + // this fails the whole test run and cancels the pool + fail: (error: Error) => { + defer.resolve() + clearTimeout(timeout) + pool.reject(error) + }, }) return defer } diff --git a/packages/vitest/src/node/types/browser.ts b/packages/vitest/src/node/types/browser.ts index 152d482c06a5..1fdedcefbc1f 100644 --- a/packages/vitest/src/node/types/browser.ts +++ b/packages/vitest/src/node/types/browser.ts @@ -239,7 +239,7 @@ export interface BrowserCommandContext { export interface BrowserServerStateSession { project: TestProject connected: () => void - reject: (v: unknown) => void + fail: (v: Error) => void } export interface BrowserOrchestrator { From 3b2d9024516f99cec3bd92e3541fdeb05bac9660 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Mon, 17 Mar 2025 12:56:53 +0100 Subject: [PATCH 13/24] chore: chore --- packages/browser/src/client/public/error-catcher.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/browser/src/client/public/error-catcher.js b/packages/browser/src/client/public/error-catcher.js index 77262bd488fb..081b9024f17a 100644 --- a/packages/browser/src/client/public/error-catcher.js +++ b/packages/browser/src/client/public/error-catcher.js @@ -77,9 +77,9 @@ async function reportUnexpectedError( if (!state.runTests || !__vitest_worker__.current) { channel.postMessage({ - type: 'done', - filenames: state.files, - id: state.iframeId, + // TODO: what to do in this case now? + event: 'response:???', + iframeId: state.iframeId, }) } } From e67a7ebe1b32324beaf4c6366411d55e4cae7533 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Mon, 24 Mar 2025 17:14:46 +0100 Subject: [PATCH 14/24] chore: fix snapshot --- test/core/test/__snapshots__/mocked.test.ts.snap | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/core/test/__snapshots__/mocked.test.ts.snap b/test/core/test/__snapshots__/mocked.test.ts.snap index 0e56affcf4bd..a2eb4faa5f69 100644 --- a/test/core/test/__snapshots__/mocked.test.ts.snap +++ b/test/core/test/__snapshots__/mocked.test.ts.snap @@ -74,10 +74,6 @@ Number of calls: 3 exports[`mocked function which fails on toReturnWith > zero call 1`] = ` "expected "spy" to return with: 2 at least once -Received: - - - Number of calls: 0 " `; From 741fd9688df88d605ebe34f1160f5d88ace9a02c Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Mon, 24 Mar 2025 17:43:25 +0100 Subject: [PATCH 15/24] fix: throw an error if createTester hangs --- packages/browser/src/node/pool.ts | 8 ++++---- packages/browser/src/node/rpc.ts | 4 +++- packages/vitest/src/node/browser/sessions.ts | 2 +- test/browser/specs/browser-crash.test.ts | 4 +++- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/browser/src/node/pool.ts b/packages/browser/src/node/pool.ts index 92ec0d33762b..b9cef63d25bc 100644 --- a/packages/browser/src/node/pool.ts +++ b/packages/browser/src/node/pool.ts @@ -142,6 +142,7 @@ function escapePathToRegexp(path: string): string { class BrowserPool { private _queue: string[] = [] private _promise: DeferPromise | undefined + private _providedContext: string | undefined private readySessions = new Set() @@ -176,6 +177,8 @@ class BrowserPool { return this._promise } + this._providedContext = stringify(this.project.getProvidedContext()) + this._queue.push(...files) this.readySessions.forEach((sessionId) => { @@ -256,16 +259,13 @@ class BrowserPool { files: [file], // this will be parsed by the test iframe, not the orchestrator // so we need to stringify it first to avoid double serialization - providedContext: stringify(this.project.getProvidedContext()), + providedContext: this._providedContext || '[]', }, ) .then(() => { this.runNextTest(method, sessionId) }) .catch((error) => { - if (error instanceof Error && error.message.startsWith('[birpc] rpc is closed')) { - return - } this.reject(error) }) }).catch(err => this.reject(err)) diff --git a/packages/browser/src/node/rpc.ts b/packages/browser/src/node/rpc.ts index 6557dd4e756c..ad90f8a3f0a6 100644 --- a/packages/browser/src/node/rpc.ts +++ b/packages/browser/src/node/rpc.ts @@ -85,8 +85,10 @@ export function setupBrowserRpc(globalServer: ParentBrowserProject): void { clients.delete(rpcId) globalServer.removeCDPHandler(rpcId) if (type === 'orchestrator') { - vitest._browserSessions.forgetSession(sessionId) + vitest._browserSessions.destroySession(sessionId) } + // this will reject any hanging methods if there are any + rpc.$close() }) }) }) diff --git a/packages/vitest/src/node/browser/sessions.ts b/packages/vitest/src/node/browser/sessions.ts index ece30ff3e783..c197a56c9ca5 100644 --- a/packages/vitest/src/node/browser/sessions.ts +++ b/packages/vitest/src/node/browser/sessions.ts @@ -9,7 +9,7 @@ export class BrowserSessions { return this.sessions.get(sessionId) } - forgetSession(sessionId: string): void { + destroySession(sessionId: string): void { this.sessions.delete(sessionId) } diff --git a/test/browser/specs/browser-crash.test.ts b/test/browser/specs/browser-crash.test.ts index 89d0b950e64e..85a8244df434 100644 --- a/test/browser/specs/browser-crash.test.ts +++ b/test/browser/specs/browser-crash.test.ts @@ -13,5 +13,7 @@ test.runIf(provider === 'playwright')('fails gracefully when browser crashes', a }, }) - expect(stderr).toContain('Page crashed when executing tests') + // TODO 2025-03-24 whtn https://github.com/antfu/birpc/pull/23 is merged, + // provide a better error message + expect(stderr).toContain('rpc is closed, cannot call "createTesters"') }) From 67f95779d5b20f8b59b7c2337b56d3b7b74989c2 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Mon, 24 Mar 2025 17:50:19 +0100 Subject: [PATCH 16/24] test: fix cdp test --- packages/browser/src/node/providers/playwright.ts | 14 ++------------ test/browser/test/cdp.test.ts | 3 ++- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/packages/browser/src/node/providers/playwright.ts b/packages/browser/src/node/providers/playwright.ts index 97b157e077d3..a20910553deb 100644 --- a/packages/browser/src/node/providers/playwright.ts +++ b/packages/browser/src/node/providers/playwright.ts @@ -10,6 +10,7 @@ import type { import type { BrowserProvider, BrowserProviderInitializationOptions, + CDPSession, TestProject, } from 'vitest/node' @@ -191,12 +192,6 @@ export class PlaywrightBrowserProvider implements BrowserProvider { }) } - // unhandled page crashes will hang vitest process - page.on('crash', () => { - const session = this.project.vitest._browserSessions.getSession(sessionId) - session?.fail(new Error('Page crashed when executing tests')) - }) - return page } @@ -206,12 +201,7 @@ export class PlaywrightBrowserProvider implements BrowserProvider { await browserPage.goto(url, { timeout: 0 }) } - async getCDPSession(sessionid: string): Promise<{ - send: (method: string, params: any) => Promise - on: (event: string, listener: (...args: any[]) => void) => void - off: (event: string, listener: (...args: any[]) => void) => void - once: (event: string, listener: (...args: any[]) => void) => void - }> { + async getCDPSession(sessionid: string): Promise { const page = this.getPage(sessionid) const cdp = await page.context().newCDPSession(page) return { diff --git a/test/browser/test/cdp.test.ts b/test/browser/test/cdp.test.ts index 026d9adbe200..fce095e4165a 100644 --- a/test/browser/test/cdp.test.ts +++ b/test/browser/test/cdp.test.ts @@ -7,9 +7,10 @@ describe.runIf( it('cdp sends events correctly', async () => { const messageAdded = vi.fn() + await cdp().send('Console.enable') + cdp().on('Console.messageAdded', messageAdded) - await cdp().send('Console.enable') onTestFinished(async () => { await cdp().send('Console.disable') }) From 1f552647870cfb93f4d7ebe37012e9223b626c44 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Mon, 24 Mar 2025 18:33:06 +0100 Subject: [PATCH 17/24] fix: check for iframe if UI rerendered it --- packages/browser/src/client/orchestrator.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/browser/src/client/orchestrator.ts b/packages/browser/src/client/orchestrator.ts index 5f88ca048465..4ac95255dee1 100644 --- a/packages/browser/src/client/orchestrator.ts +++ b/packages/browser/src/client/orchestrator.ts @@ -91,6 +91,13 @@ export class IframeOrchestrator { } private async runNonIsolatedTests(container: HTMLDivElement, options: BrowserTesterOptions) { + // if the iframe was somehow removed from the DOM, recreate it + const existingIframe = this.iframes.get(ID_ALL) + if (existingIframe && !document.body.contains(existingIframe)) { + debug('recreating iframe due to the missing element') + this.recreateNonIsolatedIframe = true + } + if (this.recreateNonIsolatedIframe) { // recreate a new non-isolated iframe during watcher reruns // because we called "cleanup" in the previous run @@ -98,9 +105,11 @@ export class IframeOrchestrator { this.recreateNonIsolatedIframe = false this.iframes.get(ID_ALL)!.remove() this.iframes.delete(ID_ALL) + debug('recreate non-isolated iframe') } if (!this.iframes.has(ID_ALL)) { + debug('preparing non-isolated iframe') await this.prepareIframe(container, ID_ALL) } @@ -109,6 +118,7 @@ export class IframeOrchestrator { const iframe = this.iframes.get(ID_ALL)! await setIframeViewport(iframe, width, height) + debug('run non-isolated tests', options.files.join(', ')) await sendEventToIframe({ event: 'execute', iframeId: ID_ALL, From a404acf20cebf05cd44c66eb7b18795fdd030143 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Mon, 24 Mar 2025 18:33:10 +0100 Subject: [PATCH 18/24] chore: add debugs --- packages/browser/src/node/pool.ts | 46 ++++++++++++++++++------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/packages/browser/src/node/pool.ts b/packages/browser/src/node/pool.ts index b9cef63d25bc..96590a3a7f32 100644 --- a/packages/browser/src/node/pool.ts +++ b/packages/browser/src/node/pool.ts @@ -10,10 +10,9 @@ import crypto from 'node:crypto' import * as nodeos from 'node:os' import { createDefer } from '@vitest/utils' import { stringify } from 'flatted' -// import { relative } from 'pathe' -// import { createDebugger } from 'vitest/node' +import { createDebugger } from 'vitest/node' -// const debug = createDebugger('vitest:browser:pool') +const debug = createDebugger('vitest:browser:pool') export function createBrowserPool(vitest: Vitest): ProcessPool { const providers = new Set() @@ -27,18 +26,6 @@ export function createBrowserPool(vitest: Vitest): ProcessPool { ? Math.max(Math.floor(numCpus / 2), 1) : Math.max(numCpus - 1, 1) - const executeTests = async ( - method: 'run' | 'collect', - pool: BrowserPool, - project: TestProject, - files: string[], - ) => { - vitest.state.clearFiles(project, files) - providers.add(project.browser!.provider) - - await pool.runTests(method, files) - } - const projectPools = new WeakMap() const ensurePool = (project: TestProject) => { @@ -46,6 +33,8 @@ export function createBrowserPool(vitest: Vitest): ProcessPool { return projectPools.get(project)! } + debug?.('creating pool for project %s', project.name) + const resolvedUrls = project.browser!.vite.resolvedUrls const origin = resolvedUrls?.local[0] ?? resolvedUrls?.network[0] @@ -97,7 +86,10 @@ export function createBrowserPool(vitest: Vitest): ProcessPool { } const pool = ensurePool(project) - await executeTests(method, pool, project, files) + vitest.state.clearFiles(project, files) + providers.add(project.browser!.provider) + + await pool.runTests(method, files) }), ) } @@ -226,8 +218,7 @@ class BrowserPool { private getOrchestrator(sessionId: string) { const orchestrator = this.orchestrators.get(sessionId) if (!orchestrator) { - // TODO: handle this error - throw new Error(`Orchestrator not found for session ${sessionId}`) + throw new Error(`Orchestrator not found for session ${sessionId}. This is a bug in Vitest. Please, open a new issue with reproduction.`) } return orchestrator } @@ -235,13 +226,15 @@ class BrowserPool { private runNextTest(method: 'run' | 'collect', sessionId: string) { const file = this._queue.shift() if (!file) { + debug?.('[%s] no more tests to run', sessionId) const orchestrator = this.getOrchestrator(sessionId) - orchestrator.cleanupTesters().finally(() => { + orchestrator.cleanupTesters().catch(error => this.reject(error)).finally(() => { this.readySessions.add(sessionId) // the last worker finished running tests if (this.readySessions.size === this.orchestrators.size) { this._promise?.resolve() this._promise = undefined + debug?.('all tests finished running') } }) return @@ -250,6 +243,7 @@ class BrowserPool { throw new Error(`Unexpected empty queue`) } const orchestrator = this.getOrchestrator(sessionId) + debug?.('[%s] run test %s', sessionId, file) this.setBreakpoint(sessionId, file).then(() => { // this starts running tests inside the orchestrator @@ -263,9 +257,22 @@ class BrowserPool { }, ) .then(() => { + debug?.('[%s] test %s finished running', sessionId, file) this.runNextTest(method, sessionId) }) .catch((error) => { + // if user cancells the test run manually, ignore the error and exit gracefully + if ( + this.project.vitest.isCancelling + && error instanceof Error + && error.message.startsWith('[birpc] rpc is closed') + ) { + this.cancel() + this._promise?.resolve() + this._promise = undefined + return + } + debug?.('[%s] error during %s test run: %s', sessionId, file, error) this.reject(error) }) }).catch(err => this.reject(err)) @@ -282,6 +289,7 @@ class BrowserPool { throw new Error('Unable to set breakpoint, CDP not supported') } + debug?.('[%s] set breakpoint for %s', sessionId, file) const session = await provider.getCDPSession(sessionId) await session.send('Debugger.enable', {}) await session.send('Debugger.setBreakpointByUrl', { From 3275269749e5ceaa23ba442cf90a0ba82e1f0ebb Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Mon, 24 Mar 2025 19:08:11 +0100 Subject: [PATCH 19/24] fix: correctly run tests --- packages/browser/src/client/orchestrator.ts | 13 ++++----- packages/browser/src/client/tester/tester.ts | 29 +++++++++----------- packages/browser/src/node/pool.ts | 2 +- 3 files changed, 19 insertions(+), 25 deletions(-) diff --git a/packages/browser/src/client/orchestrator.ts b/packages/browser/src/client/orchestrator.ts index 4ac95255dee1..747364f995f0 100644 --- a/packages/browser/src/client/orchestrator.ts +++ b/packages/browser/src/client/orchestrator.ts @@ -37,7 +37,11 @@ export class IframeOrchestrator { if (config.browser.ui) { container.className = 'absolute origin-top mt-[8px]' container.parentElement!.setAttribute('data-ready', 'true') - container.textContent = '' + // in non-isolated mode this will also remove the iframe, + // so we only do this once + if (container.textContent) { + container.textContent = '' + } } if (config.browser.isolate === false) { @@ -91,13 +95,6 @@ export class IframeOrchestrator { } private async runNonIsolatedTests(container: HTMLDivElement, options: BrowserTesterOptions) { - // if the iframe was somehow removed from the DOM, recreate it - const existingIframe = this.iframes.get(ID_ALL) - if (existingIframe && !document.body.contains(existingIframe)) { - debug('recreating iframe due to the missing element') - this.recreateNonIsolatedIframe = true - } - if (this.recreateNonIsolatedIframe) { // recreate a new non-isolated iframe during watcher reruns // because we called "cleanup" in the previous run diff --git a/packages/browser/src/client/tester/tester.ts b/packages/browser/src/client/tester/tester.ts index 4acfd3bc231c..7c4463d6a02c 100644 --- a/packages/browser/src/client/tester/tester.ts +++ b/packages/browser/src/client/tester/tester.ts @@ -20,11 +20,16 @@ import { createSafeRpc } from './rpc' import { browserHashMap, initiateRunner } from './runner' import { CommandsManager } from './utils' +const debugVar = getConfig().env.VITEST_BROWSER_DEBUG +const debug = debugVar && debugVar !== 'false' + ? (...args: unknown[]) => client.rpc.debug?.(...args.map(String)) + : undefined + channel.addEventListener('message', async (e) => { await client.waitForConnection() const data = e.data - debug('event from orchestrator', e.data) + debug?.('event from orchestrator', JSON.stringify(e.data)) if (!isEvent(data)) { const error = new Error(`Unknown message: ${JSON.stringify(e.data)}`) @@ -90,7 +95,7 @@ getBrowserState().iframeId = iframeId let contextSwitched = false async function prepareTestEnvironment() { - debug('trying to resolve runner', `${reloadStart}`) + debug?.('trying to resolve runner', `${reloadStart}`) const config = getConfig() const rpc = createSafeRpc(client) @@ -139,6 +144,8 @@ async function prepareTestEnvironment() { }) } + state.durations.prepare = performance.now() - state.durations.prepare + return { runner, config, @@ -155,7 +162,7 @@ async function executeTests(method: 'run' | 'collect', files: string[]) { throw new Error(`Data was not properly initialized. This is a bug in Vitest. Please, open a new issue with reproduction.`) } - debug('runner resolved successfully') + debug?.('runner resolved successfully') const { runner, state } = preparedData @@ -170,9 +177,7 @@ async function executeTests(method: 'run' | 'collect', files: string[]) { } }) - state.durations.prepare = performance.now() - state.durations.prepare - - debug('prepare time', state.durations.prepare, 'ms') + debug?.('prepare time', state.durations.prepare, 'ms') for (const file of files) { state.filepath = file @@ -190,13 +195,13 @@ async function prepare() { preparedData = await prepareTestEnvironment() // page is reloading - debug('runner resolved successfully') + debug?.('runner resolved successfully') const { config, state } = preparedData state.durations.prepare = performance.now() - state.durations.prepare - debug('prepare time', state.durations.prepare, 'ms') + debug?.('prepare time', state.durations.prepare, 'ms') await Promise.all([ setupCommonEnv(config), @@ -250,14 +255,6 @@ function unhandledError(e: Error, type: string) { stack: e.stack, }, type).catch(() => {}) } - -function debug(...args: unknown[]) { - const debug = getConfig().env.VITEST_BROWSER_DEBUG - if (debug && debug !== 'false') { - client.rpc.debug(...args.map(String)) - } -} - function isEvent(data: unknown): data is IframeChannelEvent { return typeof data === 'object' && !!data && 'event' in data } diff --git a/packages/browser/src/node/pool.ts b/packages/browser/src/node/pool.ts index 96590a3a7f32..ebaeb6d2058d 100644 --- a/packages/browser/src/node/pool.ts +++ b/packages/browser/src/node/pool.ts @@ -253,7 +253,7 @@ class BrowserPool { files: [file], // this will be parsed by the test iframe, not the orchestrator // so we need to stringify it first to avoid double serialization - providedContext: this._providedContext || '[]', + providedContext: this._providedContext || '[{}]', }, ) .then(() => { From 05ae5619fab8919d9f5dbea060c1f498f0424da9 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Tue, 25 Mar 2025 06:45:35 +0100 Subject: [PATCH 20/24] feat: update the error message when the page crushes --- packages/browser/src/node/rpc.ts | 4 ++- pnpm-lock.yaml | 17 +++++++---- pnpm-workspace.yaml | 2 +- .../fixtures/browser-crash/vitest.config.ts | 30 ++++++++++++++----- test/browser/specs/browser-crash.test.ts | 10 ++----- 5 files changed, 40 insertions(+), 23 deletions(-) diff --git a/packages/browser/src/node/rpc.ts b/packages/browser/src/node/rpc.ts index ad90f8a3f0a6..e6a209387be0 100644 --- a/packages/browser/src/node/rpc.ts +++ b/packages/browser/src/node/rpc.ts @@ -88,7 +88,9 @@ export function setupBrowserRpc(globalServer: ParentBrowserProject): void { vitest._browserSessions.destroySession(sessionId) } // this will reject any hanging methods if there are any - rpc.$close() + rpc.$close( + new Error(`[vitest] Browser connection was closed while running tests. Was the page closed unexpectedly?`), + ) }) }) }) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6c7d2999fe0c..f7bbaa6e9e1d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -55,8 +55,8 @@ catalogs: specifier: ^8.3.4 version: 8.3.4 birpc: - specifier: 0.2.19 - version: 0.2.19 + specifier: 2.3.0 + version: 2.3.0 cac: specifier: ^6.7.14 version: 6.7.14 @@ -525,7 +525,7 @@ importers: version: 9.9.0 birpc: specifier: 'catalog:' - version: 0.2.19 + version: 2.3.0 flatted: specifier: 'catalog:' version: 3.3.3 @@ -874,7 +874,7 @@ importers: version: 0.7.2 birpc: specifier: 'catalog:' - version: 0.2.19 + version: 2.3.0 codemirror: specifier: ^5.65.18 version: 5.65.18 @@ -1084,7 +1084,7 @@ importers: version: 8.3.4 birpc: specifier: 'catalog:' - version: 0.2.19 + version: 2.3.0 cac: specifier: 'catalog:' version: 6.7.14(patch_hash=a8f0f3517a47ce716ed90c0cfe6ae382ab763b021a664ada2a608477d0621588) @@ -1151,7 +1151,7 @@ importers: dependencies: birpc: specifier: 'catalog:' - version: 0.2.19 + version: 2.3.0 flatted: specifier: 'catalog:' version: 3.3.3 @@ -4898,6 +4898,9 @@ packages: birpc@0.2.19: resolution: {integrity: sha512-5WeXXAvTmitV1RqJFppT5QtUiz2p1mRSYU000Jkft5ZUCLJIk4uQriYNO50HknxKwM6jd8utNc66K1qGIwwWBQ==} + birpc@2.3.0: + resolution: {integrity: sha512-ijbtkn/F3Pvzb6jHypHRyve2QApOCZDR25D/VnkY2G/lBNcXCTsnsCxgY4k4PkVB7zfwzYbY3O9Lcqe3xufS5g==} + bl@4.1.0: resolution: {integrity: sha512-1W07cM9gS6DcLperZfFSj+bWLtaPGSOHWhPiGzXmvVJbRLdG82sH/Kn8EtW1VqWVA54AKf2h5k5BbnIbwF3h6w==} @@ -13292,6 +13295,8 @@ snapshots: birpc@0.2.19: {} + birpc@2.3.0: {} + bl@4.1.0: dependencies: buffer: 5.7.1 diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index f63ca962d0c3..597404d6ac15 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -21,7 +21,7 @@ catalog: '@vitejs/plugin-vue': ^5.2.1 '@vueuse/core': ^12.7.0 acorn-walk: ^8.3.4 - birpc: 0.2.19 + birpc: 2.3.0 cac: ^6.7.14 chai: ^5.2.0 debug: ^4.4.0 diff --git a/test/browser/fixtures/browser-crash/vitest.config.ts b/test/browser/fixtures/browser-crash/vitest.config.ts index 3f23dfde5b97..f6fec5a38a0a 100644 --- a/test/browser/fixtures/browser-crash/vitest.config.ts +++ b/test/browser/fixtures/browser-crash/vitest.config.ts @@ -4,16 +4,30 @@ import { instances, provider } from '../../settings' import { BrowserCommand } from 'vitest/node' const forceCrash: BrowserCommand<[]> = async (context) => { - const browser = context.context.browser().browserType().name() - if (browser === 'chromium') { - await context.page.goto('chrome://crash') - } + if (context.provider.name === 'playwright') { + const browser = context.context.browser().browserType().name() + if (browser === 'chromium') { + await context.page.goto('chrome://crash') + } - if (browser === 'firefox') { - await context.page.goto('about:crashcontent') - } + if (browser === 'firefox') { + await context.page.goto('about:crashcontent') + } - throw new Error(`Browser crash not supported for ${browser}`) + throw new Error(`Browser crash not supported for ${browser}`) + } + if (context.provider.name === 'webdriverio') { + // @ts-expect-error not typed + const browser = context.browser as any + const name = context.project.config.browser.name + if (name === 'chrome') { + await browser.url('chrome://crash') + } + if (name === 'firefox') { + await browser.url('about:crashcontent') + } + throw new Error(`Browser crash not supported for ${name}`) + } } export default defineConfig({ diff --git a/test/browser/specs/browser-crash.test.ts b/test/browser/specs/browser-crash.test.ts index 85a8244df434..daaa1865bcd7 100644 --- a/test/browser/specs/browser-crash.test.ts +++ b/test/browser/specs/browser-crash.test.ts @@ -1,9 +1,7 @@ import { expect, test } from 'vitest' -import { instances, provider, runBrowserTests } from './utils' +import { instances, runBrowserTests } from './utils' -// TODO handle webdriverio. Currently they -// expose no trustable way to detect browser crashes. -test.runIf(provider === 'playwright')('fails gracefully when browser crashes', async () => { +test('fails gracefully when browser crashes', async () => { const { stderr } = await runBrowserTests({ root: './fixtures/browser-crash', reporters: [['verbose', { isTTY: false }]], @@ -13,7 +11,5 @@ test.runIf(provider === 'playwright')('fails gracefully when browser crashes', a }, }) - // TODO 2025-03-24 whtn https://github.com/antfu/birpc/pull/23 is merged, - // provide a better error message - expect(stderr).toContain('rpc is closed, cannot call "createTesters"') + expect(stderr).toContain('Browser connection was closed while running tests. Was the page closed unexpectedly?') }) From d7559b74586b048789f4892a92aefde9857a9f55 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Wed, 26 Mar 2025 08:12:54 +0100 Subject: [PATCH 21/24] chore: support array in toReportPassedTest --- packages/ui/client/auto-imports.d.ts | 1 + .../project1/vitest.config.ts | 1 + test/browser/setup.unit.ts | 25 +++++++++++++++---- test/browser/specs/setup-file.test.ts | 5 +--- 4 files changed, 23 insertions(+), 9 deletions(-) diff --git a/packages/ui/client/auto-imports.d.ts b/packages/ui/client/auto-imports.d.ts index d25b21e28fe7..8a28dee615ef 100644 --- a/packages/ui/client/auto-imports.d.ts +++ b/packages/ui/client/auto-imports.d.ts @@ -31,6 +31,7 @@ declare global { const createInjectionState: typeof import('@vueuse/core')['createInjectionState'] const createModuleLabelItem: typeof import('./composables/module-graph')['createModuleLabelItem'] const createReactiveFn: typeof import('@vueuse/core')['createReactiveFn'] + const createRef: typeof import('@vueuse/core')['createRef'] const createReusableTemplate: typeof import('@vueuse/core')['createReusableTemplate'] const createSharedComposable: typeof import('@vueuse/core')['createSharedComposable'] const createTemplatePromise: typeof import('@vueuse/core')['createTemplatePromise'] diff --git a/test/browser/fixtures/mocking-out-of-root/project1/vitest.config.ts b/test/browser/fixtures/mocking-out-of-root/project1/vitest.config.ts index 87eb625028f0..87ea3c9c9ae5 100644 --- a/test/browser/fixtures/mocking-out-of-root/project1/vitest.config.ts +++ b/test/browser/fixtures/mocking-out-of-root/project1/vitest.config.ts @@ -12,6 +12,7 @@ export default defineConfig({ enabled: true, provider: provider, screenshotFailures: false, + headless: true, instances, }, }, diff --git a/test/browser/setup.unit.ts b/test/browser/setup.unit.ts index dd69c73378c1..4cd3a2b24d82 100644 --- a/test/browser/setup.unit.ts +++ b/test/browser/setup.unit.ts @@ -1,3 +1,4 @@ +import type { BrowserInstanceOption } from 'vitest/node' import { expect } from 'vitest' interface SummaryOptions { @@ -5,12 +6,26 @@ interface SummaryOptions { } expect.extend({ - toReportPassedTest(stdout: string, testName: string, testProject?: string) { - const includePattern = `✓ ${testProject ? `|${testProject}| ` : ''}${testName}` - const pass = stdout.includes(`✓ ${testProject ? `|${testProject}| ` : ''}${testName}`) + toReportPassedTest(stdout: string, testName: string, testProject?: string | BrowserInstanceOption[]) { + const checks: BrowserInstanceOption[] | undefined = Array.isArray(testProject) + ? testProject + : (testProject && [{ browser: testProject }]) + + const pass = checks?.length + ? checks.every(({ browser }) => { + const includePattern = `✓ |${browser}| ${testName}` + return stdout.includes(includePattern) + }) + : stdout.includes(`✓ ${testName}`) + return { pass, - message: () => `expected ${pass ? 'not ' : ''}to have "${includePattern}" in the report.\n\nstdout:\n${stdout}`, + message: () => { + const includePattern = checks?.length + ? checks.map(check => `✓ |${check}| ${testName}`).join('\n') + : `✓ ${testName}` + return `expected ${pass ? 'not ' : ''}to have "${includePattern}" in the report.\n\nstdout:\n${stdout}` + }, } }, toReportSummaryTestFiles(stdout: string, { passed }: SummaryOptions) { @@ -42,7 +57,7 @@ declare module 'vitest' { // eslint-disable-next-line unused-imports/no-unused-vars interface Assertion { // eslint-disable-next-line ts/method-signature-style - toReportPassedTest(testName: string, testProject?: string): void + toReportPassedTest(testName: string, testProject?: string | BrowserInstanceOption[]): void // eslint-disable-next-line ts/method-signature-style toReportSummaryTestFiles(options: SummaryOptions): void // eslint-disable-next-line ts/method-signature-style diff --git a/test/browser/specs/setup-file.test.ts b/test/browser/specs/setup-file.test.ts index 73699beb1b6e..0d17e733772c 100644 --- a/test/browser/specs/setup-file.test.ts +++ b/test/browser/specs/setup-file.test.ts @@ -11,8 +11,5 @@ test('setup file imports the same modules', async () => { ) expect(stderr).toReportNoErrors() - - instances.forEach(({ browser }) => { - expect(stdout).toReportPassedTest('module-equality.test.ts', browser) - }) + expect(stdout).toReportPassedTest('module-equality.test.ts', instances) }) From 57c78ae8595a788675c6648a1b3ceeaaa109fd47 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Wed, 26 Mar 2025 10:54:24 +0100 Subject: [PATCH 22/24] refactor: cleanup pool --- packages/browser/src/node/pool.ts | 50 +++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/packages/browser/src/node/pool.ts b/packages/browser/src/node/pool.ts index ebaeb6d2058d..9a6177cea0c8 100644 --- a/packages/browser/src/node/pool.ts +++ b/packages/browser/src/node/pool.ts @@ -47,9 +47,6 @@ export function createBrowserPool(vitest: Vitest): ProcessPool { const pool: BrowserPool = new BrowserPool(project, { maxWorkers: getThreadsCount(project), origin, - session(sessionId) { - return project.vitest._browserSessions.createSession(sessionId, project, pool) - }, }) projectPools.set(project, pool) vitest.onCancel(() => { @@ -143,7 +140,6 @@ class BrowserPool { private options: { maxWorkers: number origin: string - session: (sessionId: string) => Promise }, ) {} @@ -205,7 +201,11 @@ class BrowserPool { } private async openPage(sessionId: string) { - const sessionPromise = this.options.session(sessionId) + const sessionPromise = this.project.vitest._browserSessions.createSession( + sessionId, + this.project, + this, + ) const url = new URL('/', this.options.origin) url.searchParams.set('sessionId', sessionId) const pagePromise = this.project.browser!.provider.openPage( @@ -223,25 +223,43 @@ class BrowserPool { return orchestrator } - private runNextTest(method: 'run' | 'collect', sessionId: string) { + private finishSession(sessionId: string): void { + this.readySessions.add(sessionId) + + // the last worker finished running tests + if (this.readySessions.size === this.orchestrators.size) { + this._promise?.resolve() + this._promise = undefined + debug?.('all tests finished running') + } + } + + private runNextTest(method: 'run' | 'collect', sessionId: string): void { const file = this._queue.shift() + if (!file) { debug?.('[%s] no more tests to run', sessionId) + const isolate = this.project.config.browser.isolate + // we don't need to cleanup testers if isolation is enabled, + // because cleanup is done at the end of every test + if (isolate) { + this.finishSession(sessionId) + return + } + + // we need to cleanup testers first because there is only + // one iframe and it does the cleanup only after everything is completed const orchestrator = this.getOrchestrator(sessionId) - orchestrator.cleanupTesters().catch(error => this.reject(error)).finally(() => { - this.readySessions.add(sessionId) - // the last worker finished running tests - if (this.readySessions.size === this.orchestrators.size) { - this._promise?.resolve() - this._promise = undefined - debug?.('all tests finished running') - } - }) + orchestrator.cleanupTesters() + .catch(error => this.reject(error)) + .finally(() => this.finishSession(sessionId)) return } + if (!this._promise) { throw new Error(`Unexpected empty queue`) } + const orchestrator = this.getOrchestrator(sessionId) debug?.('[%s] run test %s', sessionId, file) @@ -265,7 +283,7 @@ class BrowserPool { if ( this.project.vitest.isCancelling && error instanceof Error - && error.message.startsWith('[birpc] rpc is closed') + && error.message.startsWith('Browser connection was closed while running tests') ) { this.cancel() this._promise?.resolve() From 4f13278e36e256a9616430c539d3e13841f95cf4 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Wed, 26 Mar 2025 10:54:42 +0100 Subject: [PATCH 23/24] chore: debug logs in setup file --- test/browser/specs/setup-file.test.ts | 6 ++++++ test/browser/specs/utils.ts | 4 +++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/test/browser/specs/setup-file.test.ts b/test/browser/specs/setup-file.test.ts index 0d17e733772c..4542555a82db 100644 --- a/test/browser/specs/setup-file.test.ts +++ b/test/browser/specs/setup-file.test.ts @@ -8,6 +8,12 @@ test('setup file imports the same modules', async () => { { root: './fixtures/setup-file', }, + undefined, + {}, + { + // TODO 2025-03-26 remove after debugging + std: 'inherit', + }, ) expect(stderr).toReportNoErrors() diff --git a/test/browser/specs/utils.ts b/test/browser/specs/utils.ts index f46242609e6e..aa47f88985c4 100644 --- a/test/browser/specs/utils.ts +++ b/test/browser/specs/utils.ts @@ -1,5 +1,6 @@ import type { UserConfig as ViteUserConfig } from 'vite' import type { UserConfig } from 'vitest/node' +import type { VitestRunnerCLIOptions } from '../../test-utils' import { runVitest } from '../../test-utils' import { browser } from '../settings' @@ -9,6 +10,7 @@ export async function runBrowserTests( config?: Omit & { browser?: Partial }, include?: string[], viteOverrides?: Partial, + runnerOptions?: VitestRunnerCLIOptions, ) { return runVitest({ watch: false, @@ -18,5 +20,5 @@ export async function runBrowserTests( headless: browser !== 'safari', ...config?.browser, } as UserConfig['browser'], - }, include, 'test', viteOverrides) + }, include, 'test', viteOverrides, runnerOptions) } From c58a06efc69f0ba08ad3e968faef520c42eede71 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Mon, 31 Mar 2025 17:44:28 +0200 Subject: [PATCH 24/24] chore: cleanup --- packages/browser/src/client/tester/tester.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/browser/src/client/tester/tester.ts b/packages/browser/src/client/tester/tester.ts index e12306dd21c6..44e7abb9790e 100644 --- a/packages/browser/src/client/tester/tester.ts +++ b/packages/browser/src/client/tester/tester.ts @@ -104,8 +104,6 @@ async function prepareTestEnvironment() { state.onCancel = onCancel state.rpc = rpc as any - getBrowserState().commands = new CommandsManager() - const interceptor = createModuleMockerInterceptor() const mocker = new VitestBrowserClientMocker( interceptor,