Skip to content

Commit

Permalink
fix(browser): resolved failure to find arbitrarily-named snapshot fil…
Browse files Browse the repository at this point in the history
…es when using `expect(...).toMatchFileSnapshot()` matcher. (#4839)

Co-authored-by: Zac Mullett <zac@Zacs-Air.modem>
Co-authored-by: Vladimir <sleuths.slews0s@icloud.com>
  • Loading branch information
3 people committed Jan 4, 2024
1 parent 345a25d commit b8140fc
Show file tree
Hide file tree
Showing 10 changed files with 61 additions and 15 deletions.
2 changes: 0 additions & 2 deletions packages/snapshot/src/manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'>) {
Expand All @@ -30,7 +29,6 @@ export class SnapshotManager {
})

const path = resolver(testPath, this.extension)
this.resolvedPaths.add(path)
return path
}

Expand Down
25 changes: 17 additions & 8 deletions packages/vitest/src/api/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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>(
{
Expand Down Expand Up @@ -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')
},
Expand All @@ -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)
},
Expand Down Expand Up @@ -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,
},
)
Expand Down
1 change: 1 addition & 0 deletions packages/vitest/src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
22 changes: 22 additions & 0 deletions packages/vitest/src/utils/serialization.ts
Original file line number Diff line number Diff line change
@@ -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
}
2 changes: 1 addition & 1 deletion test/benchmark/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
8 changes: 6 additions & 2 deletions test/browser/specs/runner.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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')
})
Expand Down Expand Up @@ -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')
})
1 change: 1 addition & 0 deletions test/browser/test/__snapshots__/custom/my_snapshot
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
my snapshot content
2 changes: 1 addition & 1 deletion test/browser/test/__snapshots__/snapshot.test.ts.snap
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html

exports[`file snapshot 1`] = `1`;
exports[`snapshot 1`] = `1`;
6 changes: 6 additions & 0 deletions test/browser/test/failing.snaphot.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { expect, test } from 'vitest'

test('file snapshot', async () => {
await expect('inaccessible snapshot content')
.toMatchFileSnapshot('/inaccesible/path')
})
7 changes: 6 additions & 1 deletion test/browser/test/snapshot.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
})

0 comments on commit b8140fc

Please sign in to comment.