From b92c76653d09c1033e5f21de076d208fa4b9bb15 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 18 May 2023 16:32:27 -0500 Subject: [PATCH 1/6] fix: makeApiContextWrapper and createMockProxy #1311 --- .../jsapi-components/src/HookTestUtils.tsx | 13 ++++- packages/utils/src/TestUtils.ts | 58 ++++++++++++++----- 2 files changed, 54 insertions(+), 17 deletions(-) diff --git a/packages/jsapi-components/src/HookTestUtils.tsx b/packages/jsapi-components/src/HookTestUtils.tsx index b550cf9757..465f3f9747 100644 --- a/packages/jsapi-components/src/HookTestUtils.tsx +++ b/packages/jsapi-components/src/HookTestUtils.tsx @@ -1,11 +1,18 @@ -import React, { ReactNode } from 'react'; +import React, { forwardRef, ReactNode } from 'react'; import { ApiContext } from '@deephaven/jsapi-bootstrap'; import type { dh as DhType } from '@deephaven/jsapi-types'; export function makeApiContextWrapper(dh: DhType) { - return function ApiContextWrapper({ children }: { children?: ReactNode }) { + return forwardRef(function ApiContextWrapper( + { + children, + }: { + children?: ReactNode; + }, + _ref + ) { return {children}; - }; + }); } export default { diff --git a/packages/utils/src/TestUtils.ts b/packages/utils/src/TestUtils.ts index 2f058a4502..bb943b713c 100644 --- a/packages/utils/src/TestUtils.ts +++ b/packages/utils/src/TestUtils.ts @@ -179,41 +179,71 @@ class TestUtils { * optionally be set via the constructor. Any prop that is not set will be set * to a jest.fn() instance on first access with the exeption of "then" which * will not be automatically proxied. - * @param props Optional props to explicitly set on the Proxy. + * @param overrides Optional props to explicitly set on the Proxy. * @returns */ - static createMockProxy(props: Partial = {}): T { + static createMockProxy(overrides: Partial = {}): T { + /* eslint-disable no-underscore-dangle */ return new Proxy( { - props: { - // Disable auto-proxying of certain properties that cause trouble. - // - Symbol.iterator - returning a jest.fn() throws a TypeError - // - then - avoid issues with `await` treating object as "thenable" + // Set default values on certain properties so they don't get automatically + // proxied as jest.fn() instances. + __mockProxyDefaultProps: { + // `Symbol.iterator` - returning a jest.fn() throws a TypeError + // `then` - avoid issues with `await` treating object as "thenable" [Symbol.iterator]: undefined, then: undefined, - ...props, + // Jest makes calls to `asymmetricMatch`, `hasAttribute`, `nodeType` + // `tagName`, and `toJSON` + asymmetricMatch: undefined, + hasAttribute: undefined, + nodeType: undefined, + tagName: undefined, + toJSON: undefined, }, - proxies: {} as Record, + __mockProxyOverrides: overrides, + __mockProxyProxies: {} as Record, }, { get(target, name) { - if (name in target.props) { - return target.props[name as keyof T]; + if (name === Symbol.toStringTag) { + return 'Mock Proxy'; } - if (target.proxies[name as keyof T] == null) { + // Reserved attributes for the proxy + if (String(name).startsWith('__mockProxy')) { + return target[name as keyof typeof target]; + } + + // Properties that have been explicitly overriden + if (name in target.__mockProxyOverrides) { + return target.__mockProxyOverrides[name as keyof T]; + } + + // Properties that have defaults set + if (name in target.__mockProxyDefaultProps) { + return target.__mockProxyDefaultProps[ + name as keyof typeof target.__mockProxyDefaultProps + ]; + } + + // Any other property access will create and cache a jest.fn() instance + if (target.__mockProxyProxies[name as keyof T] == null) { // eslint-disable-next-line no-param-reassign - target.proxies[name as keyof T] = jest.fn(); + target.__mockProxyProxies[name as keyof T] = jest + .fn() + .mockName(String(name)); } - return target.proxies[name as keyof T]; + return target.__mockProxyProxies[name as keyof T]; }, // Only consider explicitly defined props as "in" the proxy has(target, name) { - return name in target.props; + return name in target.__mockProxyOverrides; }, } ) as T; + /* eslint-enable no-underscore-dangle */ } /** From 0595cf042216f6e9439d01cc55658f110f724be5 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 18 May 2023 17:22:23 -0500 Subject: [PATCH 2/6] Test coverage #1311 --- packages/utils/src/TestUtils.test.tsx | 55 +++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/packages/utils/src/TestUtils.test.tsx b/packages/utils/src/TestUtils.test.tsx index 667b1e2677..b388fa8b02 100644 --- a/packages/utils/src/TestUtils.test.tsx +++ b/packages/utils/src/TestUtils.test.tsx @@ -179,6 +179,61 @@ describe('createMockProxy', () => { expect('age' in mock).toBeTruthy(); expect('blah' in mock).toBeFalsy(); }); + + it.each([ + Symbol.iterator, + 'then', + 'asymmetricMatch', + 'hasAttribute', + 'nodeType', + 'tagName', + 'toJSON', + ])('should return undefined for default props', prop => { + const mock = TestUtils.createMockProxy(); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect((mock as any)[prop]).toBeUndefined(); + }); + + it('should return custom Symbol.toStringTag', () => { + const mock = TestUtils.createMockProxy(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect((mock as any)[Symbol.toStringTag]).toEqual('Mock Proxy'); + }); + + it('should return internal storage by name', () => { + const overrides = { + name: 'mock.nam', + age: 42, + }; + + const mock = TestUtils.createMockProxy<{ + name: string; + age: number; + testMethod: () => void; + }>(overrides); + + mock.testMethod(); + + /* eslint-disable @typescript-eslint/no-explicit-any, no-underscore-dangle */ + expect((mock as any).__mockProxyDefaultProps).toEqual({ + then: undefined, + asymmetricMatch: undefined, + hasAttribute: undefined, + nodeType: undefined, + tagName: undefined, + toJSON: undefined, + [Symbol.iterator]: undefined, + }); + + expect((mock as any).__mockProxyOverrides).toEqual(overrides); + + expect((mock as any).__mockProxyProxies).toEqual({ + testMethod: expect.any(Function), + }); + expect(mock.testMethod).toBeInstanceOf(jest.fn().constructor); + /* eslint-enable @typescript-eslint/no-explicit-any, no-underscore-dangle */ + }); }); describe('extractCallArgs', () => { From 241c38983b7cf61a24f4440f82d6b5c33793f01e Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 18 May 2023 17:26:04 -0500 Subject: [PATCH 3/6] typo #1311 --- packages/utils/src/TestUtils.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/utils/src/TestUtils.test.tsx b/packages/utils/src/TestUtils.test.tsx index b388fa8b02..a27105cfa3 100644 --- a/packages/utils/src/TestUtils.test.tsx +++ b/packages/utils/src/TestUtils.test.tsx @@ -203,7 +203,7 @@ describe('createMockProxy', () => { it('should return internal storage by name', () => { const overrides = { - name: 'mock.nam', + name: 'mock.name', age: 42, }; From 3fe7e920522fd7d962146a445526edd0d16c6835 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Mon, 22 May 2023 08:49:16 -0500 Subject: [PATCH 4/6] Split mock proxy into its own module #1311 --- packages/utils/src/MockProxy.test.ts | 86 ++++++++++++++++++++++++ packages/utils/src/MockProxy.ts | 95 +++++++++++++++++++++++++++ packages/utils/src/TestUtils.test.tsx | 86 +----------------------- packages/utils/src/TestUtils.ts | 66 +------------------ 4 files changed, 185 insertions(+), 148 deletions(-) create mode 100644 packages/utils/src/MockProxy.test.ts create mode 100644 packages/utils/src/MockProxy.ts diff --git a/packages/utils/src/MockProxy.test.ts b/packages/utils/src/MockProxy.test.ts new file mode 100644 index 0000000000..e15238bc5d --- /dev/null +++ b/packages/utils/src/MockProxy.test.ts @@ -0,0 +1,86 @@ +import createMockProxy, { MockProxySymbol } from './MockProxy'; + +describe('createMockProxy', () => { + it('should proxy property access as jest.fn() unless explicitly set', () => { + const mock = createMockProxy>({ + name: 'mock.name', + }); + + expect(mock.name).toEqual('mock.name'); + expect(mock.propA).toBeInstanceOf(jest.fn().constructor); + expect(mock.propB).toBeInstanceOf(jest.fn().constructor); + }); + + it('should not interfere with `await` by not proxying `then` property', async () => { + const mock = createMockProxy>({}); + expect(mock.then).toBeUndefined(); + + const result = await mock; + + expect(result).toBe(mock); + }); + + it('should only show `in` for explicit properties', () => { + const mock = createMockProxy>({ + name: 'mock.name', + age: 42, + }); + + expect('name' in mock).toBeTruthy(); + expect('age' in mock).toBeTruthy(); + expect('blah' in mock).toBeFalsy(); + }); + + it.each([ + Symbol.iterator, + 'then', + 'asymmetricMatch', + 'hasAttribute', + 'nodeType', + 'tagName', + 'toJSON', + ])('should return undefined for default props', prop => { + const mock = createMockProxy(); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect((mock as any)[prop]).toBeUndefined(); + }); + + it('should return custom Symbol.toStringTag', () => { + const mock = createMockProxy(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + expect((mock as any)[Symbol.toStringTag]).toEqual('Mock Proxy'); + }); + + it('should return internal storage by name', () => { + const overrides = { + name: 'mock.name', + age: 42, + }; + + const mock = createMockProxy<{ + name: string; + age: number; + testMethod: () => void; + }>(overrides); + + mock.testMethod(); + + expect(mock[MockProxySymbol.defaultProps]).toEqual({ + then: undefined, + asymmetricMatch: undefined, + hasAttribute: undefined, + nodeType: undefined, + tagName: undefined, + toJSON: undefined, + [Symbol.iterator]: undefined, + }); + + expect(mock[MockProxySymbol.overrides]).toEqual(overrides); + + expect(mock[MockProxySymbol.proxies]).toEqual({ + testMethod: expect.any(Function), + }); + expect(mock.testMethod).toBeInstanceOf(jest.fn().constructor); + }); +}); diff --git a/packages/utils/src/MockProxy.ts b/packages/utils/src/MockProxy.ts new file mode 100644 index 0000000000..6bdbf80b82 --- /dev/null +++ b/packages/utils/src/MockProxy.ts @@ -0,0 +1,95 @@ +const defaultPropsSymbol: unique symbol = Symbol('mockProxyDefaultProps'); +const overridesSymbol: unique symbol = Symbol('mockProxyOverrides'); +const proxiesSymbol: unique symbol = Symbol('mockProxyProxies'); + +export const MockProxySymbol = { + defaultProps: defaultPropsSymbol, + overrides: overridesSymbol, + proxies: proxiesSymbol, +} as const; + +// Set default values on certain properties so they don't get automatically +// proxied as jest.fn() instances. +const mockProxyDefaultProps = { + // `Symbol.iterator` - returning a jest.fn() throws a TypeError + // `then` - avoid issues with `await` treating object as "thenable" + [Symbol.iterator]: undefined, + then: undefined, + // Jest makes calls to `asymmetricMatch`, `hasAttribute`, `nodeType` + // `tagName`, and `toJSON` + asymmetricMatch: undefined, + hasAttribute: undefined, + nodeType: undefined, + tagName: undefined, + toJSON: undefined, +}; + +/** + * The proxy target contains state + configuration for the proxy + */ +export interface MockProxyTarget { + [MockProxySymbol.defaultProps]: typeof mockProxyDefaultProps; + [MockProxySymbol.overrides]: Partial; + [MockProxySymbol.proxies]: Record; +} + +/** + * Creates a mock object for a type `T` using a Proxy object. Each prop can + * optionally be set via the constructor. Any prop that is not set will be set + * to a jest.fn() instance on first access with the exeption of "then" which + * will not be automatically proxied. + * @param overrides Optional props to explicitly set on the Proxy. + * @returns + */ +export default function createMockProxy( + overrides: Partial = {} +): T & MockProxyTarget { + const targetDef: MockProxyTarget = { + [MockProxySymbol.defaultProps]: mockProxyDefaultProps, + [MockProxySymbol.overrides]: overrides, + [MockProxySymbol.proxies]: {} as Record, + }; + + return new Proxy(targetDef, { + get(target, name) { + if (name === Symbol.toStringTag) { + return 'Mock Proxy'; + } + + // Reserved attributes for the proxy + if ( + MockProxySymbol.defaultProps === name || + MockProxySymbol.overrides === name || + MockProxySymbol.proxies === name + ) { + return target[name as keyof typeof target]; + } + + // Properties that have been explicitly overriden + if (name in target[MockProxySymbol.overrides]) { + return target[MockProxySymbol.overrides][name as keyof Partial]; + } + + // Properties that have defaults set + if (name in target[MockProxySymbol.defaultProps]) { + return target[MockProxySymbol.defaultProps][ + name as keyof typeof mockProxyDefaultProps + ]; + } + + // Any other property access will create and cache a jest.fn() instance + if (target[MockProxySymbol.proxies][name as keyof T] == null) { + // eslint-disable-next-line no-param-reassign + target[MockProxySymbol.proxies][name as keyof T] = jest + .fn() + .mockName(String(name)); + } + + return target[MockProxySymbol.proxies][name as keyof T]; + }, + // Only consider explicitly defined props as "in" the proxy + has(target, name) { + return name in target[MockProxySymbol.overrides]; + }, + }) as T & typeof targetDef; +} diff --git a/packages/utils/src/TestUtils.test.tsx b/packages/utils/src/TestUtils.test.tsx index a27105cfa3..b5979ed40c 100644 --- a/packages/utils/src/TestUtils.test.tsx +++ b/packages/utils/src/TestUtils.test.tsx @@ -3,6 +3,7 @@ import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import TestUtils from './TestUtils'; +import createMockProxy from './MockProxy'; beforeEach(() => { jest.clearAllMocks(); @@ -150,90 +151,7 @@ describe('click', () => { }); describe('createMockProxy', () => { - it('should proxy property access as jest.fn() unless explicitly set', () => { - const mock = TestUtils.createMockProxy>({ - name: 'mock.name', - }); - - expect(mock.name).toEqual('mock.name'); - expect(mock.propA).toBeInstanceOf(jest.fn().constructor); - expect(mock.propB).toBeInstanceOf(jest.fn().constructor); - }); - - it('should not interfere with `await` by not proxying `then` property', async () => { - const mock = TestUtils.createMockProxy>({}); - expect(mock.then).toBeUndefined(); - - const result = await mock; - - expect(result).toBe(mock); - }); - - it('should only show `in` for explicit properties', () => { - const mock = TestUtils.createMockProxy>({ - name: 'mock.name', - age: 42, - }); - - expect('name' in mock).toBeTruthy(); - expect('age' in mock).toBeTruthy(); - expect('blah' in mock).toBeFalsy(); - }); - - it.each([ - Symbol.iterator, - 'then', - 'asymmetricMatch', - 'hasAttribute', - 'nodeType', - 'tagName', - 'toJSON', - ])('should return undefined for default props', prop => { - const mock = TestUtils.createMockProxy(); - - // eslint-disable-next-line @typescript-eslint/no-explicit-any - expect((mock as any)[prop]).toBeUndefined(); - }); - - it('should return custom Symbol.toStringTag', () => { - const mock = TestUtils.createMockProxy(); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - expect((mock as any)[Symbol.toStringTag]).toEqual('Mock Proxy'); - }); - - it('should return internal storage by name', () => { - const overrides = { - name: 'mock.name', - age: 42, - }; - - const mock = TestUtils.createMockProxy<{ - name: string; - age: number; - testMethod: () => void; - }>(overrides); - - mock.testMethod(); - - /* eslint-disable @typescript-eslint/no-explicit-any, no-underscore-dangle */ - expect((mock as any).__mockProxyDefaultProps).toEqual({ - then: undefined, - asymmetricMatch: undefined, - hasAttribute: undefined, - nodeType: undefined, - tagName: undefined, - toJSON: undefined, - [Symbol.iterator]: undefined, - }); - - expect((mock as any).__mockProxyOverrides).toEqual(overrides); - - expect((mock as any).__mockProxyProxies).toEqual({ - testMethod: expect.any(Function), - }); - expect(mock.testMethod).toBeInstanceOf(jest.fn().constructor); - /* eslint-enable @typescript-eslint/no-explicit-any, no-underscore-dangle */ - }); + expect(TestUtils.createMockProxy).toBe(createMockProxy); }); describe('extractCallArgs', () => { diff --git a/packages/utils/src/TestUtils.ts b/packages/utils/src/TestUtils.ts index bb943b713c..1d8184ffa3 100644 --- a/packages/utils/src/TestUtils.ts +++ b/packages/utils/src/TestUtils.ts @@ -1,4 +1,5 @@ import type userEvent from '@testing-library/user-event'; +import createMockProxy from './MockProxy'; interface MockContext { arc: jest.Mock; @@ -180,71 +181,8 @@ class TestUtils { * to a jest.fn() instance on first access with the exeption of "then" which * will not be automatically proxied. * @param overrides Optional props to explicitly set on the Proxy. - * @returns */ - static createMockProxy(overrides: Partial = {}): T { - /* eslint-disable no-underscore-dangle */ - return new Proxy( - { - // Set default values on certain properties so they don't get automatically - // proxied as jest.fn() instances. - __mockProxyDefaultProps: { - // `Symbol.iterator` - returning a jest.fn() throws a TypeError - // `then` - avoid issues with `await` treating object as "thenable" - [Symbol.iterator]: undefined, - then: undefined, - // Jest makes calls to `asymmetricMatch`, `hasAttribute`, `nodeType` - // `tagName`, and `toJSON` - asymmetricMatch: undefined, - hasAttribute: undefined, - nodeType: undefined, - tagName: undefined, - toJSON: undefined, - }, - __mockProxyOverrides: overrides, - __mockProxyProxies: {} as Record, - }, - { - get(target, name) { - if (name === Symbol.toStringTag) { - return 'Mock Proxy'; - } - - // Reserved attributes for the proxy - if (String(name).startsWith('__mockProxy')) { - return target[name as keyof typeof target]; - } - - // Properties that have been explicitly overriden - if (name in target.__mockProxyOverrides) { - return target.__mockProxyOverrides[name as keyof T]; - } - - // Properties that have defaults set - if (name in target.__mockProxyDefaultProps) { - return target.__mockProxyDefaultProps[ - name as keyof typeof target.__mockProxyDefaultProps - ]; - } - - // Any other property access will create and cache a jest.fn() instance - if (target.__mockProxyProxies[name as keyof T] == null) { - // eslint-disable-next-line no-param-reassign - target.__mockProxyProxies[name as keyof T] = jest - .fn() - .mockName(String(name)); - } - - return target.__mockProxyProxies[name as keyof T]; - }, - // Only consider explicitly defined props as "in" the proxy - has(target, name) { - return name in target.__mockProxyOverrides; - }, - } - ) as T; - /* eslint-enable no-underscore-dangle */ - } + static createMockProxy = createMockProxy; /** * Attempt to extract the args for the nth call to a given function. This will From 10c5f3f8b4c4972edb683332c2d0e2fe15223679 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Mon, 22 May 2023 09:51:54 -0500 Subject: [PATCH 5/6] Fixed generic type on makeApiContextWrapper #1311 --- packages/jsapi-components/src/HookTestUtils.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/jsapi-components/src/HookTestUtils.tsx b/packages/jsapi-components/src/HookTestUtils.tsx index 465f3f9747..fd80ad6095 100644 --- a/packages/jsapi-components/src/HookTestUtils.tsx +++ b/packages/jsapi-components/src/HookTestUtils.tsx @@ -2,8 +2,8 @@ import React, { forwardRef, ReactNode } from 'react'; import { ApiContext } from '@deephaven/jsapi-bootstrap'; import type { dh as DhType } from '@deephaven/jsapi-types'; -export function makeApiContextWrapper(dh: DhType) { - return forwardRef(function ApiContextWrapper( +export function makeApiContextWrapper(dh: DhType) { + return forwardRef(function ApiContextWrapper( { children, }: { From a1b2bbb07cb3a79400cdf39ac7a21bf4eb6e011a Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Tue, 23 May 2023 11:57:40 -0500 Subject: [PATCH 6/6] Added ref exclusion to MockProxy #1311 --- packages/jsapi-components/src/HookTestUtils.tsx | 15 ++++----------- packages/utils/src/MockProxy.test.ts | 2 ++ packages/utils/src/MockProxy.ts | 3 ++- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/packages/jsapi-components/src/HookTestUtils.tsx b/packages/jsapi-components/src/HookTestUtils.tsx index fd80ad6095..b550cf9757 100644 --- a/packages/jsapi-components/src/HookTestUtils.tsx +++ b/packages/jsapi-components/src/HookTestUtils.tsx @@ -1,18 +1,11 @@ -import React, { forwardRef, ReactNode } from 'react'; +import React, { ReactNode } from 'react'; import { ApiContext } from '@deephaven/jsapi-bootstrap'; import type { dh as DhType } from '@deephaven/jsapi-types'; -export function makeApiContextWrapper(dh: DhType) { - return forwardRef(function ApiContextWrapper( - { - children, - }: { - children?: ReactNode; - }, - _ref - ) { +export function makeApiContextWrapper(dh: DhType) { + return function ApiContextWrapper({ children }: { children?: ReactNode }) { return {children}; - }); + }; } export default { diff --git a/packages/utils/src/MockProxy.test.ts b/packages/utils/src/MockProxy.test.ts index e15238bc5d..4c9696be12 100644 --- a/packages/utils/src/MockProxy.test.ts +++ b/packages/utils/src/MockProxy.test.ts @@ -37,6 +37,7 @@ describe('createMockProxy', () => { 'asymmetricMatch', 'hasAttribute', 'nodeType', + 'ref', 'tagName', 'toJSON', ])('should return undefined for default props', prop => { @@ -71,6 +72,7 @@ describe('createMockProxy', () => { asymmetricMatch: undefined, hasAttribute: undefined, nodeType: undefined, + ref: undefined, tagName: undefined, toJSON: undefined, [Symbol.iterator]: undefined, diff --git a/packages/utils/src/MockProxy.ts b/packages/utils/src/MockProxy.ts index 6bdbf80b82..81b1cd376b 100644 --- a/packages/utils/src/MockProxy.ts +++ b/packages/utils/src/MockProxy.ts @@ -16,8 +16,9 @@ const mockProxyDefaultProps = { [Symbol.iterator]: undefined, then: undefined, // Jest makes calls to `asymmetricMatch`, `hasAttribute`, `nodeType` - // `tagName`, and `toJSON` + // `tagName`, and `toJSON`. react-test-renderer checks `ref` asymmetricMatch: undefined, + ref: undefined, hasAttribute: undefined, nodeType: undefined, tagName: undefined,