From b8140fcadc114fd2d05ca997da4f7aa4e226417d Mon Sep 17 00:00:00 2001 From: Zac Mullett Date: Thu, 4 Jan 2024 21:53:33 +1030 Subject: [PATCH] fix(browser): resolved failure to find arbitrarily-named snapshot files when using `expect(...).toMatchFileSnapshot()` matcher. (#4839) Co-authored-by: Zac Mullett Co-authored-by: Vladimir --- packages/snapshot/src/manager.ts | 2 -- packages/vitest/src/api/setup.ts | 25 +++++++++++++------ packages/vitest/src/utils/index.ts | 1 + packages/vitest/src/utils/serialization.ts | 22 ++++++++++++++++ test/benchmark/package.json | 2 +- test/browser/specs/runner.test.mjs | 8 ++++-- .../test/__snapshots__/custom/my_snapshot | 1 + .../test/__snapshots__/snapshot.test.ts.snap | 2 +- test/browser/test/failing.snaphot.test.ts | 6 +++++ test/browser/test/snapshot.test.ts | 7 +++++- 10 files changed, 61 insertions(+), 15 deletions(-) create mode 100644 packages/vitest/src/utils/serialization.ts create mode 100644 test/browser/test/__snapshots__/custom/my_snapshot create mode 100644 test/browser/test/failing.snaphot.test.ts diff --git a/packages/snapshot/src/manager.ts b/packages/snapshot/src/manager.ts index 941fd2eaf072..c01244e834b6 100644 --- a/packages/snapshot/src/manager.ts +++ b/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() extension = '.snap' constructor(public options: Omit) { @@ -30,7 +29,6 @@ export class SnapshotManager { }) const path = resolver(testPath, this.extension) - this.resolvedPaths.add(path) return path } diff --git a/packages/vitest/src/api/setup.ts b/packages/vitest/src/api/setup.ts index a0bd2b9a0b20..7f21120f7382 100644 --- a/packages/vitest/src/api/setup.ts +++ b/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>() - ;(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( { @@ -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, }, ) diff --git a/packages/vitest/src/utils/index.ts b/packages/vitest/src/utils/index.ts index 5464456d13b3..229d489adf57 100644 --- a/packages/vitest/src/utils/index.ts +++ b/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() { diff --git a/packages/vitest/src/utils/serialization.ts b/packages/vitest/src/utils/serialization.ts new file mode 100644 index 000000000000..d620c78f8c3a --- /dev/null +++ b/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 +} diff --git a/test/benchmark/package.json b/test/benchmark/package.json index a97c69e0cb2b..2e72a0df4901 100644 --- a/test/benchmark/package.json +++ b/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" }, diff --git a/test/browser/specs/runner.test.mjs b/test/browser/specs/runner.test.mjs index ed8fd9fd4dbe..556b749f2a14 100644 --- a/test/browser/specs/runner.test.mjs +++ b/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') +}) diff --git a/test/browser/test/__snapshots__/custom/my_snapshot b/test/browser/test/__snapshots__/custom/my_snapshot new file mode 100644 index 000000000000..ae046bd2438f --- /dev/null +++ b/test/browser/test/__snapshots__/custom/my_snapshot @@ -0,0 +1 @@ +my snapshot content \ No newline at end of file diff --git a/test/browser/test/__snapshots__/snapshot.test.ts.snap b/test/browser/test/__snapshots__/snapshot.test.ts.snap index 13300a736557..76ed625eab31 100644 --- a/test/browser/test/__snapshots__/snapshot.test.ts.snap +++ b/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`; diff --git a/test/browser/test/failing.snaphot.test.ts b/test/browser/test/failing.snaphot.test.ts new file mode 100644 index 000000000000..f843ae582202 --- /dev/null +++ b/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') +}) diff --git a/test/browser/test/snapshot.test.ts b/test/browser/test/snapshot.test.ts index ed80a694376c..8098a578776a 100644 --- a/test/browser/test/snapshot.test.ts +++ b/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') +})