chore(test-runner): rewrite poll, soft, wrap (#13335)

packages/playwright-test/src/expect.ts

@@ -45,8 +45,7 @@ import {
   toHaveValue
 } from './matchers/matchers';
 import { toMatchSnapshot, toHaveScreenshot as _toHaveScreenshot } from './matchers/toMatchSnapshot';
-import type { Expect, TestError } from './types';
+import type { Expect } from './types';
-import matchers from 'expect/build/matchers';
 import { currentTestInfo } from './globals';
 import { serializeError, captureStackTrace, currentExpectTimeout } from './util';
 import { monotonicTime } from 'playwright-core/lib/utils/utils';
@@ -99,8 +98,8 @@ export const printReceivedStringContainExpectedResult = (
 
 type ExpectMessageOrOptions = undefined | string | { message?: string, timeout?: number };
 
-function createExpect(actual: unknown, messageOrOptions: ExpectMessageOrOptions, isSoft: boolean, isPoll: boolean) {
+function createExpect(actual: unknown, messageOrOptions: ExpectMessageOrOptions, isSoft: boolean, isPoll: boolean, generator?: Generator) {
-  return new Proxy(expectLibrary(actual), new ExpectMetaInfoProxyHandler(messageOrOptions, isSoft, isPoll));
+  return new Proxy(expectLibrary(actual), new ExpectMetaInfoProxyHandler(messageOrOptions, isSoft, isPoll, generator));
 }
 
 export const expect: Expect = new Proxy(expectLibrary as any, {
@@ -115,7 +114,9 @@ expect.soft = (actual: unknown, messageOrOptions: ExpectMessageOrOptions) => {
 };
 
 expect.poll = (actual: unknown, messageOrOptions: ExpectMessageOrOptions) => {
-  return createExpect(actual, messageOrOptions, false /* isSoft */, true /* isPoll */);
+  if (typeof actual !== 'function')
+    throw new Error('`expect.poll()` accepts only function as a first argument');
+  return createExpect(actual, messageOrOptions, false /* isSoft */, true /* isPoll */, actual as any);
 };
 
 expectLibrary.setState({ expand: false });
@@ -144,20 +145,22 @@ const customMatchers = {
   _toHaveScreenshot,
 };
 
+type Generator = () => any;
+
 type ExpectMetaInfo = {
   message?: string;
+  isNot: boolean;
   isSoft: boolean;
   isPoll: boolean;
   pollTimeout?: number;
+  generator?: Generator;
 };
 
-let expectCallMetaInfo: undefined|ExpectMetaInfo = undefined;
-
 class ExpectMetaInfoProxyHandler {
   private _info: ExpectMetaInfo;
 
-  constructor(messageOrOptions: ExpectMessageOrOptions, isSoft: boolean, isPoll: boolean) {
+  constructor(messageOrOptions: ExpectMessageOrOptions, isSoft: boolean, isPoll: boolean, generator?: Generator) {
-    this._info = { isSoft, isPoll };
+    this._info = { isSoft, isPoll, generator, isNot: false };
     if (typeof messageOrOptions === 'string') {
       this._info.message = messageOrOptions;
     } else {
@@ -166,100 +169,36 @@ class ExpectMetaInfoProxyHandler {
     }
   }
 
-  get(target: any, prop: any, receiver: any): any {
+  get(target: any, matcherName: any, receiver: any): any {
-    const value = Reflect.get(target, prop, receiver);
+    const matcher = Reflect.get(target, matcherName, receiver);
-    if (value === undefined)
+    if (matcher === undefined)
-      throw new Error(`expect: Property '${prop}' not found.`);
+      throw new Error(`expect: Property '${matcherName}' not found.`);
-    if (typeof value !== 'function')
+    if (typeof matcher !== 'function') {
-      return new Proxy(value, this);
+      if (matcherName === 'not')
+        this._info.isNot = !this._info.isNot;
+      return new Proxy(matcher, this);
+    }
     return (...args: any[]) => {
       const testInfo = currentTestInfo();
       if (!testInfo)
-        return value.call(target, ...args);
+        return matcher.call(target, ...args);
-      const handleError = (e: Error) => {
-        if (this._info.isSoft)
-          testInfo._failWithError(serializeError(e), false /* isHardError */);
-        else
-          throw e;
-      };
-      try {
-        expectCallMetaInfo = {
-          message: this._info.message,
-          isSoft: this._info.isSoft,
-          isPoll: this._info.isPoll,
-          pollTimeout: this._info.pollTimeout,
-        };
-        let result = value.call(target, ...args);
-        if ((result instanceof Promise))
-          result = result.catch(handleError);
-        return result;
-      } catch (e) {
-        handleError(e);
-      } finally {
-        expectCallMetaInfo = undefined;
-      }
-    };
-  }
-}
 
-async function pollMatcher(matcher: any, timeout: number, thisArg: any, generator: () => any, ...args: any[]) {
+      const stackTrace = captureStackTrace();
-  let result: { pass: boolean, message: () => string } | undefined = undefined;
+      const stackLines = stackTrace.frameTexts;
-  const startTime = monotonicTime();
+      const frame = stackTrace.frames[0];
-  const pollIntervals = [100, 250, 500];
+      const customMessage = this._info.message || '';
-  while (true) {
+      const defaultTitle = `expect${this._info.isPoll ? '.poll' : ''}${this._info.isSoft ? '.soft' : ''}${this._info.isNot ? '.not' : ''}.${matcherName}`;
-    const elapsed = monotonicTime() - startTime;
+      const step = testInfo._addStep({
-    if (timeout !== 0 && elapsed > timeout)
+        location: frame && frame.file ? { file: path.resolve(process.cwd(), frame.file), line: frame.line || 0, column: frame.column || 0 } : undefined,
-      break;
+        category: 'expect',
-    const received = timeout !== 0 ? await raceAgainstTimeout(generator, timeout - elapsed) : await generator();
+        title: customMessage || defaultTitle,
-    if (received.timedOut)
+        canHaveChildren: true,
-      break;
+        forceNoParent: false
-    result = matcher.call(thisArg, received.result, ...args);
+      });
-    const success = result!.pass !== thisArg.isNot;
+      testInfo.currentStep = step;
-    if (success)
-      return result;
-    await new Promise(x => setTimeout(x, pollIntervals.shift() ?? 1000));
-  }
-  const timeoutMessage = `Timeout ${timeout}ms exceeded while waiting on the predicate`;
-  const message = result ? [
-    result.message(),
-    '',
-    `Call Log:`,
-    `- ${timeoutMessage}`,
-  ].join('\n') : timeoutMessage;
-  return {
-    pass: thisArg.isNot,
-    message: () => message,
-  };
-}
 
-function wrap(matcherName: string, matcher: any) {
+      const reportStepError = (jestError: Error) => {
-  return function(this: any, ...args: any[]) {
+        const message = jestError.message;
-    const testInfo = currentTestInfo();
-    if (!testInfo)
-      return matcher.call(this, ...args);
-
-    const stackTrace = captureStackTrace();
-    const stackLines = stackTrace.frameTexts;
-    const frame = stackTrace.frames[0];
-    const customMessage = expectCallMetaInfo?.message ?? '';
-    const isSoft = expectCallMetaInfo?.isSoft ?? false;
-    const isPoll = expectCallMetaInfo?.isPoll ?? false;
-    const pollTimeout = expectCallMetaInfo?.pollTimeout;
-    const defaultTitle = `expect${isPoll ? '.poll' : ''}${isSoft ? '.soft' : ''}${this.isNot ? '.not' : ''}.${matcherName}`;
-    const step = testInfo._addStep({
-      location: frame && frame.file ? { file: path.resolve(process.cwd(), frame.file), line: frame.line || 0, column: frame.column || 0 } : undefined,
-      category: 'expect',
-      title: customMessage || defaultTitle,
-      canHaveChildren: true,
-      forceNoParent: false
-    });
-
-    const reportStepEnd = (result: any, options: { refinedTitle?: string }) => {
-      const success = result.pass !== this.isNot;
-      let error: TestError | undefined;
-      if (!success) {
-        const message = result.message();
-        error = { message, stack: message + '\n' + stackLines.join('\n') };
         if (customMessage) {
           const messageLines = message.split('\n');
           // Jest adds something like the following error to all errors:
@@ -273,53 +212,74 @@ function wrap(matcherName: string, matcher: any) {
               messageLines.splice(uselessMatcherLineIndex, 1);
           }
           const newMessage = [
-            customMessage,
+            'Error: ' + customMessage,
             '',
             ...messageLines,
           ].join('\n');
-          result.message = () => newMessage;
+          jestError.message = newMessage;
+          jestError.stack = newMessage + '\n' + stackLines.join('\n');
         }
+
+        const serializerError = serializeError(jestError);
+        step.complete({ error: serializerError });
+        if (this._info.isSoft)
+          testInfo._failWithError(serializerError, false /* isHardError */);
+        else
+          throw jestError;
+      };
+
+      try {
+        let result;
+        if (this._info.isPoll) {
+          if ((customMatchers as any)[matcherName] || matcherName === 'resolves' || matcherName === 'rejects')
+            throw new Error(`\`expect.poll()\` does not support "${matcherName}" matcher.`);
+          result = pollMatcher(matcherName, this._info.isNot, currentExpectTimeout({ timeout: this._info.pollTimeout }), this._info.generator!, ...args);
+        } else {
+          result = matcher.call(target, ...args);
+        }
+        if ((result instanceof Promise))
+          return result.then(() => step.complete({})).catch(reportStepError);
+        else
+          step.complete({});
+      } catch (e) {
+        reportStepError(e);
       }
-      step.complete({ ...options, error });
-      return result;
     };
-
+  }
-    const reportStepError = (error: Error) => {
-      step.complete({ error: serializeError(error) });
-      throw error;
-    };
-
-    const refineTitle = (result: SyncExpectationResult & { titleSuffix?: string }): string | undefined => {
-      return !customMessage && result.titleSuffix ? defaultTitle + result.titleSuffix : undefined;
-    };
-
-    try {
-      let result;
-      const [receivedOrGenerator, ...otherArgs] = args;
-      if (isPoll) {
-        if (typeof receivedOrGenerator !== 'function')
-          throw new Error('`expect.poll()` accepts only function as a first argument');
-        if ((customMatchers as any)[matcherName] || matcherName === 'resolves' || matcherName === 'rejects')
-          throw new Error(`\`expect.poll()\` does not support "${matcherName}" matcher.`);
-        result = pollMatcher(matcher, currentExpectTimeout({ timeout: pollTimeout }), this, receivedOrGenerator, ...otherArgs);
-      } else {
-        if (typeof receivedOrGenerator === 'function')
-          throw new Error('Cannot accept function as a first argument; did you mean to use `expect.poll()`?');
-        result = matcher.call(this, ...args);
-      }
-      if (result instanceof Promise)
-        return result.then(result => reportStepEnd(result, { refinedTitle: refineTitle(result) })).catch(reportStepError);
-      return reportStepEnd(result, { refinedTitle: refineTitle(result) });
-    } catch (e) {
-      reportStepError(e);
-    }
-  };
 }
 
-const wrappedMatchers: any = {};
+async function pollMatcher(matcherName: any, isNot: boolean, timeout: number, generator: () => any, ...args: any[]) {
-for (const matcherName in matchers)
+  let matcherError;
-  wrappedMatchers[matcherName] = wrap(matcherName, matchers[matcherName]);
+  const startTime = monotonicTime();
-for (const matcherName in customMatchers)
+  const pollIntervals = [100, 250, 500];
-  wrappedMatchers[matcherName] = wrap(matcherName, (customMatchers as any)[matcherName]);
+  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;
+    try {
+      let expectInstance = expectLibrary(received.result) as any;
+      if (isNot)
+        expectInstance = expectInstance.not;
+      expectInstance[matcherName].call(expectInstance, ...args);
+      return;
+    } catch (e) {
+      matcherError = e;
+    }
+    await new Promise(x => setTimeout(x, pollIntervals.shift() ?? 1000));
+  }
 
-expectLibrary.extend(wrappedMatchers);
+  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);

packages/playwright-test/src/matchers/toMatchSnapshot.ts

@@ -110,6 +110,7 @@ class SnapshotHelper<T extends ImageComparatorOptions> {
       }
     }
 
+    testInfo.currentStep!.refinedTitle = `${testInfo.currentStep!.title}(${path.basename(this.snapshotName)})`;
     options = {
       ...configOptions,
       ...options,
@@ -148,23 +149,19 @@ class SnapshotHelper<T extends ImageComparatorOptions> {
     this.kind = this.mimeType.startsWith('image/') ? 'Screenshot' : 'Snapshot';
   }
 
-  decorateTitle(result: SyncExpectationResult): SyncExpectationResult & { titleSuffix: string } {
-    return { ...result, titleSuffix: `(${path.basename(this.snapshotName)})` };
-  }
-
   handleMissingNegated() {
     const isWriteMissingMode = this.updateSnapshots === 'all' || this.updateSnapshots === 'missing';
     const message = `${this.snapshotPath} is missing in snapshots${isWriteMissingMode ? ', matchers using ".not" won\'t write them automatically.' : '.'}`;
-    return this.decorateTitle({
+    return {
       // NOTE: 'isNot' matcher implies inversed value.
       pass: true,
       message: () => message,
-    });
+    };
   }
 
   handleDifferentNegated() {
     // NOTE: 'isNot' matcher implies inversed value.
-    return this.decorateTitle({ pass: false, message: () => '' });
+    return { pass: false, message: () => '' };
   }
 
   handleMatchingNegated() {
@@ -174,7 +171,7 @@ class SnapshotHelper<T extends ImageComparatorOptions> {
       indent('Expected result should be different from the actual one.', '  '),
     ].join('\n');
     // NOTE: 'isNot' matcher implies inversed value.
-    return this.decorateTitle({ pass: true, message: () => message });
+    return { pass: true, message: () => message };
   }
 
   handleMissing(actual: Buffer | string) {
@@ -187,13 +184,13 @@ class SnapshotHelper<T extends ImageComparatorOptions> {
     if (this.updateSnapshots === 'all') {
       /* eslint-disable no-console */
       console.log(message);
-      return this.decorateTitle({ pass: true, message: () => message });
+      return { pass: true, message: () => message };
     }
     if (this.updateSnapshots === 'missing') {
       this.testInfo._failWithError(serializeError(new Error(message)), false /* isHardError */);
-      return this.decorateTitle({ pass: true, message: () => '' });
+      return { pass: true, message: () => '' };
     }
-    return this.decorateTitle({ pass: false, message: () => message });
+    return { pass: false, message: () => message };
   }
 
   handleDifferent(
@@ -235,11 +232,11 @@ class SnapshotHelper<T extends ImageComparatorOptions> {
       this.testInfo.attachments.push({ name: addSuffixToFilePath(this.snapshotName, '-diff'), contentType: this.mimeType, path: this.diffPath });
       output.push(`    Diff: ${colors.yellow(this.diffPath)}`);
     }
-    return this.decorateTitle({ pass: false, message: () => output.join('\n'), });
+    return { pass: false, message: () => output.join('\n'), };
   }
 
   handleMatching() {
-    return this.decorateTitle({ pass: true, message: () => '' });
+    return { pass: true, message: () => '' };
   }
 }
 
@@ -248,7 +245,7 @@ export function toMatchSnapshot(
   received: Buffer | string,
   nameOrOptions: NameOrSegments | { name?: NameOrSegments } & ImageComparatorOptions = {},
   optOptions: ImageComparatorOptions = {}
-): SyncExpectationResult & { titleSuffix: string } {
+): SyncExpectationResult {
   const testInfo = currentTestInfo();
   if (!testInfo)
     throw new Error(`toMatchSnapshot() must be called during the test`);
@@ -278,7 +275,7 @@ export function toMatchSnapshot(
     writeFileSync(helper.snapshotPath, received);
     /* eslint-disable no-console */
     console.log(helper.snapshotPath + ' does not match, writing actual.');
-    return helper.decorateTitle({ pass: true, message: () => helper.snapshotPath + ' running with --update-snapshots, writing actual.' });
+    return { pass: true, message: () => helper.snapshotPath + ' running with --update-snapshots, writing actual.' };
   }
 
   return helper.handleDifferent(received, expected, undefined, result.diff, result.errorMessage, undefined);

packages/playwright-test/src/testInfo.ts

@@ -62,6 +62,7 @@ export class TestInfoImpl implements TestInfo {
   readonly outputDir: string;
   readonly snapshotDir: string;
   errors: TestError[] = [];
+  currentStep: TestStepInternal | undefined;
 
   get error(): TestError | undefined {
     return this.errors.length > 0 ? this.errors[0] : undefined;

packages/playwright-test/src/types.ts

@@ -27,12 +27,13 @@ export type FixturesWithLocation = {
 export type Annotation = { type: string, description?: string };
 
 export interface TestStepInternal {
-  complete(result: { refinedTitle?: string, error?: Error | TestError }): void;
+  complete(result: { error?: Error | TestError }): void;
   title: string;
   category: string;
   canHaveChildren: boolean;
   forceNoParent: boolean;
   location?: Location;
+  refinedTitle?: string;
 }
 
 /**

packages/playwright-test/src/workerRunner.ts

@@ -214,7 +214,7 @@ export class WorkerRunner extends EventEmitter {
           const error = result.error instanceof Error ? serializeError(result.error) : result.error;
           const payload: StepEndPayload = {
             testId: test._id,
-            refinedTitle: result.refinedTitle,
+            refinedTitle: step.refinedTitle,
             stepId,
             wallTime: Date.now(),
             error,

tests/playwright-test/expect-poll.spec.ts

@@ -74,6 +74,7 @@ test('should respect timeout', async ({ runInlineTest }) => {
   });
   expect(result.exitCode).toBe(1);
   expect(stripAnsi(result.output)).toContain('Timeout 100ms exceeded while waiting on the predicate');
+  expect(stripAnsi(result.output)).toContain('Received: false');
   expect(stripAnsi(result.output)).toContain(`
   7 |         await test.expect.poll(() => false, { timeout: 100 }).
   `.trim());
@@ -144,3 +145,36 @@ test('should support .not predicate', async ({ runInlineTest }) => {
   });
   expect(result.exitCode).toBe(0);
 });
+
+test('should support custom matchers', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'a.spec.ts': `
+      expect.extend({
+        toBeWithinRange(received, floor, ceiling) {
+          const pass = received >= floor && received <= ceiling;
+          if (pass) {
+            return {
+              message: () =>
+                "expected " + received + " not to be within range " + floor + " - " + ceiling,
+              pass: true,
+            };
+          } else {
+            return {
+              message: () =>
+                "expected " + received + " to be within range " + floor + " - " + ceiling,
+              pass: false,
+            };
+          }
+        },
+      });
+
+      const { test } = pwt;
+      test('should poll', async () => {
+        let i = 0;
+        await test.expect.poll(() => ++i).toBeWithinRange(3, Number.MAX_VALUE);
+      });
+    `
+  });
+  expect(result.exitCode).toBe(0);
+  expect(result.passed).toBe(1);
+});

tests/playwright-test/expect-soft.spec.ts

@@ -44,19 +44,6 @@ test('soft expects should work', async ({ runInlineTest }) => {
   expect(stripAnsi(result.output)).toContain('woof-woof');
 });
 
-test('should fail when passed in function', async ({ runInlineTest }) => {
-  const result = await runInlineTest({
-    'a.spec.ts': `
-      const { test } = pwt;
-      test('should work', () => {
-        test.expect.soft(() => 1+1).toBe(2);
-      });
-    `
-  });
-  expect(result.exitCode).toBe(1);
-  expect(stripAnsi(result.output)).toContain('Cannot accept function as a first argument; did you mean to use `expect.poll()`?');
-});
-
 test('should report a mixture of soft and non-soft errors', async ({ runInlineTest }) => {
   const result = await runInlineTest({
     'a.spec.ts': `

tests/playwright-test/test-step.spec.ts

@@ -321,3 +321,61 @@ test('should report expect step locations', async ({ runInlineTest }) => {
     },
   ]);
 });
+
+test('should report custom expect steps', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'reporter.ts': stepHierarchyReporter,
+    'playwright.config.ts': `
+      module.exports = {
+        reporter: './reporter',
+      };
+    `,
+    'a.test.ts': `
+      expect.extend({
+        toBeWithinRange(received, floor, ceiling) {
+          const pass = received >= floor && received <= ceiling;
+          if (pass) {
+            return {
+              message: () =>
+                "expected " + received + " not to be within range " + floor + " - " + ceiling,
+              pass: true,
+            };
+          } else {
+            return {
+              message: () =>
+                "expected " + received + " to be within range " + floor + " - " + ceiling,
+              pass: false,
+            };
+          }
+        },
+      });
+
+      const { test } = pwt;
+      test('pass', async ({}) => {
+        expect(15).toBeWithinRange(10, 20);
+      });
+    `
+  }, { reporter: '', workers: 1 });
+
+  expect(result.exitCode).toBe(0);
+  const objects = result.output.split('\n').filter(line => line.startsWith('%% ')).map(line => line.substring(3).trim()).filter(Boolean).map(line => JSON.parse(line));
+  expect(objects).toEqual([
+    {
+      category: 'hook',
+      title: 'Before Hooks',
+    },
+    {
+      category: 'expect',
+      location: {
+        column: 'number',
+        file: 'a.test.ts',
+        line: 'number',
+      },
+      title: 'expect.toBeWithinRange',
+    },
+    {
+      category: 'hook',
+      title: 'After Hooks',
+    },
+  ]);
+});