chore(e2e): increase test timeout #3103
Conversation
gartnera
requested review from
brewmaster012,
kingpinXD,
lumtis,
ws4charlie,
skosito,
swift1337 and
fbac
as code owners
|
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 📝 Walkthrough📝 WalkthroughWalkthroughThe pull request modifies the Changes
Possibly related issues
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
|
![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
🧹 Outside diff range and nitpick comments (2)
cmd/zetae2e/local/local.go (2)
53-53: LGTM! Consider making the timeout configurable.The timeout increase from 15 to 20 minutes is reasonable given the comprehensive test suite and complex setup requirements. However, consider making this value configurable via a command-line flag to allow for different timeout durations based on the specific test scenarios being run.
Consider adding a timeout flag:
var ( - TestTimeout = 20 * time.Minute + defaultTestTimeout = 20 * time.Minute ) func NewLocalCmd() *cobra.Command { cmd := &cobra.Command{ Use: "local", Short: "Run Local E2E tests", Run: localE2ETest, } + cmd.Flags().Duration("timeout", defaultTestTimeout, "timeout duration for the test suite") // ... other flags ...Then update the timer goroutine to use the configured timeout:
- time.Sleep(TestTimeout) - logger.Error("Test timed out after %s", TestTimeout.String()) + timeout := must(cmd.Flags().GetDuration("timeout")) + time.Sleep(timeout) + logger.Error("Test timed out after %s", timeout.String())
Line range hint
291-297: Consider implementing test-specific timeouts.The current implementation uses a global timeout for all test scenarios. Consider implementing test-specific timeouts to better handle varying test durations and improve test reliability.
Consider modifying the test routine functions to accept a context with timeout:
-func erc20TestRoutine(conf *config.Config, runner *runner.Runner, verbose bool, tests ...string) func() error { +func erc20TestRoutine(ctx context.Context, conf *config.Config, runner *runner.Runner, verbose bool, tests ...string) func() error { return func() error { logger := runner.NewLogger(verbose, color.FgGreen, "erc20") + // Use a shorter timeout for ERC20 tests + ctx, cancel := context.WithTimeout(ctx, 5*time.Minute) + defer cancel() // ... test execution ... } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
cmd/zetae2e/local/local.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
cmd/zetae2e/local/local.go (1)
Pattern **/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3103 +/- ##
========================================
Coverage 63.38% 63.38%
========================================
Files 422 422
Lines 29915 29915
========================================
Hits 18963 18963
Misses 10111 10111
Partials 841 841 |
e2e test runs have been timing out on push to
develop, let's bump the default timeout.Summary by CodeRabbit