From eafa6fda13bd037fb709e622051c3651cc633a0a Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Fri, 20 Jan 2023 15:47:24 -0800 Subject: [PATCH] fix(expect): poll/toPass should not wait over specified timeout (#20266) Drive-by: unflake some timeout-dependent tests. --- packages/playwright-core/src/utils/timeoutRunner.ts | 7 ++++++- packages/playwright-test/src/matchers/matchers.ts | 2 +- tests/library/global-fetch.spec.ts | 4 ++-- tests/playwright-test/expect-poll.spec.ts | 6 ++++-- tests/playwright-test/expect-to-pass.spec.ts | 10 ++++++---- 5 files changed, 19 insertions(+), 10 deletions(-) diff --git a/packages/playwright-core/src/utils/timeoutRunner.ts b/packages/playwright-core/src/utils/timeoutRunner.ts index 981f0f517a208..72a1c4f1ab53f 100644 --- a/packages/playwright-core/src/utils/timeoutRunner.ts +++ b/packages/playwright-core/src/utils/timeoutRunner.ts @@ -126,7 +126,12 @@ export async function pollAgainstTimeout(callback: () => Promise<{ continuePo lastResult = received.result.result; if (!received.result.continuePolling) return { result: received.result.result, timedOut: false }; - await new Promise(x => setTimeout(x, pollIntervals!.shift() ?? lastPollInterval)); + let interval = pollIntervals!.shift() ?? lastPollInterval; + if (timeout !== 0) { + const elapsed = monotonicTime() - startTime; + interval = Math.min(interval, Math.max(timeout - elapsed, 1)); + } + await new Promise(x => setTimeout(x, interval)); } return { timedOut: true, result: lastResult }; } diff --git a/packages/playwright-test/src/matchers/matchers.ts b/packages/playwright-test/src/matchers/matchers.ts index f73702892198d..ec13f53eae835 100644 --- a/packages/playwright-test/src/matchers/matchers.ts +++ b/packages/playwright-test/src/matchers/matchers.ts @@ -311,7 +311,7 @@ export async function toBeOK( export async function toPass( this: ReturnType, - callback: () => void, + callback: () => any, options: { intervals?: number[]; timeout?: number, diff --git a/tests/library/global-fetch.spec.ts b/tests/library/global-fetch.spec.ts index 194eb773e271f..ed51f18828c9d 100644 --- a/tests/library/global-fetch.spec.ts +++ b/tests/library/global-fetch.spec.ts @@ -56,10 +56,10 @@ it('should support global userAgent option', async ({ playwright, server }) => { }); it('should support global timeout option', async ({ playwright, server }) => { - const request = await playwright.request.newContext({ timeout: 1 }); + const request = await playwright.request.newContext({ timeout: 100 }); server.setRoute('/empty.html', (req, res) => {}); const error = await request.get(server.EMPTY_PAGE).catch(e => e); - expect(error.message).toContain('Request timed out after 1ms'); + expect(error.message).toContain('Request timed out after 100ms'); }); it('should propagate extra http headers with redirects', async ({ playwright, server }) => { diff --git a/tests/playwright-test/expect-poll.spec.ts b/tests/playwright-test/expect-poll.spec.ts index c6ceac13e693f..b3283bf06560d 100644 --- a/tests/playwright-test/expect-poll.spec.ts +++ b/tests/playwright-test/expect-poll.spec.ts @@ -208,9 +208,11 @@ test('should respect interval', async ({ runInlineTest }) => { const { test } = pwt; test('should fail', async () => { let probes = 0; - await test.expect.poll(() => ++probes, { timeout: 1000, intervals: [600] }).toBe(3).catch(() => {}); - // Probe at 0s, at 0.6s. + const startTime = Date.now(); + await test.expect.poll(() => ++probes, { timeout: 1000, intervals: [0, 10000] }).toBe(3).catch(() => {}); + // Probe at 0 and epsilon. expect(probes).toBe(2); + expect(Date.now() - startTime).toBeLessThan(5000); }); ` }); diff --git a/tests/playwright-test/expect-to-pass.spec.ts b/tests/playwright-test/expect-to-pass.spec.ts index 82f630843805e..1a92e0b81ddac 100644 --- a/tests/playwright-test/expect-to-pass.spec.ts +++ b/tests/playwright-test/expect-to-pass.spec.ts @@ -100,12 +100,14 @@ test('should respect interval', async ({ runInlineTest }) => { const { test } = pwt; test('should fail', async () => { let probes = 0; + const startTime = Date.now(); await test.expect(() => { - ++probes + ++probes; expect(1).toBe(2); - }).toPass({ timeout: 1000, intervals: [600] }).catch(() => {}); - // Probe at 0s, at 0.6s. + }).toPass({ timeout: 1000, intervals: [0, 10000] }).catch(() => {}); + // Probe at 0 and epsilon. expect(probes).toBe(2); + expect(Date.now() - startTime).toBeLessThan(5000); }); ` }); @@ -157,7 +159,7 @@ test('should work with soft', async ({ runInlineTest }) => { test('should respect soft', async () => { await test.expect.soft(() => { expect(1).toBe(3); - }).toPass({ timeout: 1 }); + }).toPass({ timeout: 1000 }); expect.soft(2).toBe(3); }); `