chore(cassandra): use Run function #3321
Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary by CodeRabbit
WalkthroughReplaces manual ContainerRequest + GenericContainer usage in the Cassandra module with testcontainers.Run and option builders; uses string port "9042/tcp" instead of nat.Port, applies WithExposedPorts/WithEnv/WithWaitStrategy via a moduleOpts slice, updates error text, and moves a docker connections dependency to indirect in go.mod. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller as Caller
participant Module as Cassandra Module
participant RunAPI as testcontainers.Run
participant Engine as Container Engine
Caller->>Module: Run(ctx, image, userOpts...)
Module->>Module: Build moduleOpts (WithExposedPorts "9042/tcp", WithEnv, WithWaitStrategy, +userOpts)
Module->>RunAPI: Run(ctx, image, moduleOpts...)
RunAPI->>Engine: create & start container
Engine-->>RunAPI: container handle / error
RunAPI-->>Module: container (ctr) / error
Module-->>Caller: CassandraContainer (with Container=ctr) / error
note right of Module: Port as string "9042/tcp" and wait strategy via WithWaitStrategy
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
modules/cassandra/cassandra.go (1)
23-27: Fix comment: Cassandra native port is 9042, not 9000.Adjust the doc to avoid confusion for users reading the API.
-// ConnectionHost returns the host and port of the cassandra container, using the default, native 9000 port, and +// ConnectionHost returns the host and port of the Cassandra container, using the default native 9042 port, and
🧹 Nitpick comments (3)
modules/cassandra/cassandra.go (3)
33-41: YAML config shouldn’t be executable; use 0644 instead of 0755.No need to grant execute bits to cassandra.yaml.
- FileMode: 0o755, + FileMode: 0o644,
45-64: Avoid collisions when multiple scripts share the same basename.Copying all scripts to "/" risks overwriting if basenames repeat. Either prefix to make paths unique or fail fast on duplicates.
func WithInitScripts(scripts ...string) testcontainers.CustomizeRequestOption { return func(req *testcontainers.GenericContainerRequest) error { - var initScripts []testcontainers.ContainerFile + var initScripts []testcontainers.ContainerFile var execs []testcontainers.Executable - for _, script := range scripts { + // prevent overwriting when basenames collide + seen := make(map[string]struct{}, len(scripts)) + for _, script := range scripts { + base := filepath.Base(script) + if _, ok := seen[base]; ok { + return fmt.Errorf("duplicate init script basename %q; please rename or ensure unique basenames", base) + } + seen[base] = struct{}{} - cf := testcontainers.ContainerFile{ - HostFilePath: script, - ContainerFilePath: "/" + filepath.Base(script), + cf := testcontainers.ContainerFile{ + HostFilePath: script, + ContainerFilePath: "/" + base, FileMode: 0o755, } initScripts = append(initScripts, cf) execs = append(execs, initScript{File: cf.ContainerFilePath}) } req.Files = append(req.Files, initScripts...) return testcontainers.WithAfterReadyCommand(execs...)(req) } }
84-91: Increase wait deadline; Cassandra often takes >60s to bootstrap.testcontainers.WithWaitStrategy defaults to 60s. Cassandra initialization can exceed that, causing flaky tests. Recommend using a longer deadline.
+import ( + // ... + "time" +) @@ - testcontainers.WithWaitStrategy(wait.ForAll( + testcontainers.WithWaitStrategyAndDeadline(3*time.Minute, wait.ForAll( wait.ForListeningPort(port), wait.ForExec([]string{"cqlsh", "-e", "SELECT bootstrapped FROM system.local"}).WithResponseMatcher(func(body io.Reader) bool { data, _ := io.ReadAll(body) return strings.Contains(string(data), "COMPLETED") }), - )), + )),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
modules/cassandra/cassandra.go(2 hunks)modules/cassandra/go.mod(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-29T15:08:18.668Z
Learnt from: mdelapenya
PR: testcontainers/testcontainers-go#3320
File: modules/artemis/artemis.go:98-103
Timestamp: 2025-09-29T15:08:18.668Z
Learning: In testcontainers-go, nat.Port is a type alias for string, so untyped string constants can be passed directly to functions expecting nat.Port (like wait.ForListeningPort) without explicit type conversion - the Go compiler handles the implicit conversion automatically.
Applied to files:
modules/cassandra/cassandra.go
📚 Learning: 2025-09-18T08:24:27.479Z
Learnt from: mdelapenya
PR: testcontainers/testcontainers-go#3254
File: .github/dependabot.yml:21-21
Timestamp: 2025-09-18T08:24:27.479Z
Learning: In the testcontainers-go repository, submodules like atlaslocal that are part of a parent module (e.g., mongodb) share the same go.mod file and should not have separate Dependabot entries. They are already monitored through the parent module's Dependabot configuration entry.
Applied to files:
modules/cassandra/go.mod
🧬 Code graph analysis (1)
modules/cassandra/cassandra.go (5)
options.go (4)
ContainerCustomizer(22-24)WithExposedPorts(464-469)WithEnv(75-85)WithWaitStrategy(376-378)wait/all.go (1)
ForAll(44-48)wait/host_port.go (1)
ForListeningPort(67-69)wait/exec.go (1)
ForExec(71-73)modules/azure/azurite/azurite.go (2)
Run(74-119)Container(31-34)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint (modules/cassandra) / lint: modules/cassandra
- GitHub Check: Analyze (go)
🔇 Additional comments (3)
modules/cassandra/cassandra.go (2)
15-16: Port constant as string is fine here.Passing a string to wait.ForListeningPort works because nat.Port is a type alias for string. No need for explicit conversion.
Based on learnings
74-76: Migration to Run with composed options looks solid.Cleaner option composition, opts appended in the right order, error wrapping is explicit, and container wrapping is safe.
Please ensure no behavior regressions in CI by running a Cassandra smoke test that exercises:
- WithConfigFile + init scripts
- Custom image override via Run(ctx, img, ...)
Also applies to: 93-106
modules/cassandra/go.mod (1)
26-26: Approve dependency scope change; no direct imports detected.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
modules/cassandra/cassandra.go (3)
29-43: YAML shouldn’t be executable; fix mode and align comment with behavior
- Set cassandra.yaml to 0644 instead of 0755.
- The comment says this also sets a "configFile" CLI parameter, but the code only copies the file. Update the comment for accuracy.
-// It will also set the "configFile" parameter to the path of the config file -// as a command line argument to the container. +// It copies the provided YAML into /etc/cassandra/cassandra.yaml inside the container. @@ - FileMode: 0o755, + FileMode: 0o644,
74-84: Env setup is fine; consider avoiding duplicate snitch vars (optional)You set both CASSANDRA_SNITCH and CASSANDRA_ENDPOINT_SNITCH to the same value. If not required for backward compatibility, drop one to reduce ambiguity (I’d keep CASSANDRA_ENDPOINT_SNITCH).
- "CASSANDRA_SNITCH": "GossipingPropertyFileSnitch",If older images in your matrix depend on CASSANDRA_SNITCH, keep both. Please confirm test coverage across images.
84-91: Increase wait deadline to prevent flakes; minor matcher hardeningDefault WithWaitStrategy uses a 60s deadline, which is tight for Cassandra bootstrap. Bump to a more forgiving deadline (e.g., 3m). Also, make the matcher resilient to read errors.
- testcontainers.WithWaitStrategy(wait.ForAll( + testcontainers.WithWaitStrategyAndDeadline(3*time.Minute, wait.ForAll( wait.ForListeningPort(port), wait.ForExec([]string{"cqlsh", "-e", "SELECT bootstrapped FROM system.local"}).WithResponseMatcher(func(body io.Reader) bool { - data, _ := io.ReadAll(body) - return strings.Contains(string(data), "COMPLETED") + data, err := io.ReadAll(body) + return err == nil && strings.Contains(string(data), "COMPLETED") }), - )), + )),Add missing import:
import ( "context" "fmt" "io" + "time" "path/filepath" "strings"I can parameterize the deadline via an option if you want this configurable per test.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
modules/cassandra/cassandra.go(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: mdelapenya
PR: testcontainers/testcontainers-go#3319
File: modules/arangodb/arangodb.go:46-57
Timestamp: 2025-09-29T13:57:14.629Z
Learning: In testcontainers-go ArangoDB module, the wait strategy combines port listening check with HTTP readiness check using wait.ForAll - both strategies are required and complementary, not redundant.
📚 Learning: 2025-09-29T15:08:18.668Z
Learnt from: mdelapenya
PR: testcontainers/testcontainers-go#3320
File: modules/artemis/artemis.go:98-103
Timestamp: 2025-09-29T15:08:18.668Z
Learning: In testcontainers-go, nat.Port is a type alias for string, so untyped string constants can be passed directly to functions expecting nat.Port (like wait.ForListeningPort) without explicit type conversion - the Go compiler handles the implicit conversion automatically.
Applied to files:
modules/cassandra/cassandra.go
🧬 Code graph analysis (1)
modules/cassandra/cassandra.go (4)
modules/azure/azurite/azurite.go (2)
Container(31-34)Run(74-119)options.go (4)
ContainerCustomizer(22-24)WithExposedPorts(464-469)WithEnv(75-85)WithWaitStrategy(376-378)wait/host_port.go (1)
ForListeningPort(67-69)wait/exec.go (1)
ForExec(71-73)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test (1.25.x, modules/cassandra) / test: modules/cassandra/1.25.x
- GitHub Check: test (1.24.x, modules/cassandra) / test: modules/cassandra/1.24.x
- GitHub Check: Analyze (go)
🔇 Additional comments (4)
modules/cassandra/cassandra.go (4)
15-15: Port as string is correct and idiomaticUsing a string constant for the port works with both WithExposedPorts and wait.ForListeningPort because nat.Port is a type alias for string. No explicit conversion needed. Based on learnings
23-27: Doc tweak looks goodDocstring now reflects the native 9042 port and ConnectionHost correctly delegates to PortEndpoint.
93-93: Good: user opts applied after defaultsAppending opts after module defaults preserves sensible defaults while allowing overrides.
95-103: Run pattern and error wrapping look solidWrapping the returned container even on error mirrors other modules and helps with cleanup; error context “run cassandra” is clear.
1 participant
What does this PR do?
Use Run function in the Cassandra module
Why is it important?
Migrate modules to the new API
Related issues