diff --git a/packages/playwright-core/src/server/trace/recorder/tracing.ts b/packages/playwright-core/src/server/trace/recorder/tracing.ts index 3eaa227297..6ac2cd5be0 100644 --- a/packages/playwright-core/src/server/trace/recorder/tracing.ts +++ b/packages/playwright-core/src/server/trace/recorder/tracing.ts @@ -54,8 +54,8 @@ export type TracerOptions = { type RecordingState = { options: TracerOptions, traceName: string, - networkFile: string, - traceFile: string, + networkFile: { file: string, buffer: trace.TraceEvent[] }, + traceFile: { file: string, buffer: trace.TraceEvent[] }, tracesDir: string, resourcesDir: string, chunkOrdinal: number, @@ -132,15 +132,24 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps // TODO: passing the same name for two contexts makes them write into a single file // and conflict. const traceName = options.name || createGuid(); - // Init the state synchronously. - this._state = { options, traceName, traceFile: '', networkFile: '', tracesDir: '', resourcesDir: '', chunkOrdinal: 0, traceSha1s: new Set(), networkSha1s: new Set(), recording: false }; - const state = this._state; - state.tracesDir = await this._createTracesDirIfNeeded(); - state.resourcesDir = path.join(state.tracesDir, 'resources'); - state.traceFile = path.join(state.tracesDir, traceName + '.trace'); - state.networkFile = path.join(state.tracesDir, traceName + '.network'); - this._writeChain = fs.promises.mkdir(state.resourcesDir, { recursive: true }).then(() => fs.promises.writeFile(state.networkFile, '')); + const tracesDir = this._createTracesDirIfNeeded(); + + // Init the state synchronously. + this._state = { + options, + traceName, + tracesDir, + traceFile: { file: path.join(tracesDir, traceName + '.trace'), buffer: [] }, + networkFile: { file: path.join(tracesDir, traceName + '.network'), buffer: [] }, + resourcesDir: path.join(tracesDir, 'resources'), + chunkOrdinal: 0, + traceSha1s: new Set(), + networkSha1s: new Set(), + recording: false + }; + const state = this._state; + this._writeChain = fs.promises.mkdir(state.resourcesDir, { recursive: true }).then(() => fs.promises.writeFile(state.networkFile.file, '')); if (options.snapshots) this._harTracer.start(); } @@ -157,15 +166,19 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps const state = this._state; const suffix = state.chunkOrdinal ? `-${state.chunkOrdinal}` : ``; state.chunkOrdinal++; - state.traceFile = path.join(state.tracesDir, `${state.traceName}${suffix}.trace`); + state.traceFile = { + file: path.join(state.tracesDir, `${state.traceName}${suffix}.trace`), + buffer: [], + }; state.recording = true; if (options.name && options.name !== this._state.traceName) this._changeTraceName(this._state, options.name); this._appendTraceOperation(async () => { - await mkdirIfNeeded(state.traceFile); - await fs.promises.appendFile(state.traceFile, JSON.stringify({ ...this._contextCreatedEvent, title: options.title, wallTime: Date.now() }) + '\n'); + await mkdirIfNeeded(state.traceFile.file); + const event: trace.TraceEvent = { ...this._contextCreatedEvent, title: options.title, wallTime: Date.now() }; + await appendEventAndFlushIfNeeded(state.traceFile, event); }); this._context.instrumentation.addListener(this, this._context); @@ -199,12 +212,21 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps private async _changeTraceName(state: RecordingState, name: string) { await this._appendTraceOperation(async () => { + await flushTraceFile(state.traceFile); + await flushTraceFile(state.networkFile); + const oldNetworkFile = state.networkFile; state.traceName = name; - state.traceFile = path.join(state.tracesDir, name + '.trace'); - state.networkFile = path.join(state.tracesDir, name + '.network'); + state.traceFile = { + file: path.join(state.tracesDir, name + '.trace'), + buffer: [], + }; + state.networkFile = { + file: path.join(state.tracesDir, name + '.network'), + buffer: [], + }; // Network file survives across chunks, so make a copy with the new name. - await fs.promises.copyFile(oldNetworkFile, state.networkFile); + await fs.promises.copyFile(oldNetworkFile.file, state.networkFile.file); }); } @@ -225,10 +247,10 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps await removeFolders([this._tracesTmpDir]); } - private async _createTracesDirIfNeeded() { + private _createTracesDirIfNeeded() { if (this._precreatedTracesDir) return this._precreatedTracesDir; - this._tracesTmpDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'playwright-tracing-')); + this._tracesTmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'playwright-tracing-')); return this._tracesTmpDir; } @@ -265,6 +287,9 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps if (params.mode === 'discard') return {}; + await flushTraceFile(state.traceFile); + await flushTraceFile(state.networkFile); + // Network file survives across chunks, make a snapshot before returning the resulting entries. // We should pick a name starting with "traceName" and ending with .network. // Something like someSuffixHere.network. @@ -272,10 +297,10 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps // We can use -.network, but "-pwnetcopy-0" suffix is more readable // and makes it easier to debug future issues. const networkFile = path.join(state.tracesDir, state.traceName + `-pwnetcopy-${state.chunkOrdinal}.network`); - await fs.promises.copyFile(state.networkFile, networkFile); + await fs.promises.copyFile(state.networkFile.file, networkFile); const entries: NameValue[] = []; - entries.push({ name: 'trace.trace', value: state.traceFile }); + entries.push({ name: 'trace.trace', value: state.traceFile.file }); entries.push({ name: 'trace.network', value: networkFile }); for (const sha1 of new Set([...state.traceSha1s, ...state.networkSha1s])) entries.push({ name: path.join('resources', sha1), value: path.join(state.resourcesDir, sha1) }); @@ -373,7 +398,7 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps const event: trace.ResourceSnapshotTraceEvent = { type: 'resource-snapshot', snapshot: entry }; this._appendTraceOperation(async () => { const visited = visitTraceEvent(event, this._state!.networkSha1s); - await fs.promises.appendFile(this._state!.networkFile, JSON.stringify(visited) + '\n'); + await appendEventAndFlushIfNeeded(this._state!.networkFile, visited); }); } @@ -438,7 +463,7 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps private _appendTraceEvent(event: trace.TraceEvent) { this._appendTraceOperation(async () => { const visited = visitTraceEvent(event, this._state!.traceSha1s); - await fs.promises.appendFile(this._state!.traceFile, JSON.stringify(visited) + '\n'); + await appendEventAndFlushIfNeeded(this._state!.traceFile, visited); }); } @@ -539,3 +564,19 @@ function createAfterActionTraceEvent(metadata: CallMetadata): trace.AfterActionT result: metadata.result, }; } + +async function appendEventAndFlushIfNeeded(file: { file: string, buffer: trace.TraceEvent[] }, event: trace.TraceEvent) { + file.buffer.push(event); + + // Do not flush events, they are too noisy. + if (event.type === 'event' || event.type === 'object') + return; + + await flushTraceFile(file); +} + +async function flushTraceFile(file: { file: string, buffer: trace.TraceEvent[] }) { + const data = file.buffer.map(e => Buffer.from(JSON.stringify(e) + '\n')); + await fs.promises.appendFile(file.file, Buffer.concat(data)); + file.buffer = []; +} diff --git a/tests/library/tracing.spec.ts b/tests/library/tracing.spec.ts index edc40ccba9..7c68098553 100644 --- a/tests/library/tracing.spec.ts +++ b/tests/library/tracing.spec.ts @@ -21,6 +21,7 @@ import { browserTest, contextTest as test, expect } from '../config/browserTest' import { parseTraceRaw } from '../config/utils'; import type { StackFrame } from '@protocol/channels'; import type { ActionTraceEvent } from '../../packages/trace/src/trace'; +import { artifactsFolderName } from '../../packages/playwright-test/src/isomorphic/folders'; test.skip(({ trace }) => trace === 'on'); @@ -634,6 +635,69 @@ test('should store postData for global request', async ({ request, server }, tes })); }); +test('should not flush console events', async ({ context, page }, testInfo) => { + await context.tracing.start(); + const promise = new Promise(f => { + let counter = 0; + page.on('console', () => { + if (++counter === 100) + f(); + }); + }); + + await page.evaluate(() => { + setTimeout(() => { + for (let i = 0; i < 100; ++i) + console.log('hello ' + i); + }, 10); + return 31415926; + }); + + await promise; + + const dir = path.join(testInfo.project.outputDir, artifactsFolderName(testInfo.workerIndex), 'traces'); + + let content: string; + await expect(async () => { + const traceName = fs.readdirSync(dir).find(name => name.endsWith('.trace')); + content = await fs.promises.readFile(path.join(dir, traceName), 'utf8'); + expect(content).toContain('page.evaluate'); + expect(content).toContain('31415926'); + }).toPass(); + expect(content).not.toContain('hello 0'); + + await page.evaluate(() => 42); + + await expect(async () => { + const traceName = fs.readdirSync(dir).find(name => name.endsWith('.trace')); + const content = await fs.promises.readFile(path.join(dir, traceName), 'utf8'); + expect(content).toContain('hello 0'); + expect(content).toContain('hello 99'); + }).toPass(); +}); + +test('should flush console events on tracing stop', async ({ context, page }, testInfo) => { + await context.tracing.start(); + const promise = new Promise(f => { + let counter = 0; + page.on('console', () => { + if (++counter === 100) + f(); + }); + }); + await page.evaluate(() => { + setTimeout(() => { + for (let i = 0; i < 100; ++i) + console.log('hello ' + i); + }); + }); + await promise; + const tracePath = testInfo.outputPath('trace.zip'); + await context.tracing.stop({ path: tracePath }); + const trace = await parseTraceRaw(tracePath); + const events = trace.events.filter(e => e.method === 'console'); + expect(events).toHaveLength(100); +}); function expectRed(pixels: Buffer, offset: number) { const r = pixels.readUInt8(offset);