Improve tests that use discrete events (#20667)

rickhanlonii · web-flow · commit 9c32622cf04e · 2021-01-26T19:15:06.000-05:00
diff --git a/packages/react-dom/src/__tests__/ReactDOMHooks-test.js b/packages/react-dom/src/__tests__/ReactDOMHooks-test.js
@@ -12,6 +12,7 @@
 let React;
 let ReactDOM;
 let Scheduler;
+let act;
 
 describe('ReactDOMHooks', () => {
   let container;
@@ -22,6 +23,7 @@ describe('ReactDOMHooks', () => {
     React = require('react');
     ReactDOM = require('react-dom');
     Scheduler = require('scheduler');
+    act = require('react-dom/test-utils').unstable_concurrentAct;
 
     container = document.createElement('div');
     document.body.appendChild(container);
@@ -106,7 +108,7 @@ describe('ReactDOMHooks', () => {
   });
 
   // @gate experimental
-  it('should not bail out when an update is scheduled from within an event handler in Concurrent Mode', () => {
+  it('should not bail out when an update is scheduled from within an event handler in Concurrent Mode', async () => {
     const {createRef, useCallback, useState} = React;
 
     const Example = ({inputRef, labelRef}) => {
@@ -132,11 +134,14 @@ describe('ReactDOMHooks', () => {
     Scheduler.unstable_flushAll();
 
     inputRef.current.value = 'abc';
-    inputRef.current.dispatchEvent(
-      new Event('input', {bubbles: true, cancelable: true}),
-    );
-
-    Scheduler.unstable_flushAll();
+    await act(async () => {
+      inputRef.current.dispatchEvent(
+        new Event('input', {
+          bubbles: true,
+          cancelable: true,
+        }),
+      );
+    });
 
     expect(labelRef.current.innerHTML).toBe('abc');
   });
diff --git a/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js b/packages/react-dom/src/__tests__/ReactTestUtilsAct-test.js
@@ -187,7 +187,7 @@ function runActTests(label, render, unmount, rerender) {
         expect(Scheduler).toHaveYielded([100]);
       });
 
-      it('flushes effects on every call', () => {
+      it('flushes effects on every call', async () => {
         function App() {
           const [ctr, setCtr] = React.useState(0);
           React.useEffect(() => {
@@ -209,16 +209,16 @@ function runActTests(label, render, unmount, rerender) {
           button.dispatchEvent(new MouseEvent('click', {bubbles: true}));
         }
 
-        act(() => {
+        await act(async () => {
           click();
           click();
           click();
         });
         // it consolidates the 3 updates, then fires the effect
         expect(Scheduler).toHaveYielded([3]);
-        act(click);
+        await act(async () => click());
         expect(Scheduler).toHaveYielded([4]);
-        act(click);
+        await act(async () => click());
         expect(Scheduler).toHaveYielded([5]);
         expect(button.innerHTML).toBe('5');
       });
diff --git a/packages/react-dom/src/events/plugins/__tests__/ChangeEventPlugin-test.js b/packages/react-dom/src/events/plugins/__tests__/ChangeEventPlugin-test.js
@@ -685,7 +685,7 @@ describe('ChangeEventPlugin', () => {
     });
 
     // @gate experimental
-    it('is async for non-input events', () => {
+    it('is async for non-input events', async () => {
       const root = ReactDOM.unstable_createRoot(container);
       let input;
 
diff --git a/packages/react-dom/src/events/plugins/__tests__/SimpleEventPlugin-test.js b/packages/react-dom/src/events/plugins/__tests__/SimpleEventPlugin-test.js
@@ -13,6 +13,7 @@ describe('SimpleEventPlugin', function() {
   let React;
   let ReactDOM;
   let Scheduler;
+  let TestUtils;
 
   let onClick;
   let container;
@@ -39,6 +40,7 @@ describe('SimpleEventPlugin', function() {
     React = require('react');
     ReactDOM = require('react-dom');
     Scheduler = require('scheduler');
+    TestUtils = require('react-dom/test-utils');
 
     onClick = jest.fn();
   });
@@ -314,7 +316,7 @@ describe('SimpleEventPlugin', function() {
     });
 
     // @gate experimental
-    it('end result of many interactive updates is deterministic', () => {
+    it('end result of many interactive updates is deterministic', async () => {
       container = document.createElement('div');
       const root = ReactDOM.unstable_createRoot(container);
       document.body.appendChild(container);
@@ -361,12 +363,14 @@ describe('SimpleEventPlugin', function() {
       expect(button.textContent).toEqual('Count: 0');
 
       // Click the button many more times
-      click();
-      click();
-      click();
-      click();
-      click();
-      click();
+      await TestUtils.act(async () => {
+        click();
+        click();
+        click();
+        click();
+        click();
+        click();
+      });
 
       // Flush the remaining work
       Scheduler.unstable_flushAll();
@@ -375,7 +379,7 @@ describe('SimpleEventPlugin', function() {
     });
 
     // @gate experimental
-    it('flushes discrete updates in order', () => {
+    it('flushes discrete updates in order', async () => {
       container = document.createElement('div');
       document.body.appendChild(container);
 
diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js
@@ -232,7 +232,8 @@ describe('ReactIncrementalErrorHandling', () => {
     expect(ReactNoop.getChildren()).toEqual([span('Caught an error: oops!')]);
   });
 
-  it("retries at a lower priority if there's additional pending work", () => {
+  // @gate experimental
+  it("retries at a lower priority if there's additional pending work", async () => {
     function App(props) {
       if (props.isBroken) {
         Scheduler.unstable_yieldValue('error');
@@ -252,14 +253,14 @@ describe('ReactIncrementalErrorHandling', () => {
       });
     }
 
-    ReactNoop.discreteUpdates(() => {
-      ReactNoop.render(<App isBroken={true} />, onCommit);
-    });
+    ReactNoop.render(<App isBroken={true} />, onCommit);
     expect(Scheduler).toFlushAndYieldThrough(['error']);
     interrupt();
 
-    // This update is in a separate batch
-    ReactNoop.render(<App isBroken={false} />, onCommit);
+    React.unstable_startTransition(() => {
+      // This update is in a separate batch
+      ReactNoop.render(<App isBroken={false} />, onCommit);
+    });
 
     expect(Scheduler).toFlushAndYieldThrough([
       // The first render fails. But because there's a lower priority pending
@@ -311,16 +312,16 @@ describe('ReactIncrementalErrorHandling', () => {
       });
     }
 
-    ReactNoop.discreteUpdates(() => {
-      ReactNoop.render(<App isBroken={true} />, onCommit);
-    });
+    ReactNoop.render(<App isBroken={true} />, onCommit);
     expect(Scheduler).toFlushAndYieldThrough(['error']);
     interrupt();
 
     expect(ReactNoop).toMatchRenderedOutput(null);
 
-    // This update is in a separate batch
-    ReactNoop.render(<App isBroken={false} />, onCommit);
+    React.unstable_startTransition(() => {
+      // This update is in a separate batch
+      ReactNoop.render(<App isBroken={false} />, onCommit);
+    });
 
     expect(Scheduler).toFlushAndYieldThrough([
       // The first render fails. But because there's a lower priority pending
@@ -1786,6 +1787,7 @@ describe('ReactIncrementalErrorHandling', () => {
     });
   }
 
+  // @gate experimental
   it('uncaught errors should be discarded if the render is aborted', async () => {
     const root = ReactNoop.createRoot();
 
@@ -1795,22 +1797,24 @@ describe('ReactIncrementalErrorHandling', () => {
     }
 
     await ReactNoop.act(async () => {
-      ReactNoop.discreteUpdates(() => {
-        root.render(<Oops />);
-      });
+      root.render(<Oops />);
+
       // Render past the component that throws, then yield.
       expect(Scheduler).toFlushAndYieldThrough(['Oops']);
       expect(root).toMatchRenderedOutput(null);
       // Interleaved update. When the root completes, instead of throwing the
       // error, it should try rendering again. This update will cause it to
       // recover gracefully.
-      root.render('Everything is fine.');
+      React.unstable_startTransition(() => {
+        root.render('Everything is fine.');
+      });
     });
 
     // Should finish without throwing.
     expect(root).toMatchRenderedOutput('Everything is fine.');
   });
 
+  // @gate experimental
   it('uncaught errors are discarded if the render is aborted, case 2', async () => {
     const {useState} = React;
     const root = ReactNoop.createRoot();
@@ -1835,21 +1839,20 @@ describe('ReactIncrementalErrorHandling', () => {
     });
 
     await ReactNoop.act(async () => {
-      // Schedule a high pri and a low pri update on the root.
-      ReactNoop.discreteUpdates(() => {
-        root.render(<Oops />);
+      // Schedule a default pri and a low pri update on the root.
+      root.render(<Oops />);
+      React.unstable_startTransition(() => {
+        root.render(<AllGood />);
       });
-      root.render(<AllGood />);
-      // Render through just the high pri update. The low pri update remains on
+
+      // Render through just the default pri update. The low pri update remains on
       // the queue.
       expect(Scheduler).toFlushAndYieldThrough(['Everything is fine.']);
 
-      // Schedule a high pri update on a child that triggers an error.
+      // Schedule a default pri update on a child that triggers an error.
       // The root should capture this error. But since there's still a pending
       // update on the root, the error should be suppressed.
-      ReactNoop.discreteUpdates(() => {
-        setShouldThrow(true);
-      });
+      setShouldThrow(true);
     });
     // Should render the final state without throwing the error.
     expect(Scheduler).toHaveYielded(['Everything is fine.']);
diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js
@@ -570,14 +570,13 @@ describe('ReactSuspenseWithNoopRenderer', () => {
       );
     }
 
-    // Schedule a high pri update and a low pri update, without rendering in
-    // between.
-    ReactNoop.discreteUpdates(() => {
-      // High pri
-      ReactNoop.render(<App />);
-    });
+    // Schedule a default pri update and a low pri update, without rendering in between.
+    // Default pri
+    ReactNoop.render(<App />);
     // Low pri
-    ReactNoop.render(<App hide={true} />);
+    React.unstable_startTransition(() => {
+      ReactNoop.render(<App hide={true} />);
+    });
 
     expect(Scheduler).toFlushAndYield([
       // The first update suspends
@@ -1879,9 +1878,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
     ReactNoop.render(<Foo />);
     expect(Scheduler).toFlushAndYield(['Foo']);
 
-    ReactNoop.discreteUpdates(() =>
-      ReactNoop.render(<Foo renderContent={true} />),
-    );
+    ReactNoop.render(<Foo renderContent={true} />);
     expect(Scheduler).toFlushAndYieldThrough(['Foo']);
 
     // Advance some time.
@@ -3080,48 +3077,48 @@ describe('ReactSuspenseWithNoopRenderer', () => {
         // Schedule an update inside the Suspense boundary that suspends.
         setAppText('B');
         expect(Scheduler).toFlushAndYield(['Suspend! [B]', 'Loading...']);
+      });
 
-        // Commit the placeholder
-        await advanceTimers(250);
-        expect(root).toMatchRenderedOutput(
-          <>
-            <span hidden={true} prop="A" />
-            <span prop="Loading..." />
-          </>,
-        );
+      expect(root).toMatchRenderedOutput(
+        <>
+          <span hidden={true} prop="A" />
+          <span prop="Loading..." />
+        </>,
+      );
 
-        // Schedule a high pri update on the boundary, and a lower pri update
-        // on the fallback. We're testing to make sure the fallback can still
-        // update even though the primary tree is suspended.{
-        ReactNoop.discreteUpdates(() => {
-          setAppText('C');
+      // Schedule a default pri update on the boundary, and a lower pri update
+      // on the fallback. We're testing to make sure the fallback can still
+      // update even though the primary tree is suspended.
+      await ReactNoop.act(async () => {
+        setAppText('C');
+        React.unstable_startTransition(() => {
+          setFallbackText('Still loading...');
         });
-        setFallbackText('Still loading...');
+      });
 
-        expect(Scheduler).toFlushAndYield([
-          // First try to render the high pri update. Still suspended.
-          'Suspend! [C]',
-          'Loading...',
+      expect(Scheduler).toHaveYielded([
+        // First try to render the high pri update. Still suspended.
+        'Suspend! [C]',
+        'Loading...',
 
-          // In the expiration times model, once the high pri update suspends,
-          // we can't be sure if there's additional work at a lower priority
-          // that might unblock the tree. We do know that there's a lower
-          // priority update *somehwere* in the entire root, though (the update
-          // to the fallback). So we try rendering one more time, just in case.
-          // TODO: We shouldn't need to do this with lanes, because we always
-          // know exactly which lanes have pending work in each tree.
-          'Suspend! [C]',
-
-          // Then complete the update to the fallback.
-          'Still loading...',
-        ]);
-        expect(root).toMatchRenderedOutput(
-          <>
-            <span hidden={true} prop="A" />
-            <span prop="Still loading..." />
-          </>,
-        );
-      });
+        // In the expiration times model, once the high pri update suspends,
+        // we can't be sure if there's additional work at a lower priority
+        // that might unblock the tree. We do know that there's a lower
+        // priority update *somehwere* in the entire root, though (the update
+        // to the fallback). So we try rendering one more time, just in case.
+        // TODO: We shouldn't need to do this with lanes, because we always
+        // know exactly which lanes have pending work in each tree.
+        'Suspend! [C]',
+
+        // Then complete the update to the fallback.
+        'Still loading...',
+      ]);
+      expect(root).toMatchRenderedOutput(
+        <>
+          <span hidden={true} prop="A" />
+          <span prop="Still loading..." />
+        </>,
+      );
     },
   );
 
diff --git a/packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js b/packages/react-reconciler/src/__tests__/useMutableSource-test.internal.js
@@ -1397,12 +1397,13 @@ describe('useMutableSource', () => {
       // Now mutate A. Both hooks should update.
       // This is at high priority so that it doesn't get batched with default
       // priority updates that might fire during the passive effect
-      ReactNoop.discreteUpdates(() => {
-        mutateA('a1');
+      await ReactNoop.act(async () => {
+        ReactNoop.discreteUpdates(() => {
+          mutateA('a1');
+        });
       });
-      expect(Scheduler).toFlushUntilNextPaint([]);
 
-      expect(root.getChildrenAsJSX()).toEqual('first: a1, second: a1');
+      expect(root).toMatchRenderedOutput('first: a1, second: a1');
     });
 
     expect(root.getChildrenAsJSX()).toEqual('first: a1, second: a1');
diff --git a/packages/react-refresh/src/__tests__/ReactFresh-test.js b/packages/react-refresh/src/__tests__/ReactFresh-test.js
