fix(e2e): always wait for at least height 1 #3415
Conversation
gartnera
added
no-changelog
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis pull request encompasses modifications across multiple files in the Zeta project, focusing on test infrastructure and logging adjustments. The changes include updating the default wait-for-height flag in local end-to-end testing, reducing script wait times, enhancing block height waiting mechanism robustness, and adjusting logging verbosity for block event tracking. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
Documentation and Community
|
gartnera
force-pushed
the
e2e-wait-one
branch
from
a8e69af to
e6427bd
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3415 +/- ##
===========================================
- Coverage 63.01% 62.87% -0.15%
===========================================
Files 436 436
Lines 30683 30759 +76
===========================================
+ Hits 19336 19339 +3
- Misses 10535 10608 +73
Partials 812 812
|
![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 (2)
e2e/utils/zetacore.go (2)
317-317: One-second polling may affect performance on large-scale tests.While a 1-second interval is straightforward, consider a more adaptive approach if system load necessitates shorter or dynamic backoff intervals.
328-330: Adding a 100-attempt limit prevents infinite loops.Capping the retries is prudent. If deployments or nodes are particularly slow, consider allowing a configurable upper limit through a flag or environment variable.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cmd/zetae2e/local/local.go(2 hunks)contrib/localnet/scripts/start-zetaclientd.sh(1 hunks)e2e/utils/zetacore.go(1 hunks)zetaclient/zetacore/client_subscriptions.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
contrib/localnet/scripts/start-zetaclientd.sh (1)
Pattern **/*.sh: Review the shell scripts, point out issues relative to security, performance, and maintainability.
e2e/utils/zetacore.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
zetaclient/zetacore/client_subscriptions.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
cmd/zetae2e/local/local.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: start-e2e-test / e2e
🔇 Additional comments (5)
zetaclient/zetacore/client_subscriptions.go (1)
82-82: Lowering log level reduces spam and is beneficial.Switching from info to debug helps keep the logs succinct while still monitoring block events for debugging use cases.
cmd/zetae2e/local/local.go (2)
64-64: Good default to always wait for at least height 1.This ensures that the test environment is initialized past genesis height, reducing flakiness.
153-153: Enforcing block wait unconditionally improves consistency.Relying on
utils.WaitForBlockHeightwithout conditional checks guarantees a stable testing environment, helping mitigate race conditions observed earlier.contrib/localnet/scripts/start-zetaclientd.sh (1)
47-47: Reduced wait time improves responsiveness with caution.Shortening the sleep from 20s to 10s accelerates startup but might impact test stability in slower setups. Confirm that nodes reliably become available within this timeframe.
✅ Verification successful
Sleep duration aligns with codebase patterns
The 10-second wait time matches existing timeouts in end-to-end testing scripts while exceeding the 1-second delay used by core services, providing adequate buffer for node availability.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Checking logs for delayed node availability beyond 10s # The script looks at all logs in the local environment for "Waiting for zetacore0 rpc" messages lasting longer than 10s rg "Waiting for zetacore0 rpc" -A 5Length of output: 1645
e2e/utils/zetacore.go (1)
320-320: Continuing instead of erroring promotes resilience.By retrying when an RPC status call fails, the tests are more robust. Be mindful of masking persistent errors that might persist across intervals.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
e2e/utils/zetacore.go (1)
317-317: Introducing a 1-second sleep helps reduce CPU usage when waiting
This approach avoids busy waiting but may prolong total test runtime on slower machines.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cmd/zetae2e/local/local.go(2 hunks)contrib/localnet/scripts/start-zetaclientd.sh(1 hunks)e2e/utils/zetacore.go(1 hunks)zetaclient/zetacore/client_subscriptions.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
contrib/localnet/scripts/start-zetaclientd.sh (1)
Pattern **/*.sh: Review the shell scripts, point out issues relative to security, performance, and maintainability.
zetaclient/zetacore/client_subscriptions.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
cmd/zetae2e/local/local.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
e2e/utils/zetacore.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: start-e2e-test / e2e
🔇 Additional comments (5)
zetaclient/zetacore/client_subscriptions.go (1)
82-82: Lowering the log level to debug is more appropriate for frequent block event messages
This helps reduce log spam while preserving important visibility for debugging.e2e/utils/zetacore.go (2)
320-320: Swallowing errors might mask legitimate connection issues
Consider logging them at debug level or applying an exponential backoff for repeated failures.
328-330: Imposing a maximum of 100 attempts prevents infinite loops
This approach is sensible, yet it may be beneficial to allow runtime configuration or environment override for certain CI environments.cmd/zetae2e/local/local.go (1)
64-64: Always waiting for at least block height 1
This ensures minimal readiness before proceeding, potentially preventing invalid height errors.contrib/localnet/scripts/start-zetaclientd.sh (1)
47-47: Reducing wait time from 20 to 10 seconds
This speeds up initialization but might pose a risk if the node occasionally needs more time to start.
e2e/utils/zetacore.go
Outdated
| // prevent spamming logs | ||
| if i%10 == 0 { | ||
| logger.Info("waiting for block: %d, current height: %d\n", desiredHeight, currentHeight) | ||
| } | ||
| if i > 100 { |
There was a problem hiding this comment.
Maybe 100 seconds is too long. set to 30?
Seeing increased e2e failure rates after #3357. Let's always wait for at least height 1 on e2e.
Also change this info log spam to debug:
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
zetacore0RPC availability from 20 to 10 secondsWaitForBlockHeightfunction with improved error handling and retry mechanismConfiguration Changes
waitForHeightflag from 0 to 1 in local command tests