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

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

@@ -1,9 +1,9 @@
-import type { CancelReason, Custom, ExtendedContext, Suite, TaskContext, Test, VitestRunner, VitestRunnerImportSource } from '@vitest/runner'
+import type { CancelReason, Custom, ExtendedContext, File, Suite, TaskContext, Test, VitestRunner, VitestRunnerImportSource } from '@vitest/runner'
 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'
@@ -27,7 +27,17 @@ export class VitestTestRunner implements VitestRunner {
     this.snapshotClient.clear()
   }
 
-  async onAfterRunFiles() {
+  async onAfterRunFiles(files?: File[]) {
+    // mark snapshots in skipped tests as not obsolete
+    // TODO: this probably doesn't work when `VitestTestRunner` is handling multiple files concurrently,
+    //       but `snapshotClient.startCurrentRun/finishCurrentRun` doesn't work in that case already?
+    for (const test of getTests(files ?? [])) {
+      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)
@@ -57,10 +67,8 @@ export class VitestTestRunner implements VitestRunner {
     if (this.cancelRun)
       test.mode = 'skip'
 
-    if (test.mode !== 'run') {
-      this.snapshotClient.skipTestSnapshots(name)
+    if (test.mode !== 'run')
       return
-    }
 
     clearModuleMocks(this.config)
     await this.snapshotClient.startCurrentRun(test.file!.filepath, name, this.workerState.config.snapshotOptions)

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,19 @@
+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()
+})
+
+// requires at least one non-skipped test to trigger
+// `SnapshotClient.startCurrentRun` on current file
+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,
     },