test: add basic docker E2E which writes headers to DA network #2378
Conversation
WalkthroughThis update introduces a comprehensive Docker-based end-to-end testing suite for blockchain components, specifically targeting Celestia and Rollkit chains. It adds new test infrastructure, updates the CI workflow to build multi-platform Docker images and run Docker E2E tests, and enhances test scripts to support directory exclusions and a new Docker E2E test target. Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI Workflow
participant Docker as Docker Build
participant Test as Docker E2E Test Job
participant Repo as Repository
CI->>Repo: Checkout code
CI->>Docker: Build and push multi-platform Docker image
Docker-->>CI: Docker image tag
CI->>Test: Run Docker E2E tests (make test-docker-e2e)
Test->>Docker: Start Celestia and Rollkit nodes in containers
Test->>Test: Orchestrate node startup, funding, and checks
Test-->>CI: Test results
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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).
|
| Repository: "ghcr.io/celestiaorg/celestia-node", | ||
| Version: "pr-4283", |
There was a problem hiding this comment.
has a fix which enables the signer type access wallets funded after initialization
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2378 +/- ##
=======================================
Coverage 71.69% 71.69%
=======================================
Files 64 64
Lines 6306 6306
=======================================
Hits 4521 4521
Misses 1383 1383
Partials 402 402
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:
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
.gitignore (1)
7-7: Addition is fine; consider whether you also want to ignorevendor/**Plain
vendoris usually sufficient, but some tooling (especially on Windows) can create nested vendor directories.
If you want to be fully safe, add a glob:vendor +vendor/**scripts/test_cover.go (1)
44-47: Formatting glitch in log stringThe newline is inside the format string; the explanatory text appears on the next line with no prefix:
- fmt.Printf("--> Skipping tests in: %s\n as they are marked as excluded", modDir) + fmt.Printf("--> Skipping tests in: %s as it is in the exclude list\n", modDir)scripts/test.mk (1)
41-45: Declare new target as.PHONYWithout the
.PHONYdeclaration, a file namedtest-docker-e2ein the repo would prevent the rule from running.@cd test/docker-e2e && go test -mod=readonly -failfast -timeout=30m ./... +.PHONY: test-docker-e2etest/docker-e2e/base_test.go (1)
38-40: Use a context with timeout to prevent hanging CIA stuck Docker container can block CI for 30 min. Wrap the root context:
ctx, cancel := context.WithTimeout(context.Background(), 25*time.Minute) defer cancel().github/workflows/test.yml (1)
39-53: Add GHCR login (optional)
The Docker E2E tests willdocker pullthe freshly-pushed image from GHCR. Pulling anonymously can run into rate-limits; logging in (same as the build job) avoids that and keeps CI deterministic.- name: Log in to GHCR uses: docker/login-action@v3 with: registry: ghcr.io username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }}Place this before the
make test-docker-e2estep.test/docker-e2e/docker_test.go (2)
210-228: Consider context timeouts when starting Rollkit nodes
rollkitNode.InitandrollkitNode.Startare called with the parent context; if the test hangs these calls can block indefinitely and stall CI. Wrapping the calls incontext.WithTimeout(e.g. 5 min) makes failures surface earlier and releases runners.
232-248: Expose image repo via CI to avoid hard-coding
getRollkitImagefalls back toghcr.io/rollkit/rollkit. If the repo ever changes (e.g. forks, private images) the tests break silently. PassROLLKIT_IMAGE_REPOfrom the workflow (same place you setROLLKIT_IMAGE_TAG) so the value is always explicit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
test/docker-e2e/go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
.github/workflows/test.yml(1 hunks).gitignore(1 hunks)scripts/test.mk(1 hunks)scripts/test_cover.go(2 hunks)test/docker-e2e/base_test.go(1 hunks)test/docker-e2e/docker_test.go(1 hunks)test/docker-e2e/go.mod(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (3)
scripts/test_cover.go (1)
13-15: Importing standard‐libraryslicesbumps the minimum Go version to 1.21That’s OK, but please ensure
go.modin the root module declaresgo 1.21(or higher) so the build will not fail on older builders.
If you need to stay < 1.21 you can fall back togolang.org/x/exp/slices.test/docker-e2e/go.mod (1)
3-3:go 1.24.2is not released yetThe highest released Go version is currently 1.22.x.
Using a future version will break all builders and CI runners that fetch toolchains automatically.Change to the latest stable (e.g.
go 1.22) unless you have a private toolchain.-go 1.24.2 +go 1.22Likely an incorrect or invalid review comment.
.github/workflows/test.yml (1)
36-36: Multi-platform build looks good – building for both amd64 + arm64 broadens compatibility without extra complexity.
scripts/test_cover.go
Outdated
| excludeDirs := []string{"test/docker-e2e"} | ||
|
|
There was a problem hiding this comment.
Hard-coded path separators break on Windows
excludeDirs is compared with modDir, which on Windows will contain back-slashes (\).
Use filepath.Clean + filepath.ToSlash (or the reverse) on both sides before comparison:
- excludeDirs := []string{"test/docker-e2e"}
+ excludeDirs := []string{filepath.ToSlash("test/docker-e2e")}and
- if slices.Contains(excludeDirs, modDir) {
+ if slices.Contains(excludeDirs, filepath.ToSlash(modDir)) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| excludeDirs := []string{"test/docker-e2e"} | |
| // Normalize the hard-coded path for excludeDirs | |
| excludeDirs := []string{filepath.ToSlash("test/docker-e2e")} | |
| // … later, when checking modDir against excludeDirs, normalize it too | |
| if slices.Contains(excludeDirs, filepath.ToSlash(modDir)) { | |
| // … | |
| } |
🤖 Prompt for AI Agents
In scripts/test_cover.go around lines 20 to 21, the excludeDirs slice contains
hard-coded path separators that will break on Windows because modDir uses
backslashes. To fix this, normalize both excludeDirs entries and modDir by
applying filepath.Clean followed by filepath.ToSlash before comparing them,
ensuring consistent path formats across platforms.
Overview
This PR adds a basic test which
Summary by CodeRabbit
New Features
Chores
Tests