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

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<DataInspectorProps> = 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<DataInspectorProps> = 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<string>, isExpanded: boolean) => {
@@ -387,10 +395,12 @@ export const DataInspectorNode: React.FC<DataInspectorProps> = 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<string>) => {

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: {

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: <T extends object, C>(
   onRangeChange,
   onUpdateAutoScroll,
   emptyRenderer,
-  _testHeight,
 }: DataSourceProps<any, any>) {
   /**
    * Virtualization
@@ -100,13 +100,12 @@ export const DataSourceRenderer: <T extends object, C>(
   const [, setForceUpdate] = useState(0);
   const forceHeightRecalculation = useRef(0);
   const parentRef = React.useRef<null | HTMLDivElement>(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: <T extends object, C>(
           // 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: <T extends object, C>(
         dataSource.view.setListener(undefined);
       };
     },
-    [dataSource, setForceUpdate, useFixedRowHeight, _testHeight],
+    [dataSource, setForceUpdate, useFixedRowHeight, isUnitTest],
   );
 
   useEffect(() => {

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<T = any> {
   columns: DataTableColumn<T>[];
@@ -57,7 +58,6 @@ interface DataTableProps<T = any> {
   onRowStyle?(record: T): CSSProperties | undefined;
   // multiselect?: true
   tableManagerRef?: RefObject<DataTableManager<T> | 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<T extends object>(
   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<DataSourceVirtualizer | undefined>();
   const [tableState, dispatch] = useReducer(
     dataTableManagerReducer as DataTableReducer<T>,
@@ -273,7 +272,7 @@ export function DataTable<T extends object>(
         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<T extends object>(
   );
 
   /** Context menu */
-  const contexMenu = props._testHeight
+  const contexMenu = isUnitTest
     ? undefined
     : // eslint-disable-next-line
     useCallback(
@@ -446,7 +445,6 @@ export function DataTable<T extends object>(
           onRangeChange={onRangeChange}
           onUpdateAutoScroll={onUpdateAutoScroll}
           emptyRenderer={emptyRenderer}
-          _testHeight={props._testHeight}
         />
       </Layout.Top>
     ) : (
@@ -479,7 +477,7 @@ export function DataTable<T extends object>(
           />
         </AutoScroller>
       )}
-      {range && <RangeFinder>{range}</RangeFinder>}
+      {range && !isUnitTest && <RangeFinder>{range}</RangeFinder>}
     </Layout.Container>
   );
 }

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<typeof useVirtual>;

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<DataTableManager<Todo>>();
   const rendering = render(
-    <DataTable
-      dataSource={ds}
-      columns={columns}
-      tableManagerRef={ref}
-      _testHeight={400}
-    />,
+    <DataTable dataSource={ds} columns={columns} tableManagerRef={ref} />,
   );
   {
     const elem = await rendering.findAllByText('test DataTable');
@@ -100,12 +95,7 @@ test('column visibility', async () => {
   const ds = createTestDataSource();
   const ref = createRef<DataTableManager<Todo>>();
   const rendering = render(
-    <DataTable
-      dataSource={ds}
-      columns={columns}
-      tableManagerRef={ref}
-      _testHeight={400}
-    />,
+    <DataTable dataSource={ds} columns={columns} tableManagerRef={ref} />,
   );
   {
     const elem = await rendering.findAllByText('test DataTable');
@@ -176,12 +166,7 @@ test('sorting', async () => {
   });
   const ref = createRef<DataTableManager<Todo>>();
   const rendering = render(
-    <DataTable
-      dataSource={ds}
-      columns={columns}
-      tableManagerRef={ref}
-      _testHeight={400}
-    />,
+    <DataTable dataSource={ds} columns={columns} tableManagerRef={ref} />,
   );
   // insertion order
   {
@@ -256,7 +241,6 @@ test('search', async () => {
       columns={columns}
       tableManagerRef={ref}
       extraActions={<Button>Test Button</Button>}
-      _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]);
       }}

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(
-    <DataTable records={records} columns={columns} _testHeight={400} />,
-  );
+  const rendering = render(<DataTable records={records} columns={columns} />);
   {
     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(
-      <DataTable records={records} columns={columns} _testHeight={400} />,
-    );
+    rendering.rerender(<DataTable records={records} columns={columns} />);
   }
 
   // append

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';
+}