From 9aefb50b16bafa9e86680e8af06576d7e33e5561 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Fri, 18 Apr 2025 12:26:33 +0200 Subject: [PATCH 01/11] fix(browser): correctly inherit CLI options --- .../vitest/src/node/config/resolveConfig.ts | 26 ++-- packages/vitest/src/node/plugins/workspace.ts | 70 +++++---- .../src/node/workspace/resolveWorkspace.ts | 3 +- test/config/test/browser-configs.test.ts | 134 ++++++++++++++++++ 4 files changed, 184 insertions(+), 49 deletions(-) diff --git a/packages/vitest/src/node/config/resolveConfig.ts b/packages/vitest/src/node/config/resolveConfig.ts index 1c4f6d6f6178..7492eeccbc63 100644 --- a/packages/vitest/src/node/config/resolveConfig.ts +++ b/packages/vitest/src/node/config/resolveConfig.ts @@ -235,25 +235,17 @@ export function resolveConfig( const browser = resolved.browser if (browser.enabled) { - 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 + 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) } - 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('')) - } + 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('')) } } diff --git a/packages/vitest/src/node/plugins/workspace.ts b/packages/vitest/src/node/plugins/workspace.ts index 40f2cfba5106..ed88068ed993 100644 --- a/packages/vitest/src/node/plugins/workspace.ts +++ b/packages/vitest/src/node/plugins/workspace.ts @@ -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' @@ -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, @@ -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) diff --git a/packages/vitest/src/node/workspace/resolveWorkspace.ts b/packages/vitest/src/node/workspace/resolveWorkspace.ts index 2d7ec50f22f9..4476efaf3885 100644 --- a/packages/vitest/src/node/workspace/resolveWorkspace.ts +++ b/packages/vitest/src/node/workspace/resolveWorkspace.ts @@ -311,8 +311,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 diff --git a/test/config/test/browser-configs.test.ts b/test/config/test/browser-configs.test.ts index d2deccf6d50c..9ded697af5f4 100644 --- a/test/config/test/browser-configs.test.ts +++ b/test/config/test/browser-configs.test.ts @@ -1,7 +1,9 @@ import type { ViteUserConfig } from 'vitest/config' import type { UserConfig, VitestOptions } from 'vitest/node' +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) @@ -303,3 +305,135 @@ 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 = {}) { + 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') + }, + }, + ], + ...${JSON.stringify(options)} + } + } + `, + }) + return runVitestCli( + '--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) +}) From 8db97d513f2acc4f1ae65f43a8b27c657f06094c Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Fri, 18 Apr 2025 12:35:17 +0200 Subject: [PATCH 02/11] chore: add crypto --- test/config/test/browser-configs.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/config/test/browser-configs.test.ts b/test/config/test/browser-configs.test.ts index 9ded697af5f4..ab03d11b91ca 100644 --- a/test/config/test/browser-configs.test.ts +++ b/test/config/test/browser-configs.test.ts @@ -1,5 +1,6 @@ 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' From 3a0d1088cd0255340c0871a4f0e9b0620617878b Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Fri, 18 Apr 2025 12:39:25 +0200 Subject: [PATCH 03/11] chore: pass down CI flag --- test/config/test/browser-configs.test.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/config/test/browser-configs.test.ts b/test/config/test/browser-configs.test.ts index ab03d11b91ca..bd82341b802e 100644 --- a/test/config/test/browser-configs.test.ts +++ b/test/config/test/browser-configs.test.ts @@ -350,6 +350,13 @@ function getCliConfig(options: UserConfig, cli: string[], fs: Record Date: Fri, 18 Apr 2025 12:44:01 +0200 Subject: [PATCH 04/11] chore: try --- test/config/test/browser-configs.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/config/test/browser-configs.test.ts b/test/config/test/browser-configs.test.ts index bd82341b802e..983260c25a48 100644 --- a/test/config/test/browser-configs.test.ts +++ b/test/config/test/browser-configs.test.ts @@ -354,6 +354,7 @@ function getCliConfig(options: UserConfig, cli: string[], fs: Record Date: Fri, 18 Apr 2025 15:39:45 +0200 Subject: [PATCH 05/11] fix: throw and error if name and instances are not set --- packages/vitest/src/node/config/resolveConfig.ts | 7 ++++++- test/config/test/failures.test.ts | 6 ++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/vitest/src/node/config/resolveConfig.ts b/packages/vitest/src/node/config/resolveConfig.ts index 7492eeccbc63..39f66bdd5b01 100644 --- a/packages/vitest/src/node/config/resolveConfig.ts +++ b/packages/vitest/src/node/config/resolveConfig.ts @@ -234,7 +234,12 @@ 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) { + if (!browser.name && !browser.instances) { + throw new Error(`Vitest Browser Mode requires "browser.name" (deprecated) or "browser.instances" options, none were set.`) + } + const instances = browser.instances if (browser.name && browser.instances) { // --browser=chromium filters configs to a single one diff --git a/test/config/test/failures.test.ts b/test/config/test/failures.test.ts index 1a8b469d5786..975e7af3e2a0 100644 --- a/test/config/test/failures.test.ts +++ b/test/config/test/failures.test.ts @@ -426,6 +426,12 @@ test('browser.instances is empty', async () => { expect(stderr).toMatch('"browser.instances" was set in the config, but the array is empty. Define at least one browser config.') }) +test('browser.name or browser.instances are required', async () => { + const { stderr, exitCode } = await runVitestCli('--browser.enabled') + expect(exitCode).toBe(1) + expect(stderr).toMatch('Vitest Browser Mode requires "browser.name" (deprecated) or "browser.instances" options, none were set.') +}) + test('browser.name filters all browser.instances are required', async () => { const { stderr } = await runVitest({ browser: { From d89ad563b7f05b0299ed1c61f8ea005a49d9b88c Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Fri, 18 Apr 2025 16:43:11 +0200 Subject: [PATCH 06/11] chore: fix error --- packages/vitest/src/node/core.ts | 11 +++- .../src/node/workspace/resolveWorkspace.ts | 5 +- .../browser-no-config/vitest.config.ts | 9 +++ test/config/test/failures.test.ts | 63 +++++++++++-------- 4 files changed, 59 insertions(+), 29 deletions(-) create mode 100644 test/config/fixtures/browser-no-config/vitest.config.ts diff --git a/packages/vitest/src/node/core.ts b/packages/vitest/src/node/core.ts index bc55de624f19..0da193d54544 100644 --- a/packages/vitest/src/node/core.ts +++ b/packages/vitest/src/node/core.ts @@ -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) diff --git a/packages/vitest/src/node/workspace/resolveWorkspace.ts b/packages/vitest/src/node/workspace/resolveWorkspace.ts index 4476efaf3885..791ddfd9dd87 100644 --- a/packages/vitest/src/node/workspace/resolveWorkspace.ts +++ b/packages/vitest/src/node/workspace/resolveWorkspace.ts @@ -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, diff --git a/test/config/fixtures/browser-no-config/vitest.config.ts b/test/config/fixtures/browser-no-config/vitest.config.ts new file mode 100644 index 000000000000..76c887590f00 --- /dev/null +++ b/test/config/fixtures/browser-no-config/vitest.config.ts @@ -0,0 +1,9 @@ +import { defineConfig } from 'vitest/config'; + +export default defineConfig({ + test: { + browser: { + enabled: false, + }, + }, +}) \ No newline at end of file diff --git a/test/config/test/failures.test.ts b/test/config/test/failures.test.ts index 975e7af3e2a0..f056b2790b15 100644 --- a/test/config/test/failures.test.ts +++ b/test/config/test/failures.test.ts @@ -1,3 +1,4 @@ +import type { UserConfig as ViteUserConfig } from 'vite' import type { UserConfig } from 'vitest/node' import type { VitestRunnerCLIOptions } from '../../test-utils' import { normalize, resolve } from 'pathe' @@ -10,8 +11,8 @@ const providers = ['playwright', 'webdriverio', 'preview'] as const const names = ['edge', 'chromium', 'webkit', 'chrome', 'firefox', 'safari'] as const const browsers = providers.map(provider => names.map(name => ({ name, provider }))).flat() -function runVitest(config: NonNullable & { shard?: any }, runnerOptions?: VitestRunnerCLIOptions) { - return testUtils.runVitest({ root: './fixtures/test', ...config }, [], undefined, {}, runnerOptions) +function runVitest(config: NonNullable & { shard?: any }, viteOverrides: ViteUserConfig = {}, runnerOptions?: VitestRunnerCLIOptions) { + return testUtils.runVitest({ root: './fixtures/test', ...config }, [], undefined, viteOverrides, runnerOptions) } function runVitestCli(...cliArgs: string[]) { @@ -290,7 +291,7 @@ test('v8 coverage provider cannot be used in workspace without playwright + chro const { stderr } = await runVitest({ coverage: { enabled: true }, workspace: './fixtures/workspace/browser/workspace-with-browser.ts', - }, { fails: true }) + }, {}, { fails: true }) expect(stderr).toMatch( `Error: @vitest/coverage-v8 does not work with { @@ -416,45 +417,57 @@ test('maxConcurrency 0 prints a warning', async () => { }) test('browser.instances is empty', async () => { - const { stderr } = await runVitest({ - browser: { - enabled: true, - provider: 'playwright', - instances: [], + const { stderr } = await runVitest({}, { + test: { + browser: { + enabled: true, + provider: 'playwright', + instances: [], + }, }, }) expect(stderr).toMatch('"browser.instances" was set in the config, but the array is empty. Define at least one browser config.') }) test('browser.name or browser.instances are required', async () => { - const { stderr, exitCode } = await runVitestCli('--browser.enabled') + const { stderr, exitCode } = await runVitestCli('--browser.enabled', '--root=./fixtures/browser-no-config') expect(exitCode).toBe(1) expect(stderr).toMatch('Vitest Browser Mode requires "browser.name" (deprecated) or "browser.instances" options, none were set.') }) +test('--browser flag without browser configuration throws an error', async () => { + const { stderr, exitCode } = await runVitestCli('--browser.enabled') + expect(exitCode).toBe(1) + expect(stderr).toMatch('Vitest received --browser flag, but no project had a browser configuration.') +}) + test('browser.name filters all browser.instances are required', async () => { - const { stderr } = await runVitest({ - browser: { - enabled: true, - name: 'chromium', - provider: 'playwright', - instances: [ - { browser: 'firefox' }, - ], + const { stderr } = await runVitest({}, { + test: { + browser: { + enabled: true, + name: 'chromium', + provider: 'playwright', + instances: [ + { browser: 'firefox' }, + ], + }, }, }) expect(stderr).toMatch('"browser.instances" was set in the config, but the array is empty. Define at least one browser config. The "browser.name" was set to "chromium" which filtered all configs (firefox). Did you mean to use another name?') }) test('browser.instances throws an error if no custom name is provided', async () => { - const { stderr } = await runVitest({ - browser: { - enabled: true, - provider: 'playwright', - instances: [ - { browser: 'firefox' }, - { browser: 'firefox' }, - ], + const { stderr } = await runVitest({}, { + test: { + browser: { + enabled: true, + provider: 'playwright', + instances: [ + { browser: 'firefox' }, + { browser: 'firefox' }, + ], + }, }, }) expect(stderr).toMatch('Cannot define a nested project for a firefox browser. The project name "firefox" was already defined. If you have multiple instances for the same browser, make sure to define a custom "name". All projects in a workspace should have unique names. Make sure your configuration is correct.') From 82d14e3ec005eef26fc32cac5946307091865f52 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Fri, 25 Apr 2025 18:17:21 +0200 Subject: [PATCH 07/11] test: refactor --- test/config/test/browser-configs.test.ts | 75 +++++++++++++++++------- test/test-utils/index.ts | 4 +- 2 files changed, 56 insertions(+), 23 deletions(-) diff --git a/test/config/test/browser-configs.test.ts b/test/config/test/browser-configs.test.ts index 983260c25a48..e7fd24b9748e 100644 --- a/test/config/test/browser-configs.test.ts +++ b/test/config/test/browser-configs.test.ts @@ -1,5 +1,6 @@ import type { ViteUserConfig } from 'vitest/config' import type { UserConfig, VitestOptions } from 'vitest/node' +import type { TestFsStructure } from '../../test-utils' import crypto from 'node:crypto' import { resolve } from 'pathe' import { expect, onTestFinished, test } from 'vitest' @@ -307,7 +308,7 @@ test('can enable browser-cli options for multi-project workspace', async () => { expect(projects[1].config.browser.headless).toBe(true) }) -function getCliConfig(options: UserConfig, cli: string[], fs: Record = {}) { +function getCliConfig(options: UserConfig, cli: string[], fs: TestFsStructure = {}) { const root = resolve(process.cwd(), `vitest-test-${crypto.randomUUID()}`) useFS(root, { ...fs, @@ -324,6 +325,12 @@ function getCliConfig(options: UserConfig, cli: string[], fs: Record ({ + name: p.name, + headless: p.config.browser.headless, + browser: p.config.browser.enabled, + ui: p.config.browser.ui, + }) console.log(JSON.stringify({ browser: { headless: browser.headless, @@ -332,10 +339,8 @@ function getCliConfig(options: UserConfig, cli: string[], fs: Record { return { - name: p.name, - headless: p.config.browser.headless, - browser: p.config.browser.enabled, - ui: p.config.browser.ui, + ...workspace(p), + parent: p._parent ? workspace(p._parent) : null, } }) })) @@ -379,6 +384,7 @@ test('[e2e] CLI options correctly override inline workspace options', async () = browser: { enabled: true, headless: true, + provider: 'playwright', instances: [ { browser: 'chromium', @@ -393,16 +399,29 @@ test('[e2e] CLI options correctly override inline workspace options', async () = 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[0]).toEqual({ + name: 'unit', + headless: false, + browser: false, + ui: true, + parent: null, + }) - 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) + expect(config.workspace[1]).toEqual({ + name: 'browser (chromium)', + headless: false, + browser: true, + ui: true, + parent: { + name: 'browser', + headless: false, + browser: true, + ui: true, + }, + }) }) -test('[e2e] CLI options correctly override file workspace options', async () => { +test('[e2e] CLI options correctly override config file workspace options', async () => { const vitest = await getCliConfig( { workspace: [ @@ -416,13 +435,13 @@ test('[e2e] CLI options correctly override file workspace options', async () => }, ['--browser.headless=false'], { - 'vitest.browser.config.ts': /* ts */ ` - export default { + 'vitest.browser.config.ts': { test: { name: 'browser', browser: { enabled: true, headless: true, + provider: 'playwright', instances: [ { browser: 'chromium', @@ -430,19 +449,31 @@ test('[e2e] CLI options correctly override file workspace options', async () => ], }, }, - } - `, + }, }, ) 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[0]).toEqual({ + name: 'unit', + headless: false, + browser: false, + ui: true, + parent: null, + }) - 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) + expect(config.workspace[1]).toEqual({ + name: 'browser (chromium)', + headless: false, + browser: true, + ui: true, + parent: { + name: 'browser', + headless: false, + browser: true, + ui: true, + }, + }) }) diff --git a/test/test-utils/index.ts b/test/test-utils/index.ts index d96cf9c37037..7a59306ca56d 100644 --- a/test/test-utils/index.ts +++ b/test/test-utils/index.ts @@ -269,7 +269,9 @@ export function resolvePath(baseUrl: string, path: string) { return resolve(dirname(filename), path) } -export function useFS(root: string, structure: Record) { +export type TestFsStructure = Record + +export function useFS(root: string, structure: TestFsStructure) { const files = new Set() const hasConfig = Object.keys(structure).some(file => file.includes('.config.')) if (!hasConfig) { From 8a34a9409598d05a713f9d2bf7088cc1c8b7a9be Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Fri, 25 Apr 2025 18:25:59 +0200 Subject: [PATCH 08/11] test: check config before without passing down CLI flags --- test/config/test/browser-configs.test.ts | 220 ++++++++++++++--------- 1 file changed, 139 insertions(+), 81 deletions(-) diff --git a/test/config/test/browser-configs.test.ts b/test/config/test/browser-configs.test.ts index e7fd24b9748e..4b201cfb651c 100644 --- a/test/config/test/browser-configs.test.ts +++ b/test/config/test/browser-configs.test.ts @@ -3,7 +3,7 @@ import type { UserConfig, VitestOptions } from 'vitest/node' import type { TestFsStructure } from '../../test-utils' import crypto from 'node:crypto' import { resolve } from 'pathe' -import { expect, onTestFinished, test } from 'vitest' +import { describe, expect, onTestFinished, test } from 'vitest' import { createVitest } from 'vitest/node' import { runVitestCli, useFS } from '../../test-utils' @@ -370,110 +370,168 @@ function getCliConfig(options: UserConfig, cli: string[], fs: TestFsStructure = ) } -test('[e2e] CLI options correctly override inline workspace options', async () => { - const vitest = await getCliConfig({ - workspace: [ - { - test: { - name: 'unit', +describe('[e2e] workspace configs are affected by the CLI options', () => { + test('UI is not enabled by default in headless config', async () => { + const vitest = await getCliConfig({ + workspace: [ + { + test: { + name: 'unit', + }, }, - }, - { - test: { - name: 'browser', - browser: { - enabled: true, - headless: true, - provider: 'playwright', - instances: [ - { - browser: 'chromium', - }, - ], + { + test: { + name: 'browser', + browser: { + enabled: true, + headless: true, + provider: 'playwright', + instances: [ + { + browser: 'chromium', + }, + ], + }, }, }, - }, - ], - }, ['--browser.headless=false']) - - const config = JSON.parse(vitest.stdout) + ], + }, []) - expect(config.workspace).toHaveLength(2) - expect(config.workspace[0]).toEqual({ - name: 'unit', - headless: false, - browser: false, - ui: true, - parent: null, - }) + const config = JSON.parse(vitest.stdout) - expect(config.workspace[1]).toEqual({ - name: 'browser (chromium)', - headless: false, - browser: true, - ui: true, - parent: { - name: 'browser', + expect(config.workspace).toHaveLength(2) + expect(config.workspace[0]).toEqual({ + name: 'unit', headless: false, - browser: true, + browser: false, ui: true, - }, + parent: null, + }) + + expect(config.workspace[1]).toEqual({ + name: 'browser (chromium)', + // headless was set in the config + headless: true, + browser: true, + // UI is false necause headless is enabled + ui: false, + parent: { + name: 'browser', + headless: true, + browser: true, + ui: false, + }, + }) }) -}) -test('[e2e] CLI options correctly override config file workspace options', async () => { - const vitest = await getCliConfig( - { + test('CLI options correctly override inline workspace options', async () => { + const vitest = await getCliConfig({ workspace: [ { test: { name: 'unit', }, }, - './vitest.browser.config.ts', + { + test: { + name: 'browser', + browser: { + enabled: true, + headless: true, + provider: 'playwright', + instances: [ + { + browser: 'chromium', + }, + ], + }, + }, + }, ], - }, - ['--browser.headless=false'], - { - 'vitest.browser.config.ts': { - test: { - name: 'browser', - browser: { - enabled: true, - headless: true, - provider: 'playwright', - instances: [ - { - browser: 'chromium', - }, - ], + }, ['--browser.headless=false']) + + const config = JSON.parse(vitest.stdout) + + expect(config.workspace).toHaveLength(2) + expect(config.workspace[0]).toEqual({ + name: 'unit', + headless: false, + browser: false, + ui: true, + parent: null, + }) + + expect(config.workspace[1]).toEqual({ + name: 'browser (chromium)', + // headless was overriden by CLI options + headless: false, + browser: true, + // UI should be true because we always set CI to false, + // if headless was `true`, ui would be `false` + ui: true, + parent: { + name: 'browser', + headless: false, + browser: true, + ui: true, + }, + }) + }) + + test('CLI options correctly override config file workspace options', async () => { + const vitest = await getCliConfig( + { + workspace: [ + { + test: { + name: 'unit', + }, + }, + './vitest.browser.config.ts', + ], + }, + ['--browser.headless=false'], + { + 'vitest.browser.config.ts': { + test: { + name: 'browser', + browser: { + enabled: true, + headless: true, + provider: 'playwright', + instances: [ + { + browser: 'chromium', + }, + ], + }, }, }, }, - }, - ) + ) - const config = JSON.parse(vitest.stdout) + const config = JSON.parse(vitest.stdout) - expect(config.workspace).toHaveLength(2) - expect(config.workspace[0]).toEqual({ - name: 'unit', - headless: false, - browser: false, - ui: true, - parent: null, - }) + expect(config.workspace).toHaveLength(2) + expect(config.workspace[0]).toEqual({ + name: 'unit', + headless: false, + browser: false, + ui: true, + parent: null, + }) - expect(config.workspace[1]).toEqual({ - name: 'browser (chromium)', - headless: false, - browser: true, - ui: true, - parent: { - name: 'browser', + expect(config.workspace[1]).toEqual({ + name: 'browser (chromium)', headless: false, browser: true, ui: true, - }, + parent: { + name: 'browser', + headless: false, + browser: true, + ui: true, + }, + }) }) }) From 16e237568732e690ea57ec111bbb75303eb34090 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Fri, 25 Apr 2025 18:43:12 +0200 Subject: [PATCH 09/11] fix: throw an error if --browser is provided, but not configured --- packages/vitest/src/node/core.ts | 10 +++++++--- .../fixtures/no-browser-workspace/vitest.config.ts | 13 +++++++++++++ test/config/test/failures.test.ts | 6 ++++++ 3 files changed, 26 insertions(+), 3 deletions(-) create mode 100644 test/config/fixtures/no-browser-workspace/vitest.config.ts diff --git a/packages/vitest/src/node/core.ts b/packages/vitest/src/node/core.ts index 0da193d54544..4647caf34340 100644 --- a/packages/vitest/src/node/core.ts +++ b/packages/vitest/src/node/core.ts @@ -287,18 +287,22 @@ export class Vitest { })) })) + if (options.browser?.enabled) { + const browserProjects = this.projects.filter(p => p.config.browser.enabled) + if (!browserProjects.length) { + throw new Error(`Vitest received --browser flag, but no project had a browser configuration.`) + } + } if (!this.projects.length) { 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) } diff --git a/test/config/fixtures/no-browser-workspace/vitest.config.ts b/test/config/fixtures/no-browser-workspace/vitest.config.ts new file mode 100644 index 000000000000..6e5dcfe47d22 --- /dev/null +++ b/test/config/fixtures/no-browser-workspace/vitest.config.ts @@ -0,0 +1,13 @@ +import { defineConfig } from 'vitest/config'; + +export default defineConfig({ + test: { + workspace: [ + { + test: { + name: 'unit', + }, + }, + ], + }, +}) \ No newline at end of file diff --git a/test/config/test/failures.test.ts b/test/config/test/failures.test.ts index f056b2790b15..c0f561042ca1 100644 --- a/test/config/test/failures.test.ts +++ b/test/config/test/failures.test.ts @@ -441,6 +441,12 @@ test('--browser flag without browser configuration throws an error', async () => expect(stderr).toMatch('Vitest received --browser flag, but no project had a browser configuration.') }) +test('--browser flag without browser configuration in workspaces throws an error', async () => { + const { stderr, exitCode } = await runVitestCli('--browser.enabled', '--root=./fixtures/no-browser-workspace') + expect(exitCode).toBe(1) + expect(stderr).toMatch('Vitest received --browser flag, but no project had a browser configuration.') +}) + test('browser.name filters all browser.instances are required', async () => { const { stderr } = await runVitest({}, { test: { From 0d42aa5d277648adc0b6df11d1c40eef4af09c7b Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Fri, 25 Apr 2025 18:43:30 +0200 Subject: [PATCH 10/11] fix: don't panic if orchestrator is not set yet --- packages/browser/src/client/client.ts | 32 ++++++++++++++++++++------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/packages/browser/src/client/client.ts b/packages/browser/src/client/client.ts index 2565eea0a6e8..60fddb1076d6 100644 --- a/packages/browser/src/client/client.ts +++ b/packages/browser/src/client/client.ts @@ -2,6 +2,7 @@ import type { ModuleMocker } from '@vitest/mocker/browser' import type { CancelReason } from '@vitest/runner' import type { BirpcReturn } from 'birpc' import type { WebSocketBrowserEvents, WebSocketBrowserHandlers } from '../node/types' +import type { IframeOrchestrator } from './orchestrator' import { createBirpc } from 'birpc' import { parse, stringify } from 'flatted' import { getBrowserState } from './utils' @@ -35,6 +36,27 @@ export type BrowserRPC = BirpcReturn< WebSocketBrowserEvents > +// ws connection can be established before the orchestrator is fully loaded +// in very rare cases in the preview provider +function waitForOrchestrator() { + return new Promise((resolve, reject) => { + const type = getBrowserState().type + if (type !== 'orchestrator') { + reject(new TypeError('Only orchestrator can create testers.')) + return + } + + function check() { + const orchestrator = getBrowserState().orchestrator + if (orchestrator) { + return resolve(orchestrator) + } + setTimeout(check) + } + check() + }) +} + function createClient() { const autoReconnect = true const reconnectInterval = 2000 @@ -54,17 +76,11 @@ function createClient() { { onCancel: setCancel, async createTesters(options) { - const orchestrator = getBrowserState().orchestrator - if (!orchestrator) { - throw new TypeError('Only orchestrator can create testers.') - } + const orchestrator = await waitForOrchestrator() return orchestrator.createTesters(options) }, async cleanupTesters() { - const orchestrator = getBrowserState().orchestrator - if (!orchestrator) { - throw new TypeError('Only orchestrator can cleanup testers.') - } + const orchestrator = await waitForOrchestrator() return orchestrator.cleanupTesters() }, cdpEvent(event: string, payload: unknown) { From 3c8bf3d5d73ae03a89e4fb974b4d8f3f4eae7d11 Mon Sep 17 00:00:00 2001 From: Vladimir Sheremet Date: Fri, 25 Apr 2025 18:44:06 +0200 Subject: [PATCH 11/11] chore: typo --- test/config/test/browser-configs.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/config/test/browser-configs.test.ts b/test/config/test/browser-configs.test.ts index 4b201cfb651c..a9113914ca77 100644 --- a/test/config/test/browser-configs.test.ts +++ b/test/config/test/browser-configs.test.ts @@ -413,7 +413,7 @@ describe('[e2e] workspace configs are affected by the CLI options', () => { // headless was set in the config headless: true, browser: true, - // UI is false necause headless is enabled + // UI is false because headless is enabled ui: false, parent: { name: 'browser',