From 14a217d53b25aa9fd2039d3247de95267646b061 Mon Sep 17 00:00:00 2001 From: Vladimir Date: Tue, 25 Jun 2024 17:00:18 +0200 Subject: [PATCH] fix(browser): make userEvent more stable when running in parallel (#5974) --- docs/guide/browser.md | 5 ++-- packages/browser/providers/playwright.d.ts | 4 ++- packages/browser/src/node/cdp.ts | 4 --- packages/browser/src/node/commands/clear.ts | 4 +-- packages/browser/src/node/commands/click.ts | 4 +-- .../browser/src/node/commands/dragAndDrop.ts | 3 ++- packages/browser/src/node/commands/fill.ts | 4 +-- packages/browser/src/node/commands/hover.ts | 2 +- .../browser/src/node/commands/keyboard.ts | 6 +++-- .../browser/src/node/commands/screenshot.ts | 8 +++--- packages/browser/src/node/commands/select.ts | 6 ++--- packages/browser/src/node/commands/type.ts | 4 +-- .../browser/src/node/providers/playwright.ts | 26 +++++++++++++++---- packages/vitest/src/types/browser.ts | 1 - 14 files changed, 50 insertions(+), 31 deletions(-) diff --git a/docs/guide/browser.md b/docs/guide/browser.md index 9282c84dcc17..da74fddec2c9 100644 --- a/docs/guide/browser.md +++ b/docs/guide/browser.md @@ -953,7 +953,8 @@ Custom functions will override built-in ones if they have the same name. Vitest exposes several `playwright` specific properties on the command context. - `page` references the full page that contains the test iframe. This is the orchestrator HTML and you most likely shouldn't touch it to not break things. -- `frame` is the tester [iframe instance](https://playwright.dev/docs/api/class-frame). It has a simillar API to the page, but it doesn't support certain methods. +- `frame` is an async method that will resolve tester [`Frame`](https://playwright.dev/docs/api/class-frame). It has a simillar API to the `page`, but it doesn't support certain methods. If you need to query an element, you should prefer using `context.iframe` instead because it is more stable and faster. +- `iframe` is a [`FrameLocator`](https://playwright.dev/docs/api/class-framelocator) that should be used to query other elements on the page. - `context` refers to the unique [BrowserContext](https://playwright.dev/docs/api/class-browsercontext). ```ts @@ -961,7 +962,7 @@ import { defineCommand } from '@vitest/browser' export const myCommand = defineCommand(async (ctx, arg1, arg2) => { if (ctx.provider.name === 'playwright') { - const element = await ctx.frame.findByRole('alert') + const element = await ctx.iframe.findByRole('alert') const screenshot = await element.screenshot() // do something with the screenshot return difference diff --git a/packages/browser/providers/playwright.d.ts b/packages/browser/providers/playwright.d.ts index 61a3a5fabc2f..381ce37beabd 100644 --- a/packages/browser/providers/playwright.d.ts +++ b/packages/browser/providers/playwright.d.ts @@ -2,6 +2,7 @@ import type { BrowserContext, BrowserContextOptions, Frame, + FrameLocator, LaunchOptions, Page, CDPSession @@ -20,7 +21,8 @@ declare module 'vitest/node' { export interface BrowserCommandContext { page: Page - frame: Frame + frame(): Promise + iframe: FrameLocator context: BrowserContext } } diff --git a/packages/browser/src/node/cdp.ts b/packages/browser/src/node/cdp.ts index fac363340a07..b8f0c837a7e8 100644 --- a/packages/browser/src/node/cdp.ts +++ b/packages/browser/src/node/cdp.ts @@ -15,10 +15,6 @@ export class BrowserServerCDPHandler { return this.session.send(method, params) } - detach() { - return this.session.detach() - } - on(event: string, id: string, once = false) { if (!this.listenerIds[event]) { this.listenerIds[event] = [] diff --git a/packages/browser/src/node/commands/clear.ts b/packages/browser/src/node/commands/clear.ts index c217ad96b3c9..96bfd4c657df 100644 --- a/packages/browser/src/node/commands/clear.ts +++ b/packages/browser/src/node/commands/clear.ts @@ -8,8 +8,8 @@ export const clear: UserEventCommand = async ( xpath, ) => { if (context.provider instanceof PlaywrightBrowserProvider) { - const { frame } = context - const element = frame.locator(`xpath=${xpath}`) + const { iframe } = context + const element = iframe.locator(`xpath=${xpath}`) await element.clear({ timeout: 1000, }) diff --git a/packages/browser/src/node/commands/click.ts b/packages/browser/src/node/commands/click.ts index b263ebc0e34f..7a9b7ce39625 100644 --- a/packages/browser/src/node/commands/click.ts +++ b/packages/browser/src/node/commands/click.ts @@ -10,7 +10,7 @@ export const click: UserEventCommand = async ( ) => { const provider = context.provider if (provider instanceof PlaywrightBrowserProvider) { - const tester = context.frame + const tester = context.iframe await tester.locator(`xpath=${xpath}`).click({ timeout: 1000, ...options, @@ -33,7 +33,7 @@ export const dblClick: UserEventCommand = async ( ) => { const provider = context.provider if (provider instanceof PlaywrightBrowserProvider) { - const tester = context.frame + const tester = context.iframe await tester.locator(`xpath=${xpath}`).dblclick(options) } else if (provider instanceof WebdriverBrowserProvider) { diff --git a/packages/browser/src/node/commands/dragAndDrop.ts b/packages/browser/src/node/commands/dragAndDrop.ts index 55cd9a7d8215..03d6740d5dc5 100644 --- a/packages/browser/src/node/commands/dragAndDrop.ts +++ b/packages/browser/src/node/commands/dragAndDrop.ts @@ -10,7 +10,8 @@ export const dragAndDrop: UserEventCommand = async ( options, ) => { if (context.provider instanceof PlaywrightBrowserProvider) { - await context.frame.dragAndDrop( + const frame = await context.frame() + await frame.dragAndDrop( `xpath=${source}`, `xpath=${target}`, { diff --git a/packages/browser/src/node/commands/fill.ts b/packages/browser/src/node/commands/fill.ts index bdb3b8922519..866c2165aa14 100644 --- a/packages/browser/src/node/commands/fill.ts +++ b/packages/browser/src/node/commands/fill.ts @@ -10,8 +10,8 @@ export const fill: UserEventCommand = async ( options = {}, ) => { if (context.provider instanceof PlaywrightBrowserProvider) { - const { frame } = context - const element = frame.locator(`xpath=${xpath}`) + const { iframe } = context + const element = iframe.locator(`xpath=${xpath}`) await element.fill(text, { timeout: 1000, ...options }) } else if (context.provider instanceof WebdriverBrowserProvider) { diff --git a/packages/browser/src/node/commands/hover.ts b/packages/browser/src/node/commands/hover.ts index aa23b156cb39..043c9e21c5fa 100644 --- a/packages/browser/src/node/commands/hover.ts +++ b/packages/browser/src/node/commands/hover.ts @@ -9,7 +9,7 @@ export const hover: UserEventCommand = async ( options = {}, ) => { if (context.provider instanceof PlaywrightBrowserProvider) { - await context.frame.locator(`xpath=${xpath}`).hover({ + await context.iframe.locator(`xpath=${xpath}`).hover({ timeout: 1000, ...options, }) diff --git a/packages/browser/src/node/commands/keyboard.ts b/packages/browser/src/node/commands/keyboard.ts index 936aab1bbc25..ff8687854481 100644 --- a/packages/browser/src/node/commands/keyboard.ts +++ b/packages/browser/src/node/commands/keyboard.ts @@ -21,7 +21,8 @@ export const keyboard: UserEventCommand = async ( } if (context.provider instanceof PlaywrightBrowserProvider) { - await context.frame.evaluate(focusIframe) + const frame = await context.frame() + await frame.evaluate(focusIframe) } else if (context.provider instanceof WebdriverBrowserProvider) { await context.browser.execute(focusIframe) @@ -39,7 +40,8 @@ export const keyboard: UserEventCommand = async ( } } if (context.provider instanceof PlaywrightBrowserProvider) { - await context.frame.evaluate(selectAll) + const frame = await context.frame() + await frame.evaluate(selectAll) } else if (context.provider instanceof WebdriverBrowserProvider) { await context.browser.execute(selectAll) diff --git a/packages/browser/src/node/commands/screenshot.ts b/packages/browser/src/node/commands/screenshot.ts index b9371b47c618..a68bd6cfae5a 100644 --- a/packages/browser/src/node/commands/screenshot.ts +++ b/packages/browser/src/node/commands/screenshot.ts @@ -28,12 +28,14 @@ export const screenshot: BrowserCommand<[string, ScreenshotOptions]> = async ( if (context.provider instanceof PlaywrightBrowserProvider) { if (options.element) { const { element: elementXpath, ...config } = options - const iframe = context.frame - const element = iframe.locator(`xpath=${elementXpath}`) + const element = context.iframe.locator(`xpath=${elementXpath}`) await element.screenshot({ ...config, path: savePath }) } else { - await context.frame.locator('body').screenshot({ ...options, path: savePath }) + await context.iframe.locator('body').screenshot({ + ...options, + path: savePath, + }) } return path } diff --git a/packages/browser/src/node/commands/select.ts b/packages/browser/src/node/commands/select.ts index 3d34210699be..14adf30d99bf 100644 --- a/packages/browser/src/node/commands/select.ts +++ b/packages/browser/src/node/commands/select.ts @@ -12,14 +12,14 @@ export const selectOptions: UserEventCommand = async ) => { if (context.provider instanceof PlaywrightBrowserProvider) { const value = userValues as any as (string | { element: string })[] - const { frame } = context - const selectElement = frame.locator(`xpath=${xpath}`) + const { iframe } = context + const selectElement = iframe.locator(`xpath=${xpath}`) const values = await Promise.all(value.map(async (v) => { if (typeof v === 'string') { return v } - const elementHandler = await frame.locator(`xpath=${v.element}`).elementHandle() + const elementHandler = await iframe.locator(`xpath=${v.element}`).elementHandle() if (!elementHandler) { throw new Error(`Element not found: ${v.element}`) } diff --git a/packages/browser/src/node/commands/type.ts b/packages/browser/src/node/commands/type.ts index 2242b685ab6a..e94850943673 100644 --- a/packages/browser/src/node/commands/type.ts +++ b/packages/browser/src/node/commands/type.ts @@ -13,8 +13,8 @@ export const type: UserEventCommand = async ( const { skipClick = false, skipAutoClose = false } = options if (context.provider instanceof PlaywrightBrowserProvider) { - const { frame } = context - const element = frame.locator(`xpath=${xpath}`) + const { iframe } = context + const element = iframe.locator(`xpath=${xpath}`) if (!skipClick) { await element.focus() diff --git a/packages/browser/src/node/providers/playwright.ts b/packages/browser/src/node/providers/playwright.ts index 024204c8fbf8..c3e87da0c754 100644 --- a/packages/browser/src/node/providers/playwright.ts +++ b/packages/browser/src/node/providers/playwright.ts @@ -1,7 +1,9 @@ import type { Browser, + BrowserContext, BrowserContextOptions, + Frame, LaunchOptions, Page, } from 'playwright' @@ -105,8 +107,25 @@ export class PlaywrightBrowserProvider implements BrowserProvider { return { page, context: this.contexts.get(contextId)!, - get frame() { - return page.frame('vitest-iframe')! + frame() { + return new Promise((resolve, reject) => { + const frame = page.frame('vitest-iframe') + if (frame) { + return resolve(frame) + } + + const timeout = setTimeout(() => { + const err = new Error(`Cannot find "vitest-iframe" on the page. This is a bug in Vitest, please report it.`) + reject(err) + }, 1000) + page.on('frameattached', (frame) => { + clearTimeout(timeout) + resolve(frame) + }) + }) + }, + get iframe() { + return page.frameLocator('[data-vitest="true"]')! }, } } @@ -147,9 +166,6 @@ export class PlaywrightBrowserProvider implements BrowserProvider { once(event: string, listener: (...args: any[]) => void) { cdp.once(event as 'Accessibility.loadComplete', listener) }, - detach() { - return cdp.detach() - }, } } diff --git a/packages/vitest/src/types/browser.ts b/packages/vitest/src/types/browser.ts index bc919f81f0bd..14ae5293f0e4 100644 --- a/packages/vitest/src/types/browser.ts +++ b/packages/vitest/src/types/browser.ts @@ -14,7 +14,6 @@ export interface CDPSession { on: (event: string, listener: (...args: unknown[]) => void) => void once: (event: string, listener: (...args: unknown[]) => void) => void off: (event: string, listener: (...args: unknown[]) => void) => void - detach: () => Promise } export interface BrowserProvider {