fix: allow some flakiness for RPC tests#5973
Conversation
WalkthroughAdded retry configuration (exponential backoff, count=4, delay=5s, jitter=true) and explanatory comments to the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.config/nextest.toml (2)
38-39: Comment clarity is good; consider linking to nextest docs instead of Wikipedia.Nit: since this comment documents a nextest-specific feature, pointing to the nextest jitter docs provides a more durable reference for future maintainers. (nexte.st)
Apply within this range:
-# Jitter is enabled to avoid [thundering herd issues](https://en.wikipedia.org/wiki/Thundering_herd_problem). +# Jitter is enabled to avoid thundering herd issues; see: +# https://nexte.st/docs/features/retries/#adding-jitter
42-43: Improve retries config & verify override filter in .config/nextest.toml (lines 42–43)The exponential‐backoff retry settings are valid, but you can make them more robust by bounding the backoff delay and adding a kill grace period. Additionally, verification scripts found no disabling flags and no tests matching your
rpc_test_filter—please confirm the override pattern matches your intended tests.• File:
.config/nextest.toml(lines 42–43)
• No usages of--retriesorNEXTEST_RETRIESdetected; per-test overrides won’t be inadvertently disabled.
• No Rust tests namedrpc_test_*were found; ensure the filtertest(rpc_test_)actually matches your test names.Recommended diff:
-slow-timeout = { period = "60s", terminate-after = 2 } -retries = { backoff = "exponential", count = 4, delay = "5s", jitter = true } +slow-timeout = { period = "60s", terminate-after = 2, grace-period = "10s" } +retries = { backoff = "exponential", count = 4, delay = "5s", max-delay = "30s", jitter = true }Optional enhancements:
• Lock minimum Nextest version to ensure support forjitterandmax-delay(added in 0.9.38):nextest-version = { required = "0.9.38" }• Surface retried tests in CI logs more prominently:
[profile.default] status-level = "retry"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.config/nextest.toml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: LesnyRumcajs
PR: ChainSafe/forest#5907
File: src/rpc/methods/state.rs:523-570
Timestamp: 2025-08-06T15:44:33.467Z
Learning: LesnyRumcajs prefers to rely on BufWriter's Drop implementation for automatic flushing rather than explicit flush() calls in Forest codebase.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: cargo-publish-dry-run
- GitHub Check: Build Ubuntu
- GitHub Check: Build MacOS
- GitHub Check: All lint checks
- GitHub Check: tests
- GitHub Check: tests-release
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (rust)
Summary of changes
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Summary by CodeRabbit
Tests
Documentation