From 98945a81a8f6a843a65a194509b1f89950df3509 Mon Sep 17 00:00:00 2001
From: Andrey Lushnikov <aslushnikov@gmail.com>
Date: Mon, 9 May 2022 08:34:53 -0600
Subject: [PATCH] feat: move `toHaveScreenshot` to use old snapshot paths by
 default (#14006)

Note: all toHaveScreenshot tests still use `__screenshots__` directory
for their expectations. One more test was added to make sure that
by default, `toHaveScreenshot` uses old snapshots.
---
 .../src/matchers/toMatchSnapshot.ts           |  5 ++-
 tests/playwright-test/reporter-html.spec.ts   |  2 +
 .../to-have-screenshot.spec.ts                | 44 ++++++++++++++-----
 3 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/packages/playwright-test/src/matchers/toMatchSnapshot.ts b/packages/playwright-test/src/matchers/toMatchSnapshot.ts
index 8f92ccebeb..ba01e004c5 100644
--- a/packages/playwright-test/src/matchers/toMatchSnapshot.ts
+++ b/packages/playwright-test/src/matchers/toMatchSnapshot.ts
@@ -295,8 +295,11 @@ export async function toHaveScreenshot(
   if (!testInfo)
     throw new Error(`toHaveScreenshot() must be called during the test`);
   const config = (testInfo.project._expect as any)?.toHaveScreenshot;
+  const snapshotPathResolver = process.env.PLAYWRIGHT_EXPERIMENTAL_FEATURES && config?._useScreenshotsDir
+    ? testInfo._screenshotPath.bind(testInfo)
+    : testInfo.snapshotPath.bind(testInfo);
   const helper = new SnapshotHelper(
-      testInfo, testInfo._screenshotPath.bind(testInfo), 'png',
+      testInfo, snapshotPathResolver, 'png',
       {
         maxDiffPixels: config?.maxDiffPixels,
         maxDiffPixelRatio: config?.maxDiffPixelRatio,
diff --git a/tests/playwright-test/reporter-html.spec.ts b/tests/playwright-test/reporter-html.spec.ts
index b865cf38c7..ecbc0593a9 100644
--- a/tests/playwright-test/reporter-html.spec.ts
+++ b/tests/playwright-test/reporter-html.spec.ts
@@ -180,7 +180,9 @@ test('should include multiple image diffs', async ({ runInlineTest, page, showRe
 
   const result = await runInlineTest({
     'playwright.config.ts': `
+      process.env.PLAYWRIGHT_EXPERIMENTAL_FEATURES = 1;
       module.exports = {
+        expect: { toHaveScreenshot: { _useScreenshotsDir: true } },
         screenshotsDir: '__screenshots__',
         use: { viewport: { width: ${IMG_WIDTH}, height: ${IMG_HEIGHT} }}
       };
diff --git a/tests/playwright-test/to-have-screenshot.spec.ts b/tests/playwright-test/to-have-screenshot.spec.ts
index b04af58cae..194a204a60 100644
--- a/tests/playwright-test/to-have-screenshot.spec.ts
+++ b/tests/playwright-test/to-have-screenshot.spec.ts
@@ -97,6 +97,27 @@ test('should fail with proper error when unsupported argument is given', async (
   expect(stripAnsi(result.output)).toContain(`Expected options.clip.width not to be 0`);
 });
 
+test('should use match snapshot paths by default', async ({ runInlineTest }, testInfo) => {
+  const result = await runInlineTest({
+    // The helper function `playwrightConfig` injects a `_useScreenshotsDir` flag.
+    // Provide default config manually instead.
+    'playwright.config.js': `
+      process.env.PLAYWRIGHT_EXPERIMENTAL_FEATURES = '1';
+      module.exports = {};
+    `,
+    'a.spec.js': `
+      pwt.test('is a test', async ({ page }, testInfo) => {
+        testInfo.snapshotSuffix = '';
+        await expect(page).toHaveScreenshot('snapshot.png');
+      });
+    `
+  }, { 'update-snapshots': true });
+  expect(result.exitCode).toBe(0);
+
+  const snapshotOutputPath = testInfo.outputPath('a.spec.js-snapshots', 'snapshot.png');
+  expect(fs.existsSync(snapshotOutputPath)).toBe(true);
+});
+
 test('should have scale:css by default', async ({ runInlineTest }, testInfo) => {
   const result = await runInlineTest({
     ...playwrightConfig({ screenshotsDir: '__screenshots__' }),
@@ -692,14 +713,14 @@ test('should respect maxDiffPixels option', async ({ runInlineTest }) => {
 
   expect((await runInlineTest({
     ...playwrightConfig({
+      expect: {
+        toHaveScreenshot: {
+          maxDiffPixels: BAD_PIXELS
+        }
+      },
       projects: [
         {
           screenshotsDir: '__screenshots__',
-          expect: {
-            toHaveScreenshot: {
-              maxDiffPixels: BAD_PIXELS
-            }
-          },
         },
       ],
     }),
@@ -798,13 +819,13 @@ test('should respect maxDiffPixelRatio option', async ({ runInlineTest }) => {
 
   expect((await runInlineTest({
     ...playwrightConfig({
+      expect: {
+        toHaveScreenshot: {
+          maxDiffPixelRatio: BAD_RATIO,
+        },
+      },
       projects: [{
         screenshotsDir: '__screenshots__',
-        expect: {
-          toHaveScreenshot: {
-            maxDiffPixelRatio: BAD_RATIO,
-          },
-        },
       }],
     }),
     '__screenshots__/a.spec.js/snapshot.png': EXPECTED_SNAPSHOT,
@@ -920,6 +941,9 @@ test('should update expectations with retries', async ({ runInlineTest }, testIn
 });
 
 function playwrightConfig(obj: any) {
+  obj.expect ??= {};
+  obj.expect.toHaveScreenshot ??= {};
+  obj.expect.toHaveScreenshot._useScreenshotsDir ??= true;
   return {
     'playwright.config.js': `
       process.env.PLAYWRIGHT_EXPERIMENTAL_FEATURES = '1';