fix: Optimize E2E EVM tests #2421
Conversation
WalkthroughThis update reduces timeouts and polling intervals in E2E test code and Docker Compose healthchecks to accelerate test execution and service readiness checks. It also removes specific E2E sub-tests and sets logging environment variables for Reth-based services in Docker Compose files. Changes
Sequence Diagram(s)sequenceDiagram
participant Tester
participant DockerCompose
participant RethService
participant JwtInit
Tester->>DockerCompose: Start services (rollkit-reth, jwt-init)
DockerCompose->>JwtInit: Healthcheck (interval: 2s, timeout: 2s, retries: 2)
DockerCompose->>RethService: Set RUST_LOG=warn,lumen_rollkit=info
Tester->>RethService: Wait for readiness (shorter timeouts)
Tester->>RethService: Run E2E tests (with reduced polling/timeout)
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (3)`**/*_test.go`: Unit tests should be placed in `*_test.go` files alongside the code being tested.
📄 Source: CodeRabbit Inference Engine (CLAUDE.md) List of files the instruction was applied to:
`test/e2e/**/*_test.go`: End-to-end (E2E) tests should be placed in the `test/e2e/` directory.
📄 Source: CodeRabbit Inference Engine (CLAUDE.md) List of files the instruction was applied to:
`**/*.go`: Follow standard Go formatting (enforced by golangci-lint). Use meanin...
📄 Source: CodeRabbit Inference Engine (CLAUDE.md) List of files the instruction was applied to:
🧠 Learnings (2)📓 Common learningstest/e2e/evm_sequencer_e2e_test.go (8)⏰ Context from checks skipped due to timeout of 90000ms (8)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
|
The latest Buf updates on your PR. Results from workflow CI and Release / buf-check (pull_request).
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @Manav-Aggarwal, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly optimizes the E2E EVM test suite. The primary goal is to reduce the overall execution time of these tests by fine-tuning various timing parameters, streamlining container startup, and removing less critical test permutations. These changes are expected to make the test suite run much faster and more efficiently.
Highlights
- Test Suite Speedup: Reduced
DefaultDABlockTimeandDefaultTestTimeout, along with introducing new, shorter polling intervals and timeouts for various test operations (e.g., container readiness, P2P discovery, node startup). This aims to significantly decrease the overall execution time of the E2E tests. - Docker Container Optimization: Adjusted Docker Compose health check parameters for
jwt-generatorservices to speed up their readiness detection and enabledRUST_LOGenvironment variable to reduce log verbosity in sequencer containers, contributing to faster startup and less I/O overhead during tests. - Test Case Reduction: Removed two specific restart recovery test scenarios (
LazyToStandardRestartandLazyToLazyRestart) from both sequencer and full node E2E tests, directly contributing to faster test runs by reducing the total number of test permutations.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request optimizes E2E EVM tests by reducing timeouts, sleep durations, and log verbosity, and by removing some test cases. A function that was optimized for container readiness checks is not being used, making the optimization ineffective. Addressing this will ensure the intended performance gains are realized.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2421 +/- ##
==========================================
- Coverage 72.92% 72.82% -0.10%
==========================================
Files 67 67
Lines 6378 6378
==========================================
- Hits 4651 4645 -6
- Misses 1329 1335 +6
Partials 398 398
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tac0turtle
left a comment
tac0turtle
left a comment
There was a problem hiding this comment.
lets remove the comments added here
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/e2e/evm_sequencer_e2e_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
`**/*_test.go`: Unit tests should be placed in `*_test.go` files alongside the code being tested.
**/*_test.go: Unit tests should be placed in*_test.gofiles alongside the code being tested.
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
test/e2e/evm_sequencer_e2e_test.go
`test/e2e/**/*_test.go`: End-to-end (E2E) tests should be placed in the `test/e2e/` directory.
test/e2e/**/*_test.go: End-to-end (E2E) tests should be placed in thetest/e2e/directory.
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
test/e2e/evm_sequencer_e2e_test.go
`**/*.go`: Follow standard Go formatting (enforced by golangci-lint). Use meanin...
**/*.go: Follow standard Go formatting (enforced by golangci-lint).
Use meaningful variable names in Go code.
Keep functions small and focused in Go code.
Document exported types and functions in Go code.
Use context.Context for cancellation in Go code.
Wrap errors with context usingfmt.Errorfin Go code.
Return errors early in Go code.
Use custom error types for domain-specific errors in Go code.
Use structured logging and include relevant context in log messages in Go code.
Use appropriate log levels in Go code.
Never expose private keys in logs or errors.
Validate all inputs from external sources.
Use secure random number generation in Go code.
Be careful with concurrent access to shared state in Go code.
Be mindful of goroutine leaks in Go code.
Use buffered channels appropriately in Go code.
Profile before optimizing Go code.
Consider memory allocation in hot paths in Go code.
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
test/e2e/evm_sequencer_e2e_test.go
🧠 Learnings (2)
📓 Common learnings
Learnt from: tzdybal
PR: rollkit/rollkit#1641
File: node/full_node_test.go:266-321
Timestamp: 2024-10-08T18:35:32.960Z
Learning: Issue #1663 was created to address adding comprehensive tests for error scenarios in `TestVoteExtension` in the rollkit repository.
Learnt from: tzdybal
PR: rollkit/rollkit#1641
File: node/full_node_test.go:266-321
Timestamp: 2024-06-10T19:23:16.839Z
Learning: Issue #1663 was created to address adding comprehensive tests for error scenarios in `TestVoteExtension` in the rollkit repository.
test/e2e/evm_sequencer_e2e_test.go (6)
Learnt from: CR
PR: rollkit/rollkit#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T08:47:52.948Z
Learning: Applies to test/e2e/**/*_test.go : End-to-end (E2E) tests should be placed in the `test/e2e/` directory.
Learnt from: tzdybal
PR: rollkit/rollkit#1875
File: da/da_test.go:88-88
Timestamp: 2025-01-27T22:03:14.288Z
Learning: In the rollkit codebase, the `github.com/rollkit/go-da` package is imported with different aliases in test (`da`) and production code (`goDA`), but they refer to the same package. Suggestions to change between these aliases are unnecessary as they don't affect functionality.
Learnt from: Manav-Aggarwal
PR: rollkit/rollkit#2280
File: sequencers/based/sequencer_test.go:89-89
Timestamp: 2025-05-26T10:26:59.799Z
Learning: In DummyDA test cases, the height ticker (StartHeightTicker/StopHeightTicker) is not needed for tests that don't rely on time-based height progression, such as TestSequencer_GetNextBatch_ExceedsMaxDrift which only tests max drift behavior without requiring height advancement over time.
Learnt from: CR
PR: rollkit/rollkit#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T08:47:52.948Z
Learning: Update tests when changing functionality.
Learnt from: chatton
PR: rollkit/rollkit#2378
File: test/docker-e2e/base_test.go:22-32
Timestamp: 2025-06-18T12:52:25.919Z
Learning: In the rollkit codebase test/docker-e2e, the StartBridgeNode and StartRollkitNode methods in DockerTestSuite do not return errors. They handle error checking internally using s.Require().NoError(err) for their operations, so no external error handling is needed when calling these methods.
Learnt from: chatton
PR: rollkit/rollkit#2378
File: test/docker-e2e/base_test.go:22-32
Timestamp: 2025-06-18T12:52:25.919Z
Learning: In the rollkit codebase test/docker-e2e, the StartBridgeNode and StartRollkitNode methods in DockerTestSuite do not return errors, so no error handling is needed when calling these methods.
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: lint / markdown-lint
- GitHub Check: lint / golangci-lint
- GitHub Check: test / Build Rollkit EVM Single Docker Image
- GitHub Check: test / Run Unit Tests
- GitHub Check: test / Build All Rollkit Binaries
- GitHub Check: test / Run Integration Tests
- GitHub Check: test / Build Rollkit Docker Image
- GitHub Check: Analyze (go)
- GitHub Check: Summary
| // - LazyModeRestart: Normal start -> Lazy restart | ||
| // - LazyToStandardRestart: Lazy start -> Normal restart | ||
| // - LazyToLazyRestart: Lazy start -> Lazy restart | ||
| // 3. TestEvmInvalidTransactionRejectionE2E - Various invalid transaction type rejections |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Comment updated correctly but raises test coverage concerns.
The comment numbering update correctly reflects the removal of test functions. However, removing tests for double-spend nonce handling and sequencer restart recovery significantly reduces coverage for critical functionality.
Consider these alternatives to maintain coverage while optimizing performance:
- Move critical tests to a separate suite that runs less frequently but still validates security-critical scenarios
- Combine test scenarios into fewer, more comprehensive test cases
- Use test build tags to create fast vs comprehensive test suites
Let me verify what specific functionality was removed:
Recommendation: Ensure the removed functionality (double-spend prevention and restart recovery) is covered elsewhere in the test suite, as these are security and reliability critical features.
🏁 Script executed:
#!/bin/bash
# Description: Search for any remaining references to the removed test functions
# Expected: No references should remain if tests were completely removed
echo "Searching for references to removed test functions..."
rg -i "TestEvmDoubleSpendNonceHandling|TestEvmSequencerRestartRecovery"
echo -e "\nSearching for nonce-related test coverage..."
rg -A 3 -B 3 "nonce.*test|double.*spend" test/e2e/
echo -e "\nSearching for restart-related test coverage..."
rg -A 3 -B 3 "restart.*test|recovery.*test" test/e2e/Length of output: 5951
Update stale E2E test references and restore critical nonce coverage
The renumbering in test/e2e/evm_sequencer_e2e_test.go is correct, but we still have:
test/e2e/evm_full_node_e2e_test.gocomments referencing the removedTestEvmSequencerRestartRecoveryE2E.- No remaining dedicated coverage for double-spend nonce handling after
TestEvmDoubleSpendNonceHandlingE2Ewas removed.
Please address:
- In
test/e2e/evm_full_node_e2e_test.go, replace or remove all mentions ofTestEvmSequencerRestartRecoveryE2Eso they align with the newTestEvmSequencerFullNodeRestartE2Enaming. - Reintroduce the
TestEvmDoubleSpendNonceHandlingE2Escenario—either by:
• Moving it into a separate “slow” or “full” suite,
• Merging its logic into an existing high-throughput test, or
• Tagging it behind a build flag (e.g.// +build full_e2e) so it runs less frequently but still guards against replay/nonce bugs. - (Optional) Group all security-critical tests into a dedicated suite with build tags to balance performance and coverage.
🤖 Prompt for AI Agents
In test/e2e/evm_sequencer_e2e_test.go at line 15, the comment is fine but the
issue spans multiple files. Update test/e2e/evm_full_node_e2e_test.go to replace
all references of the removed TestEvmSequencerRestartRecoveryE2E with
TestEvmSequencerFullNodeRestartE2E to keep naming consistent. Reintroduce the
TestEvmDoubleSpendNonceHandlingE2E test by either moving it to a slower or full
test suite, merging its logic into an existing test, or gating it behind a build
flag like // +build full_e2e to maintain nonce handling coverage without
impacting regular test speed. Optionally, group security-critical tests into a
dedicated suite with build tags for better test management.
* main: feat: fix execution verification (#2433) build(deps): Bump github.com/spf13/viper from 1.19.0 to 1.20.1 (#2431) build(deps): Bump github.com/libp2p/go-libp2p-kad-dht from 0.29.1 to 0.33.1 (#2432) build(deps): Bump github.com/prometheus/client_golang from 1.21.1 to 1.22.0 (#2430) build(deps): Bump alpine from 3.18.3 to 3.22.0 (#2429) ci: fix dependabot patterns (#2427) feat: migrate logging to ipfs/go-log/v2 (#2416) fix: Consolidate some EVM E2E tests into one (#2423) feat: Support GetHeader in store (#2422) fix: Optimize E2E EVM tests (#2421) fix: getPeers size check (#2417) test: Add comprehensive EVM E2E tests (#2394) feat: migrate logging to ipfs/go-log/v2 (#2411) feat: execution verification (#2377)
Overview
Optimizes E2E EVM tests time by 39%.
8.5 minutes -> 5.2 minutes.
Summary by CodeRabbit
Chores
Tests