chore(clickhouse|k6|localstack|redpanda|registry|socat): use Run in tests #3432
Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Rate limit exceeded@mdelapenya has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 43 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughReplaces testcontainers.GenericContainer/ContainerRequest calls across module tests with testcontainers.Run and functional options, updating ports, wait strategies, networking, env, entrypoint/cmd and Dockerfile builds; test control flow, assertions, and cleanup remain unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant T as Test
participant Run as testcontainers.Run
participant Docker as Docker Engine
participant Wait as WaitStrategy
T->>Run: Run(ctx, image, options...)
Run->>Docker: Create & start container (ports, env, entrypoint/cmd, network)
Run->>Wait: Apply wait strategy (log/port/exec)
Wait-->>Run: Ready / error
Run-->>T: Container handle or error
T->>Run: Cleanup/Terminate (defer)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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. 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
♻️ Duplicate comments (4)
modules/socat/examples_test.go (1)
102-105: Same suggestion as earlier block: add wait for helloworld readinessmodules/socat/socat_test.go (3)
74-77: Same readiness wait suggestion as earlier block
121-124: Same readiness wait suggestion as earlier block
149-152: Same readiness wait suggestion as earlier block
🧹 Nitpick comments (10)
modules/k6/examples_test.go (1)
20-20: Use explicit protocol in exposed port for consistencyPrefer "80/tcp" to avoid ambiguity and match repo conventions.
- httpbin, err := testcontainers.Run(ctx, "kennethreitz/httpbin", testcontainers.WithExposedPorts("80"), testcontainers.WithWaitStrategy(wait.ForExposedPort())) + httpbin, err := testcontainers.Run(ctx, "kennethreitz/httpbin", testcontainers.WithExposedPorts("80/tcp"), testcontainers.WithWaitStrategy(wait.ForExposedPort()))modules/clickhouse/clickhouse_test.go (1)
210-210: Expose port with protocol to match wait strategyUse zkPort.String() ("2181/tcp") instead of only the numeric port.
- zkcontainer, err := testcontainers.Run(ctx, "zookeeper:3.7", testcontainers.WithExposedPorts(zkPort.Port()), testcontainers.WithWaitStrategy(wait.ForListeningPort(zkPort))) + zkcontainer, err := testcontainers.Run(ctx, "zookeeper:3.7", testcontainers.WithExposedPorts(zkPort.String()), testcontainers.WithWaitStrategy(wait.ForListeningPort(zkPort)))modules/socat/examples_test.go (1)
29-32: Add readiness wait for helloworld to reduce flakinessRecommend waiting for HTTP readiness on 8080 before proxying via socat.
- ctr, err := testcontainers.Run(ctx, "testcontainers/helloworld:1.2.0", - testcontainers.WithExposedPorts("8080/tcp"), - network.WithNetwork([]string{"helloworld"}, nw), - ) + ctr, err := testcontainers.Run(ctx, "testcontainers/helloworld:1.2.0", + testcontainers.WithExposedPorts("8080/tcp"), + testcontainers.WithWaitStrategy(wait.ForHTTP("/ping").WithPort("8080/tcp")), + network.WithNetwork([]string{"helloworld"}, nw), + )Add import if not present:
"github.com/testcontainers/testcontainers-go/wait"modules/socat/socat_test.go (1)
33-36: Add readiness wait for helloworldWait for HTTP on 8080 to avoid races before socat proxies to it.
- ctr, err := testcontainers.Run(ctx, "testcontainers/helloworld:1.2.0", - network.WithNetwork([]string{"helloworld"}, nw), - testcontainers.WithExposedPorts("8080/tcp"), - ) + ctr, err := testcontainers.Run(ctx, "testcontainers/helloworld:1.2.0", + network.WithNetwork([]string{"helloworld"}, nw), + testcontainers.WithExposedPorts("8080/tcp"), + testcontainers.WithWaitStrategy(wait.ForHTTP("/ping").WithPort("8080/tcp")), + )Add import if not present:
"github.com/testcontainers/testcontainers-go/wait"modules/registry/registry_test.go (3)
99-110: Use existing ctx instead of context.Background()Keeps cancellation/timeout consistent across the test.
- redisC, err := testcontainers.Run( - context.Background(), "", + redisC, err := testcontainers.Run( + ctx, "",
124-136: Use existing ctx instead of context.Background()Same reasoning as above.
- redisC, err := testcontainers.Run( - context.Background(), "", + redisC, err := testcontainers.Run( + ctx, "",
255-265: Use existing ctx instead of context.Background()Same reasoning as above.
- redisC, err := testcontainers.Run(context.Background(), "", + redisC, err := testcontainers.Run(ctx, "",modules/registry/examples_test.go (3)
76-87: Reuse ctx instead of context.Background()This example already defines ctx; use it for consistency and cancellation.
- redisC, err := testcontainers.Run( - context.Background(), "", + redisC, err := testcontainers.Run( + ctx, "",
156-168: Reuse ctx instead of context.Background()Same as above.
- redisC, err := testcontainers.Run(context.Background(), "", + redisC, err := testcontainers.Run(ctx, "",
245-248: Reuse ctx instead of context.Background()Same as above.
- newRedisC, err := testcontainers.Run(context.Background(), taggedImage, + newRedisC, err := testcontainers.Run(ctx, taggedImage,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
modules/clickhouse/clickhouse_test.go(1 hunks)modules/k6/examples_test.go(1 hunks)modules/localstack/localstack_test.go(1 hunks)modules/redpanda/redpanda_test.go(1 hunks)modules/registry/examples_test.go(3 hunks)modules/registry/registry_test.go(3 hunks)modules/socat/examples_test.go(2 hunks)modules/socat/socat_test.go(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
modules/clickhouse/clickhouse_test.go (2)
modules/clickhouse/clickhouse.go (1)
Run(69-123)options.go (2)
WithExposedPorts(454-459)WithWaitStrategy(366-368)
modules/socat/examples_test.go (3)
modules/dockermodelrunner/dockermodelrunner.go (1)
Run(28-71)modules/socat/socat.go (1)
Run(31-97)options.go (1)
WithExposedPorts(454-459)
modules/registry/registry_test.go (3)
options.go (4)
WithDockerfile(47-53)WithAlwaysPull(422-427)WithExposedPorts(454-459)WithWaitStrategy(366-368)container.go (1)
FromDockerfile(90-108)wait/log.go (1)
ForLog(118-120)
modules/k6/examples_test.go (3)
modules/k6/k6.go (1)
Run(159-177)options.go (2)
WithExposedPorts(454-459)WithWaitStrategy(366-368)wait/host_port.go (1)
ForExposedPort(73-75)
modules/registry/examples_test.go (4)
modules/registry/registry.go (1)
Run(275-306)options.go (4)
WithDockerfile(47-53)WithAlwaysPull(422-427)WithExposedPorts(454-459)WithWaitStrategy(366-368)container.go (1)
FromDockerfile(90-108)wait/log.go (1)
ForLog(118-120)
modules/socat/socat_test.go (2)
modules/socat/socat.go (1)
Run(31-97)options.go (1)
WithExposedPorts(454-459)
modules/localstack/localstack_test.go (5)
modules/localstack/localstack.go (2)
Run(66-103)WithNetwork(54-56)generic.go (1)
Run(122-149)network/network.go (1)
WithNetwork(139-141)options.go (4)
WithEntrypoint(438-443)WithCmd(462-467)WithEnv(75-85)WithWaitStrategy(366-368)wait/exec.go (1)
ForExec(71-73)
modules/redpanda/redpanda_test.go (2)
modules/localstack/localstack.go (1)
WithNetwork(54-56)options.go (2)
WithEntrypoint(438-443)WithCmd(462-467)
⏰ 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). (10)
- GitHub Check: test (1.25.x, modules/registry) / test: modules/registry/1.25.x
- GitHub Check: test (1.25.x, modules/socat) / test: modules/socat/1.25.x
- GitHub Check: test (1.24.x, modules/redpanda) / test: modules/redpanda/1.24.x
- GitHub Check: test (1.25.x, modules/localstack) / test: modules/localstack/1.25.x
- GitHub Check: test (1.25.x, modules/redpanda) / test: modules/redpanda/1.25.x
- GitHub Check: test (1.24.x, modules/socat) / test: modules/socat/1.24.x
- GitHub Check: test (1.24.x, modules/localstack) / test: modules/localstack/1.24.x
- GitHub Check: test (1.24.x, modules/registry) / test: modules/registry/1.24.x
- GitHub Check: test (1.24.x, modules/clickhouse) / test: modules/clickhouse/1.24.x
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
modules/localstack/localstack_test.go (1)
227-251: LGTM! Clean migration to the Run API.The migration correctly converts the AWS CLI container setup to use
testcontainers.Runwith functional options. All aspects are properly translated:
- Networking via
network.WithNetwork([]string{"cli"}, nw)- Entrypoint and command via
testcontainers.WithEntrypointandtestcontainers.WithCmd- Environment variables via
testcontainers.WithEnv- Wait strategy via
testcontainers.WithWaitStrategy(wait.ForExec(...))The wait strategy correctly uses the "localstack" network alias (configured on line 220) in the endpoint URL, ensuring proper inter-container communication. The exit code and response matchers are preserved, maintaining the same validation logic.
modules/redpanda/redpanda_test.go (1)
590-595: LGTM: concise Run usage for kcatClean switch to Run with entrypoint/cmd and network alias.
1 participant
What does this PR do?
It uses the Run function in the tests for the given modules.
Why is it important?
Migrate modules to the new API.
Related issues