fix(tracing): avoid trace file name collisions (#24191)

We have been optionally adding `-<number>` in multiple places, and these might collide in various circumstances, for example: two contexts at the same time, one of them has the second trace chunk. References #23387.
microsoft · Jul 13, 2023 · 53feeaf · 53feeaf
1 parent 53bf199
commit 53feeaf
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 14 deletions.
diff --git a/packages/playwright-core/src/server/trace/recorder/tracing.ts b/packages/playwright-core/src/server/trace/recorder/tracing.ts
@@ -166,17 +166,13 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps
       throw new Error('Cannot start a trace chunk while stopping');
 
     const state = this._state;
-    const suffix = state.chunkOrdinal ? `-${state.chunkOrdinal}` : ``;
-    state.chunkOrdinal++;
-    state.traceFile = {
-      file: path.join(state.tracesDir, `${state.traceName}${suffix}.trace`),
-      buffer: [],
-    };
     state.recording = true;
     state.callIds.clear();
 
-    if (options.name && options.name !== this._state.traceName)
-      this._changeTraceName(this._state, options.name);
+    if (options.name && options.name !== state.traceName)
+      this._changeTraceName(state, options.name);
+    else
+      this._allocateNewTraceFile(state);
 
     this._appendTraceOperation(async () => {
       await mkdirIfNeeded(state.traceFile.file);
@@ -213,17 +209,24 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps
       page.setScreencastOptions(null);
   }
 
+  private _allocateNewTraceFile(state: RecordingState) {
+    const suffix = state.chunkOrdinal ? `-chunk${state.chunkOrdinal}` : ``;
+    state.chunkOrdinal++;
+    state.traceFile = {
+      file: path.join(state.tracesDir, `${state.traceName}${suffix}.trace`),
+      buffer: [],
+    };
+  }
+
   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 = {
-        file: path.join(state.tracesDir, name + '.trace'),
-        buffer: [],
-      };
+      state.chunkOrdinal = 0;  // Reset ordinal for the new name.
+      this._allocateNewTraceFile(state);
       state.networkFile = {
         file: path.join(state.tracesDir, name + '.network'),
         buffer: [],

diff --git a/packages/playwright-test/src/index.ts b/packages/playwright-test/src/index.ts
@@ -698,9 +698,9 @@ class ArtifactsRecorder {
   private async _startTraceChunkOnContextCreation(tracing: Tracing) {
     if (this._captureTrace) {
       const title = [path.relative(this._testInfo.project.testDir, this._testInfo.file) + ':' + this._testInfo.line, ...this._testInfo.titlePath.slice(1)].join(' › ');
-      const ordinalSuffix = this._traceOrdinal ? `-${this._traceOrdinal}` : '';
+      const ordinalSuffix = this._traceOrdinal ? `-context${this._traceOrdinal}` : '';
       ++this._traceOrdinal;
-      const retrySuffix = this._testInfo.retry ? `-${this._testInfo.retry}` : '';
+      const retrySuffix = this._testInfo.retry ? `-retry${this._testInfo.retry}` : '';
       const name = `${this._testInfo.testId}${retrySuffix}${ordinalSuffix}`;
       if (!(tracing as any)[kTracingStarted]) {
         await tracing.start({ ...this._traceOptions, title, name });