chore(expect): extract polling from expect.poll and expect().toPass (#19882)

This extracts & unifies polling machinery from `expect.poll` and
`expect.toPass` methods.

Author: aslushnikov

packages/playwright-core/src/utils/timeoutRunner.ts

@@ -109,3 +109,24 @@ export async function raceAgainstTimeout<T>(cb: () => Promise<T>, timeout: numbe
     throw e;
   }
 }
+
+export async function pollAgainstTimeout<T>(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 };
+}

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<Error|undefined>(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]);
 
-  const timeoutMessage = `Timeout ${timeout}ms exceeded while waiting on the predicate`;
-  const message = matcherError ? [
-    matcherError.message,
-    '',
-    `Call Log:`,
-    `- ${timeoutMessage}`,
-  ].join('\n') : timeoutMessage;
+  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);
+    throw new Error(message);
+  }
 }
 
 expectLibrary.extend(customMatchers);

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<FrameExpectOptions, 'expectedValue'> & { 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<Error|undefined>(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]);
 
-  const timeoutMessage = `Timeout ${timeout}ms exceeded while waiting on the predicate`;
-  const message = () => matcherError ? [
-    matcherError.message,
-    '',
-    `Call Log:`,
-    `- ${timeoutMessage}`,
-  ].join('\n') : timeoutMessage;
+  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: isNot ? true : false };
-}
+    return { message, pass: this.isNot };
+  }
+  return { pass: !this.isNot, message: () => '' };
+}

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': `