fix: do not generate api call steps inside named expects (#28609)

Fixes: #28528
microsoft · Dec 13, 2023 · afe90d6 · afe90d6
1 parent 297cfdf
commit afe90d6
Show file tree

Hide file tree

Showing 3 changed files with 96 additions and 2 deletions.
diff --git a/packages/playwright-core/src/client/channelOwner.ts b/packages/playwright-core/src/client/channelOwner.ts
@@ -180,7 +180,9 @@ export abstract class ChannelOwner<T extends channels.Channel = channels.Channel
     if (!isInternal && expectZone)
       apiName = expectZone.title;
 
-    const csi = isInternal ? undefined : this._instrumentation;
+    // If we are coming from the expectZone, there is no need to generate a new
+    // step for the API call, since it will be generated by the expect itself.
+    const csi = isInternal || expectZone ? undefined : this._instrumentation;
     const callCookie: any = {};
 
     try {

diff --git a/packages/playwright/src/index.ts b/packages/playwright/src/index.ts
@@ -254,7 +254,7 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
     const csiListener: ClientInstrumentationListener = {
       onApiCallBegin: (apiName: string, params: Record<string, any>, frames: StackFrame[], wallTime: number, userData: any) => {
         const testInfo = currentTestInfo();
-        if (!testInfo || apiName.startsWith('expect.') || apiName.includes('setTestIdAttribute'))
+        if (!testInfo || apiName.includes('setTestIdAttribute'))
           return { userObject: null };
         const step = testInfo._addStep({
           location: frames[0] as any,

diff --git a/tests/playwright-test/test-step.spec.ts b/tests/playwright-test/test-step.spec.ts
@@ -1385,3 +1385,95 @@ test('should step w/ box', async ({ runInlineTest }) => {
     },
   ]);
 });
+
+test('should not generate dupes for named expects', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'reporter.ts': stepHierarchyReporter,
+    'playwright.config.ts': `
+      module.exports = {
+        reporter: './reporter',
+      };
+    `,
+    'a.test.ts': `
+      import { test, expect } from '@playwright/test';
+      test('timeout', async ({ page }) => {
+        await page.setContent('<div style="background:rgb(1,2,3)">hi</div>');
+        await expect(page.locator('div'), 'Checking color')
+            .toHaveCSS('background-color', 'rgb(1, 2, 3)');
+      });
+    `
+  }, { reporter: '', workers: 1, timeout: 2000 });
+
+  expect(result.exitCode).toBe(0);
+  const objects = result.outputLines.map(line => JSON.parse(line));
+  expect(objects).toEqual([
+    {
+      category: 'hook',
+      title: 'Before Hooks',
+      steps: [
+        {
+          category: 'fixture',
+          title: 'fixture: browser',
+          steps: [
+            {
+              category: 'pw:api',
+              title: 'browserType.launch',
+            },
+          ]
+        },
+        {
+          category: 'fixture',
+          title: 'fixture: context',
+          steps: [
+            {
+              category: 'pw:api',
+              title: 'browser.newContext',
+            },
+          ]
+        },
+        {
+          category: 'fixture',
+          title: 'fixture: page',
+          steps: [
+            {
+              category: 'pw:api',
+              title: 'browserContext.newPage',
+            },
+          ]
+        },
+      ],
+    },
+    {
+      category: 'pw:api',
+      title: 'page.setContent',
+      location: {
+        column: expect.any(Number),
+        file: 'a.test.ts',
+        line: expect.any(Number),
+      },
+    },
+    {
+      category: 'expect',
+      title: 'Checking color',
+      location: {
+        column: expect.any(Number),
+        file: 'a.test.ts',
+        line: expect.any(Number),
+      },
+    },
+    {
+      category: 'hook',
+      title: 'After Hooks',
+      steps: [
+        {
+          category: 'fixture',
+          title: 'fixture: page',
+        },
+        {
+          category: 'fixture',
+          title: 'fixture: context',
+        },
+      ],
+    },
+  ]);
+});