From 9727bc1e2ab68ba8d3509a3d242b8069abaee214 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Fri, 13 Dec 2024 10:37:27 +0000 Subject: [PATCH] fix(trace): duplicate network requests from beforeAll in serial mode --- .../playwright-core/src/client/tracing.ts | 8 +++- .../playwright-core/src/protocol/validator.ts | 1 + .../src/server/trace/recorder/tracing.ts | 5 ++- packages/playwright/src/index.ts | 5 ++- packages/playwright/src/worker/testTracing.ts | 6 +++ packages/protocol/src/channels.ts | 2 + packages/protocol/src/protocol.yml | 1 + tests/config/utils.ts | 4 +- .../playwright-test/playwright.trace.spec.ts | 39 +++++++++++++++++++ 9 files changed, 66 insertions(+), 5 deletions(-) diff --git a/packages/playwright-core/src/client/tracing.ts b/packages/playwright-core/src/client/tracing.ts index 2481741e3e..1bfabf86ff 100644 --- a/packages/playwright-core/src/client/tracing.ts +++ b/packages/playwright-core/src/client/tracing.ts @@ -46,8 +46,12 @@ export class Tracing extends ChannelOwner implements ap await this._startCollectingStacks(traceName); } - async startChunk(options: { name?: string, title?: string } = {}) { - const { traceName } = await this._channel.tracingStartChunk(options); + async startChunk(options: { name?: string, title?: string, _resetNetwork?: boolean } = {}) { + const { traceName } = await this._channel.tracingStartChunk({ + name: options.name, + title: options.title, + resetNetwork: options._resetNetwork, + }); await this._startCollectingStacks(traceName); } diff --git a/packages/playwright-core/src/protocol/validator.ts b/packages/playwright-core/src/protocol/validator.ts index 7fe92a7947..d2bd37d32a 100644 --- a/packages/playwright-core/src/protocol/validator.ts +++ b/packages/playwright-core/src/protocol/validator.ts @@ -2294,6 +2294,7 @@ scheme.TracingTracingStartResult = tOptional(tObject({})); scheme.TracingTracingStartChunkParams = tObject({ name: tOptional(tString), title: tOptional(tString), + resetNetwork: tOptional(tBoolean), }); scheme.TracingTracingStartChunkResult = tObject({ traceName: tString, diff --git a/packages/playwright-core/src/server/trace/recorder/tracing.ts b/packages/playwright-core/src/server/trace/recorder/tracing.ts index c19c0a33d9..aedce42686 100644 --- a/packages/playwright-core/src/server/trace/recorder/tracing.ts +++ b/packages/playwright-core/src/server/trace/recorder/tracing.ts @@ -158,7 +158,7 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps this._harTracer.start({ omitScripts: !options.live }); } - async startChunk(options: { name?: string, title?: string } = {}): Promise<{ traceName: string }> { + async startChunk(options: { name?: string, title?: string, resetNetwork?: boolean } = {}): Promise<{ traceName: string }> { if (this._state && this._state.recording) await this.stopChunk({ mode: 'discard' }); @@ -184,6 +184,9 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps }; this._appendTraceEvent(event); + if (options.resetNetwork) + this._fs.writeFile(this._state.networkFile, ''); + this._context.instrumentation.addListener(this, this._context); this._eventListeners.push( eventsHelper.addEventListener(this._context, BrowserContext.Events.Console, this._onConsoleMessage.bind(this)), diff --git a/packages/playwright/src/index.ts b/packages/playwright/src/index.ts index 27f1e9320b..5be6c772e3 100644 --- a/packages/playwright/src/index.ts +++ b/packages/playwright/src/index.ts @@ -685,14 +685,17 @@ class ArtifactsRecorder { private async _startTraceChunkOnContextCreation(tracing: Tracing) { const options = this._testInfo._tracing.traceOptions(); + const seenInThisTestSymbol = this._testInfo._tracing.uniqueSymbol(); if (options) { + const seenInThisTest = !!(tracing as any)[seenInThisTestSymbol]; + (tracing as any)[seenInThisTestSymbol] = true; const title = this._testInfo._tracing.traceTitle(); const name = this._testInfo._tracing.generateNextTraceRecordingName(); if (!(tracing as any)[kTracingStarted]) { await tracing.start({ ...options, title, name }); (tracing as any)[kTracingStarted] = true; } else { - await tracing.startChunk({ title, name }); + await tracing.startChunk({ title, name, _resetNetwork: seenInThisTest } as any); } } else { if ((tracing as any)[kTracingStarted]) { diff --git a/packages/playwright/src/worker/testTracing.ts b/packages/playwright/src/worker/testTracing.ts index eb0ce9d807..54c7e09249 100644 --- a/packages/playwright/src/worker/testTracing.ts +++ b/packages/playwright/src/worker/testTracing.ts @@ -43,6 +43,7 @@ export class TestTracing { private _artifactsDir: string; private _tracesDir: string; private _contextCreatedEvent: trace.ContextCreatedTraceEvent; + private _uniqueSymbol: symbol; constructor(testInfo: TestInfoImpl, artifactsDir: string) { this._testInfo = testInfo; @@ -59,6 +60,7 @@ export class TestTracing { monotonicTime: monotonicTime(), sdkLanguage: 'javascript', }; + this._uniqueSymbol = Symbol('unique'); this._appendTraceEvent(this._contextCreatedEvent); } @@ -140,6 +142,10 @@ export class TestTracing { return this._options; } + uniqueSymbol() { + return this._uniqueSymbol; + } + async stopIfNeeded() { if (!this._options) return; diff --git a/packages/protocol/src/channels.ts b/packages/protocol/src/channels.ts index 27c4242fe5..539945d014 100644 --- a/packages/protocol/src/channels.ts +++ b/packages/protocol/src/channels.ts @@ -4109,10 +4109,12 @@ export type TracingTracingStartResult = void; export type TracingTracingStartChunkParams = { name?: string, title?: string, + resetNetwork?: boolean, }; export type TracingTracingStartChunkOptions = { name?: string, title?: string, + resetNetwork?: boolean, }; export type TracingTracingStartChunkResult = { traceName: string, diff --git a/packages/protocol/src/protocol.yml b/packages/protocol/src/protocol.yml index d9597c2295..a87dd1f17b 100644 --- a/packages/protocol/src/protocol.yml +++ b/packages/protocol/src/protocol.yml @@ -3196,6 +3196,7 @@ Tracing: parameters: name: string? title: string? + resetNetwork: boolean? returns: traceName: string diff --git a/tests/config/utils.ts b/tests/config/utils.ts index 3743e97d80..deeb8ade1d 100644 --- a/tests/config/utils.ts +++ b/tests/config/utils.ts @@ -23,6 +23,7 @@ import { TraceModel } from '../../packages/trace-viewer/src/sw/traceModel'; import type { ActionTreeItem } from '../../packages/trace-viewer/src/ui/modelUtil'; import { buildActionTree, MultiTraceModel } from '../../packages/trace-viewer/src/ui/modelUtil'; import type { ActionTraceEvent, ConsoleMessageTraceEvent, EventTraceEvent, TraceEvent } from '@trace/trace'; +import type { ResourceSnapshot } from '@trace/snapshot'; import style from 'ansi-styles'; export async function attachFrame(page: Page, frameId: string, url: string): Promise { @@ -157,7 +158,7 @@ export async function parseTraceRaw(file: string): Promise<{ events: any[], reso }; } -export async function parseTrace(file: string): Promise<{ resources: Map, events: (EventTraceEvent | ConsoleMessageTraceEvent)[], actions: ActionTraceEvent[], apiNames: string[], traceModel: TraceModel, model: MultiTraceModel, actionTree: string[], errors: string[] }> { +export async function parseTrace(file: string): Promise<{ resources: Map, events: (EventTraceEvent | ConsoleMessageTraceEvent)[], actions: ActionTraceEvent[], apiNames: string[], traceModel: TraceModel, model: MultiTraceModel, actionTree: string[], errors: string[], network: ResourceSnapshot[] }> { const backend = new TraceBackend(file); const traceModel = new TraceModel(); await traceModel.load(backend, () => {}); @@ -179,6 +180,7 @@ export async function parseTrace(file: string): Promise<{ resources: Map { + const result = await runInlineTest({ + 'a.spec.ts': ` + import { test, expect } from '@playwright/test'; + let shared; + + test.beforeAll(async ({ browser }) => { + shared = await browser.newPage(); + await shared.route('**/*', route => route.fulfill({ body: 'hello' })); + await shared.goto('https://playwright.dev/'); + }); + + test('pass1', async ({ page }) => { + await page.route('**/*', route => route.fulfill({ body: 'hello' })); + await page.goto('https://playwright1.dev/'); + }); + + test('pass2', async ({ page }) => { + await page.route('**/*', route => route.fulfill({ body: 'hello' })); + await page.goto('https://playwright2.dev/'); + }); + + test.afterAll(async ({}) => { + await shared.close(); + }); + `, + }, { trace: 'on' }); + expect(result.exitCode).toBe(0); + expect(result.passed).toBe(2); + + const trace1 = await parseTrace(test.info().outputPath('test-results', 'a-pass1', 'trace.zip')); + expect(trace1.network.map(r => r.request.url).sort()).toEqual(['https://playwright.dev/', 'https://playwright1.dev/']); + + const trace2 = await parseTrace(test.info().outputPath('test-results', 'a-pass2', 'trace.zip')); + expect(trace2.network.map(r => r.request.url).sort()).toEqual(['https://playwright.dev/', 'https://playwright2.dev/']); +});