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) { diff --git a/packages/vitest/src/node/config/resolveConfig.ts b/packages/vitest/src/node/config/resolveConfig.ts index 1c4f6d6f6178..39f66bdd5b01 100644 --- a/packages/vitest/src/node/config/resolveConfig.ts +++ b/packages/vitest/src/node/config/resolveConfig.ts @@ -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) { 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('')) } } diff --git a/packages/vitest/src/node/core.ts b/packages/vitest/src/node/core.ts index bc55de624f19..4647caf34340 100644 --- a/packages/vitest/src/node/core.ts +++ b/packages/vitest/src/node/core.ts @@ -287,9 +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) { - 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 { + 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/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..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, @@ -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 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/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/browser-configs.test.ts b/test/config/test/browser-configs.test.ts index d2deccf6d50c..a9113914ca77 100644 --- a/test/config/test/browser-configs.test.ts +++ b/test/config/test/browser-configs.test.ts @@ -1,7 +1,11 @@ import type { ViteUserConfig } from 'vitest/config' import type { UserConfig, VitestOptions } from 'vitest/node' -import { expect, onTestFinished, test } from 'vitest' +import type { TestFsStructure } from '../../test-utils' +import crypto from 'node:crypto' +import { resolve } from 'pathe' +import { describe, 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 +307,231 @@ 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: TestFsStructure = {}) { + 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 + const workspace = (p) => ({ + 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, + browser: browser.enabled, + ui: browser.ui, + }, + workspace: vitest.projects.map(p => { + return { + ...workspace(p), + parent: p._parent ? workspace(p._parent) : null, + } + }) + })) + // throw an error to avoid running tests + throw new Error('stop') + }, + }, + ], + ...${JSON.stringify(options)} + } + } + `, + }) + return runVitestCli( + { + nodeOptions: { + env: { + CI: 'false', + GITHUB_ACTIONS: undefined, + }, + }, + }, + '--root', + root, + '--no-watch', + ...cli, + ) +} + +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', + }, + ], + }, + }, + }, + ], + }, []) + + 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 set in the config + headless: true, + browser: true, + // UI is false because headless is enabled + ui: false, + parent: { + name: 'browser', + headless: true, + browser: true, + ui: false, + }, + }) + }) + + test('CLI options correctly override inline workspace options', async () => { + const vitest = await getCliConfig({ + workspace: [ + { + test: { + name: 'unit', + }, + }, + { + 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) + + 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', + headless: false, + browser: true, + ui: true, + }, + }) + }) +}) diff --git a/test/config/test/failures.test.ts b/test/config/test/failures.test.ts index 1a8b469d5786..c0f561042ca1 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,39 +417,63 @@ 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', '--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 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({ - 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.') 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) {