-
Notifications
You must be signed in to change notification settings - Fork 50.5k
[tests] Fix flaky flight tests #35513
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[tests] Fix flaky flight tests #35513
Conversation
unstubbable
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I understand these tests do care about the debug info entries with the mocked time (introduced in #31716). For example, you want to assert that they are sequential and not overlapping, which is why there are also comments like Clamped to the start. So they are roughly asserting how this would lay out in the performance tracks. I've also never seen them flake. Under which circumstances did that happen to you?
|
@unstubbable I've seen CI fail quite a few times from this lately on my PRs, and on main. For example, this commit from main failed from this in ReactFlightDOMEdge. This test failed on my latest PR also from ReactFlightDOMEdge. But aside from CI, I can basically repro locally any time I run |
|
The root cause seems to be that when those tests are run alongside other tests, the timing changes due to subtle delays in execution caused by the other test runs. |
a738e51 to
e01bf9b
Compare
|
@unstubbable I found the root cause, see the new description and fixes - none of the test snapshots need to change now. |
unstubbable
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense!
Flights tests are failing locally and in CI non-deterministically because we're not disabling async hooks after tests, and GC can clear WeakRefs non-deterministically.
This PR fixes the issue by adding an afterEach to disable installed hooks, and normalizing the
valuetovalue: {value: undefined}}when snapshotting.Claude found the fix, here's the explanation:
Summary of Both Flaky Test Issues
Issue 1: ReactFlightDOMEdge-test.js - Cross-Test Async Hook Pollution
Root Cause:
The Fix: scripts/jest/setupTests.js
This ensures async hooks are disabled after each test, preventing cross-test pollution.
Issue 2: ReactFlightAsyncDebugInfo-test.js - WeakRef Garbage Collection
Root Cause:
The Fix: packages/internal-test-utils/debugInfo.js
This normalizes the output so snapshots always see a consistent structure regardless of GC timing.
★ Insight ─────────────────────────────────────
Two Patterns for Flaky Tests:
─────────────────────────────────────────────────
Both issues shared a common theme: non-deterministic behavior in sequential test execution. The first was about test isolation failure, the second was about normalizing inherently non-deterministic runtime behavior.