diff --git a/packages/playwright-core/src/client/tracing.ts b/packages/playwright-core/src/client/tracing.ts index 502a8caf09..d4011513ff 100644 --- a/packages/playwright-core/src/client/tracing.ts +++ b/packages/playwright-core/src/client/tracing.ts @@ -14,32 +14,20 @@ * limitations under the License. */ +import fs from 'fs'; import * as api from '../../types/types'; import * as channels from '../protocol/channels'; -import { ParsedStackTrace } from '../utils/stackTrace'; -import { calculateSha1 } from '../utils/utils'; import { Artifact } from './artifact'; import { BrowserContext } from './browserContext'; -import { ClientInstrumentationListener } from './clientInstrumentation'; export class Tracing implements api.Tracing { private _context: BrowserContext; - private _sources = new Set(); - private _instrumentationListener: ClientInstrumentationListener; constructor(channel: BrowserContext) { this._context = channel; - this._instrumentationListener = { - onApiCallBegin: (apiCall: string, stackTrace: ParsedStackTrace | null) => { - for (const frame of stackTrace?.frames || []) - this._sources.add(frame.file); - } - }; } async start(options: { name?: string, title?: string, snapshots?: boolean, screenshots?: boolean, sources?: boolean } = {}) { - if (options.sources) - this._context._instrumentation!.addListener(this._instrumentationListener); await this._context._wrapApiCall(async () => { await this._context._channel.tracingStart(options); await this._context._channel.tracingStartChunk({ title: options.title }); @@ -47,7 +35,6 @@ export class Tracing implements api.Tracing { } async startChunk(options: { title?: string } = {}) { - this._sources = new Set(); await this._context._channel.tracingStartChunk(options); } @@ -63,9 +50,6 @@ export class Tracing implements api.Tracing { } private async _doStopChunk(channel: channels.BrowserContextChannel, filePath: string | undefined) { - const sources = this._sources; - this._sources = new Set(); - this._context._instrumentation!.removeListener(this._instrumentationListener); const isLocal = !this._context._connection.isRemote(); const result = await channel.tracingStopChunk({ save: !!filePath, skipCompress: isLocal }); @@ -74,9 +58,8 @@ export class Tracing implements api.Tracing { return; } - const sourceEntries: channels.NameValue[] = []; - for (const value of sources) - sourceEntries.push({ name: 'resources/src@' + calculateSha1(value) + '.txt', value }); + if (filePath && fs.existsSync(filePath)) + await fs.promises.unlink(filePath); if (!isLocal) { // We run against remote Playwright, compress on remote side. @@ -85,7 +68,7 @@ export class Tracing implements api.Tracing { await artifact.delete(); } - if (isLocal || sourceEntries) - await this._context._localUtils.zip(filePath, sourceEntries.concat(result.entries)); + if (isLocal || result.sourceEntries.length) + await this._context._localUtils.zip(filePath, result.sourceEntries.concat(result.entries)); } } diff --git a/packages/playwright-core/src/dispatchers/browserContextDispatcher.ts b/packages/playwright-core/src/dispatchers/browserContextDispatcher.ts index babc02ea08..b5f03bfd3a 100644 --- a/packages/playwright-core/src/dispatchers/browserContextDispatcher.ts +++ b/packages/playwright-core/src/dispatchers/browserContextDispatcher.ts @@ -198,8 +198,8 @@ export class BrowserContextDispatcher extends Dispatcher { - const { artifact, entries } = await this._context.tracing.stopChunk(params.save, params.skipCompress); - return { artifact: artifact ? new ArtifactDispatcher(this._scope, artifact) : undefined, entries }; + const { artifact, entries, sourceEntries } = await this._context.tracing.stopChunk(params.save, params.skipCompress); + return { artifact: artifact ? new ArtifactDispatcher(this._scope, artifact) : undefined, entries, sourceEntries }; } async tracingStop(params: channels.BrowserContextTracingStopParams): Promise { diff --git a/packages/playwright-core/src/protocol/channels.ts b/packages/playwright-core/src/protocol/channels.ts index 5c8893a240..0d0ad13b29 100644 --- a/packages/playwright-core/src/protocol/channels.ts +++ b/packages/playwright-core/src/protocol/channels.ts @@ -1249,11 +1249,13 @@ export type BrowserContextTracingStartParams = { name?: string, snapshots?: boolean, screenshots?: boolean, + sources?: boolean, }; export type BrowserContextTracingStartOptions = { name?: string, snapshots?: boolean, screenshots?: boolean, + sources?: boolean, }; export type BrowserContextTracingStartResult = void; export type BrowserContextTracingStartChunkParams = { @@ -1273,6 +1275,7 @@ export type BrowserContextTracingStopChunkOptions = { export type BrowserContextTracingStopChunkResult = { artifact?: ArtifactChannel, entries: NameValue[], + sourceEntries: NameValue[], }; export type BrowserContextTracingStopParams = {}; export type BrowserContextTracingStopOptions = {}; diff --git a/packages/playwright-core/src/protocol/protocol.yml b/packages/playwright-core/src/protocol/protocol.yml index 3b6ebe3092..dbb91e0ef1 100644 --- a/packages/playwright-core/src/protocol/protocol.yml +++ b/packages/playwright-core/src/protocol/protocol.yml @@ -825,6 +825,7 @@ BrowserContext: name: string? snapshots: boolean? screenshots: boolean? + sources: boolean? tracingStartChunk: parameters: @@ -839,6 +840,9 @@ BrowserContext: entries: type: array items: NameValue + sourceEntries: + type: array + items: NameValue tracingStop: diff --git a/packages/playwright-core/src/protocol/validator.ts b/packages/playwright-core/src/protocol/validator.ts index 1c7b01d55f..a0ab0400bb 100644 --- a/packages/playwright-core/src/protocol/validator.ts +++ b/packages/playwright-core/src/protocol/validator.ts @@ -503,6 +503,7 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme { name: tOptional(tString), snapshots: tOptional(tBoolean), screenshots: tOptional(tBoolean), + sources: tOptional(tBoolean), }); scheme.BrowserContextTracingStartChunkParams = tObject({ title: tOptional(tString), diff --git a/packages/playwright-core/src/server/trace/recorder/tracing.ts b/packages/playwright-core/src/server/trace/recorder/tracing.ts index a3fdfd3a2b..b5303b8afe 100644 --- a/packages/playwright-core/src/server/trace/recorder/tracing.ts +++ b/packages/playwright-core/src/server/trace/recorder/tracing.ts @@ -14,31 +14,32 @@ * limitations under the License. */ +import { EventEmitter } from 'events'; import fs from 'fs'; import path from 'path'; import yazl from 'yazl'; -import { EventEmitter } from 'events'; -import { createGuid, mkdirIfNeeded, monotonicTime } from '../../../utils/utils'; +import { NameValue } from '../../../common/types'; +import { commandsWithTracingSnapshots } from '../../../protocol/channels'; +import { ManualPromise } from '../../../utils/async'; +import { eventsHelper, RegisteredListener } from '../../../utils/eventsHelper'; +import { calculateSha1, createGuid, mkdirIfNeeded, monotonicTime } from '../../../utils/utils'; import { Artifact } from '../../artifact'; import { BrowserContext } from '../../browserContext'; import { ElementHandle } from '../../dom'; -import { eventsHelper, RegisteredListener } from '../../../utils/eventsHelper'; import { CallMetadata, InstrumentationListener, SdkObject } from '../../instrumentation'; import { Page } from '../../page'; -import * as trace from '../common/traceEvents'; -import { commandsWithTracingSnapshots } from '../../../protocol/channels'; -import { Snapshotter, SnapshotterBlob, SnapshotterDelegate } from './snapshotter'; -import { FrameSnapshot } from '../common/snapshotTypes'; -import { HarTracer, HarTracerDelegate } from '../../supplements/har/harTracer'; import * as har from '../../supplements/har/har'; +import { HarTracer, HarTracerDelegate } from '../../supplements/har/harTracer'; +import { FrameSnapshot } from '../common/snapshotTypes'; +import * as trace from '../common/traceEvents'; import { VERSION } from '../common/traceEvents'; -import { NameValue } from '../../../common/types'; -import { ManualPromise } from '../../../utils/async'; +import { Snapshotter, SnapshotterBlob, SnapshotterDelegate } from './snapshotter'; export type TracerOptions = { name?: string; snapshots?: boolean; screenshots?: boolean; + sources?: boolean; }; type RecordingState = { @@ -47,7 +48,9 @@ type RecordingState = { networkFile: string, traceFile: string, filesCount: number, - sha1s: Set, + networkSha1s: Set, + traceSha1s: Set, + sources: Set, recording: boolean; }; @@ -102,8 +105,7 @@ export class Tracing implements InstrumentationListener, SnapshotterDelegate, Ha const traceName = options.name || createGuid(); const traceFile = path.join(this._tracesDir, traceName + '.trace'); const networkFile = path.join(this._tracesDir, traceName + '.network'); - this._state = { options, traceName, traceFile, networkFile, filesCount: 0, sha1s: new Set(), recording: false }; - + this._state = { options, traceName, traceFile, networkFile, filesCount: 0, traceSha1s: new Set(), networkSha1s: new Set(), sources: new Set(), recording: false }; this._writeChain = fs.promises.mkdir(this._resourcesDir, { recursive: true }).then(() => fs.promises.writeFile(networkFile, '')); if (options.snapshots) this._harTracer.start(); @@ -167,7 +169,7 @@ export class Tracing implements InstrumentationListener, SnapshotterDelegate, Ha await this._writeChain; } - async stopChunk(save: boolean, skipCompress: boolean): Promise<{ artifact: Artifact | null, entries: NameValue[] }> { + async stopChunk(save: boolean, skipCompress: boolean): Promise<{ artifact: Artifact | null, entries: NameValue[], sourceEntries: NameValue[] }> { if (this._isStopping) throw new Error(`Tracing is already stopping`); this._isStopping = true; @@ -176,7 +178,7 @@ export class Tracing implements InstrumentationListener, SnapshotterDelegate, Ha this._isStopping = false; if (save) throw new Error(`Must start tracing before stopping`); - return { artifact: null, entries: [] }; + return { artifact: null, entries: [], sourceEntries: [] }; } const state = this._state!; @@ -203,11 +205,8 @@ export class Tracing implements InstrumentationListener, SnapshotterDelegate, Ha // Chain the export operation against write operations, // so that neither trace files nor sha1s change during the export. return await this._appendTraceOperation(async () => { - this._isStopping = false; - state.recording = false; - if (!save) - return { artifact: null, entries: [] }; + return { artifact: null, entries: [], sourceEntries: [] }; // Har files a live, make a snapshot before returning the resulting entries. const networkFile = path.join(state.networkFile, '..', createGuid()); @@ -216,12 +215,22 @@ export class Tracing implements InstrumentationListener, SnapshotterDelegate, Ha const entries: NameValue[] = []; entries.push({ name: 'trace.trace', value: state.traceFile }); entries.push({ name: 'trace.network', value: networkFile }); - for (const sha1 of state.sha1s) + for (const sha1 of new Set([...state.traceSha1s, ...state.networkSha1s])) entries.push({ name: path.join('resources', sha1), value: path.join(this._resourcesDir, sha1) }); - const zipArtifact = skipCompress ? null : await this._exportZip(entries, state).catch(() => null); - return { artifact: zipArtifact, entries }; - }) || { artifact: null, entries: [] }; + const sourceEntries: NameValue[] = []; + for (const value of state.sources) + sourceEntries.push({ name: 'resources/src@' + calculateSha1(value) + '.txt', value }); + + const artifact = skipCompress ? null : await this._exportZip(entries, state).catch(() => null); + return { artifact, entries, sourceEntries }; + }).finally(() => { + // Only reset trace sha1s, network resources are preserved between chunks. + state.traceSha1s = new Set(); + state.sources = new Set(); + this._isStopping = false; + state.recording = false; + }) || { artifact: null, entries: [], sourceEntries: [] }; } private async _exportZip(entries: NameValue[], state: RecordingState): Promise { @@ -263,6 +272,10 @@ export class Tracing implements InstrumentationListener, SnapshotterDelegate, Ha metadata.afterSnapshot = `after@${metadata.id}`; const beforeSnapshot = this._captureSnapshot('before', sdkObject, metadata); this._pendingCalls.set(metadata.id, { sdkObject, metadata, beforeSnapshot }); + if (this._state?.options.sources) { + for (const frame of metadata.stack || []) + this._state.sources.add(frame.file); + } await beforeSnapshot; } @@ -302,7 +315,7 @@ export class Tracing implements InstrumentationListener, SnapshotterDelegate, Ha onEntryFinished(entry: har.Entry) { const event: trace.ResourceSnapshotTraceEvent = { type: 'resource-snapshot', snapshot: entry }; this._appendTraceOperation(async () => { - visitSha1s(event, this._state!.sha1s); + visitSha1s(event, this._state!.networkSha1s); await fs.promises.appendFile(this._state!.networkFile, JSON.stringify(event) + '\n'); }); } @@ -344,7 +357,7 @@ export class Tracing implements InstrumentationListener, SnapshotterDelegate, Ha private _appendTraceEvent(event: trace.TraceEvent) { this._appendTraceOperation(async () => { - visitSha1s(event, this._state!.sha1s); + visitSha1s(event, this._state!.traceSha1s); await fs.promises.appendFile(this._state!.traceFile, JSON.stringify(event) + '\n'); }); } diff --git a/tests/tracing.spec.ts b/tests/tracing.spec.ts index 5c2acf8d48..8833ec1a70 100644 --- a/tests/tracing.spec.ts +++ b/tests/tracing.spec.ts @@ -132,6 +132,63 @@ test('should collect two traces', async ({ context, page, server }, testInfo) => } }); +test('should not include trace resources from the provious chunks', async ({ context, page, server }, testInfo) => { + await context.tracing.start({ screenshots: true, snapshots: true, sources: true }); + + await context.tracing.startChunk(); + await page.goto(server.EMPTY_PAGE); + await page.setContent(''); + await page.click('"Click"'); + await context.tracing.stopChunk({ path: testInfo.outputPath('trace1.zip') }); + + await context.tracing.startChunk(); + await context.tracing.stopChunk({ path: testInfo.outputPath('trace2.zip') }); + + { + const { resources } = await parseTrace(testInfo.outputPath('trace1.zip')); + const names = Array.from(resources.keys()); + expect(names.filter(n => n.endsWith('.html')).length).toBe(1); + expect(names.filter(n => n.endsWith('.jpeg')).length).toBeGreaterThan(1); + // 1 source file for the test. + expect(names.filter(n => n.endsWith('.txt')).length).toBe(1); + } + + { + const { resources } = await parseTrace(testInfo.outputPath('trace2.zip')); + const names = Array.from(resources.keys()); + // 1 network resource should be preserved. + expect(names.filter(n => n.endsWith('.html')).length).toBe(1); + expect(names.filter(n => n.endsWith('.jpeg')).length).toBe(0); + // 1 source file for the test. + expect(names.filter(n => n.endsWith('.txt')).length).toBe(1); + } +}); + +test('should overwrite existing file', async ({ context, page, server }, testInfo) => { + await context.tracing.start({ screenshots: true, snapshots: true, sources: true }); + await page.goto(server.EMPTY_PAGE); + await page.setContent(''); + await page.click('"Click"'); + const path = testInfo.outputPath('trace1.zip'); + await context.tracing.stop({ path }); + { + const { resources } = await parseTrace(path); + const names = Array.from(resources.keys()); + expect(names.filter(n => n.endsWith('.html')).length).toBe(1); + expect(names.filter(n => n.endsWith('.jpeg')).length).toBeGreaterThan(1); + } + + await context.tracing.start({ screenshots: true, snapshots: true, sources: true }); + await context.tracing.stop({ path }); + + { + const { resources } = await parseTrace(path); + const names = Array.from(resources.keys()); + expect(names.filter(n => n.endsWith('.html')).length).toBe(0); + expect(names.filter(n => n.endsWith('.jpeg')).length).toBe(0); + } +}); + test('should collect sources', async ({ context, page, server }, testInfo) => { await context.tracing.start({ sources: true }); await page.goto(server.EMPTY_PAGE);