Cleaned up tests a bit. Profiling test uses mock timers now.

src/__tests__/__snapshots__/profiler-test.js.snap

@@ -1,110 +0,0 @@
-// Jest Snapshot v1, https://goo.gl/fbAQLP
-
-exports[`Profiler should clean up after a root has been unmounted: 1: mount 1`] = `
-[root]
-  ▾ <Parent key="A">
-      <Child key="0">
-      <Child key="1">
-      <Child key="2">
-[root]
-  ▾ <Parent key="B">
-      <Child key="0">
-      <Child key="1">
-`;
-
-exports[`Profiler should clean up after a root has been unmounted: 2: profiling started 1`] = `
-[root]
-  ▾ <Parent key="A">
-      <Child key="0">
-      <Child key="1">
-      <Child key="2">
-[root]
-  ▾ <Parent key="B">
-      <Child key="0">
-      <Child key="1">
-`;
-
-exports[`Profiler should clean up after a root has been unmounted: 3: update 1`] = `
-[root]
-  ▾ <Parent key="A">
-      <Child key="0">
-      <Child key="1">
-      <Child key="2">
-      <Child key="3">
-[root]
-  ▾ <Parent key="B">
-      <Child key="0">
-`;
-
-exports[`Profiler should clean up after a root has been unmounted: 4: unmount B 1`] = `
-[root]
-  ▾ <Parent key="A">
-      <Child key="0">
-      <Child key="1">
-      <Child key="2">
-      <Child key="3">
-`;
-
-exports[`Profiler should clean up after a root has been unmounted: 5: unmount A 1`] = ``;
-
-exports[`Profiler should clean up after a root has been unmounted: 6: profiling stopped 1`] = ``;
-
-exports[`Profiler should collect basic profiling metrics: 1: mount 1`] = `
-[root]
-  ▾ <Parent key="A">
-      <Child key="0">
-      <Child key="1">
-`;
-
-exports[`Profiler should collect basic profiling metrics: 2: add child 1`] = `
-[root]
-  ▾ <Parent key="A">
-      <Child key="0">
-      <Child key="1">
-      <Child key="2">
-`;
-
-exports[`Profiler should collect basic profiling metrics: 3: remove children 1`] = `
-[root]
-  ▾ <Parent key="A">
-      <Child key="0">
-`;
-
-exports[`Profiler should collect basic profiling metrics: 4: profiling stopped 1`] = `
-[root]
-  ▾ <Parent key="A">
-      <Child key="0">
-`;
-
-exports[`Profiler should collect basic profiling metrics: ProfilingSummary 1`] = `
-{
-  "rootID": 1,
-  "commitDurations": [
-    0,
-    1
-  ],
-  "commitTimes": [
-    0,
-    1
-  ],
-  "initialTreeBaseDurations": [
-    [
-      1,
-      0
-    ],
-    [
-      2,
-      1
-    ],
-    [
-      3,
-      2
-    ],
-    [
-      4,
-      3
-    ]
-  ],
-  "interactionCount": 0
-}
-`;

src/__tests__/__snapshots__/profiling-test.js.snap

@@ -0,0 +1,61 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`profiling profilingSummary should be collected for each commit: 1: mount 1`] = `
+[root]
+  ▾ <Parent key="A">
+      <Child key="0">
+      <Child key="1">
+`;
+
+exports[`profiling profilingSummary should be collected for each commit: 2: add child 1`] = `
+[root]
+  ▾ <Parent key="A">
+      <Child key="0">
+      <Child key="1">
+      <Child key="2">
+`;
+
+exports[`profiling profilingSummary should be collected for each commit: 3: remove children 1`] = `
+[root]
+  ▾ <Parent key="A">
+      <Child key="0">
+`;
+
+exports[`profiling profilingSummary should be collected for each commit: 4: profiling stopped 1`] = `
+[root]
+  ▾ <Parent key="A">
+      <Child key="0">
+`;
+
+exports[`profiling profilingSummary should be collected for each commit: ProfilingSummary 1`] = `
+{
+  "rootID": 1,
+  "commitDurations": [
+    16,
+    12
+  ],
+  "commitTimes": [
+    16,
+    28
+  ],
+  "initialTreeBaseDurations": [
+    [
+      1,
+      14
+    ],
+    [
+      2,
+      14
+    ],
+    [
+      3,
+      2
+    ],
+    [
+      4,
+      2
+    ]
+  ],
+  "interactionCount": 0
+}
+`;

src/__tests__/profiler-test.js

@@ -1,131 +0,0 @@
-// @flow
-
-describe('Profiler', () => {
-  let React;
-  let ReactDOM;
-  let TestRenderer;
-  let TestUtils;
-  let agent;
-  let store;
-
-  const act = (callback: Function) => {
-    TestUtils.act(() => {
-      callback();
-    });
-    jest.runAllTimers(); // Flush Bridge operations
-  };
-
-  const renderAndResolve = async (root, element) => {
-    // $FlowFixMe Flow doens't know about "await act()" yet
-    await TestUtils.act(async () => {
-      root.update(element);
-
-      // Resolve pending suspense promises
-      jest.runAllTimers();
-    });
-
-    // Re-render after resolved promises
-    jest.runAllTimers();
-  };
-
-  beforeEach(() => {
-    agent = global.agent;
-    store = global.store;
-    store.collapseNodesByDefault = false;
-
-    React = require('react');
-    ReactDOM = require('react-dom');
-    TestUtils = require('react-dom/test-utils');
-
-    // Hide the hook before requiring TestRenderer, so we don't end up with a loop.
-    const hook = global.__REACT_DEVTOOLS_GLOBAL_HOOK__;
-    delete global.__REACT_DEVTOOLS_GLOBAL_HOOK__;
-    TestRenderer = require('react-test-renderer');
-    global.__REACT_DEVTOOLS_GLOBAL_HOOK__ = hook;
-  });
-
-  it('should collect basic profiling metrics', async done => {
-    const Parent = ({ count }) =>
-      new Array(count).fill(true).map((_, index) => <Child key={index} />);
-    const Child = () => {
-      jest.advanceTimersByTime(1);
-      return null;
-    };
-
-    const container = document.createElement('div');
-
-    act(() => ReactDOM.render(<Parent key="A" count={2} />, container));
-    expect(store).toMatchSnapshot('1: mount');
-
-    act(() => store.startProfiling());
-
-    act(() => ReactDOM.render(<Parent key="A" count={3} />, container));
-    expect(store).toMatchSnapshot('2: add child');
-
-    act(() => ReactDOM.render(<Parent key="A" count={1} />, container));
-    expect(store).toMatchSnapshot('3: remove children');
-
-    act(() => store.stopProfiling());
-    expect(store).toMatchSnapshot('4: profiling stopped');
-
-    let profilingSummary;
-    function Suspender({ rendererID, rootID }) {
-      profilingSummary = store.profilingCache.ProfilingSummary.read({
-        rendererID,
-        rootID,
-      });
-      return null;
-    }
-
-    // HACK There's only one renderer for this test
-    const rendererID = Object.keys(agent._rendererInterfaces)[0];
-    const rootID = store.roots[0];
-
-    let root = TestRenderer.create();
-    await renderAndResolve(
-      root,
-      <React.Suspense fallback={null}>
-        <Suspender rendererID={rendererID} rootID={rootID} />
-      </React.Suspense>
-    );
-
-    // HACK root.toTree() doesn't handle Suspense yet
-    // but Jest serializer wouldn't work with a JSON string
-    expect(profilingSummary).toMatchSnapshot('ProfilingSummary');
-
-    done();
-  });
-
-  it('should clean up after a root has been unmounted', async () => {
-    const Parent = ({ count }) =>
-      new Array(count).fill(true).map((_, index) => <Child key={index} />);
-    const Child = () => <div>Hi!</div>;
-
-    const containerA = document.createElement('div');
-    const containerB = document.createElement('div');
-
-    act(() => {
-      ReactDOM.render(<Parent key="A" count={3} />, containerA);
-      ReactDOM.render(<Parent key="B" count={2} />, containerB);
-    });
-    expect(store).toMatchSnapshot('1: mount');
-
-    act(() => store.startProfiling());
-    expect(store).toMatchSnapshot('2: profiling started');
-
-    act(() => {
-      ReactDOM.render(<Parent key="A" count={4} />, containerA);
-      ReactDOM.render(<Parent key="B" count={1} />, containerB);
-    });
-    expect(store).toMatchSnapshot('3: update');
-
-    act(() => ReactDOM.unmountComponentAtNode(containerB));
-    expect(store).toMatchSnapshot('4: unmount B');
-
-    act(() => ReactDOM.unmountComponentAtNode(containerA));
-    expect(store).toMatchSnapshot('5: unmount A');
-
-    act(() => store.stopProfiling());
-    expect(store).toMatchSnapshot('6: profiling stopped');
-  });
-});

src/__tests__/profiling-test.js

@@ -0,0 +1,108 @@
+// @flow
+
+describe('profiling', () => {
+  let React;
+  let ReactDOM;
+  let Scheduler;
+  let TestRenderer;
+  let store;
+  let utils;
+
+  beforeEach(() => {
+    utils = require('./utils');
+    utils.beforeEachProfiling();
+
+    store = global.store;
+    store.collapseNodesByDefault = false;
+
+    React = require('react');
+    ReactDOM = require('react-dom');
+    Scheduler = require('scheduler');
+    TestRenderer = utils.requireTestRenderer();
+  });
+
+  describe('profilingSummary', () => {
+    it('should be collected for each commit', async done => {
+      const Parent = ({ count }) => {
+        Scheduler.advanceTime(10);
+        return new Array(count)
+          .fill(true)
+          .map((_, index) => <Child key={index} />);
+      };
+      const Child = () => {
+        Scheduler.advanceTime(2);
+        return null;
+      };
+
+      const container = document.createElement('div');
+
+      utils.act(() => ReactDOM.render(<Parent key="A" count={2} />, container));
+      expect(store).toMatchSnapshot('1: mount');
+
+      utils.act(() => store.startProfiling());
+
+      utils.act(() => ReactDOM.render(<Parent key="A" count={3} />, container));
+      expect(store).toMatchSnapshot('2: add child');
+
+      utils.act(() => ReactDOM.render(<Parent key="A" count={1} />, container));
+      expect(store).toMatchSnapshot('3: remove children');
+
+      utils.act(() => store.stopProfiling());
+      expect(store).toMatchSnapshot('4: profiling stopped');
+
+      let profilingSummary;
+      function Suspender({ rendererID, rootID }) {
+        profilingSummary = store.profilingCache.ProfilingSummary.read({
+          rendererID,
+          rootID,
+        });
+        return null;
+      }
+
+      const rendererID = utils.getRendererID();
+      const rootID = store.roots[0];
+
+      await utils.actSuspense(() =>
+        TestRenderer.create(
+          <React.Suspense fallback={null}>
+            <Suspender rendererID={rendererID} rootID={rootID} />
+          </React.Suspense>
+        )
+      );
+
+      expect(profilingSummary).toMatchSnapshot('ProfilingSummary');
+
+      done();
+    });
+  });
+
+  it('should remove profiling data when roots are unmounted', async () => {
+    const Parent = ({ count }) =>
+      new Array(count).fill(true).map((_, index) => <Child key={index} />);
+    const Child = () => <div>Hi!</div>;
+
+    const containerA = document.createElement('div');
+    const containerB = document.createElement('div');
+
+    utils.act(() => {
+      ReactDOM.render(<Parent key="A" count={3} />, containerA);
+      ReactDOM.render(<Parent key="B" count={2} />, containerB);
+    });
+
+    utils.act(() => store.startProfiling());
+
+    utils.act(() => {
+      ReactDOM.render(<Parent key="A" count={4} />, containerA);
+      ReactDOM.render(<Parent key="B" count={1} />, containerB);
+    });
+
+    utils.act(() => ReactDOM.unmountComponentAtNode(containerB));
+
+    utils.act(() => ReactDOM.unmountComponentAtNode(containerA));
+
+    utils.act(() => store.stopProfiling());
+
+    // Assert all maps are empty
+    store.assertExpectedRootMapSizes();
+  });
+});

src/__tests__/profilingSummarySerializer.js

@@ -17,11 +17,7 @@ export function print(profilingSummary, serialize, indent) {
   return JSON.stringify(
     {
       ...profilingSummary,
-      commitDurations: profilingSummary.commitDurations.map((_, i) => i),
-      commitTimes: profilingSummary.commitTimes.map((_, i) => i),
-      initialTreeBaseDurations: [
-        ...profilingSummary.initialTreeBaseDurations,
-      ].map(([id, _], index) => [id, index]),
+      initialTreeBaseDurations: [...profilingSummary.initialTreeBaseDurations],
     },
     null,
     2

src/__tests__/store-test.js

@@ -6,6 +6,7 @@ describe('Store', () => {
   let TestUtils;
   let agent;
   let store;
+  let utils;
 
   const act = (callback: Function) => {
     TestUtils.act(() => {
@@ -21,6 +22,7 @@ describe('Store', () => {
     React = require('react');
     ReactDOM = require('react-dom');
     TestUtils = require('react-dom/test-utils');
+    utils = require('./utils');
   });
 
   it('should not allow a root node to be collapsed', () => {
@@ -281,8 +283,7 @@ describe('Store', () => {
       );
       expect(store).toMatchSnapshot('7: only third child is suspended');
 
-      // HACK There's only one renderer for this test
-      const rendererID = Object.keys(agent._rendererInterfaces)[0];
+      const rendererID = utils.getRendererID();
       act(() =>
         agent.overrideSuspense({
           id: store.getElementIDAtIndex(4),
@@ -673,8 +674,7 @@ describe('Store', () => {
       act(() => store.toggleIsCollapsed(store.getElementIDAtIndex(1), false));
       expect(store).toMatchSnapshot('2: expand tree');
 
-      // HACK There's only one renderer for this test
-      const rendererID = Object.keys(agent._rendererInterfaces)[0];
+      const rendererID = utils.getRendererID();
       const suspenseID = store.getElementIDAtIndex(1);
 
       act(() =>

src/__tests__/storeOwners-test.js

@@ -5,23 +5,16 @@ const { printOwnersList } = require('./storeSerializer');
 describe('Store owners list', () => {
   let React;
   let ReactDOM;
-  let TestUtils;
+  let act;
   let store;
 
-  const act = (callback: Function) => {
-    TestUtils.act(() => {
-      callback();
-    });
-    jest.runAllTimers(); // Flush Bridge operations
-  };
-
   beforeEach(() => {
     store = global.store;
     store.collapseNodesByDefault = false;
 
     React = require('react');
     ReactDOM = require('react-dom');
-    TestUtils = require('react-dom/test-utils');
+    act = require('./utils').act;
   });
 
   it('should drill through intermediate components', () => {

src/__tests__/storeSerializer.js

@@ -70,9 +70,9 @@ export function printStore(store, includeWeight = false) {
     );
   }
 
-  if (store.roots.length === 0) {
-    store.assertEmptyMaps();
-  }
+  // If roots have been unmounted, verify that they've been removed from maps.
+  // This helps ensure the Store doesn't leak memory.
+  store.assertExpectedRootMapSizes();
 
   return snapshotLines.join('\n');
 }

src/__tests__/storeStressSync-test.js

@@ -3,18 +3,11 @@
 describe('StoreStress (Sync Mode)', () => {
   let React;
   let ReactDOM;
-  let TestUtils;
+  let act;
   let bridge;
   let store;
   let print;
 
-  const act = (callback: Function) => {
-    TestUtils.act(() => {
-      callback();
-    });
-    jest.runAllTimers(); // Flush Bridge operations
-  };
-
   beforeEach(() => {
     bridge = global.bridge;
     store = global.store;
@@ -22,7 +15,7 @@ describe('StoreStress (Sync Mode)', () => {
 
     React = require('react');
     ReactDOM = require('react-dom');
-    TestUtils = require('react-dom/test-utils');
+    act = require('./utils').act;
 
     print = require('./storeSerializer').print;
   });

src/__tests__/storeStressTestConcurrent-test.js

@@ -3,19 +3,11 @@
 describe('StoreStressConcurrent', () => {
   let React;
   let ReactDOM;
-  let TestUtils;
+  let act;
   let bridge;
   let store;
   let print;
 
-  const act = (callback: Function) => {
-    TestUtils.act(() => {
-      callback();
-    });
-    jest.advanceTimersByTime(1000); // Flush rendering and Suspense
-    jest.runAllTimers(); // Flush Bridge operations
-  };
-
   beforeEach(() => {
     bridge = global.bridge;
     store = global.store;
@@ -23,7 +15,7 @@ describe('StoreStressConcurrent', () => {
 
     React = require('react');
     ReactDOM = require('react-dom');
-    TestUtils = require('react-dom/test-utils');
+    act = require('./utils').act;
 
     print = require('./storeSerializer').print;
   });

src/__tests__/utils.js

@@ -0,0 +1,66 @@
+// @flow
+
+export function act(callback: Function): void {
+  const TestUtils = require('react-dom/test-utils');
+  TestUtils.act(() => {
+    callback();
+  });
+
+  // Flush Bridge operations
+  jest.runAllTimers();
+}
+
+export async function actSuspense(callback: Function) {
+  const TestUtils = require('react-dom/test-utils');
+  const Scheduler = require('scheduler');
+
+  // $FlowFixMe Flow doens't know about "await act()" yet
+  await TestUtils.act(async () => {
+    callback();
+
+    // Resolve pending suspense promises
+    jest.runAllTimers();
+  });
+
+  // Re-render after resolved promises
+  Scheduler.flushAll();
+}
+
+export function beforeEachProfiling() {
+  // Mock React's timing information so that test runs are predictable.
+  jest.mock('scheduler', () =>
+    // $FlowFixMe Flow does not konw about requireActual
+    require.requireActual('scheduler/unstable_mock')
+  );
+
+  // DevTools itself uses performance.now() to offset commit times
+  // so they appear relative to when profiling was started in the UI.
+  jest.spyOn(performance, 'now').mockImplementation(
+    // $FlowFixMe Flow does not konw about requireActual
+    require.requireActual('scheduler/unstable_mock').unstable_now
+  );
+}
+
+export function getRendererID() {
+  if (global.agent == null) {
+    throw Error('Agent unavailable.');
+  }
+  const ids = Object.keys(global.agent._rendererInterfaces);
+  if (ids.length !== 1) {
+    throw Error('Multiple renderers attached.');
+  }
+  return ids[0];
+}
+
+export function requireTestRenderer() {
+  let hook;
+  try {
+    // Hide the hook before requiring TestRenderer, so we don't end up with a loop.
+    hook = global.__REACT_DEVTOOLS_GLOBAL_HOOK__;
+    delete global.__REACT_DEVTOOLS_GLOBAL_HOOK__;
+
+    return require('react-test-renderer');
+  } finally {
+    global.__REACT_DEVTOOLS_GLOBAL_HOOK__ = hook;
+  }
+}

src/devtools/store.js

@@ -192,30 +192,49 @@ export default class Store extends EventEmitter {
   }
 
   // This is only used in tests to avoid memory leaks.
-  assertEmptyMaps() {
-    this.assertEmptyMap(this._idToElement, '_idToElement');
-    this.assertEmptyMap(this._ownersMap, '_ownersMap');
-    this.assertEmptyMap(
-      this._profilingOperationsByRootID,
-      '_profilingOperationsByRootID'
+  assertExpectedRootMapSizes() {
+    if (this.roots.length === 0) {
+      // The only safe time to assert these maps are empty is when the store is empty.
+      this.assertMapSizeMatchesRootCount(this._idToElement, '_idToElement');
+      this.assertMapSizeMatchesRootCount(this._ownersMap, '_ownersMap');
+
+      // These maps will be empty unless profiling mode has been started.
+      // After this, their size should always match the number of roots,
+      // but unless we want to track additional metadata about profiling history,
+      // the only safe time to assert this is when the store is empty.
+      this.assertMapSizeMatchesRootCount(
+        this._profilingOperationsByRootID,
+        '_profilingOperationsByRootID'
+      );
+      this.assertMapSizeMatchesRootCount(
+        this._profilingScreenshotsByRootID,
+        '_profilingScreenshotsByRootID'
+      );
+      this.assertMapSizeMatchesRootCount(
+        this._profilingSnapshotsByRootID,
+        '_profilingSnapshotsByRootID'
+      );
+    }
+
+    // These maps should always be the same size as the number of roots
+    this.assertMapSizeMatchesRootCount(
+      this._rootIDToCapabilities,
+      '_rootIDToCapabilities'
     );
-    this.assertEmptyMap(
-      this._profilingScreenshotsByRootID,
-      '_profilingScreenshotsByRootID'
+    this.assertMapSizeMatchesRootCount(
+      this._rootIDToRendererID,
+      '_rootIDToRendererID'
     );
-    this.assertEmptyMap(
-      this._profilingSnapshotsByRootID,
-      '_profilingSnapshotsByRootID'
-    );
-    this.assertEmptyMap(this._rootIDToCapabilities, '_rootIDToCapabilities');
-    this.assertEmptyMap(this._rootIDToRendererID, '_rootIDToRendererID');
   }
 
   // This is only used in tests to avoid memory leaks.
-  assertEmptyMap(map: Map<any, any>, mapName: string) {
-    if (map.size !== 0) {
+  assertMapSizeMatchesRootCount(map: Map<any, any>, mapName: string) {
+    const expectedSize = this.roots.length;
+    if (map.size !== expectedSize) {
       throw new Error(
-        `Expected ${mapName} to be empty, got ${map.size}: ${inspect(map, {
+        `Expected ${mapName} to contain ${expectedSize} items, but it contains ${
+          map.size
+        } items\n\n${inspect(map, {
           depth: 20,
         })}`
       );