Skip to content

Commit a550551

Browse files
sheremet-vasheremet-va
authored
fix(browser): correctly calculate timeout in hooks when actions are performed (#7747)
1 parent 8299709 commit a550551

File tree

9 files changed

+266
-31
lines changed

9 files changed

+266
-31
lines changed

‎packages/browser/src/client/tester/tester.ts

+1
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ async function prepareTestEnvironment(files: string[]) {
6060
setupDialogsSpy()
6161

6262
const runner = await initiateRunner(state, mocker, config)
63+
getBrowserState().runner = runner
6364

6465
const version = url.searchParams.get('browserv') || ''
6566
files.forEach((filename) => {

‎packages/browser/src/client/tester/utils.ts

+5-5
Original file line numberDiff line numberDiff line change
@@ -144,14 +144,14 @@ export function processTimeoutOptions<T extends { timeout?: number }>(options_?:
144144
if (getWorkerState().config.browser.providerOptions.actionTimeout != null) {
145145
return options_
146146
}
147-
const currentTest = getWorkerState().current
148-
const startTime = currentTest?.result?.startTime
147+
const runner = getBrowserState().runner
148+
const startTime = runner._currentTaskStartTime
149149
// ignore timeout if this is called outside of a test
150-
if (!currentTest || currentTest.type === 'suite' || !startTime) {
150+
if (!startTime) {
151151
return options_
152152
}
153-
const timeout = currentTest.timeout
154-
if (timeout === 0 || timeout === Number.POSITIVE_INFINITY) {
153+
const timeout = runner._currentTaskTimeout
154+
if (timeout === 0 || timeout == null || timeout === Number.POSITIVE_INFINITY) {
155155
return options_
156156
}
157157
options_ = options_ || {} as T

‎packages/browser/src/client/utils.ts

+2
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import type { VitestRunner } from '@vitest/runner'
12
import type { SerializedConfig, WorkerGlobalState } from 'vitest'
23
import type { CommandsManager } from './tester/utils'
34

@@ -66,6 +67,7 @@ export interface BrowserRunnerState {
6667
moduleCache: WorkerGlobalState['moduleCache']
6768
config: SerializedConfig
6869
provider: string
70+
runner: VitestRunner
6971
viteConfig: {
7072
root: string
7173
}

‎packages/runner/src/context.ts

+8
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type {
88
} from './types/tasks'
99
import { getSafeTimers } from '@vitest/utils'
1010
import { PendingError } from './errors'
11+
import { getRunner } from './suite'
1112

1213
const now = Date.now
1314

@@ -45,6 +46,9 @@ export function withTimeout<T extends (...args: any[]) => any>(
4546
// this function name is used to filter error in test/cli/test/fails.test.ts
4647
return (function runWithTimeout(...args: T extends (...args: infer A) => any ? A : never) {
4748
const startTime = now()
49+
const runner = getRunner()
50+
runner._currentTaskStartTime = startTime
51+
runner._currentTaskTimeout = timeout
4852
return new Promise((resolve_, reject_) => {
4953
const timer = setTimeout(() => {
5054
clearTimeout(timer)
@@ -58,6 +62,8 @@ export function withTimeout<T extends (...args: any[]) => any>(
5862
}
5963

6064
function resolve(result: unknown) {
65+
runner._currentTaskStartTime = undefined
66+
runner._currentTaskTimeout = undefined
6167
clearTimeout(timer)
6268
// if test/hook took too long in microtask, setTimeout won't be triggered,
6369
// but we still need to fail the test, see
@@ -70,6 +76,8 @@ export function withTimeout<T extends (...args: any[]) => any>(
7076
}
7177

7278
function reject(error: unknown) {
79+
runner._currentTaskStartTime = undefined
80+
runner._currentTaskTimeout = undefined
7381
clearTimeout(timer)
7482
reject_(error)
7583
}

‎packages/runner/src/types/runner.ts

+5
Original file line numberDiff line numberDiff line change
@@ -162,4 +162,9 @@ export interface VitestRunner {
162162
* The name of the current pool. Can affect how stack trace is inferred on the server side.
163163
*/
164164
pool?: string
165+
166+
/** @private */
167+
_currentTaskStartTime?: number
168+
/** @private */
169+
_currentTaskTimeout?: number
165170
}

‎test/browser/fixtures/timeout-hooks/hooks-timeout.test.ts

+85
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
import { page, server } from '@vitest/browser/context';
2+
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, onTestFailed, onTestFinished } from 'vitest';
3+
4+
describe.runIf(server.provider === 'playwright')('timeouts are failing correctly', () => {
5+
it('click on non-existing element fails', async () => {
6+
await new Promise(r => setTimeout(r, 100))
7+
await page.getByRole('code').click()
8+
}, 500)
9+
10+
it('expect.element on non-existing element fails', async () => {
11+
await expect.element(page.getByRole('code')).toBeVisible()
12+
}, 500)
13+
14+
describe('beforeEach', () => {
15+
beforeEach(async () => {
16+
await new Promise(r => setTimeout(r, 100))
17+
await page.getByTestId('non-existing').click()
18+
}, 500)
19+
20+
it('skipped')
21+
})
22+
23+
describe('afterEach', () => {
24+
afterEach(async () => {
25+
await new Promise(r => setTimeout(r, 100))
26+
await page.getByTestId('non-existing').click()
27+
}, 500)
28+
29+
it('skipped')
30+
})
31+
32+
describe('beforeAll', () => {
33+
beforeAll(async () => {
34+
await new Promise(r => setTimeout(r, 100))
35+
await page.getByTestId('non-existing').click()
36+
}, 500)
37+
38+
it('skipped')
39+
})
40+
41+
describe('afterAll', () => {
42+
afterAll(async () => {
43+
await new Promise(r => setTimeout(r, 100))
44+
await page.getByTestId('non-existing').click()
45+
}, 500)
46+
47+
it('skipped')
48+
})
49+
50+
describe('onTestFinished', () => {
51+
it('fails', ({ onTestFinished }) => {
52+
onTestFinished(async () => {
53+
await new Promise(r => setTimeout(r, 100))
54+
await page.getByTestId('non-existing').click()
55+
}, 500)
56+
})
57+
58+
it('fails global', () => {
59+
onTestFinished(async () => {
60+
await new Promise(r => setTimeout(r, 100))
61+
await page.getByTestId('non-existing').click()
62+
}, 500)
63+
})
64+
})
65+
66+
describe('onTestFailed', () => {
67+
it('fails', ({ onTestFailed }) => {
68+
onTestFailed(async () => {
69+
await new Promise(r => setTimeout(r, 100))
70+
await page.getByTestId('non-existing').click()
71+
}, 500)
72+
73+
expect.unreachable()
74+
})
75+
76+
it('fails global', () => {
77+
onTestFailed(async () => {
78+
await new Promise(r => setTimeout(r, 100))
79+
await page.getByTestId('non-existing').click()
80+
}, 500)
81+
82+
expect.unreachable()
83+
})
84+
})
85+
})

‎test/browser/fixtures/timeout-hooks/vitest.config.ts

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import { fileURLToPath } from 'node:url'
2+
import { defineConfig } from 'vitest/config'
3+
import { instances, provider } from '../../settings'
4+
5+
export default defineConfig({
6+
cacheDir: fileURLToPath(new URL("./node_modules/.vite", import.meta.url)),
7+
test: {
8+
browser: {
9+
enabled: true,
10+
provider,
11+
instances,
12+
screenshotFailures: false,
13+
},
14+
},
15+
})

‎test/browser/specs/runner.test.ts

+143-13
Original file line numberDiff line numberDiff line change
@@ -176,19 +176,8 @@ error with a stack
176176
expect(stderr).toMatch(/bundled-lib\/src\/b.js:2:(8|18)/)
177177
expect(stderr).toMatch(/bundled-lib\/src\/index.js:5:(15|17)/)
178178

179-
if (provider === 'playwright') {
180-
// page.getByRole('code').click()
181-
expect(stderr).toContain('locator.click: Timeout')
182-
// playwright error is proxied from the server to the client and back correctly
183-
expect(stderr).toContain('waiting for locator(\'[data-vitest="true"]\').contentFrame().getByRole(\'code\')')
184-
expect(stderr).toMatch(/test\/failing.test.ts:27:(33|39)/)
185-
// await expect.element().toBeVisible()
186-
expect(stderr).toContain('Cannot find element with locator: getByRole(\'code\')')
187-
expect(stderr).toMatch(/test\/failing.test.ts:31:(49|61)/)
188-
}
189-
190179
// index() is called from a bundled file
191-
expect(stderr).toMatch(/test\/failing.test.ts:36:(2|8)/)
180+
expect(stderr).toMatch(/test\/failing.test.ts:25:(2|8)/)
192181
})
193182

194183
test('popup apis should log a warning', () => {
@@ -219,7 +208,7 @@ test('user-event', async () => {
219208
})
220209
})
221210

222-
test('timeout', async () => {
211+
test('timeout settings', async () => {
223212
const { stderr } = await runBrowserTests({
224213
root: './fixtures/timeout',
225214
})
@@ -232,3 +221,144 @@ test('timeout', async () => {
232221
expect(stderr).toContain('Cannot find element with locator')
233222
}
234223
})
224+
225+
test.runIf(provider === 'playwright')('timeout hooks', async () => {
226+
const { stderr } = await runBrowserTests({
227+
root: './fixtures/timeout-hooks',
228+
})
229+
230+
const lines = stderr.split('\n')
231+
const timeoutErrorsIndexes = []
232+
lines.forEach((line, index) => {
233+
if (line.includes('TimeoutError:')) {
234+
timeoutErrorsIndexes.push(index)
235+
}
236+
})
237+
238+
const snapshot = timeoutErrorsIndexes.map((index) => {
239+
return [
240+
lines[index - 1],
241+
lines[index].replace(/Timeout \d+ms exceeded/, 'Timeout <ms> exceeded'),
242+
lines[index + 4],
243+
].join('\n')
244+
}).sort().join('\n\n')
245+
246+
expect(snapshot).toMatchInlineSnapshot(`
247+
" FAIL |chromium| hooks-timeout.test.ts > timeouts are failing correctly > afterAll
248+
TimeoutError: locator.click: Timeout <ms> exceeded.
249+
❯ hooks-timeout.test.ts:44:45
250+
251+
FAIL |chromium| hooks-timeout.test.ts > timeouts are failing correctly > afterEach > skipped
252+
TimeoutError: locator.click: Timeout <ms> exceeded.
253+
❯ hooks-timeout.test.ts:26:45
254+
255+
FAIL |chromium| hooks-timeout.test.ts > timeouts are failing correctly > beforeAll
256+
TimeoutError: locator.click: Timeout <ms> exceeded.
257+
❯ hooks-timeout.test.ts:35:45
258+
259+
FAIL |chromium| hooks-timeout.test.ts > timeouts are failing correctly > beforeEach > skipped
260+
TimeoutError: locator.click: Timeout <ms> exceeded.
261+
❯ hooks-timeout.test.ts:17:45
262+
263+
FAIL |chromium| hooks-timeout.test.ts > timeouts are failing correctly > click on non-existing element fails
264+
TimeoutError: locator.click: Timeout <ms> exceeded.
265+
❯ hooks-timeout.test.ts:7:33
266+
267+
FAIL |chromium| hooks-timeout.test.ts > timeouts are failing correctly > onTestFailed > fails
268+
TimeoutError: locator.click: Timeout <ms> exceeded.
269+
❯ hooks-timeout.test.ts:70:47
270+
271+
FAIL |chromium| hooks-timeout.test.ts > timeouts are failing correctly > onTestFailed > fails global
272+
TimeoutError: locator.click: Timeout <ms> exceeded.
273+
❯ hooks-timeout.test.ts:79:47
274+
275+
FAIL |chromium| hooks-timeout.test.ts > timeouts are failing correctly > onTestFinished > fails
276+
TimeoutError: locator.click: Timeout <ms> exceeded.
277+
❯ hooks-timeout.test.ts:54:47
278+
279+
FAIL |chromium| hooks-timeout.test.ts > timeouts are failing correctly > onTestFinished > fails global
280+
TimeoutError: locator.click: Timeout <ms> exceeded.
281+
❯ hooks-timeout.test.ts:61:47
282+
283+
FAIL |firefox| hooks-timeout.test.ts > timeouts are failing correctly > afterAll
284+
TimeoutError: locator.click: Timeout <ms> exceeded.
285+
❯ hooks-timeout.test.ts:44:45
286+
287+
FAIL |firefox| hooks-timeout.test.ts > timeouts are failing correctly > afterEach > skipped
288+
TimeoutError: locator.click: Timeout <ms> exceeded.
289+
❯ hooks-timeout.test.ts:26:45
290+
291+
FAIL |firefox| hooks-timeout.test.ts > timeouts are failing correctly > beforeAll
292+
TimeoutError: locator.click: Timeout <ms> exceeded.
293+
❯ hooks-timeout.test.ts:35:45
294+
295+
FAIL |firefox| hooks-timeout.test.ts > timeouts are failing correctly > beforeEach > skipped
296+
TimeoutError: locator.click: Timeout <ms> exceeded.
297+
❯ hooks-timeout.test.ts:17:45
298+
299+
FAIL |firefox| hooks-timeout.test.ts > timeouts are failing correctly > click on non-existing element fails
300+
TimeoutError: locator.click: Timeout <ms> exceeded.
301+
❯ hooks-timeout.test.ts:7:33
302+
303+
FAIL |firefox| hooks-timeout.test.ts > timeouts are failing correctly > onTestFailed > fails
304+
TimeoutError: locator.click: Timeout <ms> exceeded.
305+
❯ hooks-timeout.test.ts:70:47
306+
307+
FAIL |firefox| hooks-timeout.test.ts > timeouts are failing correctly > onTestFailed > fails global
308+
TimeoutError: locator.click: Timeout <ms> exceeded.
309+
❯ hooks-timeout.test.ts:79:47
310+
311+
FAIL |firefox| hooks-timeout.test.ts > timeouts are failing correctly > onTestFinished > fails
312+
TimeoutError: locator.click: Timeout <ms> exceeded.
313+
❯ hooks-timeout.test.ts:54:47
314+
315+
FAIL |firefox| hooks-timeout.test.ts > timeouts are failing correctly > onTestFinished > fails global
316+
TimeoutError: locator.click: Timeout <ms> exceeded.
317+
❯ hooks-timeout.test.ts:61:47
318+
319+
FAIL |webkit| hooks-timeout.test.ts > timeouts are failing correctly > afterAll
320+
TimeoutError: locator.click: Timeout <ms> exceeded.
321+
❯ hooks-timeout.test.ts:44:51
322+
323+
FAIL |webkit| hooks-timeout.test.ts > timeouts are failing correctly > afterEach > skipped
324+
TimeoutError: locator.click: Timeout <ms> exceeded.
325+
❯ hooks-timeout.test.ts:26:51
326+
327+
FAIL |webkit| hooks-timeout.test.ts > timeouts are failing correctly > beforeAll
328+
TimeoutError: locator.click: Timeout <ms> exceeded.
329+
❯ hooks-timeout.test.ts:35:51
330+
331+
FAIL |webkit| hooks-timeout.test.ts > timeouts are failing correctly > beforeEach > skipped
332+
TimeoutError: locator.click: Timeout <ms> exceeded.
333+
❯ hooks-timeout.test.ts:17:51
334+
335+
FAIL |webkit| hooks-timeout.test.ts > timeouts are failing correctly > click on non-existing element fails
336+
TimeoutError: locator.click: Timeout <ms> exceeded.
337+
❯ hooks-timeout.test.ts:7:39
338+
339+
FAIL |webkit| hooks-timeout.test.ts > timeouts are failing correctly > onTestFailed > fails
340+
TimeoutError: locator.click: Timeout <ms> exceeded.
341+
❯ hooks-timeout.test.ts:70:53
342+
343+
FAIL |webkit| hooks-timeout.test.ts > timeouts are failing correctly > onTestFailed > fails global
344+
TimeoutError: locator.click: Timeout <ms> exceeded.
345+
❯ hooks-timeout.test.ts:79:53
346+
347+
FAIL |webkit| hooks-timeout.test.ts > timeouts are failing correctly > onTestFinished > fails
348+
TimeoutError: locator.click: Timeout <ms> exceeded.
349+
❯ hooks-timeout.test.ts:54:53
350+
351+
FAIL |webkit| hooks-timeout.test.ts > timeouts are failing correctly > onTestFinished > fails global
352+
TimeoutError: locator.click: Timeout <ms> exceeded.
353+
❯ hooks-timeout.test.ts:61:53"
354+
`)
355+
356+
// page.getByRole('code').click()
357+
expect(stderr).toContain('locator.click: Timeout')
358+
// playwright error is proxied from the server to the client and back correctly
359+
expect(stderr).toContain('waiting for locator(\'[data-vitest="true"]\').contentFrame().getByRole(\'code\')')
360+
expect(stderr).toMatch(/hooks-timeout.test.ts:7:(33|39)/)
361+
// await expect.element().toBeVisible()
362+
expect(stderr).toContain('Cannot find element with locator: getByRole(\'code\')')
363+
expect(stderr).toMatch(/hooks-timeout.test.ts:11:(49|61)/)
364+
}, 120_000 * 3)

‎test/browser/test/failing.test.ts

+2-13
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
import { page, server } from '@vitest/browser/context'
1+
import { page } from '@vitest/browser/context'
22
import { index } from '@vitest/bundled-lib'
3-
import { describe, expect, it } from 'vitest'
3+
import { expect, it } from 'vitest'
44
import { throwError } from '../src/error'
55

66
document.body.innerHTML = `
@@ -21,17 +21,6 @@ it('several locator methods are not awaited', () => {
2121
page.getByRole('button').tripleClick()
2222
})
2323

24-
describe.runIf(server.provider === 'playwright')('timeouts are failing correctly', () => {
25-
it('click on non-existing element fails', async () => {
26-
await new Promise(r => setTimeout(r, 100))
27-
await page.getByRole('code').click()
28-
}, 1000)
29-
30-
it('expect.element on non-existing element fails', async () => {
31-
await expect.element(page.getByRole('code')).toBeVisible()
32-
}, 1000)
33-
})
34-
3524
it('correctly prints error from a bundled file', () => {
3625
index()
3726
})

0 commit comments

Comments
 (0)