fix(vitest): fix file snapshots in skipped suites considered obsolete (#4795)

Author: hi-ogawa

packages/vitest/src/runtime/runners/test.ts

@@ -3,7 +3,7 @@ import type { ExpectStatic } from '@vitest/expect'
 import { GLOBAL_EXPECT, getState, setState } from '@vitest/expect'
 import { getSnapshotClient } from '../../integrations/snapshot/chai'
 import { vi } from '../../integrations/vi'
-import { getFullName, getNames, getWorkerState } from '../../utils'
+import { getFullName, getNames, getTests, getWorkerState } from '../../utils'
 import { createExpect } from '../../integrations/chai/index'
 import type { ResolvedConfig } from '../../types/config'
 import type { VitestExecutor } from '../execute'
@@ -32,6 +32,14 @@ export class VitestTestRunner implements VitestRunner {
       suite.result!.heap = process.memoryUsage().heapUsed
 
     if (suite.mode !== 'skip' && typeof suite.filepath !== 'undefined') {
+      // mark snapshots in skipped tests as not obsolete
+      for (const test of getTests(suite)) {
+        if (test.mode === 'skip') {
+          const name = getNames(test).slice(1).join(' > ')
+          this.snapshotClient.skipTestSnapshots(name)
+        }
+      }
+
       const result = await this.snapshotClient.finishCurrentRun()
       if (result)
         await rpc().snapshotSaved(result)
@@ -52,15 +60,11 @@ export class VitestTestRunner implements VitestRunner {
   }
 
   async onBeforeRunTask(test: Test) {
-    const name = getNames(test).slice(1).join(' > ')
-
     if (this.cancelRun)
       test.mode = 'skip'
 
-    if (test.mode !== 'run') {
-      this.snapshotClient.skipTestSnapshots(name)
+    if (test.mode !== 'run')
       return
-    }
 
     clearModuleMocks(this.config)
 

test/snapshots/test/fixtures/skip-test/__snapshots__/repro.test.ts.snap

@@ -0,0 +1,5 @@
+// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
+
+exports[`repro suite > inner case 1`] = `"hi-1"`;
+
+exports[`top-level case 1`] = `"hi-2"`;

test/snapshots/test/fixtures/skip-test/repro.test.ts

@@ -0,0 +1,20 @@
+import { describe, expect, it } from 'vitest'
+
+const ENABLE_SKIP = process.env.ENABLE_SKIP;
+
+describe.skipIf(ENABLE_SKIP)('repro suite', () => {
+  it('inner case', () => {
+    expect('hi-1').toMatchSnapshot()
+  })
+})
+
+it.skipIf(ENABLE_SKIP)('top-level case', () => {
+  expect('hi-2').toMatchSnapshot()
+})
+
+// at least one non-skipped test is needed to reproduce a bug.
+// without this, there will be no SnapshotClient.startCurrentRun,
+// so the code to check skip/obsolete snapshot is not exercised.
+it('normal case', () => {
+  expect(0).toBe(0)
+})

test/snapshots/test/fixtures/skip-test/vitest.config.ts

@@ -0,0 +1,3 @@
+import { defineConfig } from 'vitest/config'
+
+export default defineConfig({})

test/snapshots/test/skip-test.test.ts

@@ -0,0 +1,48 @@
+import fs from 'node:fs'
+import { expect, test } from 'vitest'
+import { runVitest } from '../../test-utils'
+
+test('snapshots in skipped test/suite is not obsolete', async () => {
+  // create snapshot on first run
+  fs.rmSync('test/fixtures/skip-test/__snapshots__', { recursive: true, force: true })
+  let vitest = await runVitest({
+    root: 'test/fixtures/skip-test',
+    update: true,
+  })
+  expect(vitest.stdout).toContain('Snapshots  2 written')
+  expect(fs.readFileSync('test/fixtures/skip-test/__snapshots__/repro.test.ts.snap', 'utf-8')).toMatchInlineSnapshot(`
+    "// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
+
+    exports[\`repro suite > inner case 1\`] = \`"hi-1"\`;
+
+    exports[\`top-level case 1\`] = \`"hi-2"\`;
+    "
+  `)
+
+  // running with `skipIf` enabled should not show "obsolete"
+  vitest = await runVitest({
+    root: 'test/fixtures/skip-test',
+    env: {
+      ENABLE_SKIP: '1',
+    },
+  })
+  expect(vitest.stdout).toContain('2 skipped')
+  expect(vitest.stdout).not.toContain('obsolete')
+
+  // running with `skipIf` and `update` should keep snapshots
+  vitest = await runVitest({
+    root: 'test/fixtures/skip-test',
+    update: true,
+    env: {
+      ENABLE_SKIP: '1',
+    },
+  })
+  expect(fs.readFileSync('test/fixtures/skip-test/__snapshots__/repro.test.ts.snap', 'utf-8')).toMatchInlineSnapshot(`
+    "// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
+
+    exports[\`repro suite > inner case 1\`] = \`"hi-1"\`;
+
+    exports[\`top-level case 1\`] = \`"hi-2"\`;
+    "
+  `)
+})

test/snapshots/vitest.config.ts

@@ -1,8 +1,10 @@
 import { defineConfig } from 'vite'
+import { defaultExclude } from 'vitest/config'
 
 export default defineConfig({
   test: {
     globals: true,
+    exclude: [...defaultExclude, '**/fixtures'],
     snapshotFormat: {
       printBasicPrototype: true,
     },