From d0dfbba7b6604637da2a201d5a20828ddc7c5799 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Fri, 18 Oct 2024 10:50:18 +0200 Subject: [PATCH] Revert "wire up matcher with parent step" This reverts commit 1b8cd6dd5b1d1b349b49fdf6844a856e74f2a96d. --- packages/playwright/src/matchers/expect.ts | 13 ++++----- .../src/matchers/toMatchSnapshot.ts | 28 ++++++++----------- 2 files changed, 16 insertions(+), 25 deletions(-) diff --git a/packages/playwright/src/matchers/expect.ts b/packages/playwright/src/matchers/expect.ts index 1306681940..0d276d4101 100644 --- a/packages/playwright/src/matchers/expect.ts +++ b/packages/playwright/src/matchers/expect.ts @@ -60,7 +60,7 @@ import { printReceived, } from '../common/expectBundle'; import { zones } from 'playwright-core/lib/utils'; -import { TestInfoImpl, type TestStepInternal } from '../worker/testInfo'; +import { TestInfoImpl } from '../worker/testInfo'; import { ExpectError, isExpectError } from './matcherHint'; import { toMatchAriaSnapshot } from './toMatchAriaSnapshot'; @@ -116,8 +116,6 @@ function qualifiedMatcherName(qualifier: string[], matcherName: string) { return qualifier.join(':') + '$' + matcherName; } -export const testStepInternalSymbol = Symbol('TestStepInternal'); - function createExpect(info: ExpectMetaInfo, prefix: string[], customMatchers: Record) { const expectInstance: Expect<{}> = new Proxy(expectLibrary, { apply: function(target: any, thisArg: any, argumentsList: [unknown, ExpectMessage?]) { @@ -143,7 +141,7 @@ function createExpect(info: ExpectMetaInfo, prefix: string[], customMatchers: Re const wrappedMatchers: any = {}; const extendedMatchers: any = { ...customMatchers }; for (const [name, matcher] of Object.entries(matchers)) { - wrappedMatchers[name] = function(actual: any, step: TestStepInternal, ...args: any[]) { + wrappedMatchers[name] = function(...args: any[]) { const { isNot, promise, utils } = this; const newThis: ExpectMatcherState = { isNot, @@ -151,9 +149,8 @@ function createExpect(info: ExpectMetaInfo, prefix: string[], customMatchers: Re utils, timeout: currentExpectTimeout() }; - (newThis as any)[testStepInternalSymbol] = step; (newThis as any).equals = throwUnsupportedExpectMatcherError; - return (matcher as any).call(newThis, actual, ...args); + return (matcher as any).call(newThis, ...args); }; const key = qualifiedMatcherName(qualifier, name); wrappedMatchers[key] = wrappedMatchers[name]; @@ -304,7 +301,7 @@ class ExpectMetaInfoProxyHandler implements ProxyHandler { // We assume that the matcher will read the current expect timeout the first thing. setCurrentExpectConfigureTimeout(this._info.timeout); if (!testInfo) - return matcher.call(target, undefined, ...args); + return matcher.call(target, ...args); const customMessage = this._info.message || ''; const argsSuffix = computeArgsSuffix(matcherName, args); @@ -340,7 +337,7 @@ class ExpectMetaInfoProxyHandler implements ProxyHandler { }; try { - const callback = () => matcher.call(target, step, ...args); + const callback = () => matcher.call(target, ...args); // toPass and poll matchers can contain other steps, expects and API calls, // so they behave like a retriable step. const result = (matcherName === 'toPass' || this._info.poll) ? diff --git a/packages/playwright/src/matchers/toMatchSnapshot.ts b/packages/playwright/src/matchers/toMatchSnapshot.ts index 4cd4def0de..b3fa3f556e 100644 --- a/packages/playwright/src/matchers/toMatchSnapshot.ts +++ b/packages/playwright/src/matchers/toMatchSnapshot.ts @@ -29,11 +29,10 @@ import { colors } from 'playwright-core/lib/utilsBundle'; import fs from 'fs'; import path from 'path'; import { mime } from 'playwright-core/lib/utilsBundle'; -import type { TestInfoImpl, TestStepInternal } from '../worker/testInfo'; +import type { TestInfoImpl } from '../worker/testInfo'; import type { ExpectMatcherState } from '../../types/test'; import type { MatcherResult } from './matcherHint'; import type { FullProjectInternal } from '../common/config'; -import { testStepInternalSymbol } from './expect'; type NameOrSegments = string | string[]; const snapshotNamesSymbol = Symbol('snapshotNames'); @@ -76,7 +75,6 @@ const NonConfigProperties: (keyof ToHaveScreenshotOptions)[] = [ class SnapshotHelper { readonly testInfo: TestInfoImpl; - readonly step: TestStepInternal; readonly attachmentBaseName: string; readonly legacyExpectedPath: string; readonly previousPath: string; @@ -93,7 +91,6 @@ class SnapshotHelper { constructor( testInfo: TestInfoImpl, - step: TestStepInternal, matcherName: string, locator: Locator | undefined, anonymousSnapshotExtension: string, @@ -185,7 +182,6 @@ class SnapshotHelper { this.comparator = getComparator(this.mimeType); this.testInfo = testInfo; - this.step = step; this.kind = this.mimeType.startsWith('image/') ? 'Screenshot' : 'Snapshot'; } @@ -228,9 +224,9 @@ class SnapshotHelper { const isWriteMissingMode = this.updateSnapshots === 'all' || this.updateSnapshots === 'missing'; if (isWriteMissingMode) writeFileSync(this.expectedPath, actual); - this.step.attachments.push({ name: addSuffixToFilePath(this.attachmentBaseName, '-expected'), contentType: this.mimeType, path: this.expectedPath }); + this.testInfo.attachments.push({ name: addSuffixToFilePath(this.attachmentBaseName, '-expected'), contentType: this.mimeType, path: this.expectedPath }); writeFileSync(this.actualPath, actual); - this.step.attachments.push({ name: addSuffixToFilePath(this.attachmentBaseName, '-actual'), contentType: this.mimeType, path: this.actualPath }); + this.testInfo.attachments.push({ name: addSuffixToFilePath(this.attachmentBaseName, '-actual'), contentType: this.mimeType, path: this.actualPath }); const message = `A snapshot doesn't exist at ${this.expectedPath}${isWriteMissingMode ? ', writing actual.' : '.'}`; if (this.updateSnapshots === 'all') { /* eslint-disable no-console */ @@ -264,22 +260,22 @@ class SnapshotHelper { // Copy the expectation inside the `test-results/` folder for backwards compatibility, // so that one can upload `test-results/` directory and have all the data inside. writeFileSync(this.legacyExpectedPath, expected); - this.step.attachments.push({ name: addSuffixToFilePath(this.attachmentBaseName, '-expected'), contentType: this.mimeType, path: this.expectedPath }); + this.testInfo.attachments.push({ name: addSuffixToFilePath(this.attachmentBaseName, '-expected'), contentType: this.mimeType, path: this.expectedPath }); output.push(`\nExpected: ${colors.yellow(this.expectedPath)}`); } if (previous !== undefined) { writeFileSync(this.previousPath, previous); - this.step.attachments.push({ name: addSuffixToFilePath(this.attachmentBaseName, '-previous'), contentType: this.mimeType, path: this.previousPath }); + this.testInfo.attachments.push({ name: addSuffixToFilePath(this.attachmentBaseName, '-previous'), contentType: this.mimeType, path: this.previousPath }); output.push(`Previous: ${colors.yellow(this.previousPath)}`); } if (actual !== undefined) { writeFileSync(this.actualPath, actual); - this.step.attachments.push({ name: addSuffixToFilePath(this.attachmentBaseName, '-actual'), contentType: this.mimeType, path: this.actualPath }); + this.testInfo.attachments.push({ name: addSuffixToFilePath(this.attachmentBaseName, '-actual'), contentType: this.mimeType, path: this.actualPath }); output.push(`Received: ${colors.yellow(this.actualPath)}`); } if (diff !== undefined) { writeFileSync(this.diffPath, diff); - this.step.attachments.push({ name: addSuffixToFilePath(this.attachmentBaseName, '-diff'), contentType: this.mimeType, path: this.diffPath }); + this.testInfo.attachments.push({ name: addSuffixToFilePath(this.attachmentBaseName, '-diff'), contentType: this.mimeType, path: this.diffPath }); output.push(` Diff: ${colors.yellow(this.diffPath)}`); } @@ -303,8 +299,7 @@ export function toMatchSnapshot( optOptions: ImageComparatorOptions = {} ): MatcherResult { const testInfo = currentTestInfo(); - const step = (this as any)[testStepInternalSymbol] as TestStepInternal | undefined; - if (!testInfo || !step) + if (!testInfo) throw new Error(`toMatchSnapshot() must be called during the test`); if (received instanceof Promise) throw new Error('An unresolved Promise was passed to toMatchSnapshot(), make sure to resolve it by adding await to it.'); @@ -314,7 +309,7 @@ export function toMatchSnapshot( const configOptions = testInfo._projectInternal.expect?.toMatchSnapshot || {}; const helper = new SnapshotHelper( - testInfo, step, 'toMatchSnapshot', undefined, determineFileExtension(received), + testInfo, 'toMatchSnapshot', undefined, determineFileExtension(received), configOptions, nameOrOptions, optOptions); if (this.isNot) { @@ -361,8 +356,7 @@ export async function toHaveScreenshot( optOptions: ToHaveScreenshotOptions = {} ): Promise> { const testInfo = currentTestInfo(); - const step = (this as any)[testStepInternalSymbol] as TestStepInternal | undefined; - if (!testInfo || !step) + if (!testInfo) throw new Error(`toHaveScreenshot() must be called during the test`); if (testInfo._projectInternal.ignoreSnapshots) @@ -371,7 +365,7 @@ export async function toHaveScreenshot( expectTypes(pageOrLocator, ['Page', 'Locator'], 'toHaveScreenshot'); const [page, locator] = pageOrLocator.constructor.name === 'Page' ? [(pageOrLocator as PageEx), undefined] : [(pageOrLocator as Locator).page() as PageEx, pageOrLocator as Locator]; const configOptions = testInfo._projectInternal.expect?.toHaveScreenshot || {}; - const helper = new SnapshotHelper(testInfo, step, 'toHaveScreenshot', locator, 'png', configOptions, nameOrOptions, optOptions); + const helper = new SnapshotHelper(testInfo, 'toHaveScreenshot', locator, 'png', configOptions, nameOrOptions, optOptions); if (!helper.expectedPath.toLowerCase().endsWith('.png')) throw new Error(`Screenshot name "${path.basename(helper.expectedPath)}" must have '.png' extension`); expectTypes(pageOrLocator, ['Page', 'Locator'], 'toHaveScreenshot');