From 126db4d7af60f8d4b57d1a79ebaa5df2c1c298d5 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Tue, 5 Nov 2024 11:41:26 +0000 Subject: [PATCH] chore: followup to tracing.group --- docs/src/api/class-tracing.md | 48 +++++-- .../src/server/trace/recorder/tracing.ts | 10 +- packages/playwright-core/types/types.d.ts | 17 ++- packages/playwright/src/index.ts | 23 +-- tests/config/traceViewerFixtures.ts | 5 + tests/library/trace-viewer.spec.ts | 131 +++++------------- tests/library/tracing.spec.ts | 22 +++ tests/playwright-test/test-step.spec.ts | 40 ++++++ 8 files changed, 170 insertions(+), 126 deletions(-) diff --git a/docs/src/api/class-tracing.md b/docs/src/api/class-tracing.md index 353a220516698..4dd40d95b2ee2 100644 --- 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 index 3f278008e2742..c19c0a33d9918 100644 --- 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 { + 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 index d67c2ca8abc6d..4ebfafa37d536 100644 --- 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; /** - * 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; diff --git a/packages/playwright/src/index.ts b/packages/playwright/src/index.ts index efade476a8dae..9364e6b87997f 100644 --- a/packages/playwright/src/index.ts +++ b/packages/playwright/src/index.ts @@ -259,13 +259,10 @@ const playwrightFixtures: Fixtures = ({ const tracingGroupSteps: TestStepInternal[] = []; const csiListener: ClientInstrumentationListener = { onApiCallBegin: (apiName: string, params: Record, 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', @@ -273,13 +270,21 @@ const playwrightFixtures: Fixtures = ({ 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 index 837b953ef4f28..40737f6bc1d51 100644 --- 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 index b8a8602313f3f..0c8ad1aafd823 100644 --- 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,Hello world`); - await page.setContent(''); - 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,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,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,
Hello world
`); + 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,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]); diff --git a/tests/library/tracing.spec.ts b/tests/library/tracing.spec.ts index b13ed1b84aa40..0b3671b575706 100644 --- a/tests/library/tracing.spec.ts +++ b/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'); diff --git a/tests/playwright-test/test-step.spec.ts b/tests/playwright-test/test-step.spec.ts index cbd4ec38c4026..13f0c4717bebd 100644 --- a/tests/playwright-test/test-step.spec.ts +++ b/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(''); + 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 +`); +});