fix(toHaveScreenshot): nice error when rebasing with bad expectation (microsoft#37243)

Author: dgozman

packages/playwright-core/src/server/utils/comparators.ts

@@ -98,11 +98,11 @@ function validateBuffer(buffer: Buffer, mimeType: string): void {
   if (mimeType === 'image/png') {
     const pngMagicNumber = [137, 80, 78, 71, 13, 10, 26, 10];
     if (buffer.length < pngMagicNumber.length || !pngMagicNumber.every((byte, index) => buffer[index] === byte))
-      throw new Error('could not decode image as PNG.');
+      throw new Error('Could not decode expected image as PNG.');
   } else if (mimeType === 'image/jpeg') {
     const jpegMagicNumber = [255, 216];
     if (buffer.length < jpegMagicNumber.length || !jpegMagicNumber.every((byte, index) => buffer[index] === byte))
-      throw new Error('could not decode image as JPEG.');
+      throw new Error('Could not decode expected image as JPEG.');
   }
 }
 

packages/playwright/src/matchers/toMatchSnapshot.ts

@@ -398,9 +398,9 @@ export async function toHaveScreenshot(
   expectScreenshotOptions.expected = helper.updateSnapshots === 'all' ? undefined : expected;
 
   const { actual, previous, diff, errorMessage, log, timedOut } = await page._expectScreenshot(expectScreenshotOptions);
-  const writeFiles = () => {
-    writeFileSync(helper.expectedPath, actual!);
-    writeFileSync(helper.actualPath, actual!);
+  const writeFiles = (actualBuffer: Buffer) => {
+    writeFileSync(helper.expectedPath, actualBuffer);
+    writeFileSync(helper.actualPath, actualBuffer);
     /* eslint-disable no-console */
     console.log(helper.expectedPath + ' is re-generated, writing actual.');
     return helper.createMatcherResult(helper.expectedPath + ' running with --update-snapshots, writing actual.', true);
@@ -410,13 +410,18 @@ export async function toHaveScreenshot(
     // Screenshot is matching, but is not necessarily the same as the expected.
     if (helper.updateSnapshots === 'all' && actual && compareBuffersOrStrings(actual, expected)) {
       console.log(helper.expectedPath + ' is re-generated, writing actual.');
-      return writeFiles();
+      return writeFiles(actual);
     }
     return helper.handleMatching();
   }
 
-  if (helper.updateSnapshots === 'changed' || helper.updateSnapshots === 'all')
-    return writeFiles();
+  if (helper.updateSnapshots === 'changed' || helper.updateSnapshots === 'all') {
+    if (actual)
+      return writeFiles(actual);
+    let header = matcherHint(this, undefined, 'toHaveScreenshot', receiver, undefined, undefined, timedOut ? timeout : undefined);
+    header += '  Failed to re-generate expected.\n';
+    return helper.handleDifferent(actual, expectScreenshotOptions.expected, previous, diff, header, errorMessage, log, this._stepInfo);
+  }
 
   const header = matcherHint(this, undefined, 'toHaveScreenshot', receiver, undefined, undefined, timedOut ? timeout : undefined);
   return helper.handleDifferent(actual, expectScreenshotOptions.expected, previous, diff, header, errorMessage, log, this._stepInfo);

tests/playwright-test/to-have-screenshot.spec.ts

@@ -1350,8 +1350,26 @@ test('should throw pretty error if expected PNG file is not a PNG', async ({ run
     `,
   });
   expect(result.exitCode).toBe(1);
-  expect(result.output).toContain('could not decode image as PNG.');
-  expect(result.output).toContain('could not decode image as JPEG.');
+  expect(result.output).toContain('Could not decode expected image as PNG.');
+  expect(result.output).toContain('Could not decode expected image as JPEG.');
+});
+
+test('should throw pretty error if expected PNG file is not a PNG while rebasing', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    ...playwrightConfig({
+      snapshotPathTemplate: '__screenshots__/{testFilePath}/{arg}{ext}',
+    }),
+    '__screenshots__/a.spec.js/snapshot.png': 'not a png',
+    'a.spec.js': `
+      const { test, expect } = require('@playwright/test');
+      test('png', async ({ page }) => {
+        await expect(page).toHaveScreenshot('snapshot.png');
+      });
+    `,
+  }, { 'update-snapshots': true });
+  expect(result.exitCode).toBe(1);
+  expect(result.output).toContain('Failed to re-generate expected.');
+  expect(result.output).toContain('Could not decode expected image as PNG.');
 });
 
 test('should support maskColor option', async ({ runInlineTest }) => {