use expectZone to get current step for expects

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;
-}

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<any> {
       };
 
       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<any> {
       };
 
       const finalizer = () => {
-        setCurrentStep(undefined);
         step.complete({});
       };
 

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<NameOrSegments | { name?: NameOrSegments }, string> {
   const testInfo = currentTestInfo();
-  const step = currentStep();
+  if (!testInfo)
-  if (!testInfo || !step)
     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<MatcherResult<NameOrSegments | { name?: NameOrSegments }, string>> {
   const testInfo = currentTestInfo();
-  const step = currentStep();
+  if (!testInfo)
-  if (!testInfo || !step)
     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');

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>('expectZone')?.stepId;
+  }
+
   _addStep(data: Omit<TestStepInternal, 'complete' | 'stepId' | 'steps' | 'attachments'>, parentStep?: TestStepInternal): TestStepInternal {
     const stepId = `${data.category}@${++this._lastStepId}`;
 

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({