fix(browser): avoid updating screenshots when toMatchScreenshot passes (#9289)

Author: macarie

packages/browser/src/node/commands/screenshotMatcher/index.ts

@@ -148,7 +148,7 @@ async function determineOutcome(
   }
 
   // no reference to compare against - create one based on update settings
-  if (reference === null || updateSnapshot === 'all') {
+  if (reference === null) {
     if (updateSnapshot === 'all') {
       return {
         type: 'update-reference',
@@ -190,6 +190,16 @@ async function determineOutcome(
     return { type: 'matched-after-comparison' }
   }
 
+  if (updateSnapshot === 'all') {
+    return {
+      type: 'update-reference',
+      reference: {
+        image: screenshot,
+        path: paths.reference,
+      },
+    }
+  }
+
   return {
     type: 'mismatch',
     reference: {

test/browser/specs/to-match-screenshot.test.ts

@@ -99,53 +99,6 @@ describe('--watch', () => {
     },
   )
 
-  test(
-    'with --update creates snapshots and updates them on change',
-    async () => {
-      const { fs, stderr, vitest } = await runInlineTests(
-        {
-          [testFilename]: testContent,
-          'utils.ts': utilsContent,
-        },
-        {
-          update: true,
-        },
-      )
-
-      expect(stderr).toMatchInlineSnapshot(`""`)
-
-      const referencePath = `__screenshots__/${testFilename}/${testName}-1-${browser}-${platform()}.png`
-      const referenceStat = fs.statFile(referencePath)
-
-      fs.editFile(testFilename, content => `${content}\n`)
-
-      vitest.resetOutput()
-      await vitest.waitForStdout('Test Files  1 passed')
-
-      expect(vitest.stdout).toContain('✓ |chromium| basic.test.ts > screenshot-snapshot')
-
-      const {
-        atime,
-        atimeMs,
-        ctime,
-        ctimeMs,
-        mtime,
-        mtimeMs,
-        ...diffs
-      } = fs.statFile(referencePath)
-
-      expect(referenceStat).toEqual(expect.objectContaining(diffs))
-
-      expect(atime.getTime()).toBeGreaterThan(referenceStat.atime.getTime())
-      expect(ctime.getTime()).toBeGreaterThan(referenceStat.ctime.getTime())
-      expect(mtime.getTime()).toBeGreaterThan(referenceStat.mtime.getTime())
-
-      expect(atimeMs).toBeGreaterThan(referenceStat.atimeMs)
-      expect(ctimeMs).toBeGreaterThan(referenceStat.ctimeMs)
-      expect(mtimeMs).toBeGreaterThan(referenceStat.mtimeMs)
-    },
-  )
-
   test(
     'creates a reference and fails when changing the DOM content',
     async () => {
@@ -168,4 +121,104 @@ describe('--watch', () => {
       expect(vitest.stdout).toMatch(/\d+ pixels \(ratio 0.\d{2}\) differ\./)
     },
   )
+
+  describe('--update', () => {
+    test(
+      'creates snapshot and does NOT update it if reference matches',
+      async () => {
+        const { fs, stderr, vitest } = await runInlineTests(
+          {
+            [testFilename]: testContent,
+            'utils.ts': utilsContent,
+          },
+          {
+            update: true,
+          },
+        )
+
+        expect(stderr).toMatchInlineSnapshot(`""`)
+
+        const osPlatform = platform()
+        const referencePath = `__screenshots__/${testFilename}/${testName}-1-${browser}-${osPlatform}.png`
+        const referenceStat = fs.statFile(referencePath)
+
+        fs.editFile(testFilename, content => `${content}\n`)
+
+        vitest.resetOutput()
+        await vitest.waitForStdout('Test Files  1 passed')
+
+        expect(vitest.stdout).toContain('✓ |chromium| basic.test.ts > screenshot-snapshot')
+
+        // only atime should change since reference should NOT be updated
+
+        const {
+          atime,
+          atimeMs,
+          ...diffs
+        } = fs.statFile(referencePath)
+
+        expect(referenceStat).toEqual(expect.objectContaining(diffs))
+
+        // win32 does not update `atime` by default
+        if (osPlatform === 'win32') {
+          expect(atime.getTime()).toEqual(referenceStat.atime.getTime())
+          expect(atimeMs).toEqual(referenceStat.atimeMs)
+        }
+        else {
+          expect(atime.getTime()).toBeGreaterThan(referenceStat.atime.getTime())
+          expect(atimeMs).toBeGreaterThan(referenceStat.atimeMs)
+        }
+      },
+    )
+
+    test(
+      'creates snapshot and updates it if reference mismatches',
+      async () => {
+        const { fs, stderr, vitest } = await runInlineTests(
+          {
+            [testFilename]: testContent,
+            'utils.ts': utilsContent,
+          },
+          {
+            update: true,
+          },
+        )
+
+        expect(stderr).toMatchInlineSnapshot(`""`)
+
+        const referencePath = `__screenshots__/${testFilename}/${testName}-1-${browser}-${platform()}.png`
+        const referenceStat = fs.statFile(referencePath)
+
+        fs.editFile(testFilename, content => content.replace(bgColor, '#000'))
+
+        vitest.resetOutput()
+        await vitest.waitForStdout('Test Files  1 passed')
+
+        expect(vitest.stdout).toContain('✓ |chromium| basic.test.ts > screenshot-snapshot')
+
+        // atime, ctime, mtime, and size should change since reference should be updated
+
+        const {
+          atime,
+          atimeMs,
+          ctime,
+          ctimeMs,
+          mtime,
+          mtimeMs,
+          size,
+          ...diffs
+        } = fs.statFile(referencePath)
+
+        expect(referenceStat).toEqual(expect.objectContaining(diffs))
+
+        expect(atime.getTime()).toBeGreaterThan(referenceStat.atime.getTime())
+        expect(ctime.getTime()).toBeGreaterThan(referenceStat.ctime.getTime())
+        expect(mtime.getTime()).toBeGreaterThan(referenceStat.mtime.getTime())
+
+        expect(atimeMs).toBeGreaterThan(referenceStat.atimeMs)
+        expect(ctimeMs).toBeGreaterThan(referenceStat.ctimeMs)
+        expect(mtimeMs).toBeGreaterThan(referenceStat.mtimeMs)
+      },
+    )
+  })
 })