fix(trace-viewer): make call ids unique across trace files (#30720)

packages/trace-viewer/src/ui/modelUtil.ts

@@ -160,6 +160,56 @@ function indexModel(context: ContextEntry) {
 }
 
 function mergeActionsAndUpdateTiming(contexts: ContextEntry[]) {
+  const traceFileToContexts = new Map<string, ContextEntry[]>();
+  for (const context of contexts) {
+    const traceFile = context.traceUrl;
+    let list = traceFileToContexts.get(traceFile);
+    if (!list) {
+      list = [];
+      traceFileToContexts.set(traceFile, list);
+    }
+    list.push(context);
+  }
+
+  const result: ActionTraceEventInContext[] = [];
+  let traceFileId = 0;
+  for (const [, contexts] of traceFileToContexts) {
+    // Action ids are unique only within a trace file. If there are
+    // traces from more than one file we make the ids unique across the
+    // files. The code does not update snapshot ids as they are always
+    // retrieved from a particular trace file.
+    if (traceFileToContexts.size)
+      makeCallIdsUniqueAcrossTraceFiles(contexts, ++traceFileId);
+    // Align action times across runner and library contexts within each trace file.
+    const map = mergeActionsAndUpdateTimingSameTrace(contexts);
+    result.push(...map.values());
+  }
+  result.sort((a1, a2) => {
+    if (a2.parentId === a1.callId)
+      return -1;
+    if (a1.parentId === a2.callId)
+      return 1;
+    return a1.wallTime - a2.wallTime || a1.startTime - a2.startTime;
+  });
+
+  for (let i = 1; i < result.length; ++i)
+    (result[i] as any)[prevInListSymbol] = result[i - 1];
+
+  return result;
+}
+
+function makeCallIdsUniqueAcrossTraceFiles(contexts: ContextEntry[], traceFileId: number) {
+  for (const context of contexts) {
+    for (const action of context.actions) {
+      if (action.callId)
+        action.callId = `${traceFileId}:${action.callId}`;
+      if (action.parentId)
+        action.parentId = `${traceFileId}:${action.parentId}`;
+    }
+  }
+}
+
+function mergeActionsAndUpdateTimingSameTrace(contexts: ContextEntry[]) {
   const map = new Map<string, ActionTraceEventInContext>();
 
   // Protocol call aka isPrimary contexts have startTime/endTime as server-side times.
@@ -219,20 +269,7 @@ function mergeActionsAndUpdateTiming(contexts: ContextEntry[]) {
         frame.timestamp += timeDelta;
     }
   }
-
+  return map;
-  const result = [...map.values()];
-  result.sort((a1, a2) => {
-    if (a2.parentId === a1.callId)
-      return -1;
-    if (a1.parentId === a2.callId)
-      return 1;
-    return a1.wallTime - a2.wallTime || a1.startTime - a2.startTime;
-  });
-
-  for (let i = 1; i < result.length; ++i)
-    (result[i] as any)[prevInListSymbol] = result[i - 1];
-
-  return result;
 }
 
 export function buildActionTree(actions: ActionTraceEventInContext[]): { rootItem: ActionTreeItem, itemMap: Map<string, ActionTreeItem> } {

tests/library/trace-viewer.spec.ts

@@ -804,6 +804,21 @@ test('should open two trace files', async ({ context, page, request, server, sho
   await expect(callLine.getByText('events')).toHaveText(/events:[\d]+/);
 });
 
+test('should open two trace files of the same test', async ({ context, page, request, server, showTraceViewer, asset }, testInfo) => {
+  const traceViewer = await showTraceViewer([asset('test-trace1.zip'), asset('test-trace2.zip')]);
+  // Same actions from different test runs should not be merged.
+  await expect(traceViewer.actionTitles).toHaveText([
+    'Before Hooks',
+    'page.gotohttps://playwright.dev/',
+    'expect.toBe',
+    'After Hooks',
+    'Before Hooks',
+    'page.gotohttps://playwright.dev/',
+    'expect.toBe',
+    'After Hooks',
+  ]);
+});
+
 test('should include requestUrl in route.fulfill', async ({ page, runAndTrace, browserName }) => {
   await page.route('**/*', route => {
     void route.fulfill({