chore: allow multiple image diffs (#13202)

packages/html-reporter/src/links.tsx

@@ -76,7 +76,7 @@ export const AttachmentLink: React.FunctionComponent<{
     {attachment.body && <span>{attachment.name}</span>}
   </span>} loadChildren={attachment.body ? () => {
     return [<div className='attachment-body'>{attachment.body}</div>];
-  } : undefined} depth={0}></TreeItem>;
+  } : undefined} depth={0} style={{ lineHeight: '32px' }}></TreeItem>;
 };
 
 const kMissingContentType = 'x-playwright/missing';

packages/html-reporter/src/testResultView.tsx

@@ -179,6 +179,9 @@ const ImageDiffView: React.FunctionComponent<{
   }
   return <div className='vbox' data-testid='test-result-image-mismatch' ref={diffElement}>
     <TabbedPane tabs={tabs} selectedTab={selectedTab} setSelectedTab={setSelectedTab} />
+    <AttachmentLink attachment={diff.left!.attachment}></AttachmentLink>
+    <AttachmentLink attachment={diff.right!.attachment}></AttachmentLink>
+    {diff.diff && <AttachmentLink attachment={diff.diff.attachment}></AttachmentLink>}
   </div>;
 };
 

packages/html-reporter/src/treeItem.tsx

@@ -24,11 +24,12 @@ export const TreeItem: React.FunctionComponent<{
   onClick?: () => void,
   expandByDefault?: boolean,
   depth: number,
-  selected?: boolean
+  selected?: boolean,
-}> = ({ title, loadChildren, onClick, expandByDefault, depth, selected }) => {
+  style?:  React.CSSProperties,
+}> = ({ title, loadChildren, onClick, expandByDefault, depth, selected, style }) => {
   const [expanded, setExpanded] = React.useState(expandByDefault || false);
   const className = selected ? 'tree-item-title selected' : 'tree-item-title';
-  return <div className={'tree-item'}>
+  return <div className={'tree-item'} style={style}>
     <span className={className} style={{ whiteSpace: 'nowrap', paddingLeft: depth * 22 + 4 }} onClick={() => { onClick?.(); setExpanded(!expanded); }} >
       {loadChildren && !!expanded && icons.downArrow()}
       {loadChildren && !expanded && icons.rightArrow()}

packages/playwright-test/src/matchers/toMatchSnapshot.ts

@@ -34,10 +34,16 @@ import { TestInfoImpl } from '../testInfo';
 import { SyncExpectationResult } from '../expect';
 
 type NameOrSegments = string | string[];
-const SNAPSHOT_COUNTER = Symbol('noname-snapshot-counter');
+const snapshotNamesSymbol = Symbol('snapshotNames');
+
+type SnapshotNames = {
+  anonymousSnapshotIndex: number;
+  namedSnapshotIndex: { [key: string]: number };
+};
 
 class SnapshotHelper<T extends ImageComparatorOptions> {
   readonly testInfo: TestInfoImpl;
+  readonly snapshotName: string;
   readonly expectedPath: string;
   readonly previousPath: string;
   readonly snapshotPath: string;
@@ -68,14 +74,40 @@ class SnapshotHelper<T extends ImageComparatorOptions> {
       options = { ...nameOrOptions };
       delete (options as any).name;
     }
+
+    let snapshotNames = (testInfo as any)[snapshotNamesSymbol] as SnapshotNames;
+    if (!(testInfo as any)[snapshotNamesSymbol]) {
+      snapshotNames = {
+        anonymousSnapshotIndex: 0,
+        namedSnapshotIndex: {},
+      };
+      (testInfo as any)[snapshotNamesSymbol] = snapshotNames;
+    }
+
+    // Consider the use case below. We should save actual to different paths.
+    //
+    //   expect.toMatchSnapshot('a.png')
+    //   // noop
+    //   expect.toMatchSnapshot('a.png')
+
+    let actualModifier = '';
     if (!name) {
-      (testInfo as any)[SNAPSHOT_COUNTER] = ((testInfo as any)[SNAPSHOT_COUNTER] || 0);
       const fullTitleWithoutSpec = [
         ...testInfo.titlePath.slice(1),
-        (testInfo as any)[SNAPSHOT_COUNTER] + 1,
+        ++snapshotNames.anonymousSnapshotIndex,
       ].join(' ');
-      ++(testInfo as any)[SNAPSHOT_COUNTER];
       name = sanitizeForFilePath(trimLongString(fullTitleWithoutSpec)) + '.' + anonymousSnapshotExtension;
+      this.snapshotName = name;
+    } else {
+      const joinedName = Array.isArray(name) ? name.join(path.sep) : name;
+      snapshotNames.namedSnapshotIndex[joinedName] = (snapshotNames.namedSnapshotIndex[joinedName] || 0) + 1;
+      const index = snapshotNames.namedSnapshotIndex[joinedName];
+      if (index > 1) {
+        actualModifier = `-${index - 1}`;
+        this.snapshotName = `${joinedName}-${index - 1}`;
+      } else {
+        this.snapshotName = joinedName;
+      }
     }
 
     options = {
@@ -90,10 +122,12 @@ class SnapshotHelper<T extends ImageComparatorOptions> {
       throw new Error('`maxDiffPixelRatio` option value must be between 0 and 1');
 
     // sanitizes path if string
-    const pathSegments = Array.isArray(name) ? name : [addSuffixToFilePath(name, '', undefined, true)];
+    const inputPathSegments = Array.isArray(name) ? name : [addSuffixToFilePath(name, '', undefined, true)];
-    this.snapshotPath = snapshotPathResolver(...pathSegments);
+    const outputPathSegments = Array.isArray(name) ? name : [addSuffixToFilePath(name, actualModifier, undefined, true)];
-    const outputFile = testInfo.outputPath(...pathSegments);
+    this.snapshotPath = snapshotPathResolver(...inputPathSegments);
-    this.expectedPath = addSuffixToFilePath(outputFile, '-expected');
+    const inputFile = testInfo.outputPath(...inputPathSegments);
+    const outputFile = testInfo.outputPath(...outputPathSegments);
+    this.expectedPath = addSuffixToFilePath(inputFile, '-expected');
     this.previousPath = addSuffixToFilePath(outputFile, '-previous');
     this.actualPath = addSuffixToFilePath(outputFile, '-actual');
     this.diffPath = addSuffixToFilePath(outputFile, '-diff');
@@ -115,7 +149,7 @@ class SnapshotHelper<T extends ImageComparatorOptions> {
   }
 
   decorateTitle(result: SyncExpectationResult): SyncExpectationResult & { titleSuffix: string } {
-    return { ...result, titleSuffix: `(${path.basename(this.snapshotPath)})` };
+    return { ...result, titleSuffix: `(${path.basename(this.snapshotName)})` };
   }
 
   handleMissingNegated() {
@@ -183,22 +217,22 @@ class SnapshotHelper<T extends ImageComparatorOptions> {
 
     if (expected !== undefined) {
       writeFileSync(this.expectedPath, expected);
-      this.testInfo.attachments.push({ name: path.basename(this.expectedPath), contentType: this.mimeType, path: this.expectedPath });
+      this.testInfo.attachments.push({ name: addSuffixToFilePath(this.snapshotName, '-expected'), contentType: this.mimeType, path: this.expectedPath });
       output.push(`Expected: ${colors.yellow(this.expectedPath)}`);
     }
     if (previous !== undefined) {
       writeFileSync(this.previousPath, previous);
-      this.testInfo.attachments.push({ name: path.basename(this.previousPath), contentType: this.mimeType, path: this.previousPath });
+      this.testInfo.attachments.push({ name: addSuffixToFilePath(this.snapshotName, '-previous'), contentType: this.mimeType, path: this.previousPath });
       output.push(`Previous: ${colors.yellow(this.previousPath)}`);
     }
     if (actual !== undefined) {
       writeFileSync(this.actualPath, actual);
-      this.testInfo.attachments.push({ name: path.basename(this.actualPath), contentType: this.mimeType, path: this.actualPath });
+      this.testInfo.attachments.push({ name: addSuffixToFilePath(this.snapshotName, '-actual'), contentType: this.mimeType, path: this.actualPath });
       output.push(`Received: ${colors.yellow(this.actualPath)}`);
     }
     if (diff !== undefined) {
       writeFileSync(this.diffPath, diff);
-      this.testInfo.attachments.push({ name: path.basename(this.diffPath), contentType: this.mimeType, path: this.diffPath });
+      this.testInfo.attachments.push({ name: addSuffixToFilePath(this.snapshotName, '-diff'), contentType: this.mimeType, path: this.diffPath });
       output.push(`    Diff: ${colors.yellow(this.diffPath)}`);
     }
     return this.decorateTitle({ pass: false, message: () => output.join('\n'), });

tests/playwright-test/golden.spec.ts

@@ -774,17 +774,17 @@ test('should attach expected/actual/diff with snapshot path', async ({ runInline
     attachment.path = attachment.path.replace(/\\/g, '/').replace(/.*test-results\//, '');
   expect(attachments).toEqual([
     {
-      name: 'snapshot-expected.png',
+      name: 'test/path/snapshot-expected.png',
       contentType: 'image/png',
       path: 'a-is-a-test/test/path/snapshot-expected.png'
     },
     {
-      name: 'snapshot-actual.png',
+      name: 'test/path/snapshot-actual.png',
       contentType: 'image/png',
       path: 'a-is-a-test/test/path/snapshot-actual.png'
     },
     {
-      name: 'snapshot-diff.png',
+      name: 'test/path/snapshot-diff.png',
       contentType: 'image/png',
       path: 'a-is-a-test/test/path/snapshot-diff.png'
     }

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

@@ -149,9 +149,9 @@ test('should include image diff', async ({ runInlineTest, page, showReport }) =>
   const image = imageDiff.locator('img');
   await expect(image).toHaveAttribute('src', /.*png/);
   const actualSrc = await image.getAttribute('src');
-  await imageDiff.locator('text=Expected').click();
+  await imageDiff.locator('text="Expected"').click();
   const expectedSrc = await image.getAttribute('src');
-  await imageDiff.locator('text=Diff').click();
+  await imageDiff.locator('text="Diff"').click();
   const diffSrc = await image.getAttribute('src');
   const set = new Set([expectedSrc, actualSrc, diffSrc]);
   expect(set.size).toBe(3);
@@ -198,6 +198,39 @@ test('should include multiple image diffs', async ({ runInlineTest, page, showRe
   }
 });
 
+test('should include image diffs for same expectation', async ({ runInlineTest, page, showReport }) => {
+  const expected = Buffer.from('iVBORw0KGgoAAAANSUhEUgAAAMgAAADICAYAAACtWK6eAAAAAXNSR0IArs4c6QAAAhVJREFUeJzt07ERwCAQwLCQ/Xd+FuDcQiFN4MZrZuYDjv7bAfAyg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAgEg0AwCASDQDAIBINAMAiEDVPZBYx6ffy+AAAAAElFTkSuQmCC', 'base64');
+  const result = await runInlineTest({
+    'playwright.config.ts': `
+      module.exports = { use: { viewport: { width: 200, height: 200 }} };
+    `,
+    'a.test.js-snapshots/expected-linux.png': expected,
+    'a.test.js-snapshots/expected-darwin.png': expected,
+    'a.test.js-snapshots/expected-win32.png': expected,
+    'a.test.js': `
+      const { test } = pwt;
+      test('fails', async ({ page }, testInfo) => {
+        await page.setContent('<html>Hello World</html>');
+        const screenshot = await page.screenshot();
+        await expect.soft(screenshot).toMatchSnapshot('expected.png');
+        await expect.soft(screenshot).toMatchSnapshot('expected.png');
+        await expect.soft(screenshot).toMatchSnapshot('expected.png');
+      });
+    `,
+  }, { reporter: 'dot,html' }, { PW_TEST_HTML_REPORT_OPEN: 'never' });
+  expect(result.exitCode).toBe(1);
+  expect(result.failed).toBe(1);
+
+  await showReport();
+  await page.click('text=fails');
+  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',
+  ]);
+});
+
 test('should include image diff when screenshot failed to generate due to animation', async ({ runInlineTest, page, showReport }) => {
   const result = await runInlineTest({
     'playwright.config.ts': `
@@ -228,9 +261,9 @@ test('should include image diff when screenshot failed to generate due to animat
   const image = imageDiff.locator('img');
   await expect(image).toHaveAttribute('src', /.*png/);
   const actualSrc = await image.getAttribute('src');
-  await imageDiff.locator('text=Previous').click();
+  await imageDiff.locator('text="Previous"').click();
   const previousSrc = await image.getAttribute('src');
-  await imageDiff.locator('text=Diff').click();
+  await imageDiff.locator('text="Diff"').click();
   const diffSrc = await image.getAttribute('src');
   const set = new Set([previousSrc, actualSrc, diffSrc]);
   expect(set.size).toBe(3);