chore: box soft steps (microsoft#28749)

Fixes microsoft#28259
pavelfeldman · Dec 21, 2023 · 5d9e08a · 5d9e08a
1 parent ef81b05
commit 5d9e08a
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 16 deletions.
diff --git a/packages/playwright/src/worker/testInfo.ts b/packages/playwright/src/worker/testInfo.ts
@@ -27,6 +27,7 @@ import type { Location } from '../../types/testReporter';
 import { filteredStackTrace, getContainedPath, normalizeAndSaveAttachment, serializeError, trimLongString } from '../util';
 import { TestTracing } from './testTracing';
 import type { Attachment } from './testTracing';
+import type { StackFrame } from '@protocol/channels';
 
 export interface TestStepInternal {
   complete(result: { error?: Error | TestInfoError, attachments?: Attachment[] }): void;
@@ -35,6 +36,7 @@ export interface TestStepInternal {
   category: string;
   wallTime: number;
   location?: Location;
+  boxedStack?: StackFrame[];
   steps: TestStepInternal[];
   laxParent?: boolean;
   endWallTime?: number;
@@ -246,12 +248,9 @@ export class TestInfoImpl implements TestInfo {
 
   _addStep(data: Omit<TestStepInternal, 'complete' | 'stepId' | 'steps'>, parentStep?: TestStepInternal): TestStepInternal {
     const stepId = `${data.category}@${++this._lastStepId}`;
-    const rawStack = data.box || !data.location || !parentStep ? captureRawStack() : null;
-    const filteredStack = rawStack ? filteredStackTrace(rawStack) : [];
+    const rawStack = captureRawStack();
     if (!parentStep)
       parentStep = zones.zoneData<TestStepInternal>('stepZone', rawStack!) || undefined;
-    const boxedStack = data.box ? filteredStack.slice(1) : undefined;
-    const location = data.location || boxedStack?.[0] || filteredStack[0];
 
     // For out-of-stack calls, locate the enclosing step.
     let isLaxParent = false;
@@ -268,10 +267,17 @@ export class TestInfoImpl implements TestInfo {
       isLaxParent = !!parentStep;
     }
 
+    const filteredStack = filteredStackTrace(rawStack);
+    data.boxedStack = parentStep?.boxedStack;
+    if (!data.boxedStack && data.box) {
+      data.boxedStack = filteredStack.slice(1);
+      data.location = data.location || data.boxedStack[0];
+    }
+    data.location = data.location || filteredStack[0];
+
     const step: TestStepInternal = {
       stepId,
       ...data,
-      location,
       laxParent: isLaxParent,
       steps: [],
       complete: result => {
@@ -281,13 +287,15 @@ export class TestInfoImpl implements TestInfo {
         let error: TestInfoError | undefined;
         if (result.error instanceof Error) {
           // Step function threw an error.
-          if (boxedStack) {
+          if (data.boxedStack) {
             const errorTitle = `${result.error.name}: ${result.error.message}`;
-            result.error.stack = `${errorTitle}\n${stringifyStackFrames(boxedStack).join('\n')}`;
+            result.error.stack = `${errorTitle}\n${stringifyStackFrames(data.boxedStack).join('\n')}`;
           }
           error = serializeError(result.error);
         } else if (result.error) {
           // Internal API step reported an error.
+          if (data.boxedStack)
+            result.error.stack = `${result.error.message}\n${stringifyStackFrames(data.boxedStack).join('\n')}`;
           error = result.error;
         }
         step.error = error;
@@ -326,10 +334,10 @@ export class TestInfoImpl implements TestInfo {
       title: data.title,
       category: data.category,
       wallTime: data.wallTime,
-      location,
+      location: data.location,
     };
     this._onStepBegin(payload);
-    this._tracing.appendBeforeActionForStep(stepId, parentStep?.stepId, data.apiName || data.title, data.params, data.wallTime, location ? [location] : []);
+    this._tracing.appendBeforeActionForStep(stepId, parentStep?.stepId, data.apiName || data.title, data.params, data.wallTime, data.location ? [data.location] : []);
     return step;
   }
 

diff --git a/tests/playwright-test/test-step.spec.ts b/tests/playwright-test/test-step.spec.ts
@@ -1354,8 +1354,8 @@ test('should step w/ box', async ({ runInlineTest }) => {
       /*2*/ test('fail', async () => {
       /*3*/   const helper = async () => {
       /*4*/     await test.step('boxed step', async () => {
-      /*5*/       await expect(page.locator('body')).toHaveText('Good page', { timeout: 1 });
-      /*6*/     }, { box: 'self' });
+      /*5*/       expect(1).toBe(2);
+      /*6*/     }, { box: true });
       /*7*/   };
       /*8*/   await helper();
       /*9*/ });
@@ -1370,14 +1370,59 @@ test('should step w/ box', async ({ runInlineTest }) => {
       title: 'Before Hooks',
     },
     {
+      title: 'boxed step',
       category: 'test.step',
       error: expect.not.stringMatching(/a.test.ts:[^8]/),
-      location: {
-        column: 21,
-        file: 'a.test.ts',
-        line: 8,
-      },
+      location: { file: 'a.test.ts', line: 8, column: 21 },
+      steps: [{
+        title: 'expect.toBe',
+        category: 'expect',
+        error: expect.stringContaining('expect(received).toBe(expected)'),
+        location: { file: 'a.test.ts', column: 29, line: 5 }
+      }],
+    },
+    {
+      category: 'hook',
+      title: 'After Hooks',
+    },
+  ]);
+});
+
+test('should soft step w/ box', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'reporter.ts': stepHierarchyReporter,
+    'playwright.config.ts': `module.exports = { reporter: './reporter', };`,
+    'a.test.ts':
+    ` /*1*/ import { test, expect } from '@playwright/test';
+      /*2*/ test('fail', async () => {
+      /*3*/   const helper = async () => {
+      /*4*/     await test.step('boxed step', async () => {
+      /*5*/       expect.soft(1).toBe(2);
+      /*6*/     }, { box: true });
+      /*7*/   };
+      /*8*/   await helper();
+      /*9*/ });
+    `
+  }, { reporter: '' });
+
+  expect(result.exitCode).toBe(1);
+  const objects = result.outputLines.map(line => JSON.parse(line));
+  expect(objects).toEqual([
+    {
+      category: 'hook',
+      title: 'Before Hooks',
+    },
+    {
       title: 'boxed step',
+      category: 'test.step',
+      error: expect.not.stringMatching(/a.test.ts:[^8]/),
+      location: { file: 'a.test.ts', line: 8, column: 21 },
+      steps: [{
+        title: 'expect.soft.toBe',
+        category: 'expect',
+        error: expect.stringContaining('expect(received).toBe(expected)'),
+        location: { file: 'a.test.ts', column: 34, line: 5, }
+      }],
     },
     {
       category: 'hook',