chore: followup to tracing.group (#33450)

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.
+:::caution
-However, groups are only visualized in the trace viewer and, unlike test.step, have no effect on the test reports.
+When using Playwright test runner, we strongly recommend `test.step` instead.
-
-:::note Groups should not be used with Playwright Test!
-
-This API is intended for Playwright API users that can not use `test.step`.
 :::
 
 **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

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

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
+   * Creates a new group within the trace, assigning any subsequent API calls to this group, until
-   * [method: Tracing.groupEnd] is invoked.
+   * [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
+   * **NOTE** When using Playwright test runner, we strongly recommend `test.step` instead.
-   * 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`.
    *
    * **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>;
 

packages/playwright/src/index.ts

@@ -259,13 +259,10 @@ 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'))
+        if (!testInfo || apiName.includes('setTestIdAttribute') || apiName === 'tracing.groupEnd')
-          return { userObject: null };
+          return;
-        if (apiName === 'tracing.groupEnd') {
-          tracingGroupSteps.pop();
-          return { userObject: null };
-        }
         const step = testInfo._addStep({
           location: frames[0] as any,
           category: 'pw:api',
@@ -273,13 +270,21 @@ const playwrightFixtures: Fixtures<TestFixtures, WorkerFixtures> = ({
           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) {

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();

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,106 +106,46 @@ 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 }) => {
+test('should show tracing.group in the action list with location', 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';
-
   const traceViewer = await test.step('create trace with groups', async () => {
+    await page.context().tracing.group('ignored group');
     return await runAndTrace(async () => {
-      try {
+      await context.tracing.group('outer group');
-        await page.goto(`data:text/html,<!DOCTYPE html><html>Hello world</html>`);
+      await page.goto(`data:text/html,<!DOCTYPE html><body><div>Hello world</div></body>`);
-        await page.setContent('<!DOCTYPE html><button>Click</button>');
+      await context.tracing.group('inner group 1', { location: { file: __filename, line: 17, column: 1 } });
-        async function doClick() {
+      await page.locator('body').click();
-          await page.getByText('Click').click();
+      await context.tracing.groupEnd();
-        }
+      await context.tracing.group('inner group 2');
-        await context.tracing.group(outerGroup);  // Outer group
+      await expect(page.getByText('Hello')).toBeVisible();
-        await doClick();
+      await context.tracing.groupEnd();
-        await context.tracing.group(firstInnerGroup, { location: { file: `${__dirname}/tracing.spec.ts`, line: 100, column: 10 } });
+      await context.tracing.groupEnd();
-        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([
-    await expect(traceViewer.actionTitles).toHaveText([
+    /outer group/,
-      /page.gotodata:text\/html,<!DOCTYPE html><html>Hello world<\/html>/,
+    /page.goto/,
-      /page.setContent/,
+    /inner group 1/,
-      outerGroup,
+    /inner group 2/,
-      expandedFailure,
+    /expect.toBeVisible/,
-      /expect.toBeHiddengetByText\('Click'\)/,
+  ]);
-      /page.evaluate/,
+
-    ]);
+  await traceViewer.selectAction('inner group 1');
-    await expect(traceViewer.actionsTree.locator('.tree-view-entry.selected > .tree-view-indent')).toHaveCount(1);
+  await traceViewer.expandAction('inner group 1');
-    await expect(traceViewer.actionsTree.locator('.tree-view-entry.selected')).toHaveText(/expect.toBeHiddengetByText\('Click'\)/);
+  await expect(traceViewer.actionTitles).toHaveText([
-    await treeViewEntries.filter({ hasText: expandedFailure }).locator('.codicon-chevron-down').click();
+    /outer group/,
-  });
+    /page.goto/,
-  await test.step('check outer group', async () => {
+    /inner group 1/,
-    await treeViewEntries.filter({ hasText: outerGroup }).locator('.codicon-chevron-right').click();
+    /locator.click/,
-    await expect(traceViewer.actionTitles).toHaveText([
+    /inner group 2/,
-      /page.gotodata:text\/html,<!DOCTYPE html><html>Hello world<\/html>/,
+  ]);
-      /page.setContent/,
+  await traceViewer.showSourceTab();
-      outerGroup,
+  await expect(traceViewer.sourceCodeTab.locator('.source-line-running')).toHaveText(/DO NOT TOUCH THIS LINE/);
-      outerGroupContent,
+
-      firstInnerGroup,
+  await traceViewer.selectAction('inner group 2');
-      secondInnerGroup,
+  await expect(traceViewer.sourceCodeTab.locator('.source-line-running')).toContainText("await context.tracing.group('inner group 2');");
-      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 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();
-    });
-  });
 });
 
-
 test('should open simple trace viewer', async ({ showTraceViewer }) => {
   const traceViewer = await showTraceViewer([traceFile]);
   await expect(traceViewer.actionTitles).toHaveText([

tests/library/tracing.spec.ts

@@ -107,6 +107,28 @@ test('should not collect snapshots by default', async ({ context, page, server }
   expect(events.some(e => e.type === 'resource-snapshot')).toBeFalsy();
 });
 
+test('can call tracing.group/groupEnd at any time and auto-close', async ({ context, page, server }, testInfo) => {
+  await context.tracing.group('ignored');
+  await context.tracing.groupEnd();
+  await context.tracing.group('ignored2');
+
+  await context.tracing.start();
+  await context.tracing.group('actual');
+  await page.goto(server.EMPTY_PAGE);
+  await context.tracing.stopChunk({ path: testInfo.outputPath('trace.zip') });
+
+  await context.tracing.group('ignored3');
+  await context.tracing.groupEnd();
+  await context.tracing.groupEnd();
+  await context.tracing.groupEnd();
+
+  const { events } = await parseTraceRaw(testInfo.outputPath('trace.zip'));
+  const groups = events.filter(e => e.method === 'tracingGroup');
+  expect(groups).toHaveLength(1);
+  expect(groups[0].apiName).toBe('actual');
+  expect(events.some(e => e.type === 'after' && e.callId === groups[0].callId)).toBe(true);
+});
+
 test('should not include buffers in the trace', async ({ context, page, server }, testInfo) => {
   await context.tracing.start({ snapshots: true });
   await page.goto(server.PREFIX + '/empty.html');

tests/playwright-test/test-step.spec.ts

@@ -1267,3 +1267,43 @@ hook      |After Hooks
   });
   expect(exitCode).toBe(0);
 });
+
+test('should show tracing.group nested inside test.step', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'reporter.ts': stepIndentReporter,
+    'playwright.config.ts': `module.exports = { reporter: [['./reporter', { printErrorLocation: true }]] };`,
+    'a.test.ts': `
+      import { test, expect } from '@playwright/test';
+      test('pass', async ({ page }) => {
+        await test.step('my step 1', async () => {
+          await test.step('my step 2', async () => {
+            await page.context().tracing.group('my group 1');
+            await page.context().tracing.group('my group 2');
+            await page.setContent('<button></button>');
+            await page.context().tracing.groupEnd();
+            await page.context().tracing.groupEnd();
+          });
+        });
+      });
+    `
+  }, { reporter: '', workers: 1 });
+
+  expect(result.exitCode).toBe(0);
+  expect(stripAnsi(result.output)).toBe(`
+hook      |Before Hooks
+fixture   |  fixture: browser
+pw:api    |    browserType.launch
+fixture   |  fixture: context
+pw:api    |    browser.newContext
+fixture   |  fixture: page
+pw:api    |    browserContext.newPage
+test.step |my step 1 @ a.test.ts:4
+test.step |  my step 2 @ a.test.ts:5
+pw:api    |    my group 1 @ a.test.ts:6
+pw:api    |      my group 2 @ a.test.ts:7
+pw:api    |        page.setContent @ a.test.ts:8
+hook      |After Hooks
+fixture   |  fixture: page
+fixture   |  fixture: context
+`);
+});