Repository: facebook/react
Pull request: Update Suspense fuzz tests to use `act`

This updates the Suspense fuzz tester to use `act` to recursively flush timers instead of doing it manually.

This still isn't great because ideally the fuzz tester wouldn't fake timers at all. It should resolve promises using a custom queue instead of Jest's fake timer queue, like we've started doing in our other Suspense tests (i.e. the `resolveText` pattern). That's because our internal `act` API (not the public one, the one we use in our tests) uses Jest's fake timer queue as a way to force Suspense fallbacks to appear.

However I'm not interested in upgrading this test suite to a better strategy right now because if I were writing a Suspense fuzzer today I would probably use an entirely different approach. So this is just an incremental improvement to make it slightly less decoupled to React implementation details.

Changed files:
filename: packages/internal-test-utils/internalAct.js
patch: @@ -80,7 +80,19 @@ export async function act<T>(scope: () => Thenable<T>): Thenable<T> {
 
       if (!Scheduler.unstable_hasPendingWork()) {
         // $FlowFixMe[cannot-resolve-name]: Flow doesn't know about global Jest object
-        jest.runOnlyPendingTimers();
+        const j = jest;
+        if (j.getTimerCount() > 0) {
+          // There's a pending timer. Flush it now. We only do this in order to
+          // force Suspense fallbacks to display; the fact that it's a timer
+          // is an implementation detail. If there are other timers scheduled,
+          // those will also fire now, too, which is not ideal. (The public
+          // version of `act` doesn't do this.) For this reason, we should try
+          // to avoid using timers in our internal tests.
+          j.runOnlyPendingTimers();
+          // If a committing a fallback triggers another update, it might not
+          // get scheduled until a microtask. So wait one more time.
+          await waitForMicrotasks();
+        }
         if (Scheduler.unstable_hasPendingWork()) {
           // Committing a fallback scheduled additional work. Continue flushing.
         } else {

filename: packages/react-reconciler/src/__tests__/ReactSuspenseFuzz-test.internal.js
patch: @@ -32,6 +32,8 @@ describe('ReactSuspenseFuzz', () => {
     Random = require('random-seed');
   });
 
+  jest.setTimeout(20000);
+
   function createFuzzer() {
     const {useState, useContext, useLayoutEffect} = React;
 
@@ -57,7 +59,6 @@ describe('ReactSuspenseFuzz', () => {
             };
             const timeoutID = setTimeout(() => {
               pendingTasks.delete(task);
-              Scheduler.log(task.label);
               setStep(i + 1);
             }, remountAfter);
             pendingTasks.add(task);
@@ -87,7 +88,6 @@ describe('ReactSuspenseFuzz', () => {
             };
             const timeoutID = setTimeout(() => {
               pendingTasks.delete(task);
-              Scheduler.log(task.label);
               setStep([i + 1, suspendFor]);
             }, beginAfter);
             pendingTasks.add(task);
@@ -138,43 +138,50 @@ describe('ReactSuspenseFuzz', () => {
       return resolvedText;
     }
 
-    async function resolveAllTasks() {
-      Scheduler.unstable_flushAllWithoutAsserting();
-      let elapsedTime = 0;
-      while (pendingTasks && pendingTasks.size > 0) {
-        if ((elapsedTime += 1000) > 1000000) {
-          throw new Error('Something did not resolve properly.');
-        }
-        await act(() => {
-          ReactNoop.batchedUpdates(() => {
-            jest.advanceTimersByTime(1000);
-          });
-        });
-        Scheduler.unstable_flushAllWithoutAsserting();
-      }
-    }
-
     async function testResolvedOutput(unwrappedChildren) {
       const children = (
         <Suspense fallback="Loading...">{unwrappedChildren}</Suspense>
       );
 
+      // Render the app multiple times: once without suspending (as if all the
+      // data was already preloaded), and then again with suspensey data.
       resetCache();
       const expectedRoot = ReactNoop.createRoot();
-      expectedRoot.render(
-        <ShouldSuspendContext.Provider value={false}>
-          {children}
-        </ShouldSuspendContext.Provider>,
-      );
-      await resolveAllTasks();
+      await act(() => {
+        expectedRoot.render(
+          <ShouldSuspendContext.Provider value={false}>
+            {children}
+          </ShouldSuspendContext.Provider>,
+        );
+      });
+
       const expectedOutput = expectedRoot.getChildrenAsJSX();
 
       resetCache();
-      ReactNoop.renderLegacySyncRoot(children);
-      await resolveAllTasks();
-      const legacyOutput = ReactNoop.getChildrenAsJSX();
-      expect(legacyOutput).toEqual(expectedOutput);
-      ReactNoop.renderLegacySyncRoot(null);
+
+      const concurrentRootThatSuspends = ReactNoop.createRoot();
+      await act(() => {
+        concurrentRootThatSuspends.render(children);
+      });
+
+      resetCache();
+
+      // Do it again in legacy mode.
+      const legacyRootThatSuspends = ReactNoop.createLegacyRoot();
+      await act(() => {
+        legacyRootThatSuspends.render(children);
+      });
+
+      // Now compare the final output. It should be the same.
+      expect(concurrentRootThatSuspends.getChildrenAsJSX()).toEqual(
+        expectedOutput,
+      );
+      expect(legacyRootThatSuspends.getChildrenAsJSX()).toEqual(expectedOutput);
+
+      // TODO: There are Scheduler logs in this test file but they were only
+      // added for debugging purposes; we don't make any assertions on them.
+      // Should probably just delete.
+      Scheduler.unstable_clearLog();
     }
 
     function pickRandomWeighted(rand, options) {