diff --git a/packages/playwright-core/src/utils/timeoutRunner.ts b/packages/playwright-core/src/utils/timeoutRunner.ts index c1a848caba..981f0f517a 100644 --- a/packages/playwright-core/src/utils/timeoutRunner.ts +++ b/packages/playwright-core/src/utils/timeoutRunner.ts @@ -109,3 +109,24 @@ export async function raceAgainstTimeout(cb: () => Promise, timeout: numbe throw e; } } + +export async function pollAgainstTimeout(callback: () => Promise<{ continuePolling: boolean, result: T }>, timeout: number, pollIntervals: number[] = [100, 250, 500, 1000]): Promise<{ result?: T, timedOut: boolean }> { + const startTime = monotonicTime(); + const lastPollInterval = pollIntervals.pop() ?? 1000; + let lastResult: T|undefined; + const wrappedCallback = () => Promise.resolve().then(callback); + while (true) { + const elapsed = monotonicTime() - startTime; + if (timeout !== 0 && elapsed > timeout) + break; + const received = timeout !== 0 ? await raceAgainstTimeout(wrappedCallback, timeout - elapsed) + : await wrappedCallback().then(value => ({ result: value, timedOut: false })); + if (received.timedOut) + break; + lastResult = received.result.result; + if (!received.result.continuePolling) + return { result: received.result.result, timedOut: false }; + await new Promise(x => setTimeout(x, pollIntervals!.shift() ?? lastPollInterval)); + } + return { timedOut: true, result: lastResult }; +} diff --git a/packages/playwright-test/src/expect.ts b/packages/playwright-test/src/expect.ts index 55f6139446..048310d47f 100644 --- a/packages/playwright-test/src/expect.ts +++ b/packages/playwright-test/src/expect.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { raceAgainstTimeout } from 'playwright-core/lib/utils/timeoutRunner'; +import { pollAgainstTimeout } from 'playwright-core/lib/utils/timeoutRunner'; import path from 'path'; import { toBeChecked, @@ -44,7 +44,6 @@ import { toMatchSnapshot, toHaveScreenshot } from './matchers/toMatchSnapshot'; import type { Expect } from './types'; import { currentTestInfo } from './globals'; import { serializeError, captureStackTrace, currentExpectTimeout } from './util'; -import { monotonicTime } from 'playwright-core/lib/utils'; import { expect as expectLibrary, INVERTED_COLOR, @@ -253,38 +252,30 @@ class ExpectMetaInfoProxyHandler { } async function pollMatcher(matcherName: any, isNot: boolean, pollIntervals: number[] | undefined, timeout: number, generator: () => any, ...args: any[]) { - let matcherError; - const startTime = monotonicTime(); - pollIntervals = pollIntervals || [100, 250, 500, 1000]; - const lastPollInterval = pollIntervals[pollIntervals.length - 1] || 1000; - while (true) { - const elapsed = monotonicTime() - startTime; - if (timeout !== 0 && elapsed > timeout) - break; - const received = timeout !== 0 ? await raceAgainstTimeout(generator, timeout - elapsed) : await generator(); - if (received.timedOut) - break; + const result = await pollAgainstTimeout(async () => { + const value = await generator(); + let expectInstance = expectLibrary(value) as any; + if (isNot) + expectInstance = expectInstance.not; try { - let expectInstance = expectLibrary(received.result) as any; - if (isNot) - expectInstance = expectInstance.not; expectInstance[matcherName].call(expectInstance, ...args); - return; - } catch (e) { - matcherError = e; + return { continuePolling: false, result: undefined }; + } catch (error) { + return { continuePolling: true, result: error }; } - await new Promise(x => setTimeout(x, pollIntervals!.shift() ?? lastPollInterval)); + }, timeout, pollIntervals ?? [100, 250, 500, 1000]); + + if (result.timedOut) { + const timeoutMessage = `Timeout ${timeout}ms exceeded while waiting on the predicate`; + const message = result.result ? [ + result.result.message, + '', + `Call Log:`, + `- ${timeoutMessage}`, + ].join('\n') : timeoutMessage; + + throw new Error(message); } - - const timeoutMessage = `Timeout ${timeout}ms exceeded while waiting on the predicate`; - const message = matcherError ? [ - matcherError.message, - '', - `Call Log:`, - `- ${timeoutMessage}`, - ].join('\n') : timeoutMessage; - - throw new Error(message); } expectLibrary.extend(customMatchers); diff --git a/packages/playwright-test/src/matchers/matchers.ts b/packages/playwright-test/src/matchers/matchers.ts index b9477377e3..d18cbc0fe0 100644 --- a/packages/playwright-test/src/matchers/matchers.ts +++ b/packages/playwright-test/src/matchers/matchers.ts @@ -25,8 +25,7 @@ import { toEqual } from './toEqual'; import { toExpectedTextValues, toMatchText } from './toMatchText'; import type { ParsedStackTrace } from 'playwright-core/lib/utils/stackTrace'; import { isTextualMimeType } from 'playwright-core/lib/utils/mimeType'; -import { monotonicTime } from 'playwright-core/lib/utils'; -import { raceAgainstTimeout } from 'playwright-core/lib/utils/timeoutRunner'; +import { pollAgainstTimeout } from 'playwright-core/lib/utils/timeoutRunner'; interface LocatorEx extends Locator { _expect(customStackTrace: ParsedStackTrace, expression: string, options: Omit & { expectedValue?: any }): Promise<{ matches: boolean, received?: any, log?: string[], timedOut?: boolean }>; @@ -320,43 +319,27 @@ export async function toPass( timeout?: number, } = {}, ) { - let matcherError: Error | undefined; - const startTime = monotonicTime(); - const pollIntervals = options.intervals || [100, 250, 500, 1000]; - const lastPollInterval = pollIntervals[pollIntervals.length - 1] || 1000; const timeout = options.timeout !== undefined ? options.timeout : 0; - const isNot = this.isNot; - while (true) { - const elapsed = monotonicTime() - startTime; - if (timeout !== 0 && elapsed > timeout) - break; + const result = await pollAgainstTimeout(async () => { try { - const wrappedCallback = () => Promise.resolve().then(callback); - const received = timeout !== 0 ? await raceAgainstTimeout(wrappedCallback, timeout - elapsed) - : await wrappedCallback().then(() => ({ timedOut: false })); - if (received.timedOut) - break; - // The check passed, exit sucessfully. - if (isNot) - matcherError = new Error('Expected to fail, but passed'); - else - return { message: () => '', pass: true }; + await callback(); + return { continuePolling: this.isNot, result: undefined }; } catch (e) { - if (isNot) - return { message: () => '', pass: false }; - matcherError = e; + return { continuePolling: !this.isNot, result: e }; } - await new Promise(x => setTimeout(x, pollIntervals!.shift() ?? lastPollInterval)); + }, timeout, options.intervals || [100, 250, 500, 1000]); + + if (result.timedOut) { + const timeoutMessage = `Timeout ${timeout}ms exceeded while waiting on the predicate`; + const message = () => result.result ? [ + result.result.message, + '', + `Call Log:`, + `- ${timeoutMessage}`, + ].join('\n') : timeoutMessage; + + return { message, pass: this.isNot }; } - - const timeoutMessage = `Timeout ${timeout}ms exceeded while waiting on the predicate`; - const message = () => matcherError ? [ - matcherError.message, - '', - `Call Log:`, - `- ${timeoutMessage}`, - ].join('\n') : timeoutMessage; - - return { message, pass: isNot ? true : false }; -} + return { pass: !this.isNot, message: () => '' }; +} \ No newline at end of file diff --git a/tests/playwright-test/expect-poll.spec.ts b/tests/playwright-test/expect-poll.spec.ts index a3e727ce9e..c6ceac13e6 100644 --- a/tests/playwright-test/expect-poll.spec.ts +++ b/tests/playwright-test/expect-poll.spec.ts @@ -137,6 +137,24 @@ test('should show error that is thrown from predicate', async ({ runInlineTest } expect(stripAnsi(result.output)).toContain('foo bar baz'); }); +test('should not retry predicate that threw an error', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.spec.ts': ` + const { test } = pwt; + test('should fail', async ({ page }) => { + let iteration = 0; + await test.expect.poll(() => { + if (iteration++ === 0) + throw new Error('foo bar baz'); + return 42; + }).toBe(42); + }); + ` + }); + expect(result.exitCode).toBe(1); + expect(stripAnsi(result.output)).toContain('foo bar baz'); +}); + test('should support .not predicate', async ({ runInlineTest }) => { const result = await runInlineTest({ 'a.spec.ts': `