From 166d2d4fde2089d40206319fa7f5a35d4e4712e0 Mon Sep 17 00:00:00 2001
From: Yury Semikhatsky <yurys@chromium.org>
Date: Tue, 12 Mar 2024 16:10:43 -0700
Subject: [PATCH] chore: separate results for repeated snapshot names (#29880)

Reference #29719
---
 .../src/matchers/toMatchSnapshot.ts           |  2 +-
 tests/playwright-test/golden.spec.ts          | 70 +++++++++++++++++++
 tests/playwright-test/reporter-html.spec.ts   |  4 +-
 3 files changed, 73 insertions(+), 3 deletions(-)

diff --git a/packages/playwright/src/matchers/toMatchSnapshot.ts b/packages/playwright/src/matchers/toMatchSnapshot.ts
index 317a0144d9..4e12d18fe6 100644
--- a/packages/playwright/src/matchers/toMatchSnapshot.ts
+++ b/packages/playwright/src/matchers/toMatchSnapshot.ts
@@ -140,7 +140,7 @@ class SnapshotHelper {
       snapshotNames.namedSnapshotIndex[joinedName] = (snapshotNames.namedSnapshotIndex[joinedName] || 0) + 1;
       const index = snapshotNames.namedSnapshotIndex[joinedName];
       if (index > 1)
-        this.snapshotName = `${joinedName}-${index - 1}`;
+        this.snapshotName = addSuffixToFilePath(joinedName, `-${index - 1}`);
       else
         this.snapshotName = joinedName;
     }
diff --git a/tests/playwright-test/golden.spec.ts b/tests/playwright-test/golden.spec.ts
index 39efa3c0b8..e768e1cf60 100644
--- a/tests/playwright-test/golden.spec.ts
+++ b/tests/playwright-test/golden.spec.ts
@@ -90,6 +90,76 @@ test('should generate default name', async ({ runInlineTest }, testInfo) => {
   expect(fs.existsSync(testInfo.outputPath('a.spec.js-snapshots', 'is-a-test-5.dat'))).toBe(true);
 });
 
+test('should generate separate actual results for repeating names', async ({ runInlineTest }, testInfo) => {
+  test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/29719' });
+  const result = await runInlineTest({
+    ...files,
+    'a.spec.js-snapshots/foo.txt': `b`,
+    'a.spec.js-snapshots/bar/baz.txt': `c`,
+    'a.spec.js': `
+      const { test, expect } = require('./helper');
+      test.afterEach(async ({}, testInfo) => {
+        console.log('## ' + JSON.stringify(testInfo.attachments));
+      });
+      test('is a test', ({}) => {
+        expect.soft('a').toMatchSnapshot('foo.txt');
+        expect.soft('a').toMatchSnapshot('foo.txt');
+        expect.soft('b').toMatchSnapshot(['bar', 'baz.txt']);
+        expect.soft('b').toMatchSnapshot(['bar', 'baz.txt']);
+      });
+    `
+  });
+
+  const outputText = result.output;
+  const attachments = outputText.split('\n').filter(l => l.startsWith('## ')).map(l => l.substring(3)).map(l => JSON.parse(l))[0];
+  for (const attachment of attachments) {
+    attachment.path = attachment.path.replace(/\\/g, '/').replace(/.*test-results\//, '');
+    attachment.name = attachment.name.replace(/\\/g, '/');
+  }
+  expect(attachments).toEqual([
+    {
+      'name': 'foo-expected.txt',
+      'contentType': 'text/plain',
+      'path': 'golden-should-generate-separate-actual-results-for-repeating-names-playwright-test/a.spec.js-snapshots/foo.txt'
+    },
+    {
+      'name': 'foo-actual.txt',
+      'contentType': 'text/plain',
+      'path': 'a-is-a-test/foo-actual.txt'
+    },
+    {
+      'name': 'foo-1-expected.txt',
+      'contentType': 'text/plain',
+      'path': 'golden-should-generate-separate-actual-results-for-repeating-names-playwright-test/a.spec.js-snapshots/foo.txt'
+    },
+    {
+      'name': 'foo-1-actual.txt',
+      'contentType': 'text/plain',
+      'path': 'a-is-a-test/foo-1-actual.txt'
+    },
+    {
+      'name': 'bar/baz-expected.txt',
+      'contentType': 'text/plain',
+      'path': 'golden-should-generate-separate-actual-results-for-repeating-names-playwright-test/a.spec.js-snapshots/bar/baz.txt'
+    },
+    {
+      'name': 'bar/baz-actual.txt',
+      'contentType': 'text/plain',
+      'path': 'a-is-a-test/bar-baz-actual.txt'
+    },
+    {
+      'name': 'bar/baz-1-expected.txt',
+      'contentType': 'text/plain',
+      'path': 'golden-should-generate-separate-actual-results-for-repeating-names-playwright-test/a.spec.js-snapshots/bar/baz.txt'
+    },
+    {
+      'name': 'bar/baz-1-actual.txt',
+      'contentType': 'text/plain',
+      'path': 'a-is-a-test/bar-baz-1-actual.txt'
+    }
+  ]);
+});
+
 test('should compile with different option combinations', async ({ runTSC }) => {
   const result = await runTSC({
     'a.spec.ts': `
diff --git a/tests/playwright-test/reporter-html.spec.ts b/tests/playwright-test/reporter-html.spec.ts
index a819f25be8..ad39504c64 100644
--- a/tests/playwright-test/reporter-html.spec.ts
+++ b/tests/playwright-test/reporter-html.spec.ts
@@ -261,8 +261,8 @@ for (const useIntermediateMergeReport of [false, true] as const) {
       await expect(page.locator('data-testid=test-result-image-mismatch')).toHaveCount(3);
       await expect(page.locator('text=Image mismatch:')).toHaveText([
         'Image mismatch: expected.png',
-        'Image mismatch: expected.png-1',
-        'Image mismatch: expected.png-2',
+        'Image mismatch: expected-1.png',
+        'Image mismatch: expected-2.png',
       ]);
     });