microsoft · dgozman · Nov 5, 2024 · Nov 5, 2024
diff --git a/docs/src/api/class-tracing.md b/docs/src/api/class-tracing.md
@@ -284,14 +284,10 @@ To specify the final trace zip file name, you need to pass `path` option to
 ## async method: Tracing.group
 * since: v1.49
 
-Creates a new inline group within the trace, assigning any subsequent calls to this group until [method: Tracing.groupEnd] is invoked.
+Creates a new group within the trace, assigning any subsequent API calls to this group, until [`method: Tracing.groupEnd`] is called. Groups can be nested and will be visible in the trace viewer and test reports.
 
-Groups can be nested and are similar to `test.step` in trace.
-However, groups are only visualized in the trace viewer and, unlike test.step, have no effect on the test reports.
-
-:::note Groups should not be used with Playwright Test!
-
-This API is intended for Playwright API users that can not use `test.step`.
+:::caution
+When using Playwright test runner, we strongly recommend `test.step` instead.
 :::
 
 **Usage**
@@ -310,6 +306,38 @@ await context.tracing.groupEnd();
 // This Trace will have two groups: 'Open Playwright.dev' and 'Open API Docs of Tracing'.
 ```
 
+```java
+// All actions between group and groupEnd will be shown in the trace viewer as a group.
+page.context().tracing.group("Open Playwright.dev > API");
+page.navigate("https://playwright.dev/");
+page.getByRole(AriaRole.LINK, new Page.GetByRoleOptions().setName("API")).click();
+page.context().tracing.groupEnd();
+```
+
+```python sync
+# All actions between group and groupEnd will be shown in the trace viewer as a group.
+page.context.tracing.group("Open Playwright.dev > API")
+page.goto("https://playwright.dev/")
+page.get_by_role("link", name="API").click()
+page.context.tracing.group_end()
+```
+
+```python async
+# All actions between group and groupEnd will be shown in the trace viewer as a group.
+await page.context.tracing.group("Open Playwright.dev > API")
+await page.goto("https://playwright.dev/")
+await page.get_by_role("link", name="API").click()
+await page.context.tracing.group_end()
+```
+
+```csharp
+// All actions between group and groupEnd will be shown in the trace viewer as a group.
+await Page.Context().Tracing.GroupAsync("Open Playwright.dev > API");
+await Page.GotoAsync("https://playwright.dev/");
+await Page.GetByRole(AriaRole.Link, new() { Name = "API" }).ClickAsync();
+await Page.Context().Tracing.GroupEndAsync();
+```
+
 ### param: Tracing.group.name
 * since: v1.49
 - `name` <[string]>
@@ -321,15 +349,15 @@ Group name shown in the actions tree in trace viewer.
 - `location` ?<[Object]>
   - `file` <[string]> Source file path to be shown in the trace viewer source tab.
   - `line` ?<[int]> Line number in the source file.
-  - `column` ?<[int]> Column number in the source file
+  - `column` ?<[int]> Column number in the source file.
 
 Specifies a custom location for the group start to be shown in source tab in trace viewer.
-By default, location of the tracing.group() call is shown.
+By default, location of the [`method: Tracing.group`] call is shown.
 
 ## async method: Tracing.groupEnd
 * since: v1.49
 
-Closes the currently open inline group in the trace.
+Closes the last group created by [`method: Tracing.group`].
 
 ## async method: Tracing.stop
 * since: v1.12

diff --git a/packages/playwright-core/src/server/trace/recorder/tracing.ts b/packages/playwright-core/src/server/trace/recorder/tracing.ts
@@ -229,7 +229,7 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps
     this._appendTraceEvent(event);
   }
 
-  async groupEnd(): Promise<void> {
+  groupEnd() {
     if (!this._state)
       return;
     const callId = this._state.groupStack.pop();
@@ -285,7 +285,7 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps
       throw new Error(`Tracing is already stopping`);
     if (this._state.recording)
       throw new Error(`Must stop trace file before stopping tracing`);
-    await this._closeAllGroups();
+    this._closeAllGroups();
     this._harTracer.stop();
     this.flushHarEntries();
     await this._fs.syncAndGetError();
@@ -314,9 +314,9 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps
     await this._fs.syncAndGetError();
   }
 
-  async _closeAllGroups() {
+  private _closeAllGroups() {
     while (this._currentGroupId())
-      await this.groupEnd();
+      this.groupEnd();
   }
 
   async stopChunk(params: TracingTracingStopChunkParams): Promise<{ artifact?: Artifact, entries?: NameValue[] }> {
@@ -331,7 +331,7 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps
       return {};
     }
 
-    await this._closeAllGroups();
+    this._closeAllGroups();
 
     this._context.instrumentation.removeListener(this);
     eventsHelper.removeEventListeners(this._eventListeners);

diff --git a/packages/playwright-core/types/types.d.ts b/packages/playwright-core/types/types.d.ts
@@ -21056,13 +21056,11 @@ export interface Touchscreen {
  */
 export interface Tracing {
   /**
-   * Creates a new inline group within the trace, assigning any subsequent calls to this group until
-   * [method: Tracing.groupEnd] is invoked.
+   * Creates a new group within the trace, assigning any subsequent API calls to this group, until
+   * [tracing.groupEnd()](https://playwright.dev/docs/api/class-tracing#tracing-group-end) is called. Groups can be
+   * nested and will be visible in the trace viewer and test reports.
    *
-   * Groups can be nested and are similar to `test.step` in trace. However, groups are only visualized in the trace
-   * viewer and, unlike test.step, have no effect on the test reports.
-   *
-   * **NOTE** This API is intended for Playwright API users that can not use `test.step`.
+   * **NOTE** When using Playwright test runner, we strongly recommend `test.step` instead.
    *
    * **Usage**
    *
@@ -21086,7 +21084,7 @@ export interface Tracing {
   group(name: string, options?: {
     /**
      * Specifies a custom location for the group start to be shown in source tab in trace viewer. By default, location of
-     * the tracing.group() call is shown.
+     * the [tracing.group(name[, options])](https://playwright.dev/docs/api/class-tracing#tracing-group) call is shown.
      */
     location?: {
       /**
@@ -21100,14 +21098,15 @@ export interface Tracing {
       line?: number;
 
       /**
-       * Column number in the source file
+       * Column number in the source file.
        */
       column?: number;
     };
   }): Promise<void>;
 
   /**
-   * Closes the currently open inline group in the trace.
+   * Closes the last group created by
+   * [tracing.group(name[, options])](https://playwright.dev/docs/api/class-tracing#tracing-group).
    */
   groupEnd(): Promise<void>;
 

diff --git a/packages/playwright/src/index.ts b/packages/playwright/src/index.ts
@@ -259,27 +259,32 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
     const tracingGroupSteps: TestStepInternal[] = [];
     const csiListener: ClientInstrumentationListener = {
       onApiCallBegin: (apiName: string, params: Record<string, any>, frames: StackFrame[], userData: any, out: { stepId?: string }) => {
+        userData.apiName = apiName;
         const testInfo = currentTestInfo();
-        if (!testInfo || apiName.includes('setTestIdAttribute'))
-          return { userObject: null };
-        if (apiName === 'tracing.groupEnd') {
-          tracingGroupSteps.pop();
-          return { userObject: null };
-        }
+        if (!testInfo || apiName.includes('setTestIdAttribute') || apiName === 'tracing.groupEnd')
+          return;
         const step = testInfo._addStep({
           location: frames[0] as any,
           category: 'pw:api',
           title: renderApiCall(apiName, params),
           apiName,
           params,
         }, tracingGroupSteps[tracingGroupSteps.length - 1]);
-        userData.userObject = step;
+        userData.step = step;
         out.stepId = step.stepId;
         if (apiName === 'tracing.group')
           tracingGroupSteps.push(step);
       },
       onApiCallEnd: (userData: any, error?: Error) => {
-        const step = userData.userObject;
+        // "tracing.group" step will end later, when "tracing.groupEnd" finishes.
+        if (userData.apiName === 'tracing.group')
+          return;
+        if (userData.apiName === 'tracing.groupEnd') {
+          const step = tracingGroupSteps.pop();
+          step?.complete({ error });
+          return;
+        }
+        const step = userData.step;
         step?.complete({ error });
       },
       onWillPause: () => {
@@ -707,6 +712,8 @@ class ArtifactsRecorder {
 const paramsToRender = ['url', 'selector', 'text', 'key'];
 
 function renderApiCall(apiName: string, params: any) {
+  if (apiName === 'tracing.group')
+    return params.name;
   const paramsArray = [];
   if (params) {
     for (const name of paramsToRender) {

diff --git a/tests/config/traceViewerFixtures.ts b/tests/config/traceViewerFixtures.ts
@@ -75,6 +75,11 @@ class TraceViewerPage {
     return await this.page.waitForSelector(`.tree-view-entry:has-text("${action}") .action-icons`);
   }
 
+  @step
+  async expandAction(title: string, ordinal: number = 0) {
+    await this.actionsTree.locator('.tree-view-entry', { hasText: title }).nth(ordinal).locator('.codicon-chevron-right').click();
+  }
+
   @step
   async selectAction(title: string, ordinal: number = 0) {
     await this.page.locator(`.action-title:has-text("${title}")`).nth(ordinal).click();

diff --git a/tests/library/trace-viewer.spec.ts b/tests/library/trace-viewer.spec.ts
@@ -14,6 +14,9 @@
  * limitations under the License.
  */
 
+// DO NOT TOUCH THIS LINE
+// It is used in the tracing.group test.
+
 import type { TraceViewerFixtures } from '../config/traceViewerFixtures';
 import { traceViewerFixtures } from '../config/traceViewerFixtures';
 import fs from 'fs';
@@ -103,105 +106,45 @@ test('should open trace viewer on specific host', async ({ showTraceViewer }, te
   await expect(traceViewer.page).toHaveURL(/127.0.0.1/);
 });
 
-test('should show groups as tree in trace viewer', async ({ runAndTrace, page, context }) => {
-  const outerGroup = 'Outer Group';
-  const outerGroupContent = 'locator.clickgetByText(\'Click\')';
-  const firstInnerGroup = 'First Inner Group';
-  const firstInnerGroupContent = 'locator.clicklocator(\'button\').first()';
-  const secondInnerGroup = 'Second Inner Group';
-  const secondInnerGroupContent = 'expect.toBeVisiblegetByText(\'Click\')';
-  const expandedFailure = 'Expanded Failure';
-
+test('should show tracing.group in the action list with location', async ({ runAndTrace, page, context }) => {
   const traceViewer = await test.step('create trace with groups', async () => {
+    await page.context().tracing.group('ignored group');
     return await runAndTrace(async () => {
-      try {
-        await page.goto(`data:text/html,<!DOCTYPE html><html>Hello world</html>`);
-        await page.setContent('<!DOCTYPE html><button>Click</button>');
-        async function doClick() {
-          await page.getByText('Click').click();
-        }
-        await context.tracing.group(outerGroup);  // Outer group
-        await doClick();
-        await context.tracing.group(firstInnerGroup, { location: { file: `${__dirname}/tracing.spec.ts`, line: 100, column: 10 } });
-        await page.locator('button >> nth=0').click();
-        await context.tracing.groupEnd();
-        await context.tracing.group(secondInnerGroup, { location: { file: __filename } });
-        await expect(page.getByText('Click')).toBeVisible();
-        await context.tracing.groupEnd();
-        await context.tracing.groupEnd();
-        await context.tracing.group(expandedFailure);
-        try {
-          await expect(page.getByText('Click')).toBeHidden({ timeout: 1 });
-        } catch (e) {}
-        await context.tracing.groupEnd();
-        await page.evaluate(() => console.log('ungrouped'), null);
-      } catch (e) {}
-    });
-  }, { box: true });
-  const treeViewEntries = traceViewer.actionsTree.locator('.tree-view-entry');
-
-  await test.step('check automatic expansion of groups on failure', async () => {
-    await expect(traceViewer.actionTitles).toHaveText([
-      /page.gotodata:text\/html,<!DOCTYPE html><html>Hello world<\/html>/,
-      /page.setContent/,
-      outerGroup,
-      expandedFailure,
-      /expect.toBeHiddengetByText\('Click'\)/,
-      /page.evaluate/,
-    ]);
-    await expect(traceViewer.actionsTree.locator('.tree-view-entry.selected > .tree-view-indent')).toHaveCount(1);
-    await expect(traceViewer.actionsTree.locator('.tree-view-entry.selected')).toHaveText(/expect.toBeHiddengetByText\('Click'\)/);
-    await treeViewEntries.filter({ hasText: expandedFailure }).locator('.codicon-chevron-down').click();
-  });
-  await test.step('check outer group', async () => {
-    await treeViewEntries.filter({ hasText: outerGroup }).locator('.codicon-chevron-right').click();
-    await expect(traceViewer.actionTitles).toHaveText([
-      /page.gotodata:text\/html,<!DOCTYPE html><html>Hello world<\/html>/,
-      /page.setContent/,
-      outerGroup,
-      outerGroupContent,
-      firstInnerGroup,
-      secondInnerGroup,
-      expandedFailure,
-      /page.evaluate/,
-    ]);
-    await expect(treeViewEntries.filter({ hasText: firstInnerGroup }).locator(' > .tree-view-indent')).toHaveCount(1);
-    await expect(treeViewEntries.filter({ hasText: secondInnerGroup }).locator(' > .tree-view-indent')).toHaveCount(1);
-    await test.step('check automatic location of groups', async () => {
-      await traceViewer.showSourceTab();
-      await traceViewer.selectAction(outerGroup);
-      await expect(traceViewer.sourceCodeTab.locator('.source-tab-file-name')).toHaveAttribute('title', __filename);
-      await expect(traceViewer.sourceCodeTab.locator('.source-line-running')).toHaveText(/\d+\s+await context.tracing.group\(outerGroup\);  \/\/ Outer group/);
+      await context.tracing.group('outer group');
+      await page.goto(`data:text/html,<!DOCTYPE html><body><div>Hello world</div></body>`);
+      await context.tracing.group('inner group 1', { location: { file: __filename, line: 17, column: 1 } });
+      await page.locator('body').click();
+      await context.tracing.groupEnd();
+      await context.tracing.group('inner group 2');
+      await expect(page.getByText('Hello')).toBeVisible();
+      await context.tracing.groupEnd();
+      await context.tracing.groupEnd();
     });
   });
-  await test.step('check inner groups', async () => {
-    await treeViewEntries.filter({ hasText: firstInnerGroup }).locator('.codicon-chevron-right').click();
-    await treeViewEntries.filter({ hasText: secondInnerGroup }).locator('.codicon-chevron-right').click();
-    await expect(traceViewer.actionTitles).toHaveText([
-      /page.gotodata:text\/html,<!DOCTYPE html><html>Hello world<\/html>/,
-      /page.setContent/,
-      outerGroup,
-      outerGroupContent,
-      firstInnerGroup,
-      firstInnerGroupContent,
-      secondInnerGroup,
-      secondInnerGroupContent,
-      expandedFailure,
-      /page.evaluate/,
-    ]);
-    await expect(treeViewEntries.filter({ hasText: firstInnerGroupContent }).locator(' > .tree-view-indent')).toHaveCount(2);
-    await expect(treeViewEntries.filter({ hasText: secondInnerGroupContent }).locator(' > .tree-view-indent')).toHaveCount(2);
-    await test.step('check location with file, line, column', async () => {
-      await traceViewer.selectAction(firstInnerGroup);
-      await expect(traceViewer.sourceCodeTab.locator('.source-tab-file-name')).toHaveAttribute('title', `${__dirname}/tracing.spec.ts`);
-    });
-    await test.step('check location with file', async () => {
-      await traceViewer.selectAction(secondInnerGroup);
-      await expect(traceViewer.sourceCodeTab.getByText(/Licensed under the Apache License/)).toBeVisible();
-    });
-  });
-});
 
+  await expect(traceViewer.actionTitles).toHaveText([
+    /outer group/,
+    /page.goto/,
+    /inner group 1/,
+    /inner group 2/,
+    /expect.toBeVisible/,
+  ]);
+
+  await traceViewer.selectAction('inner group 1');
+  await traceViewer.expandAction('inner group 1');
+  await expect(traceViewer.actionTitles).toHaveText([
+    /outer group/,
+    /page.goto/,
+    /inner group 1/,
+    /locator.click/,
+    /inner group 2/,
+  ]);
+  await traceViewer.showSourceTab();
+  await expect(traceViewer.sourceCodeTab.locator('.source-line-running')).toHaveText(/DO NOT TOUCH THIS LINE/);
+
+  await traceViewer.selectAction('inner group 2');
+  await expect(traceViewer.sourceCodeTab.locator('.source-line-running')).toContainText("await context.tracing.group('inner group 2');");
+});
 
 test('should open simple trace viewer', async ({ showTraceViewer }) => {
   const traceViewer = await showTraceViewer([traceFile]);