From 154cb22de6ad0f28f1b1e25bef4ba31cde482931 Mon Sep 17 00:00:00 2001 From: Vladimir Date: Tue, 25 Jun 2024 22:04:55 +0200 Subject: [PATCH] feat(browser): add an option to take screenshots if the browser test fails (#5975) --- .gitignore | 2 +- docs/config/index.md | 14 ++++ packages/browser/src/client/tester/mocker.ts | 6 +- packages/browser/src/client/tester/runner.ts | 23 ++++-- packages/browser/src/client/tester/tester.ts | 4 +- packages/browser/src/client/utils.ts | 9 ++- packages/browser/src/client/vite.config.ts | 5 +- packages/browser/src/node/esmInjector.ts | 6 +- packages/browser/src/node/index.ts | 2 +- packages/browser/src/node/plugin.ts | 71 +++++++++++++++---- .../browser/src/node/providers/playwright.ts | 13 ++++ packages/runner/src/run.ts | 7 ++ packages/runner/src/types/runner.ts | 4 ++ packages/utils/src/source-map.ts | 5 +- packages/vitest/src/node/cli/cli-config.ts | 1 + packages/vitest/src/node/config.ts | 21 ++++++ packages/vitest/src/types/browser.ts | 7 ++ test/config/test/failures.test.ts | 3 +- 18 files changed, 172 insertions(+), 31 deletions(-) diff --git a/.gitignore b/.gitignore index 42754b1803b3..e5d7f1726288 100644 --- a/.gitignore +++ b/.gitignore @@ -22,6 +22,6 @@ docs/public/sponsors .eslintcache docs/.vitepress/cache/ !test/cli/fixtures/dotted-files/**/.cache -test/browser/test/__screenshots__/**/* +test/**/__screenshots__/**/* test/browser/fixtures/update-snapshot/basic.test.ts .vitest-reports diff --git a/docs/config/index.md b/docs/config/index.md index 94d4a34e7706..5749c8af73e7 100644 --- a/docs/config/index.md +++ b/docs/config/index.md @@ -1620,6 +1620,20 @@ Should Vitest UI be injected into the page. By default, injects UI iframe during Default iframe's viewport. +#### browser.screenshotDirectory {#browser-screenshotdirectory} + +- **Type:** `string` +- **Default:** `__snapshots__` in the test file directory + +Path to the snapshots directory relative to the `root`. + +#### browser.screenshotFailures {#browser-screenshotfailures} + +- **Type:** `boolean` +- **Default:** `!browser.ui` + +Should Vitest take screenshots if the test fails. + #### browser.orchestratorScripts {#browser-orchestratorscripts} - **Type:** `BrowserScript[]` diff --git a/packages/browser/src/client/tester/mocker.ts b/packages/browser/src/client/tester/mocker.ts index 20efe5eb54e9..8ff6b7f0567a 100644 --- a/packages/browser/src/client/tester/mocker.ts +++ b/packages/browser/src/client/tester/mocker.ts @@ -64,7 +64,7 @@ export class VitestBrowserClientMocker { const actualUrl = `${url.pathname}${ url.search ? `${url.search}&${query}` : `?${query}` }${url.hash}` - return getBrowserState().wrapModule(() => import(actualUrl)) + return getBrowserState().wrapModule(() => import(/* @vite-ignore */ actualUrl)) } public async importMock(rawId: string, importer: string) { @@ -86,11 +86,11 @@ export class VitestBrowserClientMocker { if (type === 'redirect') { const url = new URL(`/@id/${mockPath}`, location.href) - return import(url.toString()) + return import(/* @vite-ignore */ url.toString()) } const url = new URL(`/@id/${resolvedId}`, location.href) const query = url.search ? `${url.search}&t=${now()}` : `?t=${now()}` - const moduleObject = await import(`${url.pathname}${query}${url.hash}`) + const moduleObject = await import(/* @vite-ignore */ `${url.pathname}${query}${url.hash}`) return this.mockObject(moduleObject) } diff --git a/packages/browser/src/client/tester/runner.ts b/packages/browser/src/client/tester/runner.ts index ec1430e2b970..4cd4a7e9ef3c 100644 --- a/packages/browser/src/client/tester/runner.ts +++ b/packages/browser/src/client/tester/runner.ts @@ -4,7 +4,8 @@ import type { VitestExecutor } from 'vitest/execute' import { NodeBenchmarkRunner, VitestTestRunner } from 'vitest/runners' import { loadDiffConfig, loadSnapshotSerializers, takeCoverageInsideWorker } from 'vitest/browser' import { TraceMap, originalPositionFor } from 'vitest/utils' -import { importId } from '../utils' +import { page } from '@vitest/browser/context' +import { importFs, importId } from '../utils' import { globalChannel } from '../channel' import { VitestBrowserSnapshotEnvironment } from './snapshot' import { rpc } from './rpc' @@ -53,6 +54,12 @@ export function createBrowserRunner( } } + onTaskFinished = async (task: Task) => { + if (this.config.browser.screenshotFailures && task.result?.state === 'fail') { + await page.screenshot() + } + } + onCancel = (reason: CancelReason) => { super.onCancel?.(reason) globalChannel.postMessage({ type: 'cancel', reason }) @@ -123,7 +130,7 @@ export function createBrowserRunner( const prefix = `/${/^\w:/.test(filepath) ? '@fs/' : ''}` const query = `${test ? 'browserv' : 'v'}=${hash}` const importpath = `${prefix}${filepath}?${query}`.replace(/\/+/g, '/') - await import(importpath) + await import(/* @vite-ignore */ importpath) } } } @@ -140,9 +147,17 @@ export async function initiateRunner( } const runnerClass = config.mode === 'test' ? VitestTestRunner : NodeBenchmarkRunner + + const executeId = (id: string) => { + if (id[0] === '/' || id[1] === ':') { + return importFs(id) + } + return importId(id) + } + const BrowserRunner = createBrowserRunner(runnerClass, mocker, state, { takeCoverage: () => - takeCoverageInsideWorker(config.coverage, { executeId: importId }), + takeCoverageInsideWorker(config.coverage, { executeId }), }) if (!config.snapshotOptions.snapshotEnvironment) { config.snapshotOptions.snapshotEnvironment = new VitestBrowserSnapshotEnvironment() @@ -150,7 +165,7 @@ export async function initiateRunner( const runner = new BrowserRunner({ config, }) - const executor = { executeId: importId } as VitestExecutor + const executor = { executeId } as VitestExecutor const [diffOptions] = await Promise.all([ loadDiffConfig(config, executor), loadSnapshotSerializers(config, executor), diff --git a/packages/browser/src/client/tester/tester.ts b/packages/browser/src/client/tester/tester.ts index a045e93d3387..07732a53e96b 100644 --- a/packages/browser/src/client/tester/tester.ts +++ b/packages/browser/src/client/tester/tester.ts @@ -92,8 +92,8 @@ async function runTests(files: string[]) { try { preparedData = await prepareTestEnvironment(files) } - catch (error) { - debug('data cannot be loaded because it threw an error') + catch (error: any) { + debug('runner cannot be loaded because it threw an error', error.stack || error.message) await client.rpc.onUnhandledError(serializeError(error), 'Preload Error') done(files) return diff --git a/packages/browser/src/client/utils.ts b/packages/browser/src/client/utils.ts index 0633a238b9be..7a00d2a64766 100644 --- a/packages/browser/src/client/utils.ts +++ b/packages/browser/src/client/utils.ts @@ -1,8 +1,13 @@ import type { ResolvedConfig, WorkerGlobalState } from 'vitest' export async function importId(id: string) { - const name = `/@id/${id}` - return getBrowserState().wrapModule(() => import(name)) + const name = `/@id/${id}`.replace(/\\/g, '/') + return getBrowserState().wrapModule(() => import(/* @vite-ignore */ name)) +} + +export async function importFs(id: string) { + const name = `/@fs/${id}`.replace(/\\/g, '/') + return getBrowserState().wrapModule(() => import(/* @vite-ignore */ name)) } export function getConfig(): ResolvedConfig { diff --git a/packages/browser/src/client/vite.config.ts b/packages/browser/src/client/vite.config.ts index 780d35b56489..818e47607821 100644 --- a/packages/browser/src/client/vite.config.ts +++ b/packages/browser/src/client/vite.config.ts @@ -8,6 +8,9 @@ export default defineConfig({ server: { watch: { ignored: ['**/**'] }, }, + esbuild: { + legalComments: 'inline', + }, build: { minify: false, outDir: '../../dist/client', @@ -19,7 +22,7 @@ export default defineConfig({ orchestrator: resolve(__dirname, './orchestrator.html'), tester: resolve(__dirname, './tester/tester.html'), }, - external: [/__virtual_vitest__/], + external: [/__virtual_vitest__/, '@vitest/browser/context'], }, }, plugins: [ diff --git a/packages/browser/src/node/esmInjector.ts b/packages/browser/src/node/esmInjector.ts index c151983468e5..554b70029540 100644 --- a/packages/browser/src/node/esmInjector.ts +++ b/packages/browser/src/node/esmInjector.ts @@ -26,11 +26,13 @@ export function injectDynamicImport( // s.update(node.start, node.end, viImportMetaKey) }, onDynamicImport(node) { - const replace = '__vitest_browser_runner__.wrapModule(() => import(' + const replaceString = '__vitest_browser_runner__.wrapModule(() => import(' + const importSubstring = code.substring(node.start, node.end) + const hasIgnore = importSubstring.includes('/* @vite-ignore */') s.overwrite( node.start, (node.source as Positioned).start, - replace, + replaceString + (hasIgnore ? '/* @vite-ignore */ ' : ''), ) s.overwrite(node.end - 1, node.end, '))') }, diff --git a/packages/browser/src/node/index.ts b/packages/browser/src/node/index.ts index 27abee9b3b08..791c1b3752d9 100644 --- a/packages/browser/src/node/index.ts +++ b/packages/browser/src/node/index.ts @@ -25,7 +25,7 @@ export async function createBrowserServer( const vite = await createServer({ ...project.options, // spread project config inlined in root workspace config base: '/', - logLevel: 'error', + logLevel: (process.env.VITEST_BROWSER_DEBUG as 'info') ?? 'info', mode: project.config.mode, configFile: configPath, // watch is handled by Vitest diff --git a/packages/browser/src/node/plugin.ts b/packages/browser/src/node/plugin.ts index 24f6d6b8f070..f8d5ecd4689c 100644 --- a/packages/browser/src/node/plugin.ts +++ b/packages/browser/src/node/plugin.ts @@ -122,17 +122,44 @@ export default (browserServer: BrowserServer, base = '/'): Plugin[] => { define[`import.meta.env.${env}`] = stringValue } + const entries: string[] = [ + ...browserTestFiles, + ...setupFiles, + resolve(vitestDist, 'index.js'), + resolve(vitestDist, 'browser.js'), + resolve(vitestDist, 'runners.js'), + resolve(vitestDist, 'utils.js'), + ...(project.config.snapshotSerializers || []), + ] + + if (project.config.diff) { + entries.push(project.config.diff) + } + + if (project.ctx.coverageProvider) { + const coverage = project.ctx.config.coverage + const provider = coverage.provider + if (provider === 'v8') { + const path = tryResolve('@vitest/coverage-v8', [project.ctx.config.root]) + if (path) { + entries.push(path) + } + } + else if (provider === 'istanbul') { + const path = tryResolve('@vitest/coverage-istanbul', [project.ctx.config.root]) + if (path) { + entries.push(path) + } + } + else if (provider === 'custom' && coverage.customProviderModule) { + entries.push(coverage.customProviderModule) + } + } + return { define, optimizeDeps: { - entries: [ - ...browserTestFiles, - ...setupFiles, - resolve(vitestDist, 'index.js'), - resolve(vitestDist, 'browser.js'), - resolve(vitestDist, 'runners.js'), - resolve(vitestDist, 'utils.js'), - ], + entries, exclude: [ 'vitest', 'vitest/utils', @@ -163,6 +190,7 @@ export default (browserServer: BrowserServer, base = '/'): Plugin[] => { 'vitest > chai > loupe', 'vitest > @vitest/runner > p-limit', 'vitest > @vitest/utils > diff-sequences', + 'vitest > @vitest/utils > loupe', '@vitest/browser > @testing-library/user-event', '@vitest/browser > @testing-library/dom', ], @@ -235,10 +263,9 @@ export default (browserServer: BrowserServer, base = '/'): Plugin[] => { enforce: 'post', async config(viteConfig) { // Enables using ignore hint for coverage providers with @preserve keyword - if (viteConfig.esbuild !== false) { - viteConfig.esbuild ||= {} - viteConfig.esbuild.legalComments = 'inline' - } + viteConfig.esbuild ||= {} + viteConfig.esbuild.legalComments = 'inline' + const server = resolveApiServerConfig( viteConfig.test?.browser || {}, defaultBrowserPort, @@ -294,8 +321,8 @@ export default (browserServer: BrowserServer, base = '/'): Plugin[] => { { name: 'test-utils-rewrite', setup(build) { - const _require = createRequire(import.meta.url) build.onResolve({ filter: /@vue\/test-utils/ }, (args) => { + const _require = getRequire() // resolve to CJS instead of the browser because the browser version expects a global Vue object const resolved = _require.resolve(args.path, { paths: [args.importer], @@ -313,6 +340,24 @@ export default (browserServer: BrowserServer, base = '/'): Plugin[] => { ] } +function tryResolve(path: string, paths: string[]) { + try { + const _require = getRequire() + return _require.resolve(path, { paths }) + } + catch { + return undefined + } +} + +let _require: NodeRequire +function getRequire() { + if (!_require) { + _require = createRequire(import.meta.url) + } + return _require +} + function resolveCoverageFolder(project: WorkspaceProject) { const options = project.ctx.config const htmlReporter = options.coverage?.enabled diff --git a/packages/browser/src/node/providers/playwright.ts b/packages/browser/src/node/providers/playwright.ts index c3e87da0c754..2fba6668d847 100644 --- a/packages/browser/src/node/providers/playwright.ts +++ b/packages/browser/src/node/providers/playwright.ts @@ -141,6 +141,19 @@ export class PlaywrightBrowserProvider implements BrowserProvider { const page = await context.newPage() this.pages.set(contextId, page) + if (process.env.VITEST_PW_DEBUG) { + page.on('requestfailed', (request) => { + console.error( + '[PW Error]', + request.resourceType(), + 'request failed for', + request.url(), + 'url:', + request.failure()?.errorText, + ) + }) + } + return page } diff --git a/packages/runner/src/run.ts b/packages/runner/src/run.ts index 9fa34b8185f5..44291f143fa5 100644 --- a/packages/runner/src/run.ts +++ b/packages/runner/src/run.ts @@ -262,6 +262,13 @@ export async function runTest(test: Test | Custom, runner: VitestRunner) { return } + try { + await runner.onTaskFinished?.(test) + } + catch (e) { + failTask(test.result, e, runner.config.diffOptions) + } + try { await callSuiteHook(suite, test, 'afterEach', runner, [ test.context, diff --git a/packages/runner/src/types/runner.ts b/packages/runner/src/types/runner.ts index 49ec0198f7a5..ae8795c2fe24 100644 --- a/packages/runner/src/types/runner.ts +++ b/packages/runner/src/types/runner.ts @@ -80,6 +80,10 @@ export interface VitestRunner { test: Task, options: { retry: number; repeats: number } ) => unknown + /** + * When the task has finished running, but before cleanup hooks are called + */ + onTaskFinished?: (test: Task) => unknown /** * Called after result and state are set. */ diff --git a/packages/utils/src/source-map.ts b/packages/utils/src/source-map.ts index cbbd7216e945..5e26cbcea91e 100644 --- a/packages/utils/src/source-map.ts +++ b/packages/utils/src/source-map.ts @@ -32,7 +32,10 @@ const stackIgnorePatterns = [ '/node_modules/tinypool/', '/node_modules/tinyspy/', // browser related deps - '/deps/', + '/deps/chunk-', + '/deps/@vitest', + '/deps/loupe', + '/deps/chai', /node:\w+/, /__vitest_test__/, /__vitest_browser__/, diff --git a/packages/vitest/src/node/cli/cli-config.ts b/packages/vitest/src/node/cli/cli-config.ts index 46b217013f2d..50293f2e9277 100644 --- a/packages/vitest/src/node/cli/cli-config.ts +++ b/packages/vitest/src/node/cli/cli-config.ts @@ -413,6 +413,7 @@ export const cliOptionsConfig: VitestCLIOptions = { commands: null, viewport: null, screenshotDirectory: null, + screenshotFailures: null, }, }, pool: { diff --git a/packages/vitest/src/node/config.ts b/packages/vitest/src/node/config.ts index 4d369c996ac2..b009e7294d7b 100644 --- a/packages/vitest/src/node/config.ts +++ b/packages/vitest/src/node/config.ts @@ -235,6 +235,13 @@ export function resolveConfig( } } + if (resolved.coverage.enabled && resolved.coverage.provider === 'custom' && resolved.coverage.customProviderModule) { + resolved.coverage.customProviderModule = resolvePath( + resolved.coverage.customProviderModule, + resolved.root, + ) + } + resolved.expect ??= {} resolved.deps ??= {} @@ -683,6 +690,20 @@ export function resolveConfig( resolved.browser.screenshotDirectory, ) } + const isPreview = resolved.browser.provider === 'preview' + if (isPreview && resolved.browser.screenshotFailures === true) { + console.warn(c.yellow( + [ + `Browser provider "preview" doesn't support screenshots, `, + `so "browser.screenshotFailures" option is forcefully disabled. `, + `Set "browser.screenshotFailures" to false or remove it from the config to suppress this warning.`, + ].join(''), + )) + resolved.browser.screenshotFailures = false + } + else { + resolved.browser.screenshotFailures ??= !isPreview && !resolved.browser.ui + } resolved.browser.viewport ??= {} as any resolved.browser.viewport.width ??= 414 diff --git a/packages/vitest/src/types/browser.ts b/packages/vitest/src/types/browser.ts index 14ae5293f0e4..454ce12fa1ff 100644 --- a/packages/vitest/src/types/browser.ts +++ b/packages/vitest/src/types/browser.ts @@ -135,6 +135,13 @@ export interface BrowserConfigOptions { * @default __screenshots__ */ screenshotDirectory?: string + + /** + * Should Vitest take screenshots if the test fails + * @default !browser.ui + */ + screenshotFailures?: boolean + /** * Scripts injected into the tester iframe. */ diff --git a/test/config/test/failures.test.ts b/test/config/test/failures.test.ts index c4f49d16e82b..7bb8a64337da 100644 --- a/test/config/test/failures.test.ts +++ b/test/config/test/failures.test.ts @@ -109,7 +109,8 @@ test('version number is printed when coverage provider fails to load', async () }) expect(stdout).toMatch(`RUN v${version}`) - expect(stderr).toMatch('Error: Failed to load custom CoverageProviderModule from ./non-existing-module.ts') + expect(stderr).toMatch('Error: Failed to load custom CoverageProviderModule from') + expect(stderr).toMatch('non-existing-module.ts') }) test('coverage.autoUpdate cannot update thresholds when configuration file doesnt define them', async () => {