From 7a3253e89d7d19dc9825a9e9672ebda7b40244a1 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Mon, 21 Oct 2024 10:31:15 -0700 Subject: [PATCH] address comments --- docs/src/test-reporter-api/class-testerror.md | 14 +++++++------- packages/playwright/src/matchers/toBeTruthy.ts | 13 +++++++------ packages/playwright/src/matchers/toEqual.ts | 8 ++------ .../playwright/src/matchers/toMatchSnapshot.ts | 2 +- packages/playwright/src/matchers/toMatchText.ts | 2 ++ packages/playwright/src/reporters/base.ts | 7 ++----- packages/playwright/types/testReporter.d.ts | 2 +- tests/page/expect-matcher-result.spec.ts | 4 ++-- 8 files changed, 24 insertions(+), 28 deletions(-) diff --git a/docs/src/test-reporter-api/class-testerror.md b/docs/src/test-reporter-api/class-testerror.md index ebf8b5272c..8e76d6f595 100644 --- a/docs/src/test-reporter-api/class-testerror.md +++ b/docs/src/test-reporter-api/class-testerror.md @@ -22,18 +22,18 @@ Receiver's locator. Call log. -## property: TestError.message -* since: v1.10 -- type: ?<[string]> - -Error message. Set when [Error] (or its subclass) has been thrown. - ## property: TestError.matcherName * since: v1.49 - type: ?<[string]> Expect matcher name. +## property: TestError.message +* since: v1.10 +- type: ?<[string]> + +Error message. Set when [Error] (or its subclass) has been thrown. + ## property: TestError.received * since: v1.49 - type: ?<[string]> @@ -50,7 +50,7 @@ Error stack. Set when [Error] (or its subclass) has been thrown. * since: v1.49 - type: ?<[int]> -Timeout in milliseconds, if the error was caused by a timeout.. +Timeout in milliseconds, if the error was caused by a timeout. ## property: TestError.value * since: v1.10 diff --git a/packages/playwright/src/matchers/toBeTruthy.ts b/packages/playwright/src/matchers/toBeTruthy.ts index 1371d4d173..8902a14eea 100644 --- a/packages/playwright/src/matchers/toBeTruthy.ts +++ b/packages/playwright/src/matchers/toBeTruthy.ts @@ -39,20 +39,21 @@ export async function toBeTruthy( }; const timeout = options.timeout ?? this.timeout; - const { matches, log, timedOut, received } = await query(!!this.isNot, timeout); - if (matches === !this.isNot) { + const { matches: pass, log, timedOut, received } = await query(!!this.isNot, timeout); + if (pass === !this.isNot) { return { name: matcherName, message: () => '', - pass: matches, + pass, expected }; } + const notFound = received === kNoElementsFoundError ? received : undefined; - const actual = matches ? expected : unexpected; + const actual = pass ? expected : unexpected; let printedReceived: string | undefined; let printedExpected: string | undefined; - if (matches) { + if (pass) { printedExpected = `Expected: not ${expected}`; printedReceived = `Received: ${notFound ? kNoElementsFoundError : expected}`; } else { @@ -66,7 +67,7 @@ export async function toBeTruthy( }; return { message, - pass: matches, + pass, actual, name: matcherName, expected, diff --git a/packages/playwright/src/matchers/toEqual.ts b/packages/playwright/src/matchers/toEqual.ts index 48ca05c8f8..f75caf87f5 100644 --- a/packages/playwright/src/matchers/toEqual.ts +++ b/packages/playwright/src/matchers/toEqual.ts @@ -56,11 +56,9 @@ export async function toEqual( let printedReceived: string | undefined; let printedExpected: string | undefined; let printedDiff: string | undefined; - if (pass) { printedExpected = `Expected: not ${this.utils.printExpected(expected)}`; printedReceived = `Received: ${this.utils.printReceived(received)}`; - } else { printedDiff = this.utils.printDiffOrStringify( expected, @@ -69,14 +67,12 @@ export async function toEqual( RECEIVED_LABEL, false, ); -} - + } const message = () => { const header = matcherHint(this, receiver, matcherName, 'locator', undefined, matcherOptions, timedOut ? timeout : undefined); const details = printedDiff || `${printedExpected}\n${printedReceived}`; return `${header}${details}${callLogText(log)}`; - } - + }; // Passing the actual and expected objects so that a custom reporter // could access them, for example in order to display a custom visual diff, // or create a different error message diff --git a/packages/playwright/src/matchers/toMatchSnapshot.ts b/packages/playwright/src/matchers/toMatchSnapshot.ts index 837931891b..86504062d3 100644 --- a/packages/playwright/src/matchers/toMatchSnapshot.ts +++ b/packages/playwright/src/matchers/toMatchSnapshot.ts @@ -186,7 +186,6 @@ class SnapshotHelper { } createMatcherResult(message: string, pass: boolean, log?: string[]): ImageMatcherResult { - const unfiltered: ImageMatcherResult = { name: this.matcherName, expected: this.expectedPath, @@ -195,6 +194,7 @@ class SnapshotHelper { pass, message: () => message, log, + // eslint-disable-next-line @typescript-eslint/no-base-to-string ...(this.locator ? { locator: this.locator.toString() } : {}), printedExpected: this.expectedPath, printedReceived: this.actualPath, diff --git a/packages/playwright/src/matchers/toMatchText.ts b/packages/playwright/src/matchers/toMatchText.ts index da8a666794..76ed48af1e 100644 --- a/packages/playwright/src/matchers/toMatchText.ts +++ b/packages/playwright/src/matchers/toMatchText.ts @@ -66,6 +66,7 @@ export async function toMatchText( expected }; } + const stringSubstring = options.matchSubstring ? 'substring' : 'string'; const receivedString = received || ''; const messagePrefix = matcherHint(this, receiver, matcherName, 'locator', undefined, matcherOptions, timedOut ? timeout : undefined); @@ -117,6 +118,7 @@ export async function toMatchText( actual: received, log, timeout: timedOut ? timeout : undefined, + // eslint-disable-next-line @typescript-eslint/no-base-to-string locator: receiver.toString(), ...(printedReceived ? { printedReceived } : {}), ...(printedExpected ? { printedExpected } : {}), diff --git a/packages/playwright/src/reporters/base.ts b/packages/playwright/src/reporters/base.ts index 40e9475183..138820baee 100644 --- a/packages/playwright/src/reporters/base.ts +++ b/packages/playwright/src/reporters/base.ts @@ -32,9 +32,6 @@ type Annotation = { type ErrorDetails = { message: string; location?: Location; -}; - -type TestResultErrorDetails = ErrorDetails & { timeout?: number; matcherName?: string; locator?: string; @@ -374,8 +371,8 @@ function quotePathIfNeeded(path: string): string { return path; } -export function formatResultFailure(test: TestCase, result: TestResult, initialIndent: string, highlightCode: boolean): TestResultErrorDetails[] { - const errorDetails: TestResultErrorDetails[] = []; +export function formatResultFailure(test: TestCase, result: TestResult, initialIndent: string, highlightCode: boolean): ErrorDetails[] { + const errorDetails: ErrorDetails[] = []; if (result.status === 'passed' && test.expectedStatus === 'failed') { errorDetails.push({ diff --git a/packages/playwright/types/testReporter.d.ts b/packages/playwright/types/testReporter.d.ts index f894276260..5663f92f98 100644 --- a/packages/playwright/types/testReporter.d.ts +++ b/packages/playwright/types/testReporter.d.ts @@ -600,7 +600,7 @@ export interface TestError { stack?: string; /** - * Timeout in milliseconds, if the error was caused by a timeout.. + * Timeout in milliseconds, if the error was caused by a timeout. */ timeout?: number; diff --git a/tests/page/expect-matcher-result.spec.ts b/tests/page/expect-matcher-result.spec.ts index 61976a0ff9..7767ecf5f6 100644 --- a/tests/page/expect-matcher-result.spec.ts +++ b/tests/page/expect-matcher-result.spec.ts @@ -31,7 +31,7 @@ test('toMatchText-based assertions should have matcher result', async ({ page }) message: expect.stringContaining(`Timed out 1ms waiting for expect(locator).toHaveText(expected)`), name: 'toHaveText', pass: false, - locator:`locator('#node')`, + locator: `locator('#node')`, printedDiff: `Expected pattern: /Text2/ Received string: \"Text content\"`, log: expect.any(Array), @@ -58,7 +58,7 @@ Call log`); message: expect.stringContaining(`Timed out 1ms waiting for expect(locator).not.toHaveText(expected)`), name: 'toHaveText', pass: true, - locator:`locator('#node')`, + locator: `locator('#node')`, printedExpected: 'Expected pattern: not /Text/', printedReceived: `Received string: \"Text content\"`, log: expect.any(Array),