Add More Fuzz Tests

[gaearon]

There’s some things we don’t have a sufficient coverage of. Currently we catch them from production product bugs but this is not sustainable. We’re hoping some refactors will drastically simplify the model — but nevertheless we should invest in better fuzz test coverage. That’s how we caught similar bugs before at an earlier stage.

One thing we’re lacking coverage for is what happens to Suspense boundaries as updates are dispatched at different priorities in different order, and what happens when we’ve had to yield. We need to verify that Suspense always “wakes up” when Promises are resolved and there’s nothing to be suspended on. We also need to test this in combination with render phase updates.

Here’s examples of bugs that I want a fuzzer to catch: #18657 #18020 #18486 #18357 #18353 #18644 #18412.

@dubzzz I believe you were interested in this? This would take some effort but would be a major contribution.

[dubzzz]

Yes, I'll be really interested in helping on this subject.

For the moment, I already drafted some fuzz tests based on property based testing for SuspenseList (here). They detected the issue #18661.

Waiting your feedback on the following scenarii and on #18673.

---

Here are the scenarii I am covering so far with those tests - please let me know if my scenarii are based on wrong hypothesis:

• should wait all components to be resolved before showing any in "together" mode

The test generates a random tree of components with only SuspenseList having mode="together" and Suspense.

If there is still one pending Suspense, it checks that the SuspenseList is still waiting and not rendering anything. Then it resolves out-of-order one of the pending Suspense.

As soon as everything gets resolved, the test checks that everything is displayed properly.

• should display components up-to the first unresolved one as resolved, next ones should be considered unresolved in "forward" mode

Close to the test described above but for mode="forwards".

The check is obviously different: We expect all the Suspense up to the first unresolved one not included to be shown correctly. And starting from the first unresolved one we expect them to show the fallback.

In this test (it is not the case in the first one), we render a first tree of components then a second one with some nodes identical, some removed, some added. This second render might happen any time between two resolutions of Suspense.

Example:

Tree A::
<SuspenseList key="a" mode="forwards">
  <Suspense key="a.a"><A/></Suspense>
  <Suspense key="a.c"><C/></Suspense>
</SuspenseList>

Tree B::
<SuspenseList key="a" mode="forwards">
  <Suspense key="a.a"><A/></Suspense>
  <Suspense key="a.b"><B/></Suspense>
</SuspenseList>

Resolution order might be (render Tree A is done first):
- B Loaded        -- test expects [Loading A, Loading C]
- Render Tree B   -- test expects [Loading A, Loading B]
- C Loaded        -- test expects [Loading A, Loading B]
- A Loaded        -- test expects [A, B]

All the scenarii described above can be replayed again and again in case of bug as they rely on a seed (the scheduler handling the scheduling of which Suspense should resolve when is based on the seed).

[gaearon]

I would probably focus on Suspense itself (and suspending at different priority levels) before spending too much effort on SuspenseList. The core Suspense bugs are more important because you can’t “opt out” of them, like you can by removing SuspenseList.

[dubzzz]

I'll start to work on a possible way to detect those kinds of bugs with fuzzing. I'll just need to get through some of the bugs you put just to see what is the recurrent cause of bug in order to cover it (and more)

[dubzzz]

I implemented a first version only focusing on Suspense. It already detected what looks to be an issue https://codesandbox.io/s/strange-frost-d4ujl?file=/src/App.js 🤔

By the way, the issue looks similar to the one you reported in #18657. I'll need to rebase my branch to check if it still detects issues on the latest version of master.

The test renders the following component:

<Suspense fallback={<Text text="Loading..." />}>
  <App />
</Suspense>

with App:

function App() {
  const [text, setText] = React.useState(initialText);
  return <AsyncText text={text} readOrThrow={readOrThrow} />;
}

It reorders when AsyncText will resolve for a given text and applies "randomly" updates of the text by calling Scheduler.unstable_runWithPriority.

The code of the test is avalaible here: dubzzz@45b9398

[gaearon]

Wow, that’s great! Send a PR? 🙂

I think maybe it’s worth also trying to write a vanilla version per @acdlite’s point. Maybe their difference will convince him the library is worth it? Or maybe the other way around.

[dubzzz]

@gaearon @acdlite As the test might help you to detect other issues on Suspense, I opened a PR based on fast-check. I know that adding a new library might be problematic so feel free to close it. For now, I have not worked on an implementation framework-free as it will require lots of work to develop the scheduling logic part and might be error-prone.

[stale]

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

[stale]

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!