Skip to content
Merged
31 changes: 14 additions & 17 deletions packages/vitest/src/node/config/resolveConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,26 +234,23 @@ export function resolveConfig(

const browser = resolved.browser

if (browser.enabled) {
// if browser was enabled via CLI and it's configured by the user, then validate the input
if (browser.enabled && viteConfig.test?.browser) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main change is that we ignore all browser CLI flags if workspace doesn't have the browser configured

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: we throw an error if --browser is provided, but there are no configurations for it

if (!browser.name && !browser.instances) {
// CLI can enable `--browser.*` flag to change config of workspace projects
// the same flag will be applied to the root config that doesn't have to have "name" or "instances"
// in this case we just disable the browser mode
browser.enabled = false
throw new Error(`Vitest Browser Mode requires "browser.name" (deprecated) or "browser.instances" options, none were set.`)
}
else {
const instances = browser.instances
if (browser.name && browser.instances) {
// --browser=chromium filters configs to a single one
browser.instances = browser.instances.filter(instance => instance.browser === browser.name)
}

if (browser.instances && !browser.instances.length) {
throw new Error([
`"browser.instances" was set in the config, but the array is empty. Define at least one browser config.`,
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?` : '',
].join(''))
}
const instances = browser.instances
if (browser.name && browser.instances) {
// --browser=chromium filters configs to a single one
browser.instances = browser.instances.filter(instance => instance.browser === browser.name)
}

if (browser.instances && !browser.instances.length) {
throw new Error([
`"browser.instances" was set in the config, but the array is empty. Define at least one browser config.`,
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?` : '',
].join(''))
}
}

Expand Down
11 changes: 10 additions & 1 deletion packages/vitest/src/node/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,16 @@ export class Vitest {
}))

if (!this.projects.length) {
throw new Error(`No projects matched the filter "${toArray(resolved.project).join('", "')}".`)
const filter = toArray(resolved.project).join('", "')
if (filter) {
throw new Error(`No projects matched the filter "${filter}".`)
}
else if (options.browser?.enabled) {
throw new Error(`Vitest received --browser flag, but no project had a browser configuration.`)
}
else {
throw new Error(`Vitest wasn't able to resolve any project.`)
}
}
if (!this.coreWorkspaceProject) {
this.coreWorkspaceProject = TestProject._createBasicProject(this)
Expand Down
70 changes: 40 additions & 30 deletions packages/vitest/src/node/plugins/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type { ResolvedConfig, TestProjectInlineConfiguration } from '../types/co
import { existsSync, readFileSync } from 'node:fs'
import { deepMerge } from '@vitest/utils'
import { basename, dirname, relative, resolve } from 'pathe'
import { mergeConfig } from 'vite'
import { configDefaults } from '../../defaults'
import { generateScopedClassName } from '../../integrations/css/css-modules'
import { VitestFilteredOutProjectError } from '../errors'
Expand Down Expand Up @@ -63,35 +64,6 @@ export function WorkspaceVitestPlugin(
}
}

// keep project names to potentially filter it out
const workspaceNames = [name]
if (viteConfig.test?.browser?.enabled) {
if (viteConfig.test.browser.name) {
const browser = viteConfig.test.browser.name
// vitest injects `instances` in this case later on
workspaceNames.push(name ? `${name} (${browser})` : browser)
}

viteConfig.test.browser.instances?.forEach((instance) => {
// every instance is a potential project
instance.name ??= name ? `${name} (${instance.browser})` : instance.browser
workspaceNames.push(instance.name)
})
}

const filters = project.vitest.config.project
// if there is `--project=...` filter, check if any of the potential projects match
// if projects don't match, we ignore the test project altogether
// if some of them match, they will later be filtered again by `resolveWorkspace`
if (filters.length) {
const hasProject = workspaceNames.some((name) => {
return project.vitest.matchesProjectFilter(name)
})
if (!hasProject) {
throw new VitestFilteredOutProjectError()
}
}

const resolveOptions = getDefaultResolveOptions()
const config: ViteConfig = {
root,
Expand Down Expand Up @@ -138,10 +110,48 @@ export function WorkspaceVitestPlugin(
test: {
name,
},
};
}

// if this project defines a browser configuration, respect --browser flag
// otherwise if we always override the configuration, every project will run in browser mode
if (project.vitest._options.browser && viteConfig.test?.browser) {
viteConfig.test.browser = mergeConfig(
viteConfig.test.browser,
project.vitest._options.browser,
)
}

(config.test as ResolvedConfig).defines = defines

// keep project names to potentially filter it out
const workspaceNames = [name]
if (viteConfig.test?.browser?.enabled) {
if (viteConfig.test.browser.name && !viteConfig.test.browser.instances?.length) {
const browser = viteConfig.test.browser.name
// vitest injects `instances` in this case later on
workspaceNames.push(name ? `${name} (${browser})` : browser)
}

viteConfig.test.browser.instances?.forEach((instance) => {
// every instance is a potential project
instance.name ??= name ? `${name} (${instance.browser})` : instance.browser
workspaceNames.push(instance.name)
})
}

const filters = project.vitest.config.project
// if there is `--project=...` filter, check if any of the potential projects match
// if projects don't match, we ignore the test project altogether
// if some of them match, they will later be filtered again by `resolveWorkspace`
if (filters.length) {
const hasProject = workspaceNames.some((name) => {
return project.vitest.matchesProjectFilter(name)
})
if (!hasProject) {
throw new VitestFilteredOutProjectError()
}
}

const classNameStrategy
= (typeof testConfig.css !== 'boolean'
&& testConfig.css?.modules?.classNameStrategy)
Expand Down
8 changes: 3 additions & 5 deletions packages/vitest/src/node/workspace/resolveWorkspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,8 @@ export async function resolveBrowserWorkspace(
return
}
const instances = project.config.browser.instances || []
if (instances.length === 0) {
const browser = project.config.browser.name
// browser.name should be defined, otherwise the config fails in "resolveConfig"
const browser = project.config.browser.name
if (instances.length === 0 && browser) {
instances.push({
browser,
name: project.name ? `${project.name} (${browser})` : browser,
Expand Down Expand Up @@ -311,8 +310,7 @@ function cloneConfig(project: TestProject, { browser, ...config }: BrowserInstan
testerHtmlPath: testerHtmlPath ?? currentConfig.testerHtmlPath,
screenshotDirectory: screenshotDirectory ?? currentConfig.screenshotDirectory,
screenshotFailures: screenshotFailures ?? currentConfig.screenshotFailures,
// TODO: test that CLI arg is preferred over the local config
headless: project.vitest._options?.browser?.headless ?? headless ?? currentConfig.headless,
headless: headless ?? currentConfig.headless,
name: browser,
providerOptions: config,
instances: undefined, // projects cannot spawn more configs
Expand Down
9 changes: 9 additions & 0 deletions test/config/fixtures/browser-no-config/vitest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { defineConfig } from 'vitest/config';

export default defineConfig({
test: {
browser: {
enabled: false,
},
},
})
143 changes: 143 additions & 0 deletions test/config/test/browser-configs.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import type { ViteUserConfig } from 'vitest/config'
import type { UserConfig, VitestOptions } from 'vitest/node'
import crypto from 'node:crypto'
import { resolve } from 'pathe'
import { expect, onTestFinished, test } from 'vitest'
import { createVitest } from 'vitest/node'
import { runVitestCli, useFS } from '../../test-utils'

async function vitest(cliOptions: UserConfig, configValue: UserConfig = {}, viteConfig: ViteUserConfig = {}, vitestOptions: VitestOptions = {}) {
const vitest = await createVitest('test', { ...cliOptions, watch: false }, { ...viteConfig, test: configValue as any }, vitestOptions)
Expand Down Expand Up @@ -303,3 +306,143 @@ test('can enable browser-cli options for multi-project workspace', async () => {
expect(projects[1].config.browser.enabled).toBe(true)
expect(projects[1].config.browser.headless).toBe(true)
})

function getCliConfig(options: UserConfig, cli: string[], fs: Record<string, string> = {}) {
const root = resolve(process.cwd(), `vitest-test-${crypto.randomUUID()}`)
useFS(root, {
...fs,
'basic.test.ts': /* ts */`
import { test } from 'vitest'
test('basic', () => {
expect(1).toBe(1)
})
`,
'vitest.config.ts': /* ts */ `
export default {
test: {
reporters: [
{
onInit(vitest) {
const browser = vitest.config.browser
console.log(JSON.stringify({
browser: {
headless: browser.headless,
browser: browser.enabled,
ui: browser.ui,
},
workspace: vitest.projects.map(p => {
return {
name: p.name,
headless: p.config.browser.headless,
browser: p.config.browser.enabled,
ui: p.config.browser.ui,
}
})
}))
// throw an error to avoid running tests
throw new Error('stop')
Comment on lines +347 to +348
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This utility looks great - I've thought about building something like this before. Sometimes it's not enough to check resolved options just via resolveConfig as plugins and other parts can change them before tests run. Bailing out like this before pools are running saves time.

},
},
],
...${JSON.stringify(options)}
}
}
`,
})
return runVitestCli(
{
nodeOptions: {
env: {
CI: 'false',
GITHUB_ACTIONS: undefined,
},
},
},
'--root',
root,
'--no-watch',
...cli,
)
}

test('[e2e] CLI options correctly override inline workspace options', async () => {
const vitest = await getCliConfig({
workspace: [
{
test: {
name: 'unit',
},
},
{
test: {
name: 'browser',
browser: {
enabled: true,
headless: true,
instances: [
{
browser: 'chromium',
},
],
},
},
},
],
}, ['--browser.headless=false'])

const config = JSON.parse(vitest.stdout)

expect(config.workspace).toHaveLength(2)
expect(config.workspace[0].name).toBe('unit')
expect(config.workspace[0].browser).toBe(false)

expect(config.workspace[1].name).toBe('browser (chromium)')
expect(config.workspace[1].headless).toBe(false)
expect(config.workspace[1].browser).toBe(true)
expect(config.workspace[1].ui).toBe(true)
})

test('[e2e] CLI options correctly override file workspace options', async () => {
const vitest = await getCliConfig(
{
workspace: [
{
test: {
name: 'unit',
},
},
'./vitest.browser.config.ts',
],
},
['--browser.headless=false'],
{
'vitest.browser.config.ts': /* ts */ `
export default {
test: {
name: 'browser',
browser: {
enabled: true,
headless: true,
instances: [
{
browser: 'chromium',
},
],
},
},
}
`,
},
)

const config = JSON.parse(vitest.stdout)

expect(config.workspace).toHaveLength(2)
expect(config.workspace[0].name).toBe('unit')
expect(config.workspace[0].browser).toBe(false)

expect(config.workspace[1].name).toBe('browser (chromium)')
expect(config.workspace[1].headless).toBe(false)
expect(config.workspace[1].browser).toBe(true)
expect(config.workspace[1].ui).toBe(true)
})
Loading
Loading