[Testing] Fix flaky TestFailedTxWillNotChangeStateCommitment - #8639
[Testing] Fix flaky TestFailedTxWillNotChangeStateCommitment#8639janezpodhostnik wants to merge 1 commit into
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe failed transaction state commitment test now uses explicit 60-second timeouts and 100ms polling intervals for both receipt-delivery waits. ChangesExecution test wait behavior
Estimated code review effort: 1 (Trivial) | ~3 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
df287ca to
d7a8a76
Compare
d7a8a76 to
50f707b
Compare
TestFailedTxWillNotChangeStateCommitmentflaked once in a 10x full-suite campaign: itssecond receipt wait (
hub.DeliverAllEventually, waiting for 3 execution receipts) hit the10s timeout. Typical runtime is ~5.5s with a tight distribution, so the pipeline was
genuinely delayed, not deadlocked.
Each receipt crosses an ~8-hop async pipeline (follower -> ingestion -> collection fetch ->
execution -> receipt pusher -> stub network -> consensus mock), where every hop is an
independently scheduled goroutine handoff. Under full-suite parallel load, the accumulated
scheduling delay for 2 more receipts exceeded the 10s/500ms default bound of
DeliverAllEventually.Change: both receipt waits in this test now use
DeliverAllEventuallyUntilwith a 60sbound and a 100ms poll. Liveness bounds return early on success, so the common case is
unaffected; the faster poll actually speeds the test up to ~2.3s typical (was ~5.5s) by
removing tick-quantization latency.
Validation: 10x full-suite campaign with the fix, 10/10 green. In one run the test took
10.8s, which would have failed under the old 10s bound. All assertions unchanged, no
coverage reduction. Test-only change.
Related: #8634
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit