fix(expect): poll/toPass should not wait over specified timeout (#20266)

Drive-by: unflake some timeout-dependent tests.
microsoft · Jan 20, 2023 · eafa6fd · eafa6fd
1 parent d8e8ddb
commit eafa6fd
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 10 deletions.
diff --git a/packages/playwright-core/src/utils/timeoutRunner.ts b/packages/playwright-core/src/utils/timeoutRunner.ts
@@ -126,7 +126,12 @@ export async function pollAgainstTimeout<T>(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
@@ -311,7 +311,7 @@ export async function toBeOK(
 
 export async function toPass(
   this: ReturnType<Expect['getState']>,
-  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
@@ -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
@@ -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
@@ -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);
       });
     `