Revert "fix(merge-reports) only change test ids when needed"

This reverts commit 389e5711f4.

packages/playwright/src/reporters/merge.ts

@@ -194,25 +194,16 @@ async function mergeEvents(dir: string, shardReportFiles: string[], stringPool:
   printStatus(`merging events`);
 
   const reports: ReportData[] = [];
-  const testIdsUsedInOtherBlobs = new Set<string>();
 
   for (let i = 0; i < blobs.length; ++i) {
     // Generate unique salt for each blob.
     const { parsedEvents, metadata, localPath } = blobs[i];
     const eventPatchers = new JsonEventPatchers();
-    const testIdsUsedInThisBlob = new Set<string>();
+    eventPatchers.patchers.push(new IdsPatcher(stringPool, metadata.name, String(i)));
-    eventPatchers.patchers.push(new IdsPatcher(
-        stringPool,
-        metadata.name,
-        String(i),
-        testIdsUsedInThisBlob,
-        testIdsUsedInOtherBlobs,
-    ));
     // Only patch path separators if we are merging reports with explicit config.
     if (rootDirOverride)
       eventPatchers.patchers.push(new PathSeparatorPatcher(metadata.pathSeparator));
     eventPatchers.patchEvents(parsedEvents);
-    testIdsUsedInThisBlob.forEach(id => testIdsUsedInOtherBlobs.add(id));
 
     for (const event of parsedEvents) {
       if (event.method === 'onConfigure')
@@ -367,23 +358,11 @@ class IdsPatcher {
   private _stringPool: StringInternPool;
   private _botName: string | undefined;
   private _salt: string;
-  private _testIdsMap: Map<string, string>;
-  private _testIdsUsedInThisBlob: Set<string>;
-  private _testIdsUsedInOtherBlobs: Set<string>;
 
-  constructor(
+  constructor(stringPool: StringInternPool, botName: string | undefined, salt: string) {
-    stringPool: StringInternPool,
-    botName: string | undefined,
-    salt: string,
-    testIdsUsedInThisBlob: Set<string>,
-    testIdsUsedInOtherBlobs: Set<string>,
-  ) {
     this._stringPool = stringPool;
     this._botName = botName;
     this._salt = salt;
-    this._testIdsMap = new Map();
-    this._testIdsUsedInThisBlob = testIdsUsedInThisBlob;
-    this._testIdsUsedInOtherBlobs = testIdsUsedInOtherBlobs;
   }
 
   patchEvent(event: JsonEvent) {
@@ -427,20 +406,7 @@ class IdsPatcher {
   }
 
   private _mapTestId(testId: string): string {
-    const t1 = this._stringPool.internString(testId);
+    return this._stringPool.internString(testId + this._salt);
-    if (this._testIdsMap.has(t1))
-      // already mapped
-      return this._testIdsMap.get(t1)!;
-    if (this._testIdsUsedInOtherBlobs.has(t1)) {
-      // test id is used in another blob, so we need to salt it.
-      const t2 = this._stringPool.internString(testId + this._salt);
-      this._testIdsUsedInThisBlob.add(t2);
-      this._testIdsMap.set(t1, t2);
-      return t2;
-    }
-    this._testIdsUsedInThisBlob.add(t1);
-    this._testIdsMap.set(t1, t1);
-    return t1;
   }
 }
 
@@ -585,4 +551,4 @@ class BlobModernizer {
   }
 }
 
-const modernizer = new BlobModernizer();
+const modernizer = new BlobModernizer();

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

@@ -1813,78 +1813,6 @@ test('merge reports without --config preserves path separators', async ({ runInl
   expect(output).toContain(`test title: ${'tests2' + otherSeparator + 'b.test.js'}`);
 });
 
-test('merge reports must not change test ids when there is no need to', async ({ runInlineTest, mergeReports }) => {
-  const files = {
-    'echo-test-id-reporter.js': `
-      export default class EchoTestIdReporter {
-        onTestBegin(test) {
-          console.log('test.id:', test.id);
-        }
-      };
-    `,
-    'single-run.config.ts': `module.exports = {
-      reporter: [
-        ['./echo-test-id-reporter.js'],
-      ]
-    };`,
-    'shard-1.config.ts': `module.exports = {
-      fullyParallel: true,
-      shard: { total: 2, current: 1 },
-      reporter: [
-        ['./echo-test-id-reporter.js'],
-        ['blob', { outputDir: 'blob-report' }],
-      ]
-    };`,
-    'shard-2.config.ts': `module.exports = {
-      fullyParallel: true,
-      shard: { total: 2, current: 2 },
-      reporter: [
-        ['./echo-test-id-reporter.js'],
-        ['blob', { outputDir: 'blob-report' }],
-      ]
-    };`,
-    'merge.config.ts': `module.exports = {
-      reporter: [
-        ['./echo-test-id-reporter.js'],
-      ]
-    };`,
-    'a.test.js': `
-      import { test, expect } from '@playwright/test';
-      test('test 1', async ({}) => { });
-      test('test 2', async ({}) => { });
-      test('test 3', async ({}) => { });
-    `,
-  };
-  let testIdsFromSingleRun: string[];
-  let testIdsFromShard1: string[];
-  let testIdsFromShard2: string[];
-  let testIdsFromMergedReport: string[];
-  const parseTestIdsFromOutput = (output: string) => output.split('\n').filter(s => s.startsWith('test.id:')).map(s => s.split('test.id:')[1].trim()).sort();
-  {
-    const { exitCode, output } = await runInlineTest(files, { workers: 1 }, undefined, { additionalArgs: ['--config', test.info().outputPath('single-run.config.ts')] });
-    expect(exitCode).toBe(0);
-    testIdsFromSingleRun = parseTestIdsFromOutput(output);
-    expect(testIdsFromSingleRun.length).toEqual(3);
-  }
-  {
-    const { exitCode, output } = await runInlineTest(files, { workers: 1 }, {}, { additionalArgs: ['--config', test.info().outputPath('shard-1.config.ts')] });
-    expect(exitCode).toBe(0);
-    testIdsFromShard1 = parseTestIdsFromOutput(output);
-  }
-  {
-    const { exitCode, output } = await runInlineTest(files, { workers: 1 }, { PWTEST_BLOB_DO_NOT_REMOVE: '1' }, { additionalArgs: ['--config', test.info().outputPath('shard-2.config.ts')] });
-    expect(exitCode).toBe(0);
-    testIdsFromShard2 = parseTestIdsFromOutput(output);
-    expect([...testIdsFromShard1, ...testIdsFromShard2].sort()).toEqual(testIdsFromSingleRun);
-  }
-  {
-    const { exitCode, output } = await mergeReports(test.info().outputPath('blob-report'), undefined, { additionalArgs: ['--config', test.info().outputPath('merge.config.ts')] });
-    expect(exitCode).toBe(0);
-    testIdsFromMergedReport = parseTestIdsFromOutput(output);
-    expect(testIdsFromMergedReport).toEqual(testIdsFromSingleRun);
-  }
-});
-
 test('TestSuite.project() should return owning project', async ({ runInlineTest, mergeReports }) => {
   test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/29173' });
   const files1 = {