From f7c8e3a05c9dd4f32891f563efa4307dc1333f5a Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Fri, 3 May 2019 15:01:26 -0700 Subject: [PATCH 01/12] Experimenting with profiler tests --- package.json | 2 + .../__snapshots__/profiler-test.js.snap | 105 +++++++++++++++--- src/__tests__/profiler-test.js | 85 ++++++++++++-- src/__tests__/profilingSummarySerializer.js | 29 +++++ src/__tests__/setupTests.js | 10 +- yarn.lock | 10 ++ 6 files changed, 217 insertions(+), 24 deletions(-) create mode 100644 src/__tests__/profilingSummarySerializer.js diff --git a/package.json b/package.json index 479e760241..0c42947e5c 100644 --- a/package.json +++ b/package.json @@ -23,6 +23,7 @@ "/src/__tests__/setupTests" ], "snapshotSerializers": [ + "/src/__tests__/profilingSummarySerializer", "/src/__tests__/storeSerializer" ], "testMatch": [ @@ -134,6 +135,7 @@ "react-color": "^2.11.7", "react-dom": "0.0.0-fb28e9048", "react-is": "0.0.0-fb28e9048", + "react-test-renderer": "0.0.0-fb28e9048", "react-virtualized-auto-sizer": "^1.0.2", "react-window": "^1.8.0", "request-promise": "^4.2.4", diff --git a/src/__tests__/__snapshots__/profiler-test.js.snap b/src/__tests__/__snapshots__/profiler-test.js.snap index dade7370b5..3c68497cad 100644 --- a/src/__tests__/__snapshots__/profiler-test.js.snap +++ b/src/__tests__/__snapshots__/profiler-test.js.snap @@ -1,31 +1,110 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Profiler should start and stop profiling, handle root unmounting: 1: mount 1`] = ` +exports[`Profiler should clean up after a root has been unmounted: 1: mount 1`] = ` [root] - ▸ + ▾ + + + [root] - ▸ + ▾ + + `; -exports[`Profiler should start and stop profiling, handle root unmounting: 2: profiling started 1`] = ` +exports[`Profiler should clean up after a root has been unmounted: 2: profiling started 1`] = ` [root] - ▸ + ▾ + + + [root] - ▸ + ▾ + + `; -exports[`Profiler should start and stop profiling, handle root unmounting: 3: update 1`] = ` +exports[`Profiler should clean up after a root has been unmounted: 3: update 1`] = ` [root] - ▸ + ▾ + + + + [root] - ▸ + ▾ + `; -exports[`Profiler should start and stop profiling, handle root unmounting: 4: unmount B 1`] = ` +exports[`Profiler should clean up after a root has been unmounted: 4: unmount B 1`] = ` [root] - ▸ + ▾ + + + + `; -exports[`Profiler should start and stop profiling, handle root unmounting: 5: unmount A 1`] = ``; +exports[`Profiler should clean up after a root has been unmounted: 5: unmount A 1`] = ``; -exports[`Profiler should start and stop profiling, handle root unmounting: 6: profiling stopped 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] + ▾ + + +`; + +exports[`Profiler should collect basic profiling metrics: 2: add child 1`] = ` +[root] + ▾ + + + +`; + +exports[`Profiler should collect basic profiling metrics: 3: remove children 1`] = ` +[root] + ▾ + +`; + +exports[`Profiler should collect basic profiling metrics: 4: profiling stopped 1`] = ` +[root] + ▾ + +`; + +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 +} +`; diff --git a/src/__tests__/profiler-test.js b/src/__tests__/profiler-test.js index 28be905d05..98c1172ae3 100644 --- a/src/__tests__/profiler-test.js +++ b/src/__tests__/profiler-test.js @@ -3,7 +3,9 @@ describe('Profiler', () => { let React; let ReactDOM; + let TestRenderer; let TestUtils; + let agent; let store; const act = (callback: Function) => { @@ -13,15 +15,88 @@ describe('Profiler', () => { 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 start and stop profiling, handle root unmounting', async () => { + it('should collect basic profiling metrics', async done => { + const Parent = ({ count }) => + new Array(count).fill(true).map((_, index) => ); + const Child = () => { + jest.advanceTimersByTime(1); + return null; + }; + + const container = document.createElement('div'); + + act(() => ReactDOM.render(, container)); + expect(store).toMatchSnapshot('1: mount'); + + act(() => store.startProfiling()); + + act(() => ReactDOM.render(, container)); + expect(store).toMatchSnapshot('2: add child'); + + act(() => ReactDOM.render(, 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, + + + + ); + + // 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) => ); const Child = () =>
Hi!
; @@ -35,9 +110,7 @@ describe('Profiler', () => { }); expect(store).toMatchSnapshot('1: mount'); - act(() => { - store.startProfiling(); - }); + act(() => store.startProfiling()); expect(store).toMatchSnapshot('2: profiling started'); act(() => { @@ -52,9 +125,7 @@ describe('Profiler', () => { act(() => ReactDOM.unmountComponentAtNode(containerA)); expect(store).toMatchSnapshot('5: unmount A'); - act(() => { - store.stopProfiling(); - }); + act(() => store.stopProfiling()); expect(store).toMatchSnapshot('6: profiling stopped'); }); }); diff --git a/src/__tests__/profilingSummarySerializer.js b/src/__tests__/profilingSummarySerializer.js new file mode 100644 index 0000000000..45e8477dfd --- /dev/null +++ b/src/__tests__/profilingSummarySerializer.js @@ -0,0 +1,29 @@ +// test() is part of Jest's serializer API +export function test(maybeProfilingSummary) { + return ( + typeof maybeProfilingSummary === 'object' && + maybeProfilingSummary !== null && + typeof maybeProfilingSummary.rootID === 'number' && + Array.isArray(maybeProfilingSummary.commitDurations) && + Array.isArray(maybeProfilingSummary.commitTimes) && + typeof maybeProfilingSummary.initialTreeBaseDurations === 'object' && + maybeProfilingSummary.initialTreeBaseDurations !== null && + typeof maybeProfilingSummary.interactionCount === 'number' + ); +} + +// print() is part of Jest's serializer API +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]), + }, + null, + 2 + ); +} diff --git a/src/__tests__/setupTests.js b/src/__tests__/setupTests.js index ce96ed2b48..b498a014fd 100644 --- a/src/__tests__/setupTests.js +++ b/src/__tests__/setupTests.js @@ -8,10 +8,6 @@ import { installHook } from 'src/hook'; const env = jasmine.getEnv(); env.beforeEach(() => { - // It's important to reset modules between test runs; - // Without this, ReactDOM won't re-inject itself into the new hook. - jest.resetModules(); - // Fake timers let us flush Bridge operations between setup and assertions. jest.useFakeTimers(); @@ -55,4 +51,10 @@ env.beforeEach(() => { }); env.afterEach(() => { delete global.__REACT_DEVTOOLS_GLOBAL_HOOK__; + + // It's important to reset modules between test runs; + // Without this, ReactDOM won't re-inject itself into the new hook. + // It's also important to reset after tests, rather than before, + // so that we don't disconnect the ReactCurrentDispatcher ref. + jest.resetModules(); }); diff --git a/yarn.lock b/yarn.lock index 6ed41c6740..32a0156b37 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9837,6 +9837,16 @@ react-lifecycles-compat@^3.0.4: resolved "https://registry.yarnpkg.com/react-lifecycles-compat/-/react-lifecycles-compat-3.0.4.tgz#4f1a273afdfc8f3488a8c516bfda78f872352362" integrity sha512-fBASbA6LnOU9dOU2eW7aQ8xmYBSXUIWr+UmF9b1efZBazGNO+rcXT/icdKnYm2pTwcRylVUYwW7H1PHfLekVzA== +react-test-renderer@0.0.0-fb28e9048: + version "0.0.0-fb28e9048" + resolved "https://registry.yarnpkg.com/react-test-renderer/-/react-test-renderer-0.0.0-fb28e9048.tgz#1a94c8d19cbb1ac98ab37c66c0b294e5be280c52" + integrity sha512-WK/wQOh0v6+8Gbkurgb3he9hKoOKWueqQY+RFs2vM3u3vn7PMyYhzm/KkU75VvTG/GVciojNQBHBpekuvU5dYw== + dependencies: + object-assign "^4.1.1" + prop-types "^15.6.2" + react-is "0.0.0-fb28e9048" + scheduler "0.0.0-fb28e9048" + react-virtualized-auto-sizer@^1.0.2: version "1.0.2" resolved "https://registry.yarnpkg.com/react-virtualized-auto-sizer/-/react-virtualized-auto-sizer-1.0.2.tgz#a61dd4f756458bbf63bd895a92379f9b70f803bd" From dd96b3314cf0ec0ad50d38d9348b591068c492c5 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sat, 4 May 2019 09:36:13 -0700 Subject: [PATCH 02/12] Cleaned up tests a bit. Profiling test uses mock timers now. --- .../__snapshots__/profiler-test.js.snap | 110 --------------- .../__snapshots__/profiling-test.js.snap | 61 ++++++++ src/__tests__/profiler-test.js | 131 ------------------ src/__tests__/profiling-test.js | 108 +++++++++++++++ src/__tests__/profilingSummarySerializer.js | 6 +- src/__tests__/store-test.js | 8 +- src/__tests__/storeOwners-test.js | 11 +- src/__tests__/storeSerializer.js | 6 +- src/__tests__/storeStressSync-test.js | 11 +- .../storeStressTestConcurrent-test.js | 12 +- src/__tests__/utils.js | 66 +++++++++ src/devtools/store.js | 55 +++++--- 12 files changed, 286 insertions(+), 299 deletions(-) delete mode 100644 src/__tests__/__snapshots__/profiler-test.js.snap create mode 100644 src/__tests__/__snapshots__/profiling-test.js.snap delete mode 100644 src/__tests__/profiler-test.js create mode 100644 src/__tests__/profiling-test.js create mode 100644 src/__tests__/utils.js diff --git a/src/__tests__/__snapshots__/profiler-test.js.snap b/src/__tests__/__snapshots__/profiler-test.js.snap deleted file mode 100644 index 3c68497cad..0000000000 --- a/src/__tests__/__snapshots__/profiler-test.js.snap +++ /dev/null @@ -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] - ▾ - - - -[root] - ▾ - - -`; - -exports[`Profiler should clean up after a root has been unmounted: 2: profiling started 1`] = ` -[root] - ▾ - - - -[root] - ▾ - - -`; - -exports[`Profiler should clean up after a root has been unmounted: 3: update 1`] = ` -[root] - ▾ - - - - -[root] - ▾ - -`; - -exports[`Profiler should clean up after a root has been unmounted: 4: unmount B 1`] = ` -[root] - ▾ - - - - -`; - -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] - ▾ - - -`; - -exports[`Profiler should collect basic profiling metrics: 2: add child 1`] = ` -[root] - ▾ - - - -`; - -exports[`Profiler should collect basic profiling metrics: 3: remove children 1`] = ` -[root] - ▾ - -`; - -exports[`Profiler should collect basic profiling metrics: 4: profiling stopped 1`] = ` -[root] - ▾ - -`; - -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 -} -`; diff --git a/src/__tests__/__snapshots__/profiling-test.js.snap b/src/__tests__/__snapshots__/profiling-test.js.snap new file mode 100644 index 0000000000..a1e084986e --- /dev/null +++ b/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] + ▾ + + +`; + +exports[`profiling profilingSummary should be collected for each commit: 2: add child 1`] = ` +[root] + ▾ + + + +`; + +exports[`profiling profilingSummary should be collected for each commit: 3: remove children 1`] = ` +[root] + ▾ + +`; + +exports[`profiling profilingSummary should be collected for each commit: 4: profiling stopped 1`] = ` +[root] + ▾ + +`; + +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 +} +`; diff --git a/src/__tests__/profiler-test.js b/src/__tests__/profiler-test.js deleted file mode 100644 index 98c1172ae3..0000000000 --- a/src/__tests__/profiler-test.js +++ /dev/null @@ -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) => ); - const Child = () => { - jest.advanceTimersByTime(1); - return null; - }; - - const container = document.createElement('div'); - - act(() => ReactDOM.render(, container)); - expect(store).toMatchSnapshot('1: mount'); - - act(() => store.startProfiling()); - - act(() => ReactDOM.render(, container)); - expect(store).toMatchSnapshot('2: add child'); - - act(() => ReactDOM.render(, 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, - - - - ); - - // 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) => ); - const Child = () =>
Hi!
; - - const containerA = document.createElement('div'); - const containerB = document.createElement('div'); - - act(() => { - ReactDOM.render(, containerA); - ReactDOM.render(, containerB); - }); - expect(store).toMatchSnapshot('1: mount'); - - act(() => store.startProfiling()); - expect(store).toMatchSnapshot('2: profiling started'); - - act(() => { - ReactDOM.render(, containerA); - ReactDOM.render(, 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'); - }); -}); diff --git a/src/__tests__/profiling-test.js b/src/__tests__/profiling-test.js new file mode 100644 index 0000000000..e740d45e4a --- /dev/null +++ b/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) => ); + }; + const Child = () => { + Scheduler.advanceTime(2); + return null; + }; + + const container = document.createElement('div'); + + utils.act(() => ReactDOM.render(, container)); + expect(store).toMatchSnapshot('1: mount'); + + utils.act(() => store.startProfiling()); + + utils.act(() => ReactDOM.render(, container)); + expect(store).toMatchSnapshot('2: add child'); + + utils.act(() => ReactDOM.render(, 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( + + + + ) + ); + + 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) => ); + const Child = () =>
Hi!
; + + const containerA = document.createElement('div'); + const containerB = document.createElement('div'); + + utils.act(() => { + ReactDOM.render(, containerA); + ReactDOM.render(, containerB); + }); + + utils.act(() => store.startProfiling()); + + utils.act(() => { + ReactDOM.render(, containerA); + ReactDOM.render(, containerB); + }); + + utils.act(() => ReactDOM.unmountComponentAtNode(containerB)); + + utils.act(() => ReactDOM.unmountComponentAtNode(containerA)); + + utils.act(() => store.stopProfiling()); + + // Assert all maps are empty + store.assertExpectedRootMapSizes(); + }); +}); diff --git a/src/__tests__/profilingSummarySerializer.js b/src/__tests__/profilingSummarySerializer.js index 45e8477dfd..a2bcb1b5a0 100644 --- a/src/__tests__/profilingSummarySerializer.js +++ b/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 diff --git a/src/__tests__/store-test.js b/src/__tests__/store-test.js index 5942574a77..4208eb06ec 100644 --- a/src/__tests__/store-test.js +++ b/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(() => diff --git a/src/__tests__/storeOwners-test.js b/src/__tests__/storeOwners-test.js index f4853b2fe0..9ed0e53fdc 100644 --- a/src/__tests__/storeOwners-test.js +++ b/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', () => { diff --git a/src/__tests__/storeSerializer.js b/src/__tests__/storeSerializer.js index f41cb0fb84..80a5b3f1e1 100644 --- a/src/__tests__/storeSerializer.js +++ b/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'); } diff --git a/src/__tests__/storeStressSync-test.js b/src/__tests__/storeStressSync-test.js index 54d60ef13e..8877d5d999 100644 --- a/src/__tests__/storeStressSync-test.js +++ b/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; }); diff --git a/src/__tests__/storeStressTestConcurrent-test.js b/src/__tests__/storeStressTestConcurrent-test.js index 546db55066..d2a834ca98 100644 --- a/src/__tests__/storeStressTestConcurrent-test.js +++ b/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; }); diff --git a/src/__tests__/utils.js b/src/__tests__/utils.js new file mode 100644 index 0000000000..cb15590ee5 --- /dev/null +++ b/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; + } +} diff --git a/src/devtools/store.js b/src/devtools/store.js index 8321702717..d6e93d9fb9 100644 --- a/src/devtools/store.js +++ b/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, mapName: string) { - if (map.size !== 0) { + assertMapSizeMatchesRootCount(map: Map, 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, })}` ); From da1e5776b1491177c59d68e7723ad0e787e7fc49 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sat, 4 May 2019 13:11:56 -0700 Subject: [PATCH 03/12] Added a second Profiling test (for CommitDetails) and fixed some module reset prolems --- package.json | 1 - .../__snapshots__/profiling-test.js.snap | 113 +++++++++++++----- src/__tests__/profiling-test.js | 87 ++++++++++++++ src/__tests__/profilingSummarySerializer.js | 25 ---- src/__tests__/setupTests.js | 16 ++- src/__tests__/storeSerializer.js | 7 +- src/devtools/cache.js | 2 + 7 files changed, 188 insertions(+), 63 deletions(-) delete mode 100644 src/__tests__/profilingSummarySerializer.js diff --git a/package.json b/package.json index 0c42947e5c..0dfb1b3c71 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,6 @@ "/src/__tests__/setupTests" ], "snapshotSerializers": [ - "/src/__tests__/profilingSummarySerializer", "/src/__tests__/storeSerializer" ], "testMatch": [ diff --git a/src/__tests__/__snapshots__/profiling-test.js.snap b/src/__tests__/__snapshots__/profiling-test.js.snap index a1e084986e..4f9c7ae56d 100644 --- a/src/__tests__/__snapshots__/profiling-test.js.snap +++ b/src/__tests__/__snapshots__/profiling-test.js.snap @@ -1,5 +1,74 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`profiling commitDetails should be collected for each commit: 1: mount 1`] = ` +[root] + ▾ + + +`; + +exports[`profiling commitDetails should be collected for each commit: 2: add child 1`] = ` +[root] + ▾ + + + +`; + +exports[`profiling commitDetails should be collected for each commit: 3: remove children 1`] = ` +[root] + ▾ + +`; + +exports[`profiling commitDetails should be collected for each commit: 4: profiling stopped 1`] = ` +[root] + ▾ + +`; + +exports[`profiling commitDetails should be collected for each commit: 5: CommitDetails: mount 1`] = ` +Object { + "actualDurations": Map { + 1 => 14, + 2 => 14, + 3 => 2, + 4 => 2, + }, + "commitIndex": 0, + "interactions": Array [], + "rootID": 1, +} +`; + +exports[`profiling commitDetails should be collected for each commit: 6: CommitDetails: add child 1`] = ` +Object { + "actualDurations": Map { + 3 => 2, + 4 => 2, + 5 => 2, + 2 => 16, + 1 => 16, + }, + "commitIndex": 1, + "interactions": Array [], + "rootID": 1, +} +`; + +exports[`profiling commitDetails should be collected for each commit: 7: CommitDetails: remove children 1`] = ` +Object { + "actualDurations": Map { + 3 => 2, + 2 => 12, + 1 => 12, + }, + "commitIndex": 2, + "interactions": Array [], + "rootID": 1, +} +`; + exports[`profiling profilingSummary should be collected for each commit: 1: mount 1`] = ` [root] ▾ @@ -28,34 +97,22 @@ exports[`profiling profilingSummary should be collected for each commit: 4: prof `; exports[`profiling profilingSummary should be collected for each commit: ProfilingSummary 1`] = ` -{ +Object { + "commitDurations": Array [ + 16, + 12, + ], + "commitTimes": Array [ + 16, + 28, + ], + "initialTreeBaseDurations": Map { + 1 => 14, + 2 => 14, + 3 => 2, + 4 => 2, + }, + "interactionCount": 0, "rootID": 1, - "commitDurations": [ - 16, - 12 - ], - "commitTimes": [ - 16, - 28 - ], - "initialTreeBaseDurations": [ - [ - 1, - 14 - ], - [ - 2, - 14 - ], - [ - 3, - 2 - ], - [ - 4, - 2 - ] - ], - "interactionCount": 0 } `; diff --git a/src/__tests__/profiling-test.js b/src/__tests__/profiling-test.js index e740d45e4a..253270cf13 100644 --- a/src/__tests__/profiling-test.js +++ b/src/__tests__/profiling-test.js @@ -76,6 +76,93 @@ describe('profiling', () => { }); }); + describe('commitDetails', () => { + it('should be collected for each commit', async done => { + const Parent = ({ count }) => { + Scheduler.advanceTime(10); + return new Array(count) + .fill(true) + .map((_, index) => ); + }; + const Child = () => { + Scheduler.advanceTime(2); + return null; + }; + + const container = document.createElement('div'); + + utils.act(() => store.startProfiling()); + + utils.act(() => ReactDOM.render(, container)); + expect(store).toMatchSnapshot('1: mount'); + + utils.act(() => ReactDOM.render(, container)); + expect(store).toMatchSnapshot('2: add child'); + + utils.act(() => ReactDOM.render(, container)); + expect(store).toMatchSnapshot('3: remove children'); + + utils.act(() => store.stopProfiling()); + expect(store).toMatchSnapshot('4: profiling stopped'); + + let commitDetails; + function Suspender({ commitIndex, rendererID, rootID }) { + commitDetails = store.profilingCache.CommitDetails.read({ + commitIndex, + rendererID, + rootID, + }); + return null; + } + + const rendererID = utils.getRendererID(); + const rootID = store.roots[0]; + + await utils.actSuspense(() => + TestRenderer.create( + + + + ) + ); + expect(commitDetails).toMatchSnapshot('5: CommitDetails: mount'); + + await utils.actSuspense(() => + TestRenderer.create( + + + + ) + ); + expect(commitDetails).toMatchSnapshot('6: CommitDetails: add child'); + + await utils.actSuspense(() => + TestRenderer.create( + + + + ) + ); + expect(commitDetails).toMatchSnapshot( + '7: CommitDetails: remove children' + ); + + done(); + }); + }); + it('should remove profiling data when roots are unmounted', async () => { const Parent = ({ count }) => new Array(count).fill(true).map((_, index) => ); diff --git a/src/__tests__/profilingSummarySerializer.js b/src/__tests__/profilingSummarySerializer.js deleted file mode 100644 index a2bcb1b5a0..0000000000 --- a/src/__tests__/profilingSummarySerializer.js +++ /dev/null @@ -1,25 +0,0 @@ -// test() is part of Jest's serializer API -export function test(maybeProfilingSummary) { - return ( - typeof maybeProfilingSummary === 'object' && - maybeProfilingSummary !== null && - typeof maybeProfilingSummary.rootID === 'number' && - Array.isArray(maybeProfilingSummary.commitDurations) && - Array.isArray(maybeProfilingSummary.commitTimes) && - typeof maybeProfilingSummary.initialTreeBaseDurations === 'object' && - maybeProfilingSummary.initialTreeBaseDurations !== null && - typeof maybeProfilingSummary.interactionCount === 'number' - ); -} - -// print() is part of Jest's serializer API -export function print(profilingSummary, serialize, indent) { - return JSON.stringify( - { - ...profilingSummary, - initialTreeBaseDurations: [...profilingSummary.initialTreeBaseDurations], - }, - null, - 2 - ); -} diff --git a/src/__tests__/setupTests.js b/src/__tests__/setupTests.js index b498a014fd..2aec331a95 100644 --- a/src/__tests__/setupTests.js +++ b/src/__tests__/setupTests.js @@ -1,13 +1,17 @@ // @flow -import Agent from 'src/backend/agent'; -import { initBackend } from 'src/backend'; -import Bridge from 'src/bridge'; -import Store from 'src/devtools/store'; -import { installHook } from 'src/hook'; - const env = jasmine.getEnv(); env.beforeEach(() => { + // These files should be required (and re-reuired) before each test, + // rather than imported at the head of the module. + // That's because we reset modules between tests, + // which disconnects the DevTool's cache from the current dispatcher ref. + const Agent = require('src/backend/agent').default; + const { initBackend } = require('src/backend'); + const Bridge = require('src/bridge').default; + const Store = require('src/devtools/store').default; + const { installHook } = require('src/hook'); + // Fake timers let us flush Bridge operations between setup and assertions. jest.useFakeTimers(); diff --git a/src/__tests__/storeSerializer.js b/src/__tests__/storeSerializer.js index 80a5b3f1e1..6e6400aaf5 100644 --- a/src/__tests__/storeSerializer.js +++ b/src/__tests__/storeSerializer.js @@ -1,8 +1,9 @@ -import Store from 'src/devtools/store'; - // test() is part of Jest's serializer API export function test(maybeStore) { - return maybeStore instanceof Store; + // It's important to lazy-require the Store rather than imported at the head of the module. + // Because we reset modules between tests, different Store implementations will be used for each test. + // Unfortunately Jest does not reset its own serializer modules. + return maybeStore instanceof require('src/devtools/store').default; } // print() is part of Jest's serializer API diff --git a/src/devtools/cache.js b/src/devtools/cache.js index 1e12798030..7116600b60 100644 --- a/src/devtools/cache.js +++ b/src/devtools/cache.js @@ -49,6 +49,8 @@ const Pending = 0; const Resolved = 1; const Rejected = 2; +// TODO This file isn't being re-imported between tests it seems, so it's getting disconnected + const ReactCurrentDispatcher = (React: any) .__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.ReactCurrentDispatcher; From 1c49a7ef59f5e35b11d7418fee163a6d93a0856a Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sat, 4 May 2019 14:22:50 -0700 Subject: [PATCH 04/12] Added remaining profiling tests. One currently fails because of a bug with act() and interaction tracing --- .../__snapshots__/profiling-test.js.snap | 137 +++++++----- src/__tests__/profiling-test.js | 202 ++++++++++++------ 2 files changed, 225 insertions(+), 114 deletions(-) diff --git a/src/__tests__/__snapshots__/profiling-test.js.snap b/src/__tests__/__snapshots__/profiling-test.js.snap index 4f9c7ae56d..01204c9a6b 100644 --- a/src/__tests__/__snapshots__/profiling-test.js.snap +++ b/src/__tests__/__snapshots__/profiling-test.js.snap @@ -1,33 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`profiling commitDetails should be collected for each commit: 1: mount 1`] = ` -[root] - ▾ - - -`; - -exports[`profiling commitDetails should be collected for each commit: 2: add child 1`] = ` -[root] - ▾ - - - -`; - -exports[`profiling commitDetails should be collected for each commit: 3: remove children 1`] = ` -[root] - ▾ - -`; - -exports[`profiling commitDetails should be collected for each commit: 4: profiling stopped 1`] = ` -[root] - ▾ - -`; - -exports[`profiling commitDetails should be collected for each commit: 5: CommitDetails: mount 1`] = ` +exports[`profiling CommitDetails should be collected for each commit: CommitDetails commitIndex: 0 1`] = ` Object { "actualDurations": Map { 1 => 14, @@ -41,7 +14,7 @@ Object { } `; -exports[`profiling commitDetails should be collected for each commit: 6: CommitDetails: add child 1`] = ` +exports[`profiling CommitDetails should be collected for each commit: CommitDetails commitIndex: 1 1`] = ` Object { "actualDurations": Map { 3 => 2, @@ -56,7 +29,7 @@ Object { } `; -exports[`profiling commitDetails should be collected for each commit: 7: CommitDetails: remove children 1`] = ` +exports[`profiling CommitDetails should be collected for each commit: CommitDetails commitIndex: 2 1`] = ` Object { "actualDurations": Map { 3 => 2, @@ -69,42 +42,106 @@ Object { } `; -exports[`profiling profilingSummary should be collected for each commit: 1: mount 1`] = ` -[root] - ▾ - - +exports[`profiling CommitDetails should be collected for each commit: CommitDetails commitIndex: 3 1`] = ` +Object { + "actualDurations": Map { + 2 => 10, + 1 => 10, + }, + "commitIndex": 3, + "interactions": Array [], + "rootID": 1, +} `; -exports[`profiling profilingSummary should be collected for each commit: 2: add child 1`] = ` -[root] - ▾ - - - +exports[`profiling FiberCommits should be collected for each rendered fiber: FiberCommits: element 2 1`] = ` +Object { + "commitDurations": Array [ + 0, + 12, + 1, + 14, + 2, + 16, + ], + "fiberID": 2, + "rootID": 1, +} `; -exports[`profiling profilingSummary should be collected for each commit: 3: remove children 1`] = ` -[root] - ▾ - +exports[`profiling FiberCommits should be collected for each rendered fiber: FiberCommits: element 3 1`] = ` +Object { + "commitDurations": Array [ + 0, + 2, + 1, + 2, + 2, + 2, + ], + "fiberID": 3, + "rootID": 1, +} `; -exports[`profiling profilingSummary should be collected for each commit: 4: profiling stopped 1`] = ` -[root] - ▾ - +exports[`profiling FiberCommits should be collected for each rendered fiber: FiberCommits: element 4 1`] = ` +Object { + "commitDurations": Array [ + 1, + 2, + 2, + 2, + ], + "fiberID": 4, + "rootID": 1, +} `; -exports[`profiling profilingSummary should be collected for each commit: ProfilingSummary 1`] = ` +exports[`profiling FiberCommits should be collected for each rendered fiber: FiberCommits: element 5 1`] = ` +Object { + "commitDurations": Array [ + 2, + 2, + ], + "fiberID": 5, + "rootID": 1, +} +`; + +exports[`profiling Interactions should be collected for every traced interaction: Interactions 1`] = ` +Array [ + Object { + "__count": 0, + "commits": Array [ + 1, + ], + "id": 0, + "name": "one child", + "timestamp": 10, + }, + Object { + "__count": 0, + "commits": Array [ + 2, + ], + "id": 1, + "name": "two children", + "timestamp": 22, + }, +] +`; + +exports[`profiling ProfilingSummary should be collected for each commit: ProfilingSummary 1`] = ` Object { "commitDurations": Array [ 16, 12, + 10, ], "commitTimes": Array [ 16, 28, + 38, ], "initialTreeBaseDurations": Map { 1 => 14, diff --git a/src/__tests__/profiling-test.js b/src/__tests__/profiling-test.js index 253270cf13..b38c6dadd8 100644 --- a/src/__tests__/profiling-test.js +++ b/src/__tests__/profiling-test.js @@ -4,6 +4,7 @@ describe('profiling', () => { let React; let ReactDOM; let Scheduler; + let SchedulerTracing; let TestRenderer; let store; let utils; @@ -18,10 +19,11 @@ describe('profiling', () => { React = require('react'); ReactDOM = require('react-dom'); Scheduler = require('scheduler'); + SchedulerTracing = require('scheduler/tracing'); TestRenderer = utils.requireTestRenderer(); }); - describe('profilingSummary', () => { + describe('ProfilingSummary', () => { it('should be collected for each commit', async done => { const Parent = ({ count }) => { Scheduler.advanceTime(10); @@ -36,26 +38,19 @@ describe('profiling', () => { const container = document.createElement('div'); - utils.act(() => ReactDOM.render(, container)); - expect(store).toMatchSnapshot('1: mount'); - + utils.act(() => ReactDOM.render(, container)); utils.act(() => store.startProfiling()); - - utils.act(() => ReactDOM.render(, container)); - expect(store).toMatchSnapshot('2: add child'); - - utils.act(() => ReactDOM.render(, container)); - expect(store).toMatchSnapshot('3: remove children'); - + utils.act(() => ReactDOM.render(, container)); + utils.act(() => ReactDOM.render(, container)); + utils.act(() => ReactDOM.render(, container)); utils.act(() => store.stopProfiling()); - expect(store).toMatchSnapshot('4: profiling stopped'); - let profilingSummary; function Suspender({ rendererID, rootID }) { - profilingSummary = store.profilingCache.ProfilingSummary.read({ + const profilingSummary = store.profilingCache.ProfilingSummary.read({ rendererID, rootID, }); + expect(profilingSummary).toMatchSnapshot('ProfilingSummary'); return null; } @@ -70,13 +65,11 @@ describe('profiling', () => { ) ); - expect(profilingSummary).toMatchSnapshot('ProfilingSummary'); - done(); }); }); - describe('commitDetails', () => { + describe('CommitDetails', () => { it('should be collected for each commit', async done => { const Parent = ({ count }) => { Scheduler.advanceTime(10); @@ -92,26 +85,140 @@ describe('profiling', () => { const container = document.createElement('div'); utils.act(() => store.startProfiling()); - - utils.act(() => ReactDOM.render(, container)); - expect(store).toMatchSnapshot('1: mount'); - - utils.act(() => ReactDOM.render(, container)); - expect(store).toMatchSnapshot('2: add child'); - - utils.act(() => ReactDOM.render(, container)); - expect(store).toMatchSnapshot('3: remove children'); - + utils.act(() => ReactDOM.render(, container)); + utils.act(() => ReactDOM.render(, container)); + utils.act(() => ReactDOM.render(, container)); + utils.act(() => ReactDOM.render(, container)); utils.act(() => store.stopProfiling()); - expect(store).toMatchSnapshot('4: profiling stopped'); - let commitDetails; function Suspender({ commitIndex, rendererID, rootID }) { - commitDetails = store.profilingCache.CommitDetails.read({ + const commitDetails = store.profilingCache.CommitDetails.read({ commitIndex, rendererID, rootID, }); + expect(commitDetails).toMatchSnapshot( + `CommitDetails commitIndex: ${commitIndex}` + ); + return null; + } + + const rendererID = utils.getRendererID(); + const rootID = store.roots[0]; + + for (let commitIndex = 0; commitIndex <= 3; commitIndex++) { + await utils.actSuspense(() => + TestRenderer.create( + + + + ) + ); + } + + done(); + }); + }); + + describe('FiberCommits', () => { + it('should be collected for each rendered fiber', async done => { + const Parent = ({ count }) => { + Scheduler.advanceTime(10); + return new Array(count) + .fill(true) + .map((_, index) => ); + }; + const Child = () => { + Scheduler.advanceTime(2); + return null; + }; + + const container = document.createElement('div'); + + utils.act(() => store.startProfiling()); + utils.act(() => ReactDOM.render(, container)); + utils.act(() => ReactDOM.render(, container)); + utils.act(() => ReactDOM.render(, container)); + utils.act(() => store.stopProfiling()); + + function Suspender({ fiberID, rendererID, rootID }) { + const fiberCommits = store.profilingCache.FiberCommits.read({ + fiberID, + rendererID, + rootID, + }); + expect(fiberCommits).toMatchSnapshot( + `FiberCommits: element ${fiberID}` + ); + return null; + } + + const rendererID = utils.getRendererID(); + const rootID = store.roots[0]; + + for (let index = 0; index < store.numElements; index++) { + await utils.actSuspense(() => + TestRenderer.create( + + + + ) + ); + } + + done(); + }); + }); + + describe('Interactions', () => { + it('should be collected for every traced interaction', async done => { + const Parent = ({ count }) => { + Scheduler.advanceTime(10); + return new Array(count) + .fill(true) + .map((_, index) => ); + }; + const Child = () => { + Scheduler.advanceTime(2); + return null; + }; + + const container = document.createElement('div'); + + utils.act(() => store.startProfiling()); + console.log('[test] render one'); + utils.act(() => + SchedulerTracing.unstable_trace( + 'one child', + Scheduler.unstable_now(), + () => ReactDOM.render(, container) + ) + ); + console.log('[test] render two'); + utils.act(() => + SchedulerTracing.unstable_trace( + 'two children', + Scheduler.unstable_now(), + () => ReactDOM.render(, container) + ) + ); + console.log('[test] done'); + utils.act(() => store.stopProfiling()); + + function Suspender({ rendererID, rootID }) { + const interactions = store.profilingCache.Interactions.read({ + rendererID, + rootID, + }); + expect(interactions).toMatchSnapshot('Interactions'); return null; } @@ -121,43 +228,10 @@ describe('profiling', () => { await utils.actSuspense(() => TestRenderer.create( - + ) ); - expect(commitDetails).toMatchSnapshot('5: CommitDetails: mount'); - - await utils.actSuspense(() => - TestRenderer.create( - - - - ) - ); - expect(commitDetails).toMatchSnapshot('6: CommitDetails: add child'); - - await utils.actSuspense(() => - TestRenderer.create( - - - - ) - ); - expect(commitDetails).toMatchSnapshot( - '7: CommitDetails: remove children' - ); done(); }); From 269969b64564637dd2a0933b426d63059e9e3652 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sun, 5 May 2019 09:52:27 -0700 Subject: [PATCH 05/12] Updated snapshot data after ReactDOM batch fix --- src/__tests__/__snapshots__/profiling-test.js.snap | 14 +++++++------- src/__tests__/profiling-test.js | 7 ++----- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/__tests__/__snapshots__/profiling-test.js.snap b/src/__tests__/__snapshots__/profiling-test.js.snap index 01204c9a6b..c0dfd222e0 100644 --- a/src/__tests__/__snapshots__/profiling-test.js.snap +++ b/src/__tests__/__snapshots__/profiling-test.js.snap @@ -111,22 +111,22 @@ Object { exports[`profiling Interactions should be collected for every traced interaction: Interactions 1`] = ` Array [ Object { - "__count": 0, + "__count": 1, "commits": Array [ - 1, + 0, ], "id": 0, - "name": "one child", - "timestamp": 10, + "name": "mount: one child", + "timestamp": 0, }, Object { "__count": 0, "commits": Array [ - 2, + 1, ], "id": 1, - "name": "two children", - "timestamp": 22, + "name": "update: two children", + "timestamp": 12, }, ] `; diff --git a/src/__tests__/profiling-test.js b/src/__tests__/profiling-test.js index b38c6dadd8..de2d4cf04c 100644 --- a/src/__tests__/profiling-test.js +++ b/src/__tests__/profiling-test.js @@ -194,23 +194,20 @@ describe('profiling', () => { const container = document.createElement('div'); utils.act(() => store.startProfiling()); - console.log('[test] render one'); utils.act(() => SchedulerTracing.unstable_trace( - 'one child', + 'mount: one child', Scheduler.unstable_now(), () => ReactDOM.render(, container) ) ); - console.log('[test] render two'); utils.act(() => SchedulerTracing.unstable_trace( - 'two children', + 'update: two children', Scheduler.unstable_now(), () => ReactDOM.render(, container) ) ); - console.log('[test] done'); utils.act(() => store.stopProfiling()); function Suspender({ rendererID, rootID }) { From c29483122ce8297c7c136c50cdf31cb89fcb4c04 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Sun, 5 May 2019 09:56:54 -0700 Subject: [PATCH 06/12] Removed outdated TODO comment --- src/devtools/cache.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/devtools/cache.js b/src/devtools/cache.js index 7116600b60..1e12798030 100644 --- a/src/devtools/cache.js +++ b/src/devtools/cache.js @@ -49,8 +49,6 @@ const Pending = 0; const Resolved = 1; const Rejected = 2; -// TODO This file isn't being re-imported between tests it seems, so it's getting disconnected - const ReactCurrentDispatcher = (React: any) .__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.ReactCurrentDispatcher; From 705cd9b10993fab6d05844f2a7d527d555dbeea6 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 6 May 2019 09:44:01 -0700 Subject: [PATCH 07/12] Added Flow types to profiling test --- flow-typed/npm/react-test-renderer_v16.x.x.js | 81 +++++++++++++++++++ src/__tests__/profiling-test.js | 25 ++++-- src/__tests__/utils.js | 12 +-- 3 files changed, 105 insertions(+), 13 deletions(-) create mode 100644 flow-typed/npm/react-test-renderer_v16.x.x.js diff --git a/flow-typed/npm/react-test-renderer_v16.x.x.js b/flow-typed/npm/react-test-renderer_v16.x.x.js new file mode 100644 index 0000000000..87a149a1d3 --- /dev/null +++ b/flow-typed/npm/react-test-renderer_v16.x.x.js @@ -0,0 +1,81 @@ +// flow-typed signature: b6bb53397d83d2d821e258cc73818d1b +// flow-typed version: 9c71eca8ef/react-test-renderer_v16.x.x/flow_>=v0.47.x + +// Type definitions for react-test-renderer 16.x.x +// Ported from: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/react-test-renderer + +type ReactComponentInstance = React$Component; + +type ReactTestRendererJSON = { + type: string, + props: { [propName: string]: any }, + children: null | ReactTestRendererJSON[], +}; + +type ReactTestRendererTree = ReactTestRendererJSON & { + nodeType: 'component' | 'host', + instance: ?ReactComponentInstance, + rendered: null | ReactTestRendererTree, +}; + +type ReactTestInstance = { + instance: ?ReactComponentInstance, + type: string, + props: { [propName: string]: any }, + parent: null | ReactTestInstance, + children: Array, + + find(predicate: (node: ReactTestInstance) => boolean): ReactTestInstance, + findByType(type: React$ElementType): ReactTestInstance, + findByProps(props: { [propName: string]: any }): ReactTestInstance, + + findAll( + predicate: (node: ReactTestInstance) => boolean, + options?: { deep: boolean } + ): ReactTestInstance[], + findAllByType( + type: React$ElementType, + options?: { deep: boolean } + ): ReactTestInstance[], + findAllByProps( + props: { [propName: string]: any }, + options?: { deep: boolean } + ): ReactTestInstance[], +}; + +type TestRendererOptions = { + createNodeMock(element: React$Element): any, +}; + +declare module 'react-test-renderer' { + declare export type ReactTestRenderer = { + toJSON(): null | ReactTestRendererJSON, + toTree(): null | ReactTestRendererTree, + unmount(nextElement?: React$Element): void, + update(nextElement: React$Element): void, + getInstance(): ?ReactComponentInstance, + root: ReactTestInstance, + }; + + declare type Thenable = { + then(resolve: () => mixed, reject?: () => mixed): mixed, + }; + + declare function create( + nextElement: React$Element, + options?: TestRendererOptions + ): ReactTestRenderer; + + declare function act(callback: () => void): Thenable; +} + +declare module 'react-test-renderer/shallow' { + declare export default class ShallowRenderer { + static createRenderer(): ShallowRenderer; + getMountedInstance(): ReactTestInstance; + getRenderOutput>(): E; + getRenderOutput(): React$Element; + render(element: React$Element, context?: any): void; + unmount(): void; + } +} diff --git a/src/__tests__/profiling-test.js b/src/__tests__/profiling-test.js index de2d4cf04c..5bd2e037bb 100644 --- a/src/__tests__/profiling-test.js +++ b/src/__tests__/profiling-test.js @@ -1,12 +1,17 @@ // @flow +import type React from 'react'; +import type ReactDOM from 'react-dom'; +import typeof ReactTestRenderer from 'react-test-renderer'; +import type Store from 'src/devtools/store'; + describe('profiling', () => { - let React; - let ReactDOM; + let React: React; + let ReactDOM: ReactDOM; let Scheduler; let SchedulerTracing; - let TestRenderer; - let store; + let TestRenderer: ReactTestRenderer; + let store: Store; let utils; beforeEach(() => { @@ -161,17 +166,21 @@ describe('profiling', () => { const rootID = store.roots[0]; for (let index = 0; index < store.numElements; index++) { - await utils.actSuspense(() => + await utils.actSuspense(() => { + const fiberID = store.getElementIDAtIndex(index); + if (fiberID == null) { + throw Error(`Unexpected null ID for element at index ${index}`); + } TestRenderer.create( - ) - ); + ); + }); } done(); diff --git a/src/__tests__/utils.js b/src/__tests__/utils.js index cb15590ee5..1e3acc55ac 100644 --- a/src/__tests__/utils.js +++ b/src/__tests__/utils.js @@ -1,5 +1,7 @@ // @flow +import typeof ReactTestRenderer from 'react-test-renderer'; + export function act(callback: Function): void { const TestUtils = require('react-dom/test-utils'); TestUtils.act(() => { @@ -10,7 +12,7 @@ export function act(callback: Function): void { jest.runAllTimers(); } -export async function actSuspense(callback: Function) { +export async function actSuspense(callback: Function): Promise { const TestUtils = require('react-dom/test-utils'); const Scheduler = require('scheduler'); @@ -26,7 +28,7 @@ export async function actSuspense(callback: Function) { Scheduler.flushAll(); } -export function beforeEachProfiling() { +export function beforeEachProfiling(): void { // Mock React's timing information so that test runs are predictable. jest.mock('scheduler', () => // $FlowFixMe Flow does not konw about requireActual @@ -41,7 +43,7 @@ export function beforeEachProfiling() { ); } -export function getRendererID() { +export function getRendererID(): number { if (global.agent == null) { throw Error('Agent unavailable.'); } @@ -49,10 +51,10 @@ export function getRendererID() { if (ids.length !== 1) { throw Error('Multiple renderers attached.'); } - return ids[0]; + return parseInt(ids[0], 10); } -export function requireTestRenderer() { +export function requireTestRenderer(): ReactTestRenderer { let hook; try { // Hide the hook before requiring TestRenderer, so we don't end up with a loop. From d86bc1020ec6122a0a37685f57ab2c7a88be71d8 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 6 May 2019 13:04:14 -0700 Subject: [PATCH 08/12] Hardened tests to ensure expectations are flushed --- src/__tests__/profiling-test.js | 26 +++++++++++++++++++++++--- src/devtools/store.js | 4 ++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/__tests__/profiling-test.js b/src/__tests__/profiling-test.js index 5bd2e037bb..6c49272847 100644 --- a/src/__tests__/profiling-test.js +++ b/src/__tests__/profiling-test.js @@ -50,11 +50,14 @@ describe('profiling', () => { utils.act(() => ReactDOM.render(, container)); utils.act(() => store.stopProfiling()); + let suspenseResolved = false; + function Suspender({ rendererID, rootID }) { const profilingSummary = store.profilingCache.ProfilingSummary.read({ rendererID, rootID, }); + suspenseResolved = true; expect(profilingSummary).toMatchSnapshot('ProfilingSummary'); return null; } @@ -70,6 +73,8 @@ describe('profiling', () => { ) ); + expect(suspenseResolved).toBe(true); + done(); }); }); @@ -96,12 +101,15 @@ describe('profiling', () => { utils.act(() => ReactDOM.render(, container)); utils.act(() => store.stopProfiling()); + let suspenseResolved = false; + function Suspender({ commitIndex, rendererID, rootID }) { const commitDetails = store.profilingCache.CommitDetails.read({ commitIndex, rendererID, rootID, }); + suspenseResolved = true; expect(commitDetails).toMatchSnapshot( `CommitDetails commitIndex: ${commitIndex}` ); @@ -112,7 +120,8 @@ describe('profiling', () => { const rootID = store.roots[0]; for (let commitIndex = 0; commitIndex <= 3; commitIndex++) { - await utils.actSuspense(() => + suspenseResolved = false; + await utils.actSuspense(() => { TestRenderer.create( { rootID={rootID} /> - ) - ); + ); + }); + expect(suspenseResolved).toBe(true); } done(); @@ -150,12 +160,15 @@ describe('profiling', () => { utils.act(() => ReactDOM.render(, container)); utils.act(() => store.stopProfiling()); + let suspenseResolved = false; + function Suspender({ fiberID, rendererID, rootID }) { const fiberCommits = store.profilingCache.FiberCommits.read({ fiberID, rendererID, rootID, }); + suspenseResolved = true; expect(fiberCommits).toMatchSnapshot( `FiberCommits: element ${fiberID}` ); @@ -166,6 +179,7 @@ describe('profiling', () => { const rootID = store.roots[0]; for (let index = 0; index < store.numElements; index++) { + suspenseResolved = false; await utils.actSuspense(() => { const fiberID = store.getElementIDAtIndex(index); if (fiberID == null) { @@ -181,6 +195,7 @@ describe('profiling', () => { ); }); + expect(suspenseResolved).toBe(true); } done(); @@ -219,11 +234,14 @@ describe('profiling', () => { ); utils.act(() => store.stopProfiling()); + let suspenseResolved = false; + function Suspender({ rendererID, rootID }) { const interactions = store.profilingCache.Interactions.read({ rendererID, rootID, }); + suspenseResolved = true; expect(interactions).toMatchSnapshot('Interactions'); return null; } @@ -239,6 +257,8 @@ describe('profiling', () => { ) ); + expect(suspenseResolved).toBe(true); + done(); }); }); diff --git a/src/devtools/store.js b/src/devtools/store.js index d6e93d9fb9..7a84b33647 100644 --- a/src/devtools/store.js +++ b/src/devtools/store.js @@ -852,6 +852,10 @@ export default class Store extends EventEmitter { weight: 0, }); + if (this._isProfiling) { + this._profilingSnapshotsByRootID.set(id, new Map()); + } + haveRootsChanged = true; } else { parentID = ((operations[i]: any): number); From b25d996fc4d8d84afe00bbb852f7480ab4725fa6 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 6 May 2019 13:37:06 -0700 Subject: [PATCH 09/12] Added profiling chart data tests --- .../profilingCharts-test.js.snap | 333 ++++++++++++++++++ src/__tests__/profiling-test.js | 6 +- src/__tests__/profilingCharts-test.js | 292 +++++++++++++++ src/__tests__/utils.js | 17 +- 4 files changed, 641 insertions(+), 7 deletions(-) create mode 100644 src/__tests__/__snapshots__/profilingCharts-test.js.snap create mode 100644 src/__tests__/profilingCharts-test.js diff --git a/src/__tests__/__snapshots__/profilingCharts-test.js.snap b/src/__tests__/__snapshots__/profilingCharts-test.js.snap new file mode 100644 index 0000000000..36e4c8cf90 --- /dev/null +++ b/src/__tests__/__snapshots__/profilingCharts-test.js.snap @@ -0,0 +1,333 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`profiling charts flamegraph chart should contain valid data: 0: CommitTree 1`] = ` +Object { + "nodes": Map { + 1 => Object { + "children": Array [ + 2, + ], + "displayName": null, + "id": 1, + "key": null, + "parentID": 0, + "treeBaseDuration": 14, + }, + 2 => Object { + "children": Array [ + 3, + 4, + ], + "displayName": "Parent", + "id": 2, + "key": null, + "parentID": 1, + "treeBaseDuration": 14, + }, + 3 => Object { + "children": Array [], + "displayName": "Memo(Child)", + "id": 3, + "key": "first", + "parentID": 2, + "treeBaseDuration": 2, + }, + 4 => Object { + "children": Array [], + "displayName": "Memo(Child)", + "id": 4, + "key": "second", + "parentID": 2, + "treeBaseDuration": 2, + }, + }, + "rootID": 1, +} +`; + +exports[`profiling charts flamegraph chart should contain valid data: 0: FlamegraphChartData 1`] = ` +Object { + "baseDuration": 14, + "depth": 2, + "idToDepthMap": Map { + 2 => 1, + 4 => 2, + 3 => 2, + }, + "maxSelfDuration": 10, + "rows": Array [ + Array [ + Object { + "actualDuration": 14, + "didRender": true, + "id": 2, + "label": "Parent (10.0ms of 14.0ms)", + "name": "Parent", + "offset": 0, + "selfDuration": 10, + "treeBaseDuration": 14, + }, + ], + Array [ + Object { + "actualDuration": 2, + "didRender": true, + "id": 4, + "label": "Memo(Child) key=\\"second\\" (2.0ms of 2.0ms)", + "name": "Memo(Child)", + "offset": 12, + "selfDuration": 2, + "treeBaseDuration": 2, + }, + Object { + "actualDuration": 2, + "didRender": true, + "id": 3, + "label": "Memo(Child) key=\\"first\\" (2.0ms of 2.0ms)", + "name": "Memo(Child)", + "offset": 10, + "selfDuration": 2, + "treeBaseDuration": 2, + }, + ], + ], +} +`; + +exports[`profiling charts flamegraph chart should contain valid data: 1: CommitTree 1`] = ` +Object { + "nodes": Map { + 1 => Object { + "children": Array [ + 2, + ], + "displayName": null, + "id": 1, + "key": null, + "parentID": 0, + "treeBaseDuration": 14, + }, + 2 => Object { + "children": Array [ + 3, + 4, + ], + "displayName": "Parent", + "id": 2, + "key": null, + "parentID": 1, + "treeBaseDuration": 14, + }, + 3 => Object { + "children": Array [], + "displayName": "Memo(Child)", + "id": 3, + "key": "first", + "parentID": 2, + "treeBaseDuration": 2, + }, + 4 => Object { + "children": Array [], + "displayName": "Memo(Child)", + "id": 4, + "key": "second", + "parentID": 2, + "treeBaseDuration": 2, + }, + }, + "rootID": 1, +} +`; + +exports[`profiling charts flamegraph chart should contain valid data: 1: FlamegraphChartData 1`] = ` +Object { + "baseDuration": 14, + "depth": 2, + "idToDepthMap": Map { + 2 => 1, + 4 => 2, + 3 => 2, + }, + "maxSelfDuration": 10, + "rows": Array [ + Array [ + Object { + "actualDuration": 10, + "didRender": true, + "id": 2, + "label": "Parent (10.0ms of 10.0ms)", + "name": "Parent", + "offset": 0, + "selfDuration": 10, + "treeBaseDuration": 14, + }, + ], + Array [ + Object { + "actualDuration": 0, + "didRender": false, + "id": 4, + "label": "Memo(Child) key=\\"second\\"", + "name": "Memo(Child)", + "offset": 12, + "selfDuration": 0, + "treeBaseDuration": 2, + }, + Object { + "actualDuration": 0, + "didRender": false, + "id": 3, + "label": "Memo(Child) key=\\"first\\"", + "name": "Memo(Child)", + "offset": 10, + "selfDuration": 0, + "treeBaseDuration": 2, + }, + ], + ], +} +`; + +exports[`profiling charts interactions should contain valid data: Interactions 1`] = ` +Object { + "lastInteractionTime": 24, + "maxCommitDuration": 14, +} +`; + +exports[`profiling charts interactions should contain valid data: Interactions 2`] = ` +Object { + "lastInteractionTime": 24, + "maxCommitDuration": 14, +} +`; + +exports[`profiling charts ranked chart should contain valid data: 0: CommitTree 1`] = ` +Object { + "nodes": Map { + 1 => Object { + "children": Array [ + 2, + ], + "displayName": null, + "id": 1, + "key": null, + "parentID": 0, + "treeBaseDuration": 14, + }, + 2 => Object { + "children": Array [ + 3, + 4, + ], + "displayName": "Parent", + "id": 2, + "key": null, + "parentID": 1, + "treeBaseDuration": 14, + }, + 3 => Object { + "children": Array [], + "displayName": "Memo(Child)", + "id": 3, + "key": "first", + "parentID": 2, + "treeBaseDuration": 2, + }, + 4 => Object { + "children": Array [], + "displayName": "Memo(Child)", + "id": 4, + "key": "second", + "parentID": 2, + "treeBaseDuration": 2, + }, + }, + "rootID": 1, +} +`; + +exports[`profiling charts ranked chart should contain valid data: 0: RankedChartData 1`] = ` +Object { + "maxValue": 10, + "nodes": Array [ + Object { + "id": 2, + "label": "Parent (10.0ms)", + "name": "Parent", + "value": 10, + }, + Object { + "id": 3, + "label": "Memo(Child) key=\\"first\\" (2.0ms)", + "name": "Memo(Child)", + "value": 2, + }, + Object { + "id": 4, + "label": "Memo(Child) key=\\"second\\" (2.0ms)", + "name": "Memo(Child)", + "value": 2, + }, + ], +} +`; + +exports[`profiling charts ranked chart should contain valid data: 1: CommitTree 1`] = ` +Object { + "nodes": Map { + 1 => Object { + "children": Array [ + 2, + ], + "displayName": null, + "id": 1, + "key": null, + "parentID": 0, + "treeBaseDuration": 14, + }, + 2 => Object { + "children": Array [ + 3, + 4, + ], + "displayName": "Parent", + "id": 2, + "key": null, + "parentID": 1, + "treeBaseDuration": 14, + }, + 3 => Object { + "children": Array [], + "displayName": "Memo(Child)", + "id": 3, + "key": "first", + "parentID": 2, + "treeBaseDuration": 2, + }, + 4 => Object { + "children": Array [], + "displayName": "Memo(Child)", + "id": 4, + "key": "second", + "parentID": 2, + "treeBaseDuration": 2, + }, + }, + "rootID": 1, +} +`; + +exports[`profiling charts ranked chart should contain valid data: 1: RankedChartData 1`] = ` +Object { + "maxValue": 10, + "nodes": Array [ + Object { + "id": 2, + "label": "Parent (10.0ms)", + "name": "Parent", + "value": 10, + }, + ], +} +`; diff --git a/src/__tests__/profiling-test.js b/src/__tests__/profiling-test.js index 6c49272847..39dd595e49 100644 --- a/src/__tests__/profiling-test.js +++ b/src/__tests__/profiling-test.js @@ -1,13 +1,11 @@ // @flow -import type React from 'react'; -import type ReactDOM from 'react-dom'; import typeof ReactTestRenderer from 'react-test-renderer'; import type Store from 'src/devtools/store'; describe('profiling', () => { - let React: React; - let ReactDOM: ReactDOM; + let React; + let ReactDOM; let Scheduler; let SchedulerTracing; let TestRenderer: ReactTestRenderer; diff --git a/src/__tests__/profilingCharts-test.js b/src/__tests__/profilingCharts-test.js new file mode 100644 index 0000000000..3e2613b5af --- /dev/null +++ b/src/__tests__/profilingCharts-test.js @@ -0,0 +1,292 @@ +// @flow + +import typeof TestRendererType from 'react-test-renderer'; +import type Store from 'src/devtools/store'; + +describe('profiling charts', () => { + let React; + let ReactDOM; + let Scheduler; + let SchedulerTracing; + let TestRenderer: TestRendererType; + let store: 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'); + SchedulerTracing = require('scheduler/tracing'); + TestRenderer = utils.requireTestRenderer(); + }); + + describe('flamegraph chart', () => { + it('should contain valid data', async done => { + const Parent = ({ count }) => { + Scheduler.advanceTime(10); + return ( + + + + + ); + }; + + // Memoize children to verify that chart doesn't include in the update. + const Child = React.memo(function Child() { + Scheduler.advanceTime(2); + return null; + }); + + const container = document.createElement('div'); + + utils.act(() => store.startProfiling()); + utils.act(() => + SchedulerTracing.unstable_trace('mount', Scheduler.unstable_now(), () => + ReactDOM.render(, container) + ) + ); + utils.act(() => + SchedulerTracing.unstable_trace( + 'update', + Scheduler.unstable_now(), + () => ReactDOM.render(, container) + ) + ); + utils.act(() => store.stopProfiling()); + + let suspenseResolved = false; + + function Suspender({ commitIndex, rendererID, rootID }) { + const profilingSummary = store.profilingCache.ProfilingSummary.read({ + rendererID, + rootID, + }); + const commitDetails = store.profilingCache.CommitDetails.read({ + commitIndex, + rendererID, + rootID, + }); + suspenseResolved = true; + const commitTree = store.profilingCache.getCommitTree({ + commitIndex, + profilingSummary, + }); + const chartData = store.profilingCache.getFlamegraphChartData({ + commitDetails, + commitIndex, + commitTree, + }); + expect(commitTree).toMatchSnapshot(`${commitIndex}: CommitTree`); + expect(chartData).toMatchSnapshot( + `${commitIndex}: FlamegraphChartData` + ); + return null; + } + + const rendererID = utils.getRendererID(); + const rootID = store.roots[0]; + + for (let commitIndex = 0; commitIndex < 2; commitIndex++) { + suspenseResolved = false; + + await utils.actSuspense( + () => + TestRenderer.create( + + + + ), + 3 + ); + + expect(suspenseResolved).toBe(true); + } + + expect(suspenseResolved).toBe(true); + + done(); + }); + }); + + describe('ranked chart', () => { + it('should contain valid data', async done => { + const Parent = ({ count }) => { + Scheduler.advanceTime(10); + return ( + + + + + ); + }; + + // Memoize children to verify that chart doesn't include in the update. + const Child = React.memo(function Child() { + Scheduler.advanceTime(2); + return null; + }); + + const container = document.createElement('div'); + + utils.act(() => store.startProfiling()); + utils.act(() => + SchedulerTracing.unstable_trace('mount', Scheduler.unstable_now(), () => + ReactDOM.render(, container) + ) + ); + utils.act(() => + SchedulerTracing.unstable_trace( + 'update', + Scheduler.unstable_now(), + () => ReactDOM.render(, container) + ) + ); + utils.act(() => store.stopProfiling()); + + let suspenseResolved = false; + + function Suspender({ commitIndex, rendererID, rootID }) { + const profilingSummary = store.profilingCache.ProfilingSummary.read({ + rendererID, + rootID, + }); + const commitDetails = store.profilingCache.CommitDetails.read({ + commitIndex, + rendererID, + rootID, + }); + suspenseResolved = true; + const commitTree = store.profilingCache.getCommitTree({ + commitIndex, + profilingSummary, + }); + const chartData = store.profilingCache.getRankedChartData({ + commitDetails, + commitIndex, + commitTree, + }); + expect(commitTree).toMatchSnapshot(`${commitIndex}: CommitTree`); + expect(chartData).toMatchSnapshot(`${commitIndex}: RankedChartData`); + return null; + } + + const rendererID = utils.getRendererID(); + const rootID = store.roots[0]; + + for (let commitIndex = 0; commitIndex < 2; commitIndex++) { + suspenseResolved = false; + + await utils.actSuspense( + () => + TestRenderer.create( + + + + ), + 3 + ); + + expect(suspenseResolved).toBe(true); + } + + done(); + }); + }); + + describe('interactions', () => { + it('should contain valid data', async done => { + const Parent = ({ count }) => { + Scheduler.advanceTime(10); + return ( + + + + + ); + }; + + // Memoize children to verify that chart doesn't include in the update. + const Child = React.memo(function Child() { + Scheduler.advanceTime(2); + return null; + }); + + const container = document.createElement('div'); + + utils.act(() => store.startProfiling()); + utils.act(() => + SchedulerTracing.unstable_trace('mount', Scheduler.unstable_now(), () => + ReactDOM.render(, container) + ) + ); + utils.act(() => + SchedulerTracing.unstable_trace( + 'update', + Scheduler.unstable_now(), + () => ReactDOM.render(, container) + ) + ); + utils.act(() => store.stopProfiling()); + + let suspenseResolved = false; + + function Suspender({ commitIndex, rendererID, rootID }) { + const profilingSummary = store.profilingCache.ProfilingSummary.read({ + rendererID, + rootID, + }); + const interactions = store.profilingCache.Interactions.read({ + rendererID, + rootID, + }); + suspenseResolved = true; + const chartData = store.profilingCache.getInteractionsChartData({ + interactions, + profilingSummary, + }); + expect(chartData).toMatchSnapshot('Interactions'); + return null; + } + + const rendererID = utils.getRendererID(); + const rootID = store.roots[0]; + + for (let commitIndex = 0; commitIndex < 2; commitIndex++) { + suspenseResolved = false; + + await utils.actSuspense( + () => + TestRenderer.create( + + + + ), + 3 + ); + + expect(suspenseResolved).toBe(true); + } + + done(); + }); + }); +}); diff --git a/src/__tests__/utils.js b/src/__tests__/utils.js index 1e3acc55ac..89c7ffd6b1 100644 --- a/src/__tests__/utils.js +++ b/src/__tests__/utils.js @@ -12,7 +12,10 @@ export function act(callback: Function): void { jest.runAllTimers(); } -export async function actSuspense(callback: Function): Promise { +export async function actSuspense( + callback: Function, + numTimesToFlush: number = 1 +): Promise { const TestUtils = require('react-dom/test-utils'); const Scheduler = require('scheduler'); @@ -24,8 +27,16 @@ export async function actSuspense(callback: Function): Promise { jest.runAllTimers(); }); - // Re-render after resolved promises - Scheduler.flushAll(); + // Run cascading microtasks and flush scheduled React work. + // Components that suspend multiple times will need to do this once per suspend operation. + // HACK Ideally the mock scheduler would provide an API to ask if there was outstanding work. + while (--numTimesToFlush >= 0) { + // $FlowFixMe Flow doens't know about "await act()" yet + await TestUtils.act(async () => { + jest.runAllTimers(); + Scheduler.flushAll(); + }); + } } export function beforeEachProfiling(): void { From c2bf71f406e530813b023eaa267c5e5e49869d32 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 6 May 2019 13:41:36 -0700 Subject: [PATCH 10/12] Added commit tree builder test --- .../profilingCommitTreeBuilder-test.js.snap | 162 ++++++++++++++++++ .../profilingCommitTreeBuilder-test.js | 91 ++++++++++ 2 files changed, 253 insertions(+) create mode 100644 src/__tests__/__snapshots__/profilingCommitTreeBuilder-test.js.snap create mode 100644 src/__tests__/profilingCommitTreeBuilder-test.js diff --git a/src/__tests__/__snapshots__/profilingCommitTreeBuilder-test.js.snap b/src/__tests__/__snapshots__/profilingCommitTreeBuilder-test.js.snap new file mode 100644 index 0000000000..b483cd5efc --- /dev/null +++ b/src/__tests__/__snapshots__/profilingCommitTreeBuilder-test.js.snap @@ -0,0 +1,162 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`commit tree should be able to rebuild the store tree for each commit: 0: CommitTree 1`] = ` +Object { + "nodes": Map { + 1 => Object { + "children": Array [ + 2, + ], + "displayName": null, + "id": 1, + "key": null, + "parentID": 0, + "treeBaseDuration": 12, + }, + 2 => Object { + "children": Array [ + 3, + ], + "displayName": "Parent", + "id": 2, + "key": null, + "parentID": 1, + "treeBaseDuration": 12, + }, + 3 => Object { + "children": Array [], + "displayName": "Memo(Child)", + "id": 3, + "key": "0", + "parentID": 2, + "treeBaseDuration": 2, + }, + }, + "rootID": 1, +} +`; + +exports[`commit tree should be able to rebuild the store tree for each commit: 1: CommitTree 1`] = ` +Object { + "nodes": Map { + 1 => Object { + "children": Array [ + 2, + ], + "displayName": null, + "id": 1, + "key": null, + "parentID": 0, + "treeBaseDuration": 16, + }, + 2 => Object { + "children": Array [ + 3, + 4, + 5, + ], + "displayName": "Parent", + "id": 2, + "key": null, + "parentID": 1, + "treeBaseDuration": 16, + }, + 3 => Object { + "children": Array [], + "displayName": "Memo(Child)", + "id": 3, + "key": "0", + "parentID": 2, + "treeBaseDuration": 2, + }, + 4 => Object { + "children": Array [], + "displayName": "Memo(Child)", + "id": 4, + "key": "1", + "parentID": 2, + "treeBaseDuration": 2, + }, + 5 => Object { + "children": Array [], + "displayName": "Memo(Child)", + "id": 5, + "key": "2", + "parentID": 2, + "treeBaseDuration": 2, + }, + }, + "rootID": 1, +} +`; + +exports[`commit tree should be able to rebuild the store tree for each commit: 2: CommitTree 1`] = ` +Object { + "nodes": Map { + 1 => Object { + "children": Array [ + 2, + ], + "displayName": null, + "id": 1, + "key": null, + "parentID": 0, + "treeBaseDuration": 14, + }, + 2 => Object { + "children": Array [ + 3, + 4, + ], + "displayName": "Parent", + "id": 2, + "key": null, + "parentID": 1, + "treeBaseDuration": 14, + }, + 3 => Object { + "children": Array [], + "displayName": "Memo(Child)", + "id": 3, + "key": "0", + "parentID": 2, + "treeBaseDuration": 2, + }, + 4 => Object { + "children": Array [], + "displayName": "Memo(Child)", + "id": 4, + "key": "1", + "parentID": 2, + "treeBaseDuration": 2, + }, + }, + "rootID": 1, +} +`; + +exports[`commit tree should be able to rebuild the store tree for each commit: 3: CommitTree 1`] = ` +Object { + "nodes": Map { + 1 => Object { + "children": Array [ + 2, + ], + "displayName": null, + "id": 1, + "key": null, + "parentID": 0, + "treeBaseDuration": 10, + }, + 2 => Object { + "children": Array [], + "displayName": "Parent", + "id": 2, + "key": null, + "parentID": 1, + "treeBaseDuration": 10, + }, + }, + "rootID": 1, +} +`; diff --git a/src/__tests__/profilingCommitTreeBuilder-test.js b/src/__tests__/profilingCommitTreeBuilder-test.js new file mode 100644 index 0000000000..bf7b630646 --- /dev/null +++ b/src/__tests__/profilingCommitTreeBuilder-test.js @@ -0,0 +1,91 @@ +// @flow + +import typeof TestRendererType from 'react-test-renderer'; +import type Store from 'src/devtools/store'; + +describe('commit tree', () => { + let React; + let ReactDOM; + let Scheduler; + let SchedulerTracing; + let TestRenderer: TestRendererType; + let store: 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'); + SchedulerTracing = require('scheduler/tracing'); + TestRenderer = utils.requireTestRenderer(); + }); + + it('should be able to rebuild the store tree for each commit', async done => { + const Parent = ({ count }) => { + Scheduler.advanceTime(10); + return new Array(count) + .fill(true) + .map((_, index) => ); + }; + const Child = React.memo(function Child() { + Scheduler.advanceTime(2); + return null; + }); + + const container = document.createElement('div'); + + utils.act(() => store.startProfiling()); + utils.act(() => ReactDOM.render(, container)); + utils.act(() => ReactDOM.render(, container)); + utils.act(() => ReactDOM.render(, container)); + utils.act(() => ReactDOM.render(, container)); + utils.act(() => store.stopProfiling()); + + let suspenseResolved = false; + + function Suspender({ commitIndex, rendererID, rootID }) { + const profilingSummary = store.profilingCache.ProfilingSummary.read({ + rendererID, + rootID, + }); + suspenseResolved = true; + const commitTree = store.profilingCache.getCommitTree({ + commitIndex, + profilingSummary, + }); + expect(commitTree).toMatchSnapshot(`${commitIndex}: CommitTree`); + return null; + } + + const rendererID = utils.getRendererID(); + const rootID = store.roots[0]; + + for (let commitIndex = 0; commitIndex < 4; commitIndex++) { + suspenseResolved = false; + + await utils.actSuspense( + () => + TestRenderer.create( + + + + ), + 3 + ); + + expect(suspenseResolved).toBe(true); + } + + done(); + }); +}); From 6ab897234557a951ed23ec57d925f6e3447d7497 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 6 May 2019 13:52:27 -0700 Subject: [PATCH 11/12] Update to React canary release for interaction tracing bugfix --- package.json | 10 +++++----- yarn.lock | 48 ++++++++++++++++++++++++------------------------ 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/package.json b/package.json index 0dfb1b3c71..a86f97bd33 100644 --- a/package.json +++ b/package.json @@ -130,15 +130,15 @@ "opener": "^1.5.1", "prettier": "^1.16.4", "prop-types": "^15.6.2", - "react": "0.0.0-fb28e9048", + "react": "^0.0.0-6da04b5d8", "react-color": "^2.11.7", - "react-dom": "0.0.0-fb28e9048", - "react-is": "0.0.0-fb28e9048", - "react-test-renderer": "0.0.0-fb28e9048", + "react-dom": "^0.0.0-6da04b5d8", + "react-is": "^0.0.0-6da04b5d8", + "react-test-renderer": "^0.0.0-6da04b5d8", "react-virtualized-auto-sizer": "^1.0.2", "react-window": "^1.8.0", "request-promise": "^4.2.4", - "scheduler": "0.0.0-fb28e9048", + "scheduler": "^0.0.0-6da04b5d8", "semver": "^5.5.1", "style-loader": "^0.23.1", "web-ext": "^3.0.0", diff --git a/yarn.lock b/yarn.lock index 32a0156b37..4b65dd25f4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9807,20 +9807,20 @@ react-color@^2.11.7: reactcss "^1.2.0" tinycolor2 "^1.4.1" -react-dom@0.0.0-fb28e9048: - version "0.0.0-fb28e9048" - resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-0.0.0-fb28e9048.tgz#d4276f11a16f74b2cef0700bb5a8eefaee3761d2" - integrity sha512-C4bdIWINwV4pF1P0oN7fDAq+vTHOW2rxQLvGE5rDslGrK/DJ7bM5PrD81AcPkdgUnK3O/JvIkeGIvRcRgYemSw== +react-dom@^0.0.0-6da04b5d8: + version "0.0.0-6da04b5d8" + resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-0.0.0-6da04b5d8.tgz#ee78e45a40771560c756b8fc7f1fa213f2179ebe" + integrity sha512-6oyfkucrweqCB5XyLsfEnPSWhvkFnttutkU9uUQovLAljuazgpAjvBy6MBGSewKptqch2OTNwopqvR3QUMD8AQ== dependencies: loose-envify "^1.1.0" object-assign "^4.1.1" prop-types "^15.6.2" - scheduler "0.0.0-fb28e9048" + scheduler "0.0.0-6da04b5d8" -react-is@0.0.0-fb28e9048: - version "0.0.0-fb28e9048" - resolved "https://registry.yarnpkg.com/react-is/-/react-is-0.0.0-fb28e9048.tgz#f2bdccfea1005ea5234719de192f25504ff4244e" - integrity sha512-McCC7GvLXMcY29GIiJ56bvuXFVTIwBr7wcz93TcGvUN23a7l0h9KjCwUjGQfEpBRG3pbeBtMxDc6GiXuVbK6Lg== +react-is@0.0.0-6da04b5d8, react-is@^0.0.0-6da04b5d8: + version "0.0.0-6da04b5d8" + resolved "https://registry.yarnpkg.com/react-is/-/react-is-0.0.0-6da04b5d8.tgz#ded1f02e9f1e2b8456812d0d45c341d70b7bf7db" + integrity sha512-+Df3meqx+XUir+3dCqiHNAHruwmOAgXVp3TYmlUvgtPNBu+LF0OdchTMZ/xMUok1gYVoe4l/xhfs9CTEPkWt3g== react-is@^16.8.1: version "16.8.3" @@ -9837,15 +9837,15 @@ react-lifecycles-compat@^3.0.4: resolved "https://registry.yarnpkg.com/react-lifecycles-compat/-/react-lifecycles-compat-3.0.4.tgz#4f1a273afdfc8f3488a8c516bfda78f872352362" integrity sha512-fBASbA6LnOU9dOU2eW7aQ8xmYBSXUIWr+UmF9b1efZBazGNO+rcXT/icdKnYm2pTwcRylVUYwW7H1PHfLekVzA== -react-test-renderer@0.0.0-fb28e9048: - version "0.0.0-fb28e9048" - resolved "https://registry.yarnpkg.com/react-test-renderer/-/react-test-renderer-0.0.0-fb28e9048.tgz#1a94c8d19cbb1ac98ab37c66c0b294e5be280c52" - integrity sha512-WK/wQOh0v6+8Gbkurgb3he9hKoOKWueqQY+RFs2vM3u3vn7PMyYhzm/KkU75VvTG/GVciojNQBHBpekuvU5dYw== +react-test-renderer@^0.0.0-6da04b5d8: + version "0.0.0-6da04b5d8" + resolved "https://registry.yarnpkg.com/react-test-renderer/-/react-test-renderer-0.0.0-6da04b5d8.tgz#01bed04c5a4cf22339f0ae3b23f89bb45e9a8f2a" + integrity sha512-yDt5RPDXLZXTqlWS0jXMJ1IyS1e/UZXSr0L8bG0UCsna4T7A3HIYR2zChlydGXsxjGMZntKqEfm27hUhFgC06Q== dependencies: object-assign "^4.1.1" prop-types "^15.6.2" - react-is "0.0.0-fb28e9048" - scheduler "0.0.0-fb28e9048" + react-is "0.0.0-6da04b5d8" + scheduler "0.0.0-6da04b5d8" react-virtualized-auto-sizer@^1.0.2: version "1.0.2" @@ -9860,15 +9860,15 @@ react-window@^1.8.0: "@babel/runtime" "^7.0.0" memoize-one ">=3.1.1 <6" -react@0.0.0-fb28e9048: - version "0.0.0-fb28e9048" - resolved "https://registry.yarnpkg.com/react/-/react-0.0.0-fb28e9048.tgz#f3488c3e5cac772b2731d6d4bb3484410821254c" - integrity sha512-6zsjgsy9hcKC41T6X6SMbyf3fJ2VadmUxDgEXUVAJ2qV9q4SP0mnK4ttDfOwDvlFNbisqtS5HbS0hnWO/8iKxQ== +react@^0.0.0-6da04b5d8: + version "0.0.0-6da04b5d8" + resolved "https://registry.yarnpkg.com/react/-/react-0.0.0-6da04b5d8.tgz#583d81f73b26771da41170a5042a5ab0bdcfe37a" + integrity sha512-8hXBHwDCKxSVFqj5Kb4OskZz7//2fx2IpUnYyukYV8qyAHlXr0qUl3GxwuryhdPzJHlzi776WjN0YEmEEANhYA== dependencies: loose-envify "^1.1.0" object-assign "^4.1.1" prop-types "^15.6.2" - scheduler "0.0.0-fb28e9048" + scheduler "0.0.0-6da04b5d8" reactcss@^1.2.0: version "1.2.3" @@ -10524,10 +10524,10 @@ sax@>=0.6.0, sax@^1.2.4: resolved "https://registry.yarnpkg.com/sax/-/sax-1.2.4.tgz#2816234e2378bddc4e5354fab5caa895df7100d9" integrity sha512-NqVDv9TpANUjFm0N8uM5GxL36UgKi9/atZw+x7YFnQ8ckwFGKrl4xX4yWtrey3UJm5nP1kUbnYgLopqWNSRhWw== -scheduler@0.0.0-fb28e9048: - version "0.0.0-fb28e9048" - resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.0.0-fb28e9048.tgz#5e7e37a44eb69204936a3ae67f187947980438a8" - integrity sha512-2s3rauGawjL8fGNLWHv5NJkUn8y1IOwAtptE6en0IRUcz33IZQyKlBlFpwdAKyarGScRL6p9FpcLYorXZCaDuw== +scheduler@0.0.0-6da04b5d8, scheduler@^0.0.0-6da04b5d8: + version "0.0.0-6da04b5d8" + resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.0.0-6da04b5d8.tgz#5e0ec65c2b0a7f05ffdc5522fc3a6d95b693e5c9" + integrity sha512-upTrWBZvk4IjMsC/AcRpgCwjnSQl8i78+07KmcndqWOnWp7s4wauowWXhyswP9vucLtZaN5ussFdM3d7dXcTkw== dependencies: loose-envify "^1.1.0" object-assign "^4.1.1" From 7ead6997148b1628dae43078859c2f6f539598ba Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 6 May 2019 13:58:49 -0700 Subject: [PATCH 12/12] Prettier --- .../profilingCommitTreeBuilder-test.js | 2 -- src/__tests__/store-test.js | 21 +++++++------------ 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/src/__tests__/profilingCommitTreeBuilder-test.js b/src/__tests__/profilingCommitTreeBuilder-test.js index bf7b630646..1d037e778f 100644 --- a/src/__tests__/profilingCommitTreeBuilder-test.js +++ b/src/__tests__/profilingCommitTreeBuilder-test.js @@ -7,7 +7,6 @@ describe('commit tree', () => { let React; let ReactDOM; let Scheduler; - let SchedulerTracing; let TestRenderer: TestRendererType; let store: Store; let utils; @@ -22,7 +21,6 @@ describe('commit tree', () => { React = require('react'); ReactDOM = require('react-dom'); Scheduler = require('scheduler'); - SchedulerTracing = require('scheduler/tracing'); TestRenderer = utils.requireTestRenderer(); }); diff --git a/src/__tests__/store-test.js b/src/__tests__/store-test.js index 4208eb06ec..38c5b33eab 100644 --- a/src/__tests__/store-test.js +++ b/src/__tests__/store-test.js @@ -3,17 +3,10 @@ describe('Store', () => { let React; let ReactDOM; - let TestUtils; let agent; + let act; + let getRendererID; let store; - let utils; - - const act = (callback: Function) => { - TestUtils.act(() => { - callback(); - }); - jest.runAllTimers(); // Flush Bridge operations - }; beforeEach(() => { agent = global.agent; @@ -21,8 +14,10 @@ describe('Store', () => { React = require('react'); ReactDOM = require('react-dom'); - TestUtils = require('react-dom/test-utils'); - utils = require('./utils'); + + const utils = require('./utils'); + act = utils.act; + getRendererID = utils.getRendererID; }); it('should not allow a root node to be collapsed', () => { @@ -283,7 +278,7 @@ describe('Store', () => { ); expect(store).toMatchSnapshot('7: only third child is suspended'); - const rendererID = utils.getRendererID(); + const rendererID = getRendererID(); act(() => agent.overrideSuspense({ id: store.getElementIDAtIndex(4), @@ -674,7 +669,7 @@ describe('Store', () => { act(() => store.toggleIsCollapsed(store.getElementIDAtIndex(1), false)); expect(store).toMatchSnapshot('2: expand tree'); - const rendererID = utils.getRendererID(); + const rendererID = getRendererID(); const suspenseID = store.getElementIDAtIndex(1); act(() =>