From 53feeaf2702abcda86c94e22cfec1d6df2fe928a Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Wed, 12 Jul 2023 17:20:25 -0700
Subject: [PATCH] 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.
---
 .../src/server/trace/recorder/tracing.ts      | 27 ++++++++++---------
 packages/playwright-test/src/index.ts         |  4 +--
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/packages/playwright-core/src/server/trace/recorder/tracing.ts b/packages/playwright-core/src/server/trace/recorder/tracing.ts
index 859a697791c7f..23cb0d0d91f25 100644
--- 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,6 +209,15 @@ 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);
@@ -220,10 +225,8 @@ export class Tracing extends SdkObject implements InstrumentationListener, Snaps
 
       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
index 4ea5cc106ad4e..125f9fb7996dc 100644
--- 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 });