Skip to content

Commit 03660f9

Browse files
sheremet-vasheremet-va
authored
fix(browser): correctly inherit CLI options (#7858)
1 parent f2ce53c commit 03660f9

File tree

10 files changed

+402
-87
lines changed

10 files changed

+402
-87
lines changed

‎packages/browser/src/client/client.ts

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { ModuleMocker } from '@vitest/mocker/browser'
22
import type { CancelReason } from '@vitest/runner'
33
import type { BirpcReturn } from 'birpc'
44
import type { WebSocketBrowserEvents, WebSocketBrowserHandlers } from '../node/types'
5+
import type { IframeOrchestrator } from './orchestrator'
56
import { createBirpc } from 'birpc'
67
import { parse, stringify } from 'flatted'
78
import { getBrowserState } from './utils'
@@ -35,6 +36,27 @@ export type BrowserRPC = BirpcReturn<
3536
WebSocketBrowserEvents
3637
>
3738

39+
// ws connection can be established before the orchestrator is fully loaded
40+
// in very rare cases in the preview provider
41+
function waitForOrchestrator() {
42+
return new Promise<IframeOrchestrator>((resolve, reject) => {
43+
const type = getBrowserState().type
44+
if (type !== 'orchestrator') {
45+
reject(new TypeError('Only orchestrator can create testers.'))
46+
return
47+
}
48+
49+
function check() {
50+
const orchestrator = getBrowserState().orchestrator
51+
if (orchestrator) {
52+
return resolve(orchestrator)
53+
}
54+
setTimeout(check)
55+
}
56+
check()
57+
})
58+
}
59+
3860
function createClient() {
3961
const autoReconnect = true
4062
const reconnectInterval = 2000
@@ -54,17 +76,11 @@ function createClient() {
5476
{
5577
onCancel: setCancel,
5678
async createTesters(options) {
57-
const orchestrator = getBrowserState().orchestrator
58-
if (!orchestrator) {
59-
throw new TypeError('Only orchestrator can create testers.')
60-
}
79+
const orchestrator = await waitForOrchestrator()
6180
return orchestrator.createTesters(options)
6281
},
6382
async cleanupTesters() {
64-
const orchestrator = getBrowserState().orchestrator
65-
if (!orchestrator) {
66-
throw new TypeError('Only orchestrator can cleanup testers.')
67-
}
83+
const orchestrator = await waitForOrchestrator()
6884
return orchestrator.cleanupTesters()
6985
},
7086
cdpEvent(event: string, payload: unknown) {

‎packages/vitest/src/node/config/resolveConfig.ts

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -234,26 +234,23 @@ export function resolveConfig(
234234

235235
const browser = resolved.browser
236236

237-
if (browser.enabled) {
237+
// if browser was enabled via CLI and it's configured by the user, then validate the input
238+
if (browser.enabled && viteConfig.test?.browser) {
238239
if (!browser.name && !browser.instances) {
239-
// CLI can enable `--browser.*` flag to change config of workspace projects
240-
// the same flag will be applied to the root config that doesn't have to have "name" or "instances"
241-
// in this case we just disable the browser mode
242-
browser.enabled = false
240+
throw new Error(`Vitest Browser Mode requires "browser.name" (deprecated) or "browser.instances" options, none were set.`)
243241
}
244-
else {
245-
const instances = browser.instances
246-
if (browser.name && browser.instances) {
247-
// --browser=chromium filters configs to a single one
248-
browser.instances = browser.instances.filter(instance => instance.browser === browser.name)
249-
}
250242

251-
if (browser.instances && !browser.instances.length) {
252-
throw new Error([
253-
`"browser.instances" was set in the config, but the array is empty. Define at least one browser config.`,
254-
browser.name && instances?.length ? ` The "browser.name" was set to "${browser.name}" which filtered all configs (${instances.map(c => c.browser).join(', ')}). Did you mean to use another name?` : '',
255-
].join(''))
256-
}
243+
const instances = browser.instances
244+
if (browser.name && browser.instances) {
245+
// --browser=chromium filters configs to a single one
246+
browser.instances = browser.instances.filter(instance => instance.browser === browser.name)
247+
}
248+
249+
if (browser.instances && !browser.instances.length) {
250+
throw new Error([
251+
`"browser.instances" was set in the config, but the array is empty. Define at least one browser config.`,
252+
browser.name && instances?.length ? ` The "browser.name" was set to "${browser.name}" which filtered all configs (${instances.map(c => c.browser).join(', ')}). Did you mean to use another name?` : '',
253+
].join(''))
257254
}
258255
}
259256

‎packages/vitest/src/node/core.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,9 +287,22 @@ export class Vitest {
287287
}))
288288
}))
289289

290+
if (options.browser?.enabled) {
291+
const browserProjects = this.projects.filter(p => p.config.browser.enabled)
292+
if (!browserProjects.length) {
293+
throw new Error(`Vitest received --browser flag, but no project had a browser configuration.`)
294+
}
295+
}
290296
if (!this.projects.length) {
291-
throw new Error(`No projects matched the filter "${toArray(resolved.project).join('", "')}".`)
297+
const filter = toArray(resolved.project).join('", "')
298+
if (filter) {
299+
throw new Error(`No projects matched the filter "${filter}".`)
300+
}
301+
else {
302+
throw new Error(`Vitest wasn't able to resolve any project.`)
303+
}
292304
}
305+
293306
if (!this.coreWorkspaceProject) {
294307
this.coreWorkspaceProject = TestProject._createBasicProject(this)
295308
}

‎packages/vitest/src/node/plugins/workspace.ts

Lines changed: 40 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import type { ResolvedConfig, TestProjectInlineConfiguration } from '../types/co
44
import { existsSync, readFileSync } from 'node:fs'
55
import { deepMerge } from '@vitest/utils'
66
import { basename, dirname, relative, resolve } from 'pathe'
7+
import { mergeConfig } from 'vite'
78
import { configDefaults } from '../../defaults'
89
import { generateScopedClassName } from '../../integrations/css/css-modules'
910
import { VitestFilteredOutProjectError } from '../errors'
@@ -63,35 +64,6 @@ export function WorkspaceVitestPlugin(
6364
}
6465
}
6566

66-
// keep project names to potentially filter it out
67-
const workspaceNames = [name]
68-
if (viteConfig.test?.browser?.enabled) {
69-
if (viteConfig.test.browser.name) {
70-
const browser = viteConfig.test.browser.name
71-
// vitest injects `instances` in this case later on
72-
workspaceNames.push(name ? `${name} (${browser})` : browser)
73-
}
74-
75-
viteConfig.test.browser.instances?.forEach((instance) => {
76-
// every instance is a potential project
77-
instance.name ??= name ? `${name} (${instance.browser})` : instance.browser
78-
workspaceNames.push(instance.name)
79-
})
80-
}
81-
82-
const filters = project.vitest.config.project
83-
// if there is `--project=...` filter, check if any of the potential projects match
84-
// if projects don't match, we ignore the test project altogether
85-
// if some of them match, they will later be filtered again by `resolveWorkspace`
86-
if (filters.length) {
87-
const hasProject = workspaceNames.some((name) => {
88-
return project.vitest.matchesProjectFilter(name)
89-
})
90-
if (!hasProject) {
91-
throw new VitestFilteredOutProjectError()
92-
}
93-
}
94-
9567
const resolveOptions = getDefaultResolveOptions()
9668
const config: ViteConfig = {
9769
root,
@@ -138,10 +110,48 @@ export function WorkspaceVitestPlugin(
138110
test: {
139111
name,
140112
},
141-
};
113+
}
114+
115+
// if this project defines a browser configuration, respect --browser flag
116+
// otherwise if we always override the configuration, every project will run in browser mode
117+
if (project.vitest._options.browser && viteConfig.test?.browser) {
118+
viteConfig.test.browser = mergeConfig(
119+
viteConfig.test.browser,
120+
project.vitest._options.browser,
121+
)
122+
}
142123

143124
(config.test as ResolvedConfig).defines = defines
144125

126+
// keep project names to potentially filter it out
127+
const workspaceNames = [name]
128+
if (viteConfig.test?.browser?.enabled) {
129+
if (viteConfig.test.browser.name && !viteConfig.test.browser.instances?.length) {
130+
const browser = viteConfig.test.browser.name
131+
// vitest injects `instances` in this case later on
132+
workspaceNames.push(name ? `${name} (${browser})` : browser)
133+
}
134+
135+
viteConfig.test.browser.instances?.forEach((instance) => {
136+
// every instance is a potential project
137+
instance.name ??= name ? `${name} (${instance.browser})` : instance.browser
138+
workspaceNames.push(instance.name)
139+
})
140+
}
141+
142+
const filters = project.vitest.config.project
143+
// if there is `--project=...` filter, check if any of the potential projects match
144+
// if projects don't match, we ignore the test project altogether
145+
// if some of them match, they will later be filtered again by `resolveWorkspace`
146+
if (filters.length) {
147+
const hasProject = workspaceNames.some((name) => {
148+
return project.vitest.matchesProjectFilter(name)
149+
})
150+
if (!hasProject) {
151+
throw new VitestFilteredOutProjectError()
152+
}
153+
}
154+
145155
const classNameStrategy
146156
= (typeof testConfig.css !== 'boolean'
147157
&& testConfig.css?.modules?.classNameStrategy)

‎packages/vitest/src/node/workspace/resolveWorkspace.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -178,9 +178,8 @@ export async function resolveBrowserWorkspace(
178178
return
179179
}
180180
const instances = project.config.browser.instances || []
181-
if (instances.length === 0) {
182-
const browser = project.config.browser.name
183-
// browser.name should be defined, otherwise the config fails in "resolveConfig"
181+
const browser = project.config.browser.name
182+
if (instances.length === 0 && browser) {
184183
instances.push({
185184
browser,
186185
name: project.name ? `${project.name} (${browser})` : browser,
@@ -311,8 +310,7 @@ function cloneConfig(project: TestProject, { browser, ...config }: BrowserInstan
311310
testerHtmlPath: testerHtmlPath ?? currentConfig.testerHtmlPath,
312311
screenshotDirectory: screenshotDirectory ?? currentConfig.screenshotDirectory,
313312
screenshotFailures: screenshotFailures ?? currentConfig.screenshotFailures,
314-
// TODO: test that CLI arg is preferred over the local config
315-
headless: project.vitest._options?.browser?.headless ?? headless ?? currentConfig.headless,
313+
headless: headless ?? currentConfig.headless,
316314
name: browser,
317315
providerOptions: config,
318316
instances: undefined, // projects cannot spawn more configs

‎test/config/fixtures/browser-no-config/vitest.config.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import { defineConfig } from 'vitest/config';
2+
3+
export default defineConfig({
4+
test: {
5+
browser: {
6+
enabled: false,
7+
},
8+
},
9+
})

‎test/config/fixtures/no-browser-workspace/vitest.config.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import { defineConfig } from 'vitest/config';
2+
3+
export default defineConfig({
4+
test: {
5+
workspace: [
6+
{
7+
test: {
8+
name: 'unit',
9+
},
10+
},
11+
],
12+
},
13+
})

0 commit comments

Comments
 (0)