chore: unify expect message calculation (#37337)

Author: dgozman

packages/playwright/bundles/expect/src/expectBundleImpl.ts

@@ -45,5 +45,6 @@ export {
   EXPECTED_COLOR,
   INVERTED_COLOR,
   RECEIVED_COLOR,
+  DIM_COLOR,
   printReceived,
 } from 'jest-matcher-utils';

packages/playwright/src/common/expectBundle.ts

@@ -21,4 +21,5 @@ export const matcherUtils = require('./expectBundleImpl').matcherUtils;
 export const EXPECTED_COLOR: typeof import('../../bundles/expect/node_modules/jest-matcher-utils/build').EXPECTED_COLOR = require('./expectBundleImpl').EXPECTED_COLOR;
 export const INVERTED_COLOR: typeof import('../../bundles/expect/node_modules/jest-matcher-utils/build').INVERTED_COLOR = require('./expectBundleImpl').INVERTED_COLOR;
 export const RECEIVED_COLOR: typeof import('../../bundles/expect/node_modules/jest-matcher-utils/build').RECEIVED_COLOR = require('./expectBundleImpl').RECEIVED_COLOR;
+export const DIM_COLOR: typeof import('../../bundles/expect/node_modules/jest-matcher-utils/build').DIM_COLOR = require('./expectBundleImpl').DIM_COLOR;
 export const printReceived: typeof import('../../bundles/expect/node_modules/jest-matcher-utils/build').printReceived = require('./expectBundleImpl').printReceived;

packages/playwright/src/matchers/matcherHint.ts

@@ -15,22 +15,62 @@
  */
 
 import { stringifyStackFrames } from 'playwright-core/lib/utils';
+import { DIM_COLOR, RECEIVED_COLOR, EXPECTED_COLOR } from '../common/expectBundle';
 
 import type { ExpectMatcherState } from '../../types/test';
 import type { StackFrame } from '@protocol/channels';
 import type { Locator } from 'playwright-core';
 
-export function matcherHint(state: ExpectMatcherState, locator: Locator | undefined, matcherName: string, expression: any, actual: any, matcherOptions: any, timeout: number | undefined, expectedReceivedString?: string, preventExtraStatIndent: boolean = false) {
-  let header = state.utils.matcherHint(matcherName, expression, actual, matcherOptions).replace(/ \/\/ deep equality/, '') + ' failed\n\n';
-  // Extra space added after locator and timeout to match Jest's received/expected output
-  const extraSpace = preventExtraStatIndent ? '' : ' ';
-  if (locator)
-    header += `Locator: ${extraSpace}${String(locator)}\n`;
-  if (expectedReceivedString)
-    header += `${expectedReceivedString}\n`;
-  if (timeout)
-    header += `Timeout: ${extraSpace}${timeout}ms\n`;
-  return header;
+type MatcherMessageDetails = {
+  receiver?: string; // Assuming 'locator' when locator is provided, 'page' otherwise.
+  matcherName: string;
+  expectation: string;
+  locator?: Locator;
+  printedExpected?: string;
+  printedReceived?: string;
+  printedDiff?: string;
+  timedOut?: boolean;
+  timeout?: number;
+  errorMessage?: string;
+  log?: string[];
+};
+
+export function formatMatcherMessage(state: ExpectMatcherState, details: MatcherMessageDetails) {
+  const receiver = details.receiver ?? (details.locator ? 'locator' : 'page');
+  let message = DIM_COLOR('expect(') + RECEIVED_COLOR(receiver)
+      + DIM_COLOR(')' + (state.promise ? '.' + state.promise : '') + (state.isNot ? '.not' : '') + '.')
+      + details.matcherName
+      + DIM_COLOR('(') + EXPECTED_COLOR(details.expectation) + DIM_COLOR(')')
+      + ' failed\n\n';
+
+  // Sometimes diff is actually expected + received. Turn it into two lines to
+  // simplify alignment logic.
+  const diffLines = details.printedDiff?.split('\n');
+  if (diffLines?.length === 2) {
+    details.printedExpected = diffLines[0];
+    details.printedReceived = diffLines[1];
+    details.printedDiff = undefined;
+  }
+
+  const align = !details.errorMessage && details.printedExpected?.startsWith('Expected:')
+      && (!details.printedReceived || details.printedReceived.startsWith('Received:'));
+  if (details.locator)
+    message += `Locator: ${align ? ' ' : ''}${String(details.locator)}\n`;
+  if (details.printedExpected)
+    message += details.printedExpected + '\n';
+  if (details.printedReceived)
+    message += details.printedReceived + '\n';
+  if (details.timedOut && details.timeout)
+    message += `Timeout: ${align ? ' ' : ''}${details.timeout}ms\n`;
+  if (details.printedDiff)
+    message += details.printedDiff + '\n';
+  if (details.errorMessage) {
+    message += details.errorMessage;
+    if (!details.errorMessage.endsWith('\n'))
+      message += '\n';
+  }
+  message += callLogText(details.log);
+  return message;
 }
 
 export type MatcherResult<E, A> = {
@@ -71,3 +111,12 @@ export class ExpectError extends Error {
 export function isJestError(e: unknown): e is JestError {
   return e instanceof Error && 'matcherResult' in e;
 }
+
+export const callLogText = (log: string[] | undefined) => {
+  if (!log || !log.some(l => !!l))
+    return '';
+  return `
+Call log:
+${DIM_COLOR(log.join('\n'))}
+`;
+};

packages/playwright/src/matchers/matchers.ts

@@ -17,14 +17,15 @@
 import { constructURLBasedOnBaseURL, isRegExp, isString, isTextualMimeType, pollAgainstDeadline, serializeExpectedTextValues } from 'playwright-core/lib/utils';
 import { colors } from 'playwright-core/lib/utils';
 
-import { callLogText, expectTypes } from '../util';
+import { expectTypes } from '../util';
 import { toBeTruthy } from './toBeTruthy';
 import { toEqual } from './toEqual';
 import { toHaveURLWithPredicate } from './toHaveURL';
 import { toMatchText } from './toMatchText';
 import { takeFirst } from '../common/config';
 import { currentTestInfo } from '../common/globals';
 import { TestInfoImpl } from '../worker/testInfo';
+import { formatMatcherMessage } from './matcherHint';
 
 import type { ExpectMatcherState } from '../../types/test';
 import type { TestStepInfoImpl } from '../worker/testInfo';
@@ -187,7 +188,7 @@ export function toContainText(
     return toMatchText.call(this, 'toContainText', locator, 'Locator', async (isNot, timeout) => {
       const expectedText = serializeExpectedTextValues([expected], { matchSubstring: true, normalizeWhiteSpace: true, ignoreCase: options.ignoreCase });
       return await locator._expect('to.have.text', { expectedText, isNot, useInnerText: options.useInnerText, timeout });
-    }, expected, options);
+    }, expected, { ...options, matchSubstring: true });
   }
 }
 
@@ -262,7 +263,7 @@ export function toHaveClass(
     return toEqual.call(this, 'toHaveClass', locator, 'Locator', async (isNot, timeout) => {
       const expectedText = serializeExpectedTextValues(expected);
       return await locator._expect('to.have.class.array', { expectedText, isNot, timeout });
-    }, expected, options, true);
+    }, expected, options);
   } else {
     return toMatchText.call(this, 'toHaveClass', locator, 'Locator', async (isNot, timeout) => {
       const expectedText = serializeExpectedTextValues([expected]);
@@ -283,7 +284,7 @@ export function toContainClass(
     return toEqual.call(this, 'toContainClass', locator, 'Locator', async (isNot, timeout) => {
       const expectedText = serializeExpectedTextValues(expected);
       return await locator._expect('to.contain.class.array', { expectedText, isNot, timeout });
-    }, expected, options, true);
+    }, expected, options);
   } else {
     if (isRegExp(expected))
       throw new Error(`"expected" argument in toContainClass cannot be a RegExp value`);
@@ -408,7 +409,7 @@ export function toHaveTitle(
   return toMatchText.call(this, 'toHaveTitle', page, 'Page', async (isNot, timeout) => {
     const expectedText = serializeExpectedTextValues([expected], { normalizeWhiteSpace: true });
     return await (page.mainFrame() as FrameEx)._expect('to.have.title', { expectedText, isNot, timeout });
-  }, expected, { receiverLabel: 'page', ...options });
+  }, expected, options);
 }
 
 export function toHaveURL(
@@ -426,7 +427,7 @@ export function toHaveURL(
   return toMatchText.call(this, 'toHaveURL', page, 'Page', async (isNot, timeout) => {
     const expectedText = serializeExpectedTextValues([expected], { ignoreCase: options?.ignoreCase });
     return await (page.mainFrame() as FrameEx)._expect('to.have.url', { expectedText, isNot, timeout });
-  }, expected, { receiverLabel: 'page', ...options });
+  }, expected, options);
 }
 
 export async function toBeOK(
@@ -443,9 +444,12 @@ export async function toBeOK(
     isTextEncoding ? response.text() : null
   ]) : [];
 
-  const message = () => this.utils.matcherHint(matcherName, undefined, '', { isNot: this.isNot }) +
-    callLogText(log) +
-    (text === null ? '' : `\nResponse text:\n${colors.dim(text?.substring(0, 1000) || '')}`);
+  const message = () => formatMatcherMessage(this, {
+    matcherName,
+    receiver: 'response',
+    expectation: '',
+    log,
+  }) + (text === null ? '' : `\nResponse text:\n${colors.dim(text?.substring(0, 1000) || '')}`);
 
   const pass = response.ok();
   return { message, pass };

packages/playwright/src/matchers/toBeTruthy.ts

@@ -14,8 +14,8 @@
  * limitations under the License.
  */
 
-import { callLogText, expectTypes } from '../util';
-import { matcherHint } from './matcherHint';
+import { expectTypes } from '../util';
+import { formatMatcherMessage } from './matcherHint';
 import { runBrowserBackendOnError } from '../mcp/test/browserBackend';
 
 import type { MatcherResult } from './matcherHint';
@@ -25,19 +25,14 @@ import type { Locator } from 'playwright-core';
 export async function toBeTruthy(
   this: ExpectMatcherState,
   matcherName: string,
-  receiver: Locator,
+  locator: Locator,
   receiverType: string,
   expected: string,
   arg: string,
   query: (isNot: boolean, timeout: number) => Promise<{ matches: boolean, log?: string[], received?: any, timedOut?: boolean, errorMessage?: string }>,
   options: { timeout?: number } = {},
 ): Promise<MatcherResult<any, any>> {
-  expectTypes(receiver, [receiverType], matcherName);
-
-  const matcherOptions = {
-    isNot: this.isNot,
-    promise: this.promise,
-  };
+  expectTypes(locator, [receiverType], matcherName);
 
   const timeout = options.timeout ?? this.timeout;
 
@@ -56,18 +51,26 @@ export async function toBeTruthy(
   let printedExpected: string | undefined;
   if (pass) {
     printedExpected = `Expected: not ${expected}`;
-    printedReceived = errorMessage ?? `Received: ${expected}`;
+    printedReceived = errorMessage ? '' : `Received: ${expected}`;
   } else {
     printedExpected = `Expected: ${expected}`;
-    printedReceived = errorMessage ?? `Received: ${received}`;
+    printedReceived = errorMessage ? '' : `Received: ${received}`;
   }
   const message = () => {
-    const header = matcherHint(this, receiver, matcherName, 'locator', arg, matcherOptions, timedOut ? timeout : undefined, `${printedExpected}\n${printedReceived}`);
-    const logText = callLogText(log);
-    return `${header}${logText}`;
+    return formatMatcherMessage(this, {
+      matcherName,
+      expectation: arg,
+      locator,
+      timeout,
+      timedOut,
+      printedExpected,
+      printedReceived,
+      errorMessage,
+      log,
+    });
   };
 
-  await runBrowserBackendOnError(receiver.page(), message);
+  await runBrowserBackendOnError(locator.page(), message);
 
   return {
     message,

packages/playwright/src/matchers/toEqual.ts

@@ -16,8 +16,8 @@
 
 import { isRegExp } from 'playwright-core/lib/utils';
 
-import { callLogText, expectTypes } from '../util';
-import { matcherHint } from './matcherHint';
+import { expectTypes } from '../util';
+import { formatMatcherMessage } from './matcherHint';
 import { runBrowserBackendOnError } from '../mcp/test/browserBackend';
 
 import type { MatcherResult } from './matcherHint';
@@ -31,20 +31,13 @@ const RECEIVED_LABEL = 'Received';
 export async function toEqual<T>(
   this: ExpectMatcherState,
   matcherName: string,
-  receiver: Locator,
+  locator: Locator,
   receiverType: string,
   query: (isNot: boolean, timeout: number) => Promise<{ matches: boolean, received?: any, log?: string[], timedOut?: boolean, errorMessage?: string }>,
   expected: T,
   options: { timeout?: number, contains?: boolean } = {},
-  messagePreventExtraStatIndent?: boolean
 ): Promise<MatcherResult<any, any>> {
-  expectTypes(receiver, [receiverType], matcherName);
-
-  const matcherOptions = {
-    comment: options.contains ? '' : 'deep equality',
-    isNot: this.isNot,
-    promise: this.promise,
-  };
+  expectTypes(locator, [receiverType], matcherName);
 
   const timeout = options.timeout ?? this.timeout;
 
@@ -64,10 +57,9 @@ export async function toEqual<T>(
   let printedDiff: string | undefined;
   if (pass) {
     printedExpected = `Expected: not ${this.utils.printExpected(expected)}`;
-    printedReceived = errorMessage ?? `Received: ${this.utils.printReceived(received)}`;
+    printedReceived = errorMessage ? '' : `Received: ${this.utils.printReceived(received)}`;
   } else if (errorMessage) {
     printedExpected = `Expected: ${this.utils.printExpected(expected)}`;
-    printedReceived = errorMessage;
   } else if (Array.isArray(expected) && Array.isArray(received)) {
     const normalizedExpected = expected.map((exp, index) => {
       const rec = received[index];
@@ -93,12 +85,21 @@ export async function toEqual<T>(
     );
   }
   const message = () => {
-    const details = printedDiff || `${printedExpected}\n${printedReceived}`;
-    const header = matcherHint(this, receiver, matcherName, 'locator', undefined, matcherOptions, timedOut ? timeout : undefined, details, messagePreventExtraStatIndent);
-    return `${header}${callLogText(log)}`;
+    return formatMatcherMessage(this, {
+      matcherName,
+      expectation: 'expected',
+      locator,
+      timeout,
+      timedOut,
+      printedExpected,
+      printedReceived,
+      printedDiff,
+      errorMessage,
+      log,
+    });
   };
 
-  await runBrowserBackendOnError(receiver.page(), message);
+  await runBrowserBackendOnError(locator.page(), message);
 
   // Passing the actual and expected objects so that a custom reporter
   // could access them, for example in order to display a custom visual diff,