From bbcb16d8fba3a358bb1da6a25c802848104ab849 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Wed, 28 Apr 2021 06:32:09 -0700 Subject: [PATCH] Make DataTable / DataInspector unit tests predictable Summary: Having time / async / non-blocking behavior in components in unit tests is really annoying, as it makes unit tests async without an easy way to determine 'done'. This diff makes sure that DataTable & DataInspector don't break down their work in smaller tasks, but do everything block if they are running in a unit test. Reviewed By: nikoant Differential Revision: D28054487 fbshipit-source-id: 72c3b519e092ad69ed71eb1731e1fed80022f91f --- .../ui/data-inspector/DataInspectorNode.tsx | 22 ++++++++++---- .../__tests__/DataInspector.node.tsx | 29 ------------------- .../src/ui/data-table/DataSourceRenderer.tsx | 11 ++++--- .../src/ui/data-table/DataTable.tsx | 14 ++++----- .../data-table/StaticDataSourceRenderer.tsx | 1 - .../data-table/__tests__/DataTable.node.tsx | 24 ++------------- .../__tests__/DataTableRecords.node.tsx | 8 ++--- .../src/utils/useInUnitTest().tsx | 16 ++++++++++ 8 files changed, 48 insertions(+), 77 deletions(-) create mode 100644 desktop/flipper-plugin/src/utils/useInUnitTest().tsx diff --git a/desktop/flipper-plugin/src/ui/data-inspector/DataInspectorNode.tsx b/desktop/flipper-plugin/src/ui/data-inspector/DataInspectorNode.tsx index a87a6cf84..cc61f2421 100644 --- a/desktop/flipper-plugin/src/ui/data-inspector/DataInspectorNode.tsx +++ b/desktop/flipper-plugin/src/ui/data-inspector/DataInspectorNode.tsx @@ -26,6 +26,7 @@ import {useHighlighter, HighlightManager} from '../Highlight'; import {Dropdown, Menu, Tooltip} from 'antd'; import {tryGetFlipperLibImplementation} from '../../plugin/FlipperLib'; import {safeStringify} from '../../utils/safeStringify'; +import {useInUnitTest} from '../../utils/useInUnitTest()'; export {DataValueExtractor} from './DataPreview'; @@ -300,6 +301,7 @@ export const DataInspectorNode: React.FC = memo( }) { const highlighter = useHighlighter(); const getRoot = useContext(RootDataContext); + const isUnitTest = useInUnitTest(); const shouldExpand = useRef(false); const expandHandle = useRef(undefined as any); @@ -366,14 +368,20 @@ export const DataInspectorNode: React.FC = memo( if (!shouldExpand.current) { setRenderExpanded(false); } else { - expandHandle.current = requestIdleCallback(() => { + if (isUnitTest) { setRenderExpanded(true); - }); + } else { + expandHandle.current = requestIdleCallback(() => { + setRenderExpanded(true); + }); + } } return () => { - cancelIdleCallback(expandHandle.current); + if (!isUnitTest) { + cancelIdleCallback(expandHandle.current); + } }; - }, [shouldExpand.current]); + }, [shouldExpand.current, isUnitTest]); const setExpanded = useCallback( (pathParts: Array, isExpanded: boolean) => { @@ -387,10 +395,12 @@ export const DataInspectorNode: React.FC = memo( ); const handleClick = useCallback(() => { - cancelIdleCallback(expandHandle.current); + if (!isUnitTest) { + cancelIdleCallback(expandHandle.current); + } const isExpanded = shouldBeExpanded(expandedPaths, path, collapsed); setExpanded(path, !isExpanded); - }, [expandedPaths, path, collapsed]); + }, [expandedPaths, path, collapsed, isUnitTest]); const handleDelete = useCallback( (path: Array) => { diff --git a/desktop/flipper-plugin/src/ui/data-inspector/__tests__/DataInspector.node.tsx b/desktop/flipper-plugin/src/ui/data-inspector/__tests__/DataInspector.node.tsx index 893a1b903..0a0f3809d 100644 --- a/desktop/flipper-plugin/src/ui/data-inspector/__tests__/DataInspector.node.tsx +++ b/desktop/flipper-plugin/src/ui/data-inspector/__tests__/DataInspector.node.tsx @@ -13,35 +13,6 @@ import {render, fireEvent, waitFor, act} from '@testing-library/react'; import {DataInspector} from '../DataInspector'; import {sleep} from '../../../utils/sleep'; -const mocks = { - requestIdleCallback(fn: Function) { - return setTimeout(fn, 1); - }, - cancelIdleCallback(handle: any) { - clearTimeout(handle); - }, -}; - -beforeAll(() => { - Object.keys(mocks).forEach((key) => { - // @ts-ignore - if (!global[key]) { - // @ts-ignore - global[key] = mocks[key]; - } - }); -}); - -afterAll(() => { - Object.keys(mocks).forEach((key) => { - // @ts-ignore - if (global[key] === mocks[key]) { - // @ts-ignore - delete global[key]; - } - }); -}); - const json = { data: { is: { diff --git a/desktop/flipper-plugin/src/ui/data-table/DataSourceRenderer.tsx b/desktop/flipper-plugin/src/ui/data-table/DataSourceRenderer.tsx index e7535b14a..f1dc1aa82 100644 --- a/desktop/flipper-plugin/src/ui/data-table/DataSourceRenderer.tsx +++ b/desktop/flipper-plugin/src/ui/data-table/DataSourceRenderer.tsx @@ -22,6 +22,7 @@ import {DataSource} from '../../state/DataSource'; import {useVirtual} from 'react-virtual'; import styled from '@emotion/styled'; import observeRect from '@reach/observe-rect'; +import {useInUnitTest} from '../../utils/useInUnitTest()'; // how fast we update if updates are low-prio (e.g. out of window and not super significant) const LOW_PRIO_UPDATE = 1000; //ms @@ -89,7 +90,6 @@ export const DataSourceRenderer: ( onRangeChange, onUpdateAutoScroll, emptyRenderer, - _testHeight, }: DataSourceProps) { /** * Virtualization @@ -100,13 +100,12 @@ export const DataSourceRenderer: ( const [, setForceUpdate] = useState(0); const forceHeightRecalculation = useRef(0); const parentRef = React.useRef(null); + const isUnitTest = useInUnitTest(); const virtualizer = useVirtual({ size: dataSource.view.size, parentRef, - useObserver: _testHeight - ? () => ({height: _testHeight, width: 1000}) - : undefined, + useObserver: isUnitTest ? () => ({height: 500, width: 1000}) : undefined, // eslint-disable-next-line estimateSize: useCallback(() => defaultRowHeight, [forceHeightRecalculation.current, defaultRowHeight]), overscan: 0, @@ -138,7 +137,7 @@ export const DataSourceRenderer: ( // the height of some existing rows might have changed forceHeightRecalculation.current++; } - if (_testHeight) { + if (isUnitTest) { // test environment, update immediately forceUpdate(); return; @@ -202,7 +201,7 @@ export const DataSourceRenderer: ( dataSource.view.setListener(undefined); }; }, - [dataSource, setForceUpdate, useFixedRowHeight, _testHeight], + [dataSource, setForceUpdate, useFixedRowHeight, isUnitTest], ); useEffect(() => { diff --git a/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx b/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx index 81641e8f3..0678e14a6 100644 --- a/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx +++ b/desktop/flipper-plugin/src/ui/data-table/DataTable.tsx @@ -47,6 +47,7 @@ import {Formatter} from '../DataFormatter'; import {usePluginInstance} from '../../plugin/PluginContext'; import {debounce} from 'lodash'; import {StaticDataSourceRenderer} from './StaticDataSourceRenderer'; +import {useInUnitTest} from '../../utils/useInUnitTest()'; interface DataTableProps { columns: DataTableColumn[]; @@ -57,7 +58,6 @@ interface DataTableProps { onRowStyle?(record: T): CSSProperties | undefined; // multiselect?: true tableManagerRef?: RefObject | undefined>; // Actually we want a MutableRefObject, but that is not what React.createRef() returns, and we don't want to put the burden on the plugin dev to cast it... - _testHeight?: number; // exposed for unit testing only onCopyRows?(records: T[]): string; onContextMenu?: (selection: undefined | T) => React.ReactElement; searchbar?: boolean; @@ -113,11 +113,10 @@ export function DataTable( useAssertStableRef(props.columns, 'columns'); useAssertStableRef(onCopyRows, 'onCopyRows'); useAssertStableRef(onContextMenu, 'onContextMenu'); - useAssertStableRef(props._testHeight, '_testHeight'); + const isUnitTest = useInUnitTest(); - // lint disabled for conditional inclusion of a hook (_testHeight is asserted to be stable) // eslint-disable-next-line - const scope = props._testHeight ? "" : usePluginInstance().pluginKey; + const scope = isUnitTest ? "" : usePluginInstance().pluginKey; const virtualizerRef = useRef(); const [tableState, dispatch] = useReducer( dataTableManagerReducer as DataTableReducer, @@ -273,7 +272,7 @@ export function DataTable( computeDataTableFilter(search, useRegex, columns), ); }; - return props._testHeight ? setFilter : debounce(setFilter, 250); + return isUnitTest ? setFilter : debounce(setFilter, 250); }); useEffect( function updateFilter() { @@ -360,7 +359,7 @@ export function DataTable( ); /** Context menu */ - const contexMenu = props._testHeight + const contexMenu = isUnitTest ? undefined : // eslint-disable-next-line useCallback( @@ -446,7 +445,6 @@ export function DataTable( onRangeChange={onRangeChange} onUpdateAutoScroll={onUpdateAutoScroll} emptyRenderer={emptyRenderer} - _testHeight={props._testHeight} /> ) : ( @@ -479,7 +477,7 @@ export function DataTable( /> )} - {range && {range}} + {range && !isUnitTest && {range}} ); } diff --git a/desktop/flipper-plugin/src/ui/data-table/StaticDataSourceRenderer.tsx b/desktop/flipper-plugin/src/ui/data-table/StaticDataSourceRenderer.tsx index 20d04241d..8929fc0f0 100644 --- a/desktop/flipper-plugin/src/ui/data-table/StaticDataSourceRenderer.tsx +++ b/desktop/flipper-plugin/src/ui/data-table/StaticDataSourceRenderer.tsx @@ -10,7 +10,6 @@ import React, {memo, useCallback, useEffect, useState} from 'react'; import {DataSource} from '../../state/DataSource'; import {useVirtual} from 'react-virtual'; -import styled from '@emotion/styled'; import {RedrawContext} from './DataSourceRenderer'; export type DataSourceVirtualizer = ReturnType; diff --git a/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTable.node.tsx b/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTable.node.tsx index 923a2abe9..dc03c235d 100644 --- a/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTable.node.tsx +++ b/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTable.node.tsx @@ -43,12 +43,7 @@ test('update and append', async () => { const ds = createTestDataSource(); const ref = createRef>(); const rendering = render( - , + , ); { const elem = await rendering.findAllByText('test DataTable'); @@ -100,12 +95,7 @@ test('column visibility', async () => { const ds = createTestDataSource(); const ref = createRef>(); const rendering = render( - , + , ); { const elem = await rendering.findAllByText('test DataTable'); @@ -176,12 +166,7 @@ test('sorting', async () => { }); const ref = createRef>(); const rendering = render( - , + , ); // insertion order { @@ -256,7 +241,6 @@ test('search', async () => { columns={columns} tableManagerRef={ref} extraActions={} - _testHeight={400} />, ); { @@ -540,7 +524,6 @@ test('onSelect callback fires, and in order', () => { dataSource={ds} columns={columns} tableManagerRef={ref} - _testHeight={400} onSelect={(item, items) => { events.push([item, items]); }} @@ -592,7 +575,6 @@ test('selection always has the latest state', () => { dataSource={ds} columns={columns} tableManagerRef={ref} - _testHeight={400} onSelect={(item, items) => { events.push([item, items]); }} diff --git a/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTableRecords.node.tsx b/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTableRecords.node.tsx index d344f137c..4bccb1912 100644 --- a/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTableRecords.node.tsx +++ b/desktop/flipper-plugin/src/ui/data-table/__tests__/DataTableRecords.node.tsx @@ -38,9 +38,7 @@ const columns: DataTableColumn[] = [ test('update and append', async () => { let records = createTestRecords(); - const rendering = render( - , - ); + const rendering = render(); { const elem = await rendering.findAllByText('test DataTable'); expect(elem.length).toBe(1); @@ -63,9 +61,7 @@ test('update and append', async () => { } function rerender() { - rendering.rerender( - , - ); + rendering.rerender(); } // append diff --git a/desktop/flipper-plugin/src/utils/useInUnitTest().tsx b/desktop/flipper-plugin/src/utils/useInUnitTest().tsx new file mode 100644 index 000000000..42a30098c --- /dev/null +++ b/desktop/flipper-plugin/src/utils/useInUnitTest().tsx @@ -0,0 +1,16 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + */ + +/** + * Check if we are currently running a unit test. + * Use this hook to disable certain functionality that is probably not going to work as expected in the JSDom implementaiton + */ +export function useInUnitTest(): boolean { + return process.env.NODE_ENV === 'test'; +}