From b1cd7e71e0a80d49b8df5f2d47a350a35b2c1a29 Mon Sep 17 00:00:00 2001 From: Simon Abbott Date: Fri, 5 Aug 2022 10:52:13 -0500 Subject: [PATCH] fix: gracefully handle unsettable keys during automocking (#1786) Sometimes while automocking we will encounter a property that, for some unknown reason, throws an error when you try to define it. Unfortunately I can't figure out _why_ it errors, so instead I have opted to quietly skip these unsettable properties. If that becomes a problem in the future this can be revisited, but I don't forsee it being an issue since these keys are mostly deeply internal prototype stuff that 99.999% of people don't even know exists, let alone want to mock. Plus, if you _really_ need to have this behavior you can always use `__mocks__` or just shim it inline yourself. --- packages/vitest/src/runtime/mocker.ts | 28 +++++++++++++++++++-------- packages/vitest/src/utils/base.ts | 23 ++++++++++++---------- test/core/src/mockedC.ts | 3 +++ test/core/test/mocked.test.ts | 7 ++++++- 4 files changed, 42 insertions(+), 19 deletions(-) diff --git a/packages/vitest/src/runtime/mocker.ts b/packages/vitest/src/runtime/mocker.ts index 2a272ed826da..de63b87345f1 100644 --- a/packages/vitest/src/runtime/mocker.ts +++ b/packages/vitest/src/runtime/mocker.ts @@ -3,7 +3,7 @@ import { isNodeBuiltin } from 'mlly' import { basename, dirname, join, resolve } from 'pathe' import { normalizeRequestId, toFilePath } from 'vite-node/utils' import type { ModuleCacheMap } from 'vite-node/client' -import { getAllProperties, getType, getWorkerState, isWindows, mergeSlashes, slash } from '../utils' +import { getAllMockableProperties, getType, getWorkerState, isWindows, mergeSlashes, slash } from '../utils' import { distDir } from '../constants' import type { PendingSuiteMock } from '../types/mocker' import type { ExecuteOptions } from './execute' @@ -172,15 +172,24 @@ export class VitestMocker { const finalizers = new Array<() => void>() const refs = new RefTracker() + const define = (container: Record, key: Key, value: any) => { + try { + container[key] = value + return true + } + catch { + return false + } + } + const mockPropertiesOf = (container: Record, newContainer: Record) => { const containerType = getType(container) const isModule = containerType === 'Module' || !!container.__esModule - for (const property of getAllProperties(container)) { + for (const { key: property, descriptor } of getAllMockableProperties(container)) { // Modules define their exports as getters. We want to process those. if (!isModule) { // TODO: Mock getters/setters somehow? - const descriptor = Object.getOwnPropertyDescriptor(container, property) - if (descriptor?.get || descriptor?.set) + if (descriptor.get || descriptor.set) continue } @@ -194,24 +203,27 @@ export class VitestMocker { // recursion in circular objects. const refId = refs.getId(value) if (refId) { - finalizers.push(() => newContainer[property] = refs.getMockedValue(refId)) + finalizers.push(() => define(newContainer, property, refs.getMockedValue(refId))) continue } const type = getType(value) if (Array.isArray(value)) { - newContainer[property] = [] + define(newContainer, property, []) continue } const isFunction = type.includes('Function') && typeof value === 'function' if ((!isFunction || value.__isMockFunction) && type !== 'Object' && type !== 'Module') { - newContainer[property] = value + define(newContainer, property, value) continue } - newContainer[property] = isFunction ? value : {} + // Sometimes this assignment fails for some unknown reason. If it does, + // just move along. + if (!define(newContainer, property, isFunction ? value : {})) + continue if (isFunction) { spyModule.spyOn(newContainer, property).mockImplementation(() => undefined) diff --git a/packages/vitest/src/utils/base.ts b/packages/vitest/src/utils/base.ts index 21a706a0837f..e4e4c6909605 100644 --- a/packages/vitest/src/utils/base.ts +++ b/packages/vitest/src/utils/base.ts @@ -5,22 +5,25 @@ function isFinalObj(obj: any) { return obj === Object.prototype || obj === Function.prototype || obj === RegExp.prototype } -function collectOwnProperties(obj: any, collector: Set) { - const props = Object.getOwnPropertyNames(obj) - const symbols = Object.getOwnPropertySymbols(obj) - - props.forEach(prop => collector.add(prop)) - symbols.forEach(symbol => collector.add(symbol)) +function collectOwnProperties(obj: any, collector: Set | ((key: string | symbol) => void)) { + const collect = typeof collector === 'function' ? collector : (key: string | symbol) => collector.add(key) + Object.getOwnPropertyNames(obj).forEach(collect) + Object.getOwnPropertySymbols(obj).forEach(collect) } -export function getAllProperties(obj: any) { - const allProps = new Set() +export function getAllMockableProperties(obj: any) { + const allProps = new Set<{ key: string | symbol; descriptor: PropertyDescriptor }>() let curr = obj do { - // we don't need propterties from these + // we don't need properties from these if (isFinalObj(curr)) break - collectOwnProperties(curr, allProps) + + collectOwnProperties(curr, (key) => { + const descriptor = Object.getOwnPropertyDescriptor(curr, key) + if (descriptor) + allProps.add({ key, descriptor }) + }) // eslint-disable-next-line no-cond-assign } while (curr = Object.getPrototypeOf(curr)) return Array.from(allProps) diff --git a/test/core/src/mockedC.ts b/test/core/src/mockedC.ts index 36f47ebcb0ee..cead9be3bcc8 100644 --- a/test/core/src/mockedC.ts +++ b/test/core/src/mockedC.ts @@ -22,3 +22,6 @@ export async function asyncFunc(): Promise { await new Promise(resolve => resolve()) return '1234' } + +// This is here because mocking streams previously caused some problems (#1671). +export const exportedStream = process.stderr diff --git a/test/core/test/mocked.test.ts b/test/core/test/mocked.test.ts index 8025f52b8818..2082109a0c98 100644 --- a/test/core/test/mocked.test.ts +++ b/test/core/test/mocked.test.ts @@ -4,7 +4,7 @@ import { value as virtualValue } from 'virtual-module' import { two } from '../src/submodule' import * as mocked from '../src/mockedA' import { mockedB } from '../src/mockedB' -import { MockedC, asyncFunc } from '../src/mockedC' +import { MockedC, asyncFunc, exportedStream } from '../src/mockedC' import * as globalMock from '../src/global-mock' vitest.mock('../src/submodule') @@ -63,3 +63,8 @@ test('async functions should be mocked', () => { vi.mocked(asyncFunc).mockResolvedValue('foo') expect(asyncFunc()).resolves.toBe('foo') }) + +// This is here because mocking streams previously caused some problems (#1671). +test('streams', () => { + expect(exportedStream).toBeDefined() +})