fix(browser): resolved failure to find arbitrarily-named snapshot files when using expect(...).toMatchFileSnapshot() matcher. (#4839)

Co-authored-by: Zac Mullett <zac@Zacs-Air.modem>
Co-authored-by: Vladimir <sleuths.slews0s@icloud.com>

Author: 3 people

packages/snapshot/src/manager.ts

@@ -3,7 +3,6 @@ import type { SnapshotResult, SnapshotStateOptions, SnapshotSummary } from './ty
 
 export class SnapshotManager {
   summary: SnapshotSummary = undefined!
-  resolvedPaths = new Set<string>()
   extension = '.snap'
 
   constructor(public options: Omit<SnapshotStateOptions, 'snapshotEnvironment'>) {
@@ -30,7 +29,6 @@ export class SnapshotManager {
     })
 
     const path = resolver(testPath, this.extension)
-    this.resolvedPaths.add(path)
     return path
   }
 

packages/vitest/src/api/setup.ts

@@ -6,24 +6,27 @@ import { createBirpc } from 'birpc'
 import { parse, stringify } from 'flatted'
 import type { WebSocket } from 'ws'
 import { WebSocketServer } from 'ws'
+import { isFileServingAllowed } from 'vite'
 import type { ViteDevServer } from 'vite'
 import type { StackTraceParserOptions } from '@vitest/utils/source-map'
 import { API_PATH } from '../constants'
 import type { Vitest } from '../node'
 import type { File, ModuleGraphData, Reporter, TaskResultPack, UserConsoleLog } from '../types'
-import { getModuleGraph, isPrimitive } from '../utils'
+import { getModuleGraph, isPrimitive, stringifyReplace } from '../utils'
 import type { WorkspaceProject } from '../node/workspace'
 import { parseErrorStacktrace } from '../utils/source-map'
 import type { TransformResultWithSource, WebSocketEvents, WebSocketHandlers } from './types'
 
-export function setup(vitestOrWorkspace: Vitest | WorkspaceProject, server?: ViteDevServer) {
+export function setup(vitestOrWorkspace: Vitest | WorkspaceProject, _server?: ViteDevServer) {
   const ctx = 'ctx' in vitestOrWorkspace ? vitestOrWorkspace.ctx : vitestOrWorkspace
 
   const wss = new WebSocketServer({ noServer: true })
 
   const clients = new Map<WebSocket, BirpcReturn<WebSocketEvents, WebSocketHandlers>>()
 
-  ;(server || ctx.server).httpServer?.on('upgrade', (request, socket, head) => {
+  const server = _server || ctx.server
+
+  server.httpServer?.on('upgrade', (request, socket, head) => {
     if (!request.url)
       return
 
@@ -37,6 +40,11 @@ export function setup(vitestOrWorkspace: Vitest | WorkspaceProject, server?: Vit
     })
   })
 
+  function checkFileAccess(path: string) {
+    if (!isFileServingAllowed(path, server))
+      throw new Error(`Access denied to "${path}". See Vite config documentation for "server.fs": https://vitejs.dev/config/server-options.html#server-fs-strict.`)
+  }
+
   function setupClient(ws: WebSocket) {
     const rpc = createBirpc<WebSocketEvents, WebSocketHandlers>(
       {
@@ -73,7 +81,8 @@ export function setup(vitestOrWorkspace: Vitest | WorkspaceProject, server?: Vit
           return ctx.snapshot.resolveRawPath(testPath, rawPath)
         },
         async readSnapshotFile(snapshotPath) {
-          if (!ctx.snapshot.resolvedPaths.has(snapshotPath) || !existsSync(snapshotPath))
+          checkFileAccess(snapshotPath)
+          if (!existsSync(snapshotPath))
             return null
           return fs.readFile(snapshotPath, 'utf-8')
         },
@@ -88,13 +97,13 @@ export function setup(vitestOrWorkspace: Vitest | WorkspaceProject, server?: Vit
           return fs.writeFile(id, content, 'utf-8')
         },
         async saveSnapshotFile(id, content) {
-          if (!ctx.snapshot.resolvedPaths.has(id))
-            throw new Error(`Snapshot file "${id}" does not exist.`)
+          checkFileAccess(id)
           await fs.mkdir(dirname(id), { recursive: true })
           return fs.writeFile(id, content, 'utf-8')
         },
         async removeSnapshotFile(id) {
-          if (!ctx.snapshot.resolvedPaths.has(id) || !existsSync(id))
+          checkFileAccess(id)
+          if (!existsSync(id))
             throw new Error(`Snapshot file "${id}" does not exist.`)
           return fs.unlink(id)
         },
@@ -140,7 +149,7 @@ export function setup(vitestOrWorkspace: Vitest | WorkspaceProject, server?: Vit
         post: msg => ws.send(msg),
         on: fn => ws.on('message', fn),
         eventNames: ['onUserConsoleLog', 'onFinished', 'onCollected', 'onCancel'],
-        serialize: stringify,
+        serialize: (data: any) => stringify(data, stringifyReplace),
         deserialize: parse,
       },
     )

packages/vitest/src/utils/index.ts

@@ -9,6 +9,7 @@ export * from './global'
 export * from './timers'
 export * from './env'
 export * from './modules'
+export * from './serialization'
 
 export const isWindows = isNode && process.platform === 'win32'
 export function getRunMode() {

packages/vitest/src/utils/serialization.ts

@@ -0,0 +1,22 @@
+// Serialization support utils.
+
+function cloneByOwnProperties(value: any) {
+  // Clones the value's properties into a new Object. The simpler approach of
+  // Object.assign() won't work in the case that properties are not enumerable.
+  return Object.getOwnPropertyNames(value)
+    .reduce((clone, prop) => ({
+      ...clone,
+      [prop]: value[prop],
+    }), {})
+}
+
+/**
+ * Replacer function for serialization methods such as JS.stringify() or
+ * flatted.stringify().
+ */
+export function stringifyReplace(key: string, value: any) {
+  if (value instanceof Error)
+    return cloneByOwnProperties(value)
+  else
+    return value
+}

test/benchmark/package.json

@@ -3,7 +3,7 @@
   "type": "module",
   "private": true,
   "scripts": {
-    "test": "node --test specs/ && echo '1'",
+    "test": "node --test specs/* && echo '1'",
     "bench:json": "vitest bench --reporter=json",
     "bench": "vitest bench"
   },

test/browser/specs/runner.test.mjs

@@ -28,9 +28,9 @@ const passedTests = getPassed(browserResultJson.testResults)
 const failedTests = getFailed(browserResultJson.testResults)
 
 await test('tests are actually running', async () => {
-  assert.ok(browserResultJson.testResults.length === 9, 'Not all the tests have been run')
+  assert.ok(browserResultJson.testResults.length === 10, 'Not all the tests have been run')
   assert.ok(passedTests.length === 8, 'Some tests failed')
-  assert.ok(failedTests.length === 1, 'Some tests have passed but should fail')
+  assert.ok(failedTests.length === 2, 'Some tests have passed but should fail')
 
   assert.doesNotMatch(stderr, /Unhandled Error/, 'doesn\'t have any unhandled errors')
 })
@@ -80,3 +80,7 @@ await test('popup apis should log a warning', () => {
   assert.ok(stderr.includes('Vitest encountered a \`confirm\("test"\)\`'), 'prints warning for confirm')
   assert.ok(stderr.includes('Vitest encountered a \`prompt\("test"\)\`'), 'prints warning for prompt')
 })
+
+await test('snapshot inaccessible file debuggability', () => {
+  assert.ok(stdout.includes('Access denied to "/inaccesible/path".'), 'file security enforcement explained')
+})

test/browser/test/__snapshots__/custom/my_snapshot

@@ -0,0 +1 @@
+my snapshot content

test/browser/test/__snapshots__/snapshot.test.ts.snap

@@ -1,3 +1,3 @@
 // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
 
-exports[`file snapshot 1`] = `1`;
+exports[`snapshot 1`] = `1`;

test/browser/test/failing.snaphot.test.ts

@@ -0,0 +1,6 @@
+import { expect, test } from 'vitest'
+
+test('file snapshot', async () => {
+  await expect('inaccessible snapshot content')
+    .toMatchFileSnapshot('/inaccesible/path')
+})

test/browser/test/snapshot.test.ts

@@ -4,6 +4,11 @@ test('inline snapshot', () => {
   expect(1).toMatchInlineSnapshot('1')
 })
 
-test('file snapshot', () => {
+test('snapshot', () => {
   expect(1).toMatchSnapshot()
 })
+
+test('file snapshot', async () => {
+  await expect('my snapshot content')
+    .toMatchFileSnapshot('./__snapshots__/custom/my_snapshot')
+})