Update Suspense fuzz tests to use `act` (#26498)
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.
Andrew Clark committed
Mar 28, 2023 at 14:40 UTC
1f5cdf8c77182fc51910787e48384ec4620dc40d
2 files changed
+49
-30
packages/internal-test-utils/internalAct.js
+13
-1
@@ -80,7 +80,19 @@ export async function act<T>(scope: () => Thenable<T>): Thenable<T> {
80
81
if (!Scheduler.unstable_hasPendingWork()) {
82
// $FlowFixMe[cannot-resolve-name]: Flow doesn't know about global Jest object
83
- jest.runOnlyPendingTimers();
83
+ const j = jest;
84
+ if (j.getTimerCount() > 0) {
85
+ // There's a pending timer. Flush it now. We only do this in order to
86
+ // force Suspense fallbacks to display; the fact that it's a timer
87
+ // is an implementation detail. If there are other timers scheduled,
88
+ // those will also fire now, too, which is not ideal. (The public
89
+ // version of `act` doesn't do this.) For this reason, we should try
90
+ // to avoid using timers in our internal tests.
91
+ j.runOnlyPendingTimers();
92
+ // If a committing a fallback triggers another update, it might not
93
+ // get scheduled until a microtask. So wait one more time.
94
+ await waitForMicrotasks();
95
+ }
96
if (Scheduler.unstable_hasPendingWork()) {
97
// Committing a fallback scheduled additional work. Continue flushing.
98
} else {
packages/react-reconciler/src/__tests__/ReactSuspenseFuzz-test.internal.js
+36
-29
@@ -32,6 +32,8 @@ describe('ReactSuspenseFuzz', () => {
32
Random = require('random-seed');
33
});
34
35
+ jest.setTimeout(20000);
36
+
37
function createFuzzer() {
38
const {useState, useContext, useLayoutEffect} = React;
39
@@ -57,7 +59,6 @@ describe('ReactSuspenseFuzz', () => {
59
};
60
const timeoutID = setTimeout(() => {
61
pendingTasks.delete(task);
60
- Scheduler.log(task.label);
62
setStep(i + 1);
63
}, remountAfter);
64
pendingTasks.add(task);
@@ -87,7 +88,6 @@ describe('ReactSuspenseFuzz', () => {
88
};
89
const timeoutID = setTimeout(() => {
90
pendingTasks.delete(task);
90
- Scheduler.log(task.label);
91
setStep([i + 1, suspendFor]);
92
}, beginAfter);
93
pendingTasks.add(task);
@@ -138,43 +138,50 @@ describe('ReactSuspenseFuzz', () => {
138
return resolvedText;
139
}
140
141
- async function resolveAllTasks() {
142
- Scheduler.unstable_flushAllWithoutAsserting();
143
- let elapsedTime = 0;
144
- while (pendingTasks && pendingTasks.size > 0) {
145
- if ((elapsedTime += 1000) > 1000000) {
146
- throw new Error('Something did not resolve properly.');
147
- }
148
- await act(() => {
149
- ReactNoop.batchedUpdates(() => {
150
- jest.advanceTimersByTime(1000);
151
- });
152
- });
153
- Scheduler.unstable_flushAllWithoutAsserting();
154
- }
155
- }
156
-
141
async function testResolvedOutput(unwrappedChildren) {
142
const children = (
143
<Suspense fallback="Loading...">{unwrappedChildren}</Suspense>
144
);
145
146
+ // Render the app multiple times: once without suspending (as if all the
147
+ // data was already preloaded), and then again with suspensey data.
148
resetCache();
149
const expectedRoot = ReactNoop.createRoot();
164
- expectedRoot.render(
165
- <ShouldSuspendContext.Provider value={false}>
166
- {children}
167
- </ShouldSuspendContext.Provider>,
168
- );
169
- await resolveAllTasks();
150
+ await act(() => {
151
+ expectedRoot.render(
152
+ <ShouldSuspendContext.Provider value={false}>
153
+ {children}
154
+ </ShouldSuspendContext.Provider>,
155
+ );
156
+ });
157
+
158
const expectedOutput = expectedRoot.getChildrenAsJSX();
159
160
resetCache();
173
- ReactNoop.renderLegacySyncRoot(children);
174
- await resolveAllTasks();
175
- const legacyOutput = ReactNoop.getChildrenAsJSX();
176
- expect(legacyOutput).toEqual(expectedOutput);
177
- ReactNoop.renderLegacySyncRoot(null);
161
+
162
+ const concurrentRootThatSuspends = ReactNoop.createRoot();
163
+ await act(() => {
164
+ concurrentRootThatSuspends.render(children);
165
+ });
166
+
167
+ resetCache();
168
+
169
+ // Do it again in legacy mode.
170
+ const legacyRootThatSuspends = ReactNoop.createLegacyRoot();
171
+ await act(() => {
172
+ legacyRootThatSuspends.render(children);
173
+ });
174
+
175
+ // Now compare the final output. It should be the same.
176
+ expect(concurrentRootThatSuspends.getChildrenAsJSX()).toEqual(
177
+ expectedOutput,
178
+ );
179
+ expect(legacyRootThatSuspends.getChildrenAsJSX()).toEqual(expectedOutput);
180
+
181
+ // TODO: There are Scheduler logs in this test file but they were only
182
+ // added for debugging purposes; we don't make any assertions on them.
183
+ // Should probably just delete.
184
+ Scheduler.unstable_clearLog();
185
}
186
187
function pickRandomWeighted(rand, options) {