diff --git a/packages/playwright/src/isomorphic/util.ts b/packages/playwright/src/isomorphic/util.ts new file mode 100644 index 0000000000..208b5f0cd1 --- /dev/null +++ b/packages/playwright/src/isomorphic/util.ts @@ -0,0 +1,17 @@ +/** + * Copyright (c) Microsoft Corporation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export const kTopLevelAttachmentPrefix = '_attach'; diff --git a/packages/playwright/src/worker/DEPS.list b/packages/playwright/src/worker/DEPS.list index ed3973d1fc..1532b16c9f 100644 --- a/packages/playwright/src/worker/DEPS.list +++ b/packages/playwright/src/worker/DEPS.list @@ -4,3 +4,4 @@ ../util.ts ../utilBundle.ts ../matchers/** +../isomorphic/util.ts diff --git a/packages/playwright/src/worker/testInfo.ts b/packages/playwright/src/worker/testInfo.ts index ba0fb789ea..a5f64e68bf 100644 --- a/packages/playwright/src/worker/testInfo.ts +++ b/packages/playwright/src/worker/testInfo.ts @@ -414,14 +414,10 @@ export class TestInfoImpl implements TestInfo { _attach(attachment: TestInfo['attachments'][0], stepId: string | undefined) { const index = this._attachmentsPush(attachment) - 1; - if (stepId) { + if (stepId) this._stepMap.get(stepId)!.attachmentIndices.push(index); - } else { - // trace viewer has no means of representing attachments outside of a step, so we create an artificial action - const callId = `attach@${++this._lastStepId}`; - this._tracing.appendBeforeActionForStep(callId, this._findLastPredefinedStep(this._steps)?.stepId, 'attach', `attach "${attachment.name}"`, undefined, []); - this._tracing.appendAfterActionForStep(callId, undefined, [attachment]); - } + else + this._tracing.appendTopLevelAttachment(attachment); this._onAttach({ testId: this.testId, diff --git a/packages/playwright/src/worker/testTracing.ts b/packages/playwright/src/worker/testTracing.ts index 4b4390baa3..6ef860e1a6 100644 --- a/packages/playwright/src/worker/testTracing.ts +++ b/packages/playwright/src/worker/testTracing.ts @@ -20,6 +20,7 @@ import path from 'path'; import { ManualPromise, SerializedFS, calculateSha1, createGuid, monotonicTime } from 'playwright-core/lib/utils'; import { yauzl, yazl } from 'playwright-core/lib/zipBundle'; +import { kTopLevelAttachmentPrefix } from '../isomorphic/util'; import { filteredStackTrace } from '../util'; import type { TestInfoImpl } from './testInfo'; @@ -47,6 +48,7 @@ export class TestTracing { private _tracesDir: string; private _contextCreatedEvent: trace.ContextCreatedTraceEvent; private _didFinishTestFunctionAndAfterEachHooks = false; + private _lastActionId = 0; constructor(testInfo: TestInfoImpl, artifactsDir: string) { this._testInfo = testInfo; @@ -291,6 +293,16 @@ export class TestTracing { }); } + appendTopLevelAttachment(attachment: Attachment) { + // trace viewer has no means of representing attachments outside of a step, + // so we create an artificial action that's hidden in the UI + // the alternative would be to have one hidden step at the end with all top-level attachments, + // but that would delay useful information in live traces. + const callId = `${kTopLevelAttachmentPrefix}@${++this._lastActionId}`; + this.appendBeforeActionForStep(callId, undefined, kTopLevelAttachmentPrefix, `${kTopLevelAttachmentPrefix} "${attachment.name}"`, undefined, []); + this.appendAfterActionForStep(callId, undefined, [attachment]); + } + private _appendTraceEvent(event: trace.TraceEvent) { this._traceEvents.push(event); if (this._liveTraceFile) diff --git a/packages/trace-viewer/src/ui/modelUtil.ts b/packages/trace-viewer/src/ui/modelUtil.ts index 7c15bb5edd..4300733ec1 100644 --- a/packages/trace-viewer/src/ui/modelUtil.ts +++ b/packages/trace-viewer/src/ui/modelUtil.ts @@ -20,6 +20,7 @@ import type * as trace from '@trace/trace'; import type { ActionTraceEvent } from '@trace/trace'; import type { ActionEntry, ContextEntry, PageEntry } from '../types/entries'; import type { StackFrame } from '@protocol/channels'; +import { kTopLevelAttachmentPrefix } from '@testIsomorphic/util'; const contextSymbol = Symbol('context'); const nextInContextSymbol = Symbol('next'); @@ -338,6 +339,8 @@ export function buildActionTree(actions: ActionTraceEventInContext[]): { rootIte const rootItem: ActionTreeItem = { id: '', parent: undefined, children: [] }; for (const item of itemMap.values()) { + if (item.action?.apiName.startsWith(kTopLevelAttachmentPrefix)) + continue; const parent = item.action!.parentId ? itemMap.get(item.action!.parentId) || rootItem : rootItem; parent.children.push(item); item.parent = parent; diff --git a/tests/playwright-test/playwright.connect.spec.ts b/tests/playwright-test/playwright.connect.spec.ts index aee39ad244..e9d29dfe0e 100644 --- a/tests/playwright-test/playwright.connect.spec.ts +++ b/tests/playwright-test/playwright.connect.spec.ts @@ -223,7 +223,7 @@ test('should record trace', async ({ runInlineTest }) => { 'After Hooks', 'fixture: page', 'fixture: context', - 'attach "_prompt-0"', + '_attach "_prompt-0"', 'Worker Cleanup', 'fixture: browser', ]); diff --git a/tests/playwright-test/playwright.trace.spec.ts b/tests/playwright-test/playwright.trace.spec.ts index 30cb524aa9..28a7150c8e 100644 --- a/tests/playwright-test/playwright.trace.spec.ts +++ b/tests/playwright-test/playwright.trace.spec.ts @@ -129,7 +129,6 @@ test('should record api trace', async ({ runInlineTest, server }, testInfo) => { ' fixture: context', ' fixture: request', ' apiRequestContext.dispose', - ' attach "_prompt-0"', 'Worker Cleanup', ' fixture: browser', ]); @@ -328,7 +327,6 @@ test('should not override trace file in afterAll', async ({ runInlineTest, serve 'After Hooks', ' fixture: page', ' fixture: context', - ' attach "_prompt-0"', ' afterAll hook', ' fixture: request', ' apiRequest.newContext', @@ -671,7 +669,6 @@ test('should show non-expect error in trace', async ({ runInlineTest }, testInfo 'After Hooks', ' fixture: page', ' fixture: context', - ' attach "_prompt-0"', 'Worker Cleanup', ' fixture: browser', ]); @@ -734,7 +731,7 @@ test('should not throw when attachment is missing', async ({ runInlineTest }, te expect(result.exitCode).toBe(0); expect(result.passed).toBe(1); const trace = await parseTrace(testInfo.outputPath('test-results', 'a-passes', 'trace.zip')); - expect(trace.actionTree).toContain('attach "screenshot"'); + expect(trace.apiNames).toContain('_attach "screenshot"'); }); test('should not throw when screenshot on failure fails', async ({ runInlineTest, server }, testInfo) => { @@ -983,7 +980,6 @@ test('should record nested steps, even after timeout', async ({ runInlineTest }, ' page.setContent', ' fixture: page', ' fixture: context', - ' attach "_prompt-0"', ' afterAll hook', ' fixture: barPage', ' barPage setup', @@ -1043,7 +1039,6 @@ test('should attribute worker fixture teardown to the right test', async ({ runI expect(trace2.actionTree).toEqual([ 'Before Hooks', 'After Hooks', - ' attach "_prompt-0"', 'Worker Cleanup', ' fixture: foo', ' step in foo teardown', @@ -1153,7 +1148,6 @@ test('should not corrupt actions when no library trace is present', async ({ run 'After Hooks', ' fixture: foo', ' expect.toBe', - ' attach "_prompt-0"', 'Worker Cleanup', ]); }); @@ -1184,7 +1178,6 @@ test('should record trace for manually created context in a failed test', async 'page.setContent', 'expect.toBe', 'After Hooks', - ' attach "_prompt-0"', 'Worker Cleanup', ' fixture: browser', ]); @@ -1272,7 +1265,6 @@ test('should record trace after fixture teardown timeout', { 'page.evaluate', 'After Hooks', ' fixture: fixture', - ' attach "_prompt-0"', 'Worker Cleanup', ' fixture: browser', ]); diff --git a/tests/playwright-test/ui-mode-trace.spec.ts b/tests/playwright-test/ui-mode-trace.spec.ts index 47662adf97..090e39ecfe 100644 --- a/tests/playwright-test/ui-mode-trace.spec.ts +++ b/tests/playwright-test/ui-mode-trace.spec.ts @@ -458,7 +458,6 @@ test('attachments tab shows all but top-level .push attachments', async ({ runUI - treeitem /step/: - group: - treeitem /attach \\"foo-attach\\"/ - - treeitem /attach \\"bar-push\\"/ - treeitem /attach \\"bar-attach\\"/ `); await page.getByRole('tab', { name: 'Attachments' }).click(); diff --git a/tests/tsconfig.json b/tests/tsconfig.json index 1a1e3d7527..a921b85a09 100644 --- a/tests/tsconfig.json +++ b/tests/tsconfig.json @@ -12,6 +12,7 @@ "baseUrl": "..", "paths": { "@isomorphic/*": ["packages/playwright-core/src/utils/isomorphic/*"], + "@testIsomorphic/*": ["packages/playwright/src/isomorphic/*"], "@protocol/*": ["packages/protocol/src/*"], "@recorder/*": ["packages/recorder/src/*"], "@trace/*": ["packages/trace/src/*"],