fix(merge): make sure testId from different blobs are unique (#24475)

Fixes a scenario where each shard runs the same setup project.

References #24451.

packages/playwright-test/src/reporters/merge.ts

@@ -96,6 +96,7 @@ async function mergeEvents(dir: string, shardReportFiles: string[]) {
     const shardB = b.metadata.shard?.current ?? 0;
     return shardA - shardB;
   });
+  const allTestIds = new Set<string>();
   for (const { parsedEvents } of shardEvents) {
     for (const event of parsedEvents) {
       if (event.method === 'onConfigure')
@@ -105,7 +106,7 @@ async function mergeEvents(dir: string, shardReportFiles: string[]) {
       else if (event.method === 'onEnd')
         endEvents.push(event);
       else if (event.method === 'onBlobReportMetadata')
-        new ProjectNamePatcher(event.params.projectSuffix).patchEvents(parsedEvents);
+        new ProjectNamePatcher(allTestIds, event.params.projectSuffix || '').patchEvents(parsedEvents);
       else
         events.push(event);
     }
@@ -211,12 +212,12 @@ async function sortedShardFiles(dir: string) {
 }
 
 class ProjectNamePatcher {
-  constructor(private _projectNameSuffix: string) {
+  private _testIds = new Set<string>();
+
+  constructor(private _allTestIds: Set<string>, private _projectNameSuffix: string) {
   }
 
   patchEvents(events: JsonEvent[]) {
-    if (!this._projectNameSuffix)
-      return;
     for (const event of events) {
       const { method, params } = event;
       switch (method) {
@@ -234,6 +235,8 @@ class ProjectNamePatcher {
           continue;
       }
     }
+    for (const testId of this._testIds)
+      this._allTestIds.add(testId);
   }
 
   private _onBegin(config: JsonConfig, projects: JsonProject[]) {
@@ -259,11 +262,21 @@ class ProjectNamePatcher {
   }
 
   private _updateTestIds(suite: JsonSuite) {
-    suite.tests.forEach(test => test.testId = this._mapTestId(test.testId));
+    suite.tests.forEach(test => {
+      test.testId = this._mapTestId(test.testId);
+      this._testIds.add(test.testId);
+    });
     suite.suites.forEach(suite => this._updateTestIds(suite));
   }
 
   private _mapTestId(testId: string): string {
-    return testId + '-' + this._projectNameSuffix;
+    testId = testId + this._projectNameSuffix;
+    // Consider a setup project running on each shard. In this case we'll have
+    // the same testId (from setup project) in multiple blob reports.
+    // To avoid reporters being confused by clashing test ids, we automatically
+    // make them unique and produce a separate test from each blob.
+    while (this._allTestIds.has(testId))
+      testId = testId + '1';
+    return testId;
   }
 }

tests/playwright-test/reporter-blob.spec.ts

@@ -146,15 +146,25 @@ test('should call methods in right order', async ({ runInlineTest, mergeReports
   expect(lines.filter(l => l === 'onExit').length).toBe(1);
 });
 
-test('should merge into html', async ({ runInlineTest, mergeReports, showReport, page }) => {
+test('should merge into html with dependencies', async ({ runInlineTest, mergeReports, showReport, page }) => {
   const reportDir = test.info().outputPath('blob-report');
   const files = {
     'playwright.config.ts': `
       module.exports = {
         retries: 1,
-        reporter: [['blob', { outputDir: '${reportDir.replace(/\\/g, '/')}' }]]
+        reporter: [['blob', { outputDir: '${reportDir.replace(/\\/g, '/')}' }]],
+        projects: [
+          { name: 'test', dependencies: ['setup'] },
+          { name: 'setup', testMatch: /.*setup.js/ },
+        ]
       };
     `,
+    'setup.js': `
+      import { test as setup } from '@playwright/test';
+      setup('login once', async ({}) => {
+        await setup.step('login step', async () => {});
+      });
+    `,
     'a.test.js': `
       import { test, expect } from '@playwright/test';
       test('math 1', async ({}) => {
@@ -202,14 +212,24 @@ test('should merge into html', async ({ runInlineTest, mergeReports, showReport,
 
   await showReport();
 
-  await expect(page.locator('.subnav-item:has-text("All") .counter')).toHaveText('10');
+  await expect(page.locator('.subnav-item:has-text("All") .counter')).toHaveText('13');
-  await expect(page.locator('.subnav-item:has-text("Passed") .counter')).toHaveText('3');
+  await expect(page.locator('.subnav-item:has-text("Passed") .counter')).toHaveText('6');
   await expect(page.locator('.subnav-item:has-text("Failed") .counter')).toHaveText('2');
   await expect(page.locator('.subnav-item:has-text("Flaky") .counter')).toHaveText('2');
   await expect(page.locator('.subnav-item:has-text("Skipped") .counter')).toHaveText('3');
 
-  await expect(page.locator('.test-file-test .test-file-title')).toHaveText(
+  await expect(page.locator('.test-file-test .test-file-title')).toHaveText([
-      ['failing 1', 'flaky 1', 'math 1', 'skipped 1', 'failing 2', 'math 2', 'skipped 2', 'flaky 2', 'math 3', 'skipped 3']);
+    'failing 1', 'flaky 1', 'math 1', 'skipped 1',
+    'failing 2', 'math 2', 'skipped 2',
+    'flaky 2', 'math 3', 'skipped 3',
+    'login once', 'login once', 'login once',
+  ]);
+
+  for (let i = 0; i < 3; i++) {
+    await page.getByText('login once').nth(i).click();
+    await expect(page.getByText('login step')).toBeVisible();
+    await page.goBack();
+  }
 });
 
 test('be able to merge incomplete shards', async ({ runInlineTest, mergeReports, showReport, page }) => {