diff --git a/packages/playwright/src/common/globals.ts b/packages/playwright/src/common/globals.ts index f24a55fc60..746e1ec847 100644 --- a/packages/playwright/src/common/globals.ts +++ b/packages/playwright/src/common/globals.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import type { TestInfoImpl, TestStepInternal } from '../worker/testInfo'; +import type { TestInfoImpl } from '../worker/testInfo'; import type { Suite } from './test'; let currentTestInfoValue: TestInfoImpl | null = null; @@ -42,13 +42,3 @@ export function setIsWorkerProcess() { export function isWorkerProcess() { return _isWorkerProcess; } - -let currentStepValue: TestStepInternal | undefined; - -export function setCurrentStep(step: TestStepInternal | undefined) { - currentStepValue = step; -} - -export function currentStep(): TestStepInternal | undefined { - return currentStepValue; -} diff --git a/packages/playwright/src/matchers/expect.ts b/packages/playwright/src/matchers/expect.ts index 288dd18e1a..0bd116e7a1 100644 --- a/packages/playwright/src/matchers/expect.ts +++ b/packages/playwright/src/matchers/expect.ts @@ -51,7 +51,7 @@ import { } from './matchers'; import { toMatchSnapshot, toHaveScreenshot, toHaveScreenshotStepTitle } from './toMatchSnapshot'; import type { Expect, ExpectMatcherState } from '../../types/test'; -import { currentTestInfo, setCurrentStep } from '../common/globals'; +import { currentTestInfo } from '../common/globals'; import { filteredStackTrace, trimLongString } from '../util'; import { expect as expectLibrary, @@ -322,10 +322,8 @@ class ExpectMetaInfoProxyHandler implements ProxyHandler { }; const step = testInfo._addStep(stepInfo); - setCurrentStep(step); const reportStepError = (e: Error | unknown) => { - setCurrentStep(undefined); const jestError = isJestError(e) ? e : null; const error = jestError ? new ExpectError(jestError, customMessage, stackFrames) : e; if (jestError?.matcherResult.suggestedRebaseline) { @@ -340,7 +338,6 @@ class ExpectMetaInfoProxyHandler implements ProxyHandler { }; const finalizer = () => { - setCurrentStep(undefined); step.complete({}); }; diff --git a/packages/playwright/src/matchers/toMatchSnapshot.ts b/packages/playwright/src/matchers/toMatchSnapshot.ts index d04be1ab69..8d16f11fc2 100644 --- a/packages/playwright/src/matchers/toMatchSnapshot.ts +++ b/packages/playwright/src/matchers/toMatchSnapshot.ts @@ -16,7 +16,7 @@ import type { Locator, Page } from 'playwright-core'; import type { ExpectScreenshotOptions, Page as PageEx } from 'playwright-core/lib/client/page'; -import { currentStep, currentTestInfo } from '../common/globals'; +import { currentTestInfo } from '../common/globals'; import type { ImageComparatorOptions, Comparator } from 'playwright-core/lib/utils'; import { compareBuffersOrStrings, getComparator, isString, sanitizeForFilePath } from 'playwright-core/lib/utils'; import { @@ -29,7 +29,7 @@ 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 { matcherHint, type MatcherResult } from './matcherHint'; import type { FullProjectInternal } from '../common/config'; @@ -75,7 +75,6 @@ const NonConfigProperties: (keyof ToHaveScreenshotOptions)[] = [ class SnapshotHelper { readonly testInfo: TestInfoImpl; - readonly step: TestStepInternal; readonly attachmentBaseName: string; readonly legacyExpectedPath: string; readonly previousPath: string; @@ -92,7 +91,6 @@ class SnapshotHelper { constructor( testInfo: TestInfoImpl, - step: TestStepInternal, matcherName: string, locator: Locator | undefined, anonymousSnapshotExtension: string, @@ -184,7 +182,6 @@ class SnapshotHelper { this.comparator = getComparator(this.mimeType); this.testInfo = testInfo; - this.step = step; this.kind = this.mimeType.startsWith('image/') ? 'Screenshot' : 'Snapshot'; } @@ -227,9 +224,9 @@ class SnapshotHelper { const isWriteMissingMode = this.updateSnapshots !== 'none'; 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' || this.updateSnapshots === 'changed') { /* eslint-disable no-console */ @@ -257,22 +254,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)}`); } @@ -296,8 +293,7 @@ export function toMatchSnapshot( optOptions: ImageComparatorOptions = {} ): MatcherResult { const testInfo = currentTestInfo(); - const step = currentStep(); - 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.'); @@ -307,7 +303,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) { @@ -369,8 +365,7 @@ export async function toHaveScreenshot( optOptions: ToHaveScreenshotOptions = {} ): Promise> { const testInfo = currentTestInfo(); - const step = currentStep(); - if (!testInfo || !step) + if (!testInfo) throw new Error(`toHaveScreenshot() must be called during the test`); if (testInfo._projectInternal.ignoreSnapshots) @@ -379,7 +374,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'); diff --git a/packages/playwright/src/worker/testInfo.ts b/packages/playwright/src/worker/testInfo.ts index d4fdcb4c9c..ba0a03f10f 100644 --- a/packages/playwright/src/worker/testInfo.ts +++ b/packages/playwright/src/worker/testInfo.ts @@ -17,6 +17,7 @@ import fs from 'fs'; import path from 'path'; import { captureRawStack, monotonicTime, zones, sanitizeForFilePath, stringifyStackFrames } from 'playwright-core/lib/utils'; +import type { ExpectZone } from 'playwright-core/lib/utils'; import type { TestInfo, TestStatus, FullProject } from '../../types/test'; import type { AttachmentPayload, StepBeginPayload, StepEndPayload, TestInfoErrorImpl, WorkerInitParams } from '../common/ipc'; import type { TestCase } from '../common/test'; @@ -194,7 +195,7 @@ export class TestInfoImpl implements TestInfo { this._attachmentsPush = this.attachments.push.bind(this.attachments); this.attachments.push = (...attachments: TestInfo['attachments']) => { for (const a of attachments) - this._attach(a, this._parentStep()?.stepId); + this._attach(a, this._expectStepId() ?? this._parentStep()?.stepId); return this.attachments.length; }; @@ -244,6 +245,10 @@ export class TestInfoImpl implements TestInfo { ?? this._findLastStageStep(this._steps); // If no parent step on stack, assume the current stage as parent. } + private _expectStepId() { + return zones.zoneData('expectZone')?.stepId; + } + _addStep(data: Omit, parentStep?: TestStepInternal): TestStepInternal { const stepId = `${data.category}@${++this._lastStepId}`; diff --git a/tests/playwright-test/to-have-screenshot.spec.ts b/tests/playwright-test/to-have-screenshot.spec.ts index 9f1bd4c47c..3afa7a8d90 100644 --- a/tests/playwright-test/to-have-screenshot.spec.ts +++ b/tests/playwright-test/to-have-screenshot.spec.ts @@ -677,6 +677,30 @@ test('should write missing expectations locally twice and attach them', async ({ ]); }); +test('should attach missing expectations to right step', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'reporter.ts': ` + class Reporter { + onStepEnd(test, result, step) { + if (step.attachments.length > 0) + console.log(\`%%\${step.title}: \${step.attachments.map(a => a.name).join(", ")}\`); + } + } + module.exports = Reporter; + `, + ...playwrightConfig({ reporter: [['dot'], ['./reporter']] }), + 'a.spec.js': ` + const { test, expect } = require('@playwright/test'); + test('is a test', async ({ page }) => { + await expect(page).toHaveScreenshot('snapshot.png'); + }); + `, + }, { reporter: '' }); + + expect(result.exitCode).toBe(1); + expect(result.outputLines).toEqual(['expect.toHaveScreenshot(snapshot.png): snapshot-expected.png, snapshot-actual.png']); +}); + test('shouldn\'t write missing expectations locally for negated matcher', async ({ runInlineTest }, testInfo) => { const result = await runInlineTest({ ...playwrightConfig({