From a550551801c18ebbea2c40b345d7fe4488ff780d Mon Sep 17 00:00:00 2001 From: Vladimir Date: Fri, 28 Mar 2025 13:47:56 +0100 Subject: [PATCH] fix(browser): correctly calculate timeout in hooks when actions are performed (#7747) --- packages/browser/src/client/tester/tester.ts | 1 + packages/browser/src/client/tester/utils.ts | 10 +- packages/browser/src/client/utils.ts | 2 + packages/runner/src/context.ts | 8 + packages/runner/src/types/runner.ts | 5 + .../timeout-hooks/hooks-timeout.test.ts | 85 ++++++++++ .../fixtures/timeout-hooks/vitest.config.ts | 15 ++ test/browser/specs/runner.test.ts | 156 ++++++++++++++++-- test/browser/test/failing.test.ts | 15 +- 9 files changed, 266 insertions(+), 31 deletions(-) create mode 100644 test/browser/fixtures/timeout-hooks/hooks-timeout.test.ts create mode 100644 test/browser/fixtures/timeout-hooks/vitest.config.ts diff --git a/packages/browser/src/client/tester/tester.ts b/packages/browser/src/client/tester/tester.ts index dd891e15e..3ddab0f3a 100644 --- a/packages/browser/src/client/tester/tester.ts +++ b/packages/browser/src/client/tester/tester.ts @@ -60,6 +60,7 @@ async function prepareTestEnvironment(files: string[]) { setupDialogsSpy() const runner = await initiateRunner(state, mocker, config) + getBrowserState().runner = runner const version = url.searchParams.get('browserv') || '' files.forEach((filename) => { diff --git a/packages/browser/src/client/tester/utils.ts b/packages/browser/src/client/tester/utils.ts index acf8e1086..19afe9437 100644 --- a/packages/browser/src/client/tester/utils.ts +++ b/packages/browser/src/client/tester/utils.ts @@ -144,14 +144,14 @@ export function processTimeoutOptions(options_?: if (getWorkerState().config.browser.providerOptions.actionTimeout != null) { return options_ } - const currentTest = getWorkerState().current - const startTime = currentTest?.result?.startTime + const runner = getBrowserState().runner + const startTime = runner._currentTaskStartTime // ignore timeout if this is called outside of a test - if (!currentTest || currentTest.type === 'suite' || !startTime) { + if (!startTime) { return options_ } - const timeout = currentTest.timeout - if (timeout === 0 || timeout === Number.POSITIVE_INFINITY) { + const timeout = runner._currentTaskTimeout + if (timeout === 0 || timeout == null || timeout === Number.POSITIVE_INFINITY) { return options_ } options_ = options_ || {} as T diff --git a/packages/browser/src/client/utils.ts b/packages/browser/src/client/utils.ts index f46307e1c..90fbad922 100644 --- a/packages/browser/src/client/utils.ts +++ b/packages/browser/src/client/utils.ts @@ -1,3 +1,4 @@ +import type { VitestRunner } from '@vitest/runner' import type { SerializedConfig, WorkerGlobalState } from 'vitest' import type { CommandsManager } from './tester/utils' @@ -66,6 +67,7 @@ export interface BrowserRunnerState { moduleCache: WorkerGlobalState['moduleCache'] config: SerializedConfig provider: string + runner: VitestRunner viteConfig: { root: string } diff --git a/packages/runner/src/context.ts b/packages/runner/src/context.ts index a816fcc96..a15305a9f 100644 --- a/packages/runner/src/context.ts +++ b/packages/runner/src/context.ts @@ -8,6 +8,7 @@ import type { } from './types/tasks' import { getSafeTimers } from '@vitest/utils' import { PendingError } from './errors' +import { getRunner } from './suite' const now = Date.now @@ -45,6 +46,9 @@ export function withTimeout any>( // this function name is used to filter error in test/cli/test/fails.test.ts return (function runWithTimeout(...args: T extends (...args: infer A) => any ? A : never) { const startTime = now() + const runner = getRunner() + runner._currentTaskStartTime = startTime + runner._currentTaskTimeout = timeout return new Promise((resolve_, reject_) => { const timer = setTimeout(() => { clearTimeout(timer) @@ -58,6 +62,8 @@ export function withTimeout any>( } function resolve(result: unknown) { + runner._currentTaskStartTime = undefined + runner._currentTaskTimeout = undefined clearTimeout(timer) // if test/hook took too long in microtask, setTimeout won't be triggered, // but we still need to fail the test, see @@ -70,6 +76,8 @@ export function withTimeout any>( } function reject(error: unknown) { + runner._currentTaskStartTime = undefined + runner._currentTaskTimeout = undefined clearTimeout(timer) reject_(error) } diff --git a/packages/runner/src/types/runner.ts b/packages/runner/src/types/runner.ts index ff9a3b6a1..81bd8d73b 100644 --- a/packages/runner/src/types/runner.ts +++ b/packages/runner/src/types/runner.ts @@ -162,4 +162,9 @@ export interface VitestRunner { * The name of the current pool. Can affect how stack trace is inferred on the server side. */ pool?: string + + /** @private */ + _currentTaskStartTime?: number + /** @private */ + _currentTaskTimeout?: number } diff --git a/test/browser/fixtures/timeout-hooks/hooks-timeout.test.ts b/test/browser/fixtures/timeout-hooks/hooks-timeout.test.ts new file mode 100644 index 000000000..7ee586252 --- /dev/null +++ b/test/browser/fixtures/timeout-hooks/hooks-timeout.test.ts @@ -0,0 +1,85 @@ +import { page, server } from '@vitest/browser/context'; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, onTestFailed, onTestFinished } from 'vitest'; + +describe.runIf(server.provider === 'playwright')('timeouts are failing correctly', () => { + it('click on non-existing element fails', async () => { + await new Promise(r => setTimeout(r, 100)) + await page.getByRole('code').click() + }, 500) + + it('expect.element on non-existing element fails', async () => { + await expect.element(page.getByRole('code')).toBeVisible() + }, 500) + + describe('beforeEach', () => { + beforeEach(async () => { + await new Promise(r => setTimeout(r, 100)) + await page.getByTestId('non-existing').click() + }, 500) + + it('skipped') + }) + + describe('afterEach', () => { + afterEach(async () => { + await new Promise(r => setTimeout(r, 100)) + await page.getByTestId('non-existing').click() + }, 500) + + it('skipped') + }) + + describe('beforeAll', () => { + beforeAll(async () => { + await new Promise(r => setTimeout(r, 100)) + await page.getByTestId('non-existing').click() + }, 500) + + it('skipped') + }) + + describe('afterAll', () => { + afterAll(async () => { + await new Promise(r => setTimeout(r, 100)) + await page.getByTestId('non-existing').click() + }, 500) + + it('skipped') + }) + + describe('onTestFinished', () => { + it('fails', ({ onTestFinished }) => { + onTestFinished(async () => { + await new Promise(r => setTimeout(r, 100)) + await page.getByTestId('non-existing').click() + }, 500) + }) + + it('fails global', () => { + onTestFinished(async () => { + await new Promise(r => setTimeout(r, 100)) + await page.getByTestId('non-existing').click() + }, 500) + }) + }) + + describe('onTestFailed', () => { + it('fails', ({ onTestFailed }) => { + onTestFailed(async () => { + await new Promise(r => setTimeout(r, 100)) + await page.getByTestId('non-existing').click() + }, 500) + + expect.unreachable() + }) + + it('fails global', () => { + onTestFailed(async () => { + await new Promise(r => setTimeout(r, 100)) + await page.getByTestId('non-existing').click() + }, 500) + + expect.unreachable() + }) + }) +}) diff --git a/test/browser/fixtures/timeout-hooks/vitest.config.ts b/test/browser/fixtures/timeout-hooks/vitest.config.ts new file mode 100644 index 000000000..24e7abdee --- /dev/null +++ b/test/browser/fixtures/timeout-hooks/vitest.config.ts @@ -0,0 +1,15 @@ +import { fileURLToPath } from 'node:url' +import { defineConfig } from 'vitest/config' +import { instances, provider } from '../../settings' + +export default defineConfig({ + cacheDir: fileURLToPath(new URL("./node_modules/.vite", import.meta.url)), + test: { + browser: { + enabled: true, + provider, + instances, + screenshotFailures: false, + }, + }, +}) diff --git a/test/browser/specs/runner.test.ts b/test/browser/specs/runner.test.ts index c74f9e478..703f76e97 100644 --- a/test/browser/specs/runner.test.ts +++ b/test/browser/specs/runner.test.ts @@ -176,19 +176,8 @@ error with a stack expect(stderr).toMatch(/bundled-lib\/src\/b.js:2:(8|18)/) expect(stderr).toMatch(/bundled-lib\/src\/index.js:5:(15|17)/) - if (provider === 'playwright') { - // page.getByRole('code').click() - expect(stderr).toContain('locator.click: Timeout') - // playwright error is proxied from the server to the client and back correctly - expect(stderr).toContain('waiting for locator(\'[data-vitest="true"]\').contentFrame().getByRole(\'code\')') - expect(stderr).toMatch(/test\/failing.test.ts:27:(33|39)/) - // await expect.element().toBeVisible() - expect(stderr).toContain('Cannot find element with locator: getByRole(\'code\')') - expect(stderr).toMatch(/test\/failing.test.ts:31:(49|61)/) - } - // index() is called from a bundled file - expect(stderr).toMatch(/test\/failing.test.ts:36:(2|8)/) + expect(stderr).toMatch(/test\/failing.test.ts:25:(2|8)/) }) test('popup apis should log a warning', () => { @@ -219,7 +208,7 @@ test('user-event', async () => { }) }) -test('timeout', async () => { +test('timeout settings', async () => { const { stderr } = await runBrowserTests({ root: './fixtures/timeout', }) @@ -232,3 +221,144 @@ test('timeout', async () => { expect(stderr).toContain('Cannot find element with locator') } }) + +test.runIf(provider === 'playwright')('timeout hooks', async () => { + const { stderr } = await runBrowserTests({ + root: './fixtures/timeout-hooks', + }) + + const lines = stderr.split('\n') + const timeoutErrorsIndexes = [] + lines.forEach((line, index) => { + if (line.includes('TimeoutError:')) { + timeoutErrorsIndexes.push(index) + } + }) + + const snapshot = timeoutErrorsIndexes.map((index) => { + return [ + lines[index - 1], + lines[index].replace(/Timeout \d+ms exceeded/, 'Timeout exceeded'), + lines[index + 4], + ].join('\n') + }).sort().join('\n\n') + + expect(snapshot).toMatchInlineSnapshot(` + " FAIL |chromium| hooks-timeout.test.ts > timeouts are failing correctly > afterAll + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:44:45 + + FAIL |chromium| hooks-timeout.test.ts > timeouts are failing correctly > afterEach > skipped + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:26:45 + + FAIL |chromium| hooks-timeout.test.ts > timeouts are failing correctly > beforeAll + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:35:45 + + FAIL |chromium| hooks-timeout.test.ts > timeouts are failing correctly > beforeEach > skipped + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:17:45 + + FAIL |chromium| hooks-timeout.test.ts > timeouts are failing correctly > click on non-existing element fails + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:7:33 + + FAIL |chromium| hooks-timeout.test.ts > timeouts are failing correctly > onTestFailed > fails + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:70:47 + + FAIL |chromium| hooks-timeout.test.ts > timeouts are failing correctly > onTestFailed > fails global + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:79:47 + + FAIL |chromium| hooks-timeout.test.ts > timeouts are failing correctly > onTestFinished > fails + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:54:47 + + FAIL |chromium| hooks-timeout.test.ts > timeouts are failing correctly > onTestFinished > fails global + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:61:47 + + FAIL |firefox| hooks-timeout.test.ts > timeouts are failing correctly > afterAll + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:44:45 + + FAIL |firefox| hooks-timeout.test.ts > timeouts are failing correctly > afterEach > skipped + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:26:45 + + FAIL |firefox| hooks-timeout.test.ts > timeouts are failing correctly > beforeAll + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:35:45 + + FAIL |firefox| hooks-timeout.test.ts > timeouts are failing correctly > beforeEach > skipped + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:17:45 + + FAIL |firefox| hooks-timeout.test.ts > timeouts are failing correctly > click on non-existing element fails + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:7:33 + + FAIL |firefox| hooks-timeout.test.ts > timeouts are failing correctly > onTestFailed > fails + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:70:47 + + FAIL |firefox| hooks-timeout.test.ts > timeouts are failing correctly > onTestFailed > fails global + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:79:47 + + FAIL |firefox| hooks-timeout.test.ts > timeouts are failing correctly > onTestFinished > fails + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:54:47 + + FAIL |firefox| hooks-timeout.test.ts > timeouts are failing correctly > onTestFinished > fails global + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:61:47 + + FAIL |webkit| hooks-timeout.test.ts > timeouts are failing correctly > afterAll + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:44:51 + + FAIL |webkit| hooks-timeout.test.ts > timeouts are failing correctly > afterEach > skipped + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:26:51 + + FAIL |webkit| hooks-timeout.test.ts > timeouts are failing correctly > beforeAll + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:35:51 + + FAIL |webkit| hooks-timeout.test.ts > timeouts are failing correctly > beforeEach > skipped + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:17:51 + + FAIL |webkit| hooks-timeout.test.ts > timeouts are failing correctly > click on non-existing element fails + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:7:39 + + FAIL |webkit| hooks-timeout.test.ts > timeouts are failing correctly > onTestFailed > fails + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:70:53 + + FAIL |webkit| hooks-timeout.test.ts > timeouts are failing correctly > onTestFailed > fails global + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:79:53 + + FAIL |webkit| hooks-timeout.test.ts > timeouts are failing correctly > onTestFinished > fails + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:54:53 + + FAIL |webkit| hooks-timeout.test.ts > timeouts are failing correctly > onTestFinished > fails global + TimeoutError: locator.click: Timeout exceeded. + ❯ hooks-timeout.test.ts:61:53" + `) + + // page.getByRole('code').click() + expect(stderr).toContain('locator.click: Timeout') + // playwright error is proxied from the server to the client and back correctly + expect(stderr).toContain('waiting for locator(\'[data-vitest="true"]\').contentFrame().getByRole(\'code\')') + expect(stderr).toMatch(/hooks-timeout.test.ts:7:(33|39)/) + // await expect.element().toBeVisible() + expect(stderr).toContain('Cannot find element with locator: getByRole(\'code\')') + expect(stderr).toMatch(/hooks-timeout.test.ts:11:(49|61)/) +}, 120_000 * 3) diff --git a/test/browser/test/failing.test.ts b/test/browser/test/failing.test.ts index fd3cb4951..2f88b0d1f 100644 --- a/test/browser/test/failing.test.ts +++ b/test/browser/test/failing.test.ts @@ -1,6 +1,6 @@ -import { page, server } from '@vitest/browser/context' +import { page } from '@vitest/browser/context' import { index } from '@vitest/bundled-lib' -import { describe, expect, it } from 'vitest' +import { expect, it } from 'vitest' import { throwError } from '../src/error' document.body.innerHTML = ` @@ -21,17 +21,6 @@ it('several locator methods are not awaited', () => { page.getByRole('button').tripleClick() }) -describe.runIf(server.provider === 'playwright')('timeouts are failing correctly', () => { - it('click on non-existing element fails', async () => { - await new Promise(r => setTimeout(r, 100)) - await page.getByRole('code').click() - }, 1000) - - it('expect.element on non-existing element fails', async () => { - await expect.element(page.getByRole('code')).toBeVisible() - }, 1000) -}) - it('correctly prints error from a bundled file', () => { index() }) -- 2.51.2