feat: enable parallel e2e test execution with dynamic port allocation #137
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ulziibay-kernel
force-pushed
the
ulziibay-kernel/e2e-test-parallelization
branch
from
06773db to
ac54a21
Compare
ulziibay-kernel
force-pushed
the
ulziibay-kernel/e2e-test-parallelization
branch
from
ac54a21 to
da2c63f
Compare
Introduce TestContainer wrapper around testcontainers-go for managing Docker containers with dynamically allocated ports, eliminating port conflicts and enabling parallel test execution. Changes: - Add container.go using testcontainers-go for robust container management - Migrate all e2e tests to use TestContainer instead of global setup - Add t.Parallel() to test functions for concurrent execution - Remove -p 1 from Makefile now that port conflicts are resolved - Replace slog logging with t.Log/t.Logf for cleaner test output Benefits of testcontainers-go over custom implementation: - Docker SDK integration (vs shell commands) - Built-in wait strategies with timeouts - Automatic container cleanup via reaper - Battle-tested library used across the Go ecosystem
ulziibay-kernel
force-pushed
the
ulziibay-kernel/e2e-test-parallelization
branch
from
da2c63f to
f7417c8
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| } | ||
|
|
||
| for _, tc := range testCases { | ||
| tc := tc // capture range variable |
There was a problem hiding this comment.
Unnecessary loop variable capture in Go 1.22+
Low Severity
The tc := tc // capture range variable pattern is unnecessary in Go 1.22 and later. The go.mod specifies go 1.25.0, where loop variables are captured per-iteration by default. This obsolete idiom adds noise without providing any benefit.
| } | ||
| } | ||
|
|
||
| return exitCode, string(buf), nil |
There was a problem hiding this comment.
Unused Exec method is never called
Low Severity
The Exec() method on TestContainer is defined but never called anywhere in the codebase. Tests use execCombinedOutputWithClient instead which calls the API endpoint rather than using testcontainers' exec functionality. This unused method adds maintenance burden without providing value.
| // Container returns the underlying testcontainers.Container for advanced usage. | ||
| func (c *TestContainer) Container() testcontainers.Container { | ||
| return c.ctr | ||
| } |
There was a problem hiding this comment.
Unused Container method is never called
Low Severity
The Container() method that exposes the underlying testcontainers.Container is defined but never called anywhere in the codebase. This accessor for "advanced usage" has no current use cases and represents dead code.
|
|
||
| // Build container request options | ||
| opts := []testcontainers.ContainerCustomizer{ | ||
| testcontainers.WithImage(c.Image), |
There was a problem hiding this comment.
Image specified redundantly in both option and parameter
Low Severity
The container image is specified twice: once via testcontainers.WithImage(c.Image) in the options array (line 61) and again as a direct parameter to testcontainers.Run(ctx, c.Image, opts...) (line 85). This redundancy is confusing and could lead to bugs if one is changed but not the other.
Additional Locations (1)
Introduce TestContainer to manage Docker containers with dynamically allocated ports, eliminating port conflicts and enabling parallel test execution. This significantly speeds up CI runs on high-resource machines.
Changes:
Checklist
Note
Medium Risk
Primarily test harness and dependency changes, but it alters how Docker containers are launched/managed and enables parallel execution, which can introduce new CI flakiness and resource/cleanup issues.
Overview
Refactors the e2e suite to run in parallel by introducing a
TestContainerabstraction (based ontestcontainers-go) that starts per-test Docker containers with dynamically mapped API/CDP ports, plus helpers for readiness checks and no-keepalive API clients.Updates the existing e2e tests/benchmarks to use this container wrapper (and
t.Parallel()where applicable), replacing ad-hocdocker run/fixed-port assumptions and switching most logging fromslogtot.Log/b.Log. TheMakefiledropsgo test -p 1and Go module deps are updated to include Docker/testcontainers libraries to support this.Written by Cursor Bugbot for commit f7417c8. This will update automatically on new commits. Configure here.