devtool: complete raced round initialization fix - #3995
Conversation
The prior fix in #3970 only handled transactions that reverted after submission. Also tolerate initializeRound failures during gas estimation when another initializer has already completed the round.
📝 WalkthroughWalkthrough
ChangesRound initialization handling
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/devtool/devtool/devtool_utils.go (1)
298-312: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAdd focused coverage for both race windows.
Test
tolerateInitializedRoundwhen the round is initialized, not initialized, and when the state check fails; also exercise bothInitializeRoundandCheckTxcallers. This protects the CI-flake fix and verifies original errors remain preserved.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/devtool/devtool/devtool_utils.go` around lines 298 - 312, add focused tests for tolerateInitializedRound covering initialized, not initialized, and CurrentRoundInitialized failure cases, asserting successful suppression or preservation of the original initialization error as appropriate. Also exercise both InitializeRound and CheckTx callers to cover each race window and verify their original errors remain intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@cmd/devtool/devtool/devtool_utils.go`:
- Around line 298-312: add focused tests for tolerateInitializedRound covering
initialized, not initialized, and CurrentRoundInitialized failure cases,
asserting successful suppression or preservation of the original initialization
error as appropriate. Also exercise both InitializeRound and CheckTx callers to
cover each race window and verify their original errors remain intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4c10966a-050b-4a59-8cf8-a89092f6bfdf
📒 Files selected for processing (1)
cmd/devtool/devtool/devtool_utils.go
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3995 +/- ##
===================================================
- Coverage 35.42499% 35.41977% -0.00522%
===================================================
Files 174 174
Lines 45036 45037 +1
===================================================
- Hits 15954 15952 -2
- Misses 27825 27827 +2
- Partials 1257 1258 +1
... and 4 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
The prior fix in #3970 only handled transactions that reverted after submission. Also tolerate initializeRound failures during gas estimation when another initializer has already completed the round.
Attempts to fix the sporadic e2e test failures during CI
Summary by CodeRabbit