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')
+})