test(testutil): try retrying for 'panic: pebbledb: closed' #2162
Conversation
WalkthroughThis pull request introduces several enhancements and fixes across the Nibiru blockchain project, primarily focusing on the EVM, bank keeper, and testing infrastructure. The changes include improvements to gas consumption, address encoding, event handling, and test suite robustness. A new utility function Changes
Sequence DiagramsequenceDiagram
participant Test Runner
participant RetrySuiteRunIfDbClosed
participant Test Suite
participant Database
Test Runner->>RetrySuiteRunIfDbClosed: Initiate test run
RetrySuiteRunIfDbClosed->>Database: Attempt database connection
alt Database is closed
RetrySuiteRunIfDbClosed-->>Test Runner: Retry test run
else Database is open
RetrySuiteRunIfDbClosed->>Test Suite: Execute test suite
end
Possibly related PRs
Suggested reviewers
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2162 +/- ##
==========================================
- Coverage 65.20% 65.12% -0.08%
==========================================
Files 277 277
Lines 22242 22267 +25
==========================================
Hits 14502 14502
- Misses 6746 6771 +25
Partials 994 994
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
x/common/testutil/cases.go (1)
43-74: Consider adding delay between retries and making panic message configurable.The implementation is solid, but could be enhanced with:
- A delay between retries to prevent immediate retries
- A configurable panic message parameter for reusability
-func RetrySuiteRunIfDbClosed(t *testing.T, runTest func(), maxRetries int) { - panicMessage := "pebbledb: closed" +func RetrySuiteRunIfDbClosed(t *testing.T, runTest func(), maxRetries int, retryDelay time.Duration) { + panicMessage := "pebbledb: closed" // TODO: Consider making this configurable for attempt := 0; attempt < maxRetries; attempt++ { panicked := false func() { defer func() { if r := recover(); r != nil { if errMsg, ok := r.(string); ok && strings.Contains(errMsg, panicMessage) { t.Logf("Recovered from panic on attempt %d: %v", attempt, r) panicked = true } else { panic(r) // Re-panic if it's not the specific error } } }() runTest() }() if !panicked { t.Logf("Test suite succeeded on attempt %d", attempt) return } + if attempt < maxRetries-1 { + t.Logf("Waiting %v before retry attempt %d", retryDelay, attempt+1) + time.Sleep(retryDelay) + } t.Logf("Retrying test suite: attempt %d", attempt+1) } t.Fatalf("Test suite failed after %d attempts due to '%s'", maxRetries, panicMessage) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
CHANGELOG.md(1 hunks)app/wasmext/wasm_cli_test/cli_test.go(1 hunks)eth/rpc/rpcapi/eth_api_test.go(1 hunks)x/common/testutil/cases.go(2 hunks)x/common/testutil/testnetwork/network_test.go(1 hunks)x/oracle/keeper/app_test.go(1 hunks)x/sudo/cli/cli_test.go(1 hunks)x/tokenfactory/cli/cli_test.go(1 hunks)
👮 Files not reviewed due to content moderation or server errors (4)
- x/tokenfactory/cli/cli_test.go
- x/sudo/cli/cli_test.go
- eth/rpc/rpcapi/eth_api_test.go
- CHANGELOG.md
🔇 Additional comments (4)
x/common/testutil/cases.go (1)
39-42: LGTM! Well-documented function with clear purpose.The documentation clearly explains the purpose and context of the function, linking to the relevant issue.
x/common/testutil/testnetwork/network_test.go (1)
25-27: LGTM! Proper usage of retry mechanism.The implementation correctly uses the retry mechanism with a reasonable number of retries (2).
app/wasmext/wasm_cli_test/cli_test.go (1)
143-145: LGTM! Consistent implementation of retry mechanism.The implementation follows the same pattern as other files, using 2 retries.
x/oracle/keeper/app_test.go (1)
180-182: LGTM! Consistent implementation of retry mechanism.The implementation follows the same pattern as other files, using 2 retries.
Purpose / Abstract
An attempt at solving an old problem that's plagued the CI for years (#1918)
Summary by CodeRabbit
New Features
RetrySuiteRunIfDbClosedto handle test suite retriesBug Fixes
Refactor
VerifyFeefunction by removing unnecessary arguments