chore: use Run function (part 2) #3305
Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary by CodeRabbit
WalkthroughMigrates tests and docs from GenericContainer/ContainerRequest to a new Run(ctx, image, opts...) functional-options API, introduces WithReuseByName, updates log consumer configuration to WithLogConsumerConfig, and refactors code to build option slices prior to calling Run. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer / Test
participant RunAPI as Run(ctx, image, opts...)
participant Opts as Option Builders
participant Engine as Container Engine
Dev->>RunAPI: Run(ctx, image, WithExposedPorts..., WithWaitStrategy..., WithReuseByName?)
RunAPI->>Opts: Apply and compose options
Note over RunAPI,Opts: Build final container configuration
alt WithReuseByName provided
RunAPI->>Engine: Lookup container by name
alt Found
Engine-->>RunAPI: Existing container handle
RunAPI-->>Dev: Return reused container
else Not found
RunAPI->>Engine: Create & start new container
Engine-->>RunAPI: New container handle
RunAPI-->>Dev: Return new container
end
else No reuse option
RunAPI->>Engine: Create & start new container
Engine-->>RunAPI: New container handle
RunAPI-->>Dev: Return new container
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
port_forwarding.go (1)
95-99: Guard TerminateContainer when sshdContainer is nil on error pathIf newSshdContainer fails before assigning sshdContainer, passing a typed-nil to an interface can still be non-nil and cause a panic when Terminate is invoked. Add a nil check.
Apply this diff:
defer func() { - if err != nil { - err = errors.Join(err, TerminateContainer(sshdContainer)) - } + if err != nil && sshdContainer != nil { + err = errors.Join(err, TerminateContainer(sshdContainer)) + } }()
🧹 Nitpick comments (3)
docs/features/follow_logs.md (3)
29-30: Clarify Run-based usage in the preferred approach sentenceSuggest minor wording to explicitly reference Run(...) and tighten phrasing.
-This will represent the current way for associating `LogConsumer`s. You simply define your consumers, and attach them as a slice using the `WithLogConsumerConfig` functional option. +This is the preferred way to associate `LogConsumer`s when using `Run(...)`. Define your consumers and pass them via the `WithLogConsumerConfig` option.
79-79: Fix typo: StartLogProducer“StarLogProducer” → “StartLogProducer”.
-c.FollowOutput(&g) // must be called before StarLogProducer +c.FollowOutput(&g) // must be called before StartLogProducer
131-133: Align StartLogProducer usage with option patternElsewhere you document LogProductionOption and WithLogProductionTimeout; mirror that here.
- startErr := container.StartLogProducer(ctx, timeout) + startErr := container.StartLogProducer(ctx, WithLogProductionTimeout(timeout))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
docs/features/creating_container.md(3 hunks)docs/features/follow_logs.md(1 hunks)logconsumer_test.go(13 hunks)port_forwarding.go(1 hunks)reaper_test.go(3 hunks)reuse_test.go(4 hunks)wait/exec_test.go(2 hunks)wait/file_test.go(1 hunks)wait/http_test.go(9 hunks)wait/tls_test.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (10)
docs/features/follow_logs.md (4)
options.go (1)
WithLogConsumerConfig(280-285)docker.go (2)
c(444-450)c(428-433)options_test.go (2)
TestWithLogConsumerConfig(103-138)TestWithLogConsumers(80-101)modules/pulsar/pulsar.go (1)
Container(33-36)
reaper_test.go (2)
generic.go (1)
Run(122-149)options.go (1)
WithExposedPorts(464-469)
wait/tls_test.go (4)
generic.go (1)
Run(122-149)options.go (2)
WithDockerfile(47-53)WithWaitStrategy(376-378)container.go (2)
FromDockerfile(90-108)FromDockerfile(131-171)docker_auth_test.go (1)
TestBuildContainerFromDockerfile(160-173)
wait/http_test.go (5)
generic.go (1)
Run(122-149)options.go (4)
WithExposedPorts(464-469)WithWaitStrategy(376-378)WithDockerfile(47-53)ContainerCustomizer(22-24)wait/http.go (1)
ForHTTP(149-151)container.go (1)
FromDockerfile(90-108)options_test.go (1)
TestWithDockerfile(656-677)
reuse_test.go (3)
options.go (3)
ContainerCustomizer(22-24)WithExposedPorts(464-469)WithReuseByName(129-138)options_test.go (1)
TestWithReuseByName_Succeeds(736-747)modules/ollama/ollama.go (1)
RunContainer(71-73)
logconsumer_test.go (4)
options.go (11)
ContainerCustomizer(22-24)WithDockerfile(47-53)WithExposedPorts(464-469)WithWaitStrategy(376-378)WithLogConsumerConfig(280-285)WithEnv(75-85)WithHostConfigModifier(88-94)WithCmd(472-477)WithConfigModifier(56-62)WithAlwaysPull(432-437)WithNoStart(120-125)container.go (1)
FromDockerfile(90-108)generic.go (1)
Run(122-149)docker.go (2)
LogProductionOption(747-747)WithLogProductionTimeout(751-755)
wait/exec_test.go (3)
generic.go (1)
Run(122-149)options.go (2)
WithEntrypoint(448-453)WithWaitStrategy(376-378)wait/exec.go (1)
ForExec(71-73)
docs/features/creating_container.md (2)
options_test.go (2)
TestWithReuseByName_ErrorsWithoutContainerNameProvided(749-759)TestWithReuseByName_Succeeds(736-747)options.go (1)
WithReuseByName(129-138)
port_forwarding.go (3)
options.go (4)
ContainerCustomizer(22-24)WithExposedPorts(464-469)WithEnv(75-85)WithWaitStrategy(376-378)wait/host_port.go (1)
ForListeningPort(67-69)generic.go (1)
Run(122-149)
wait/file_test.go (2)
generic.go (1)
Run(122-149)testing.go (1)
CleanupContainer(91-97)
🪛 markdownlint-cli2 (0.18.1)
docs/features/creating_container.md
224-224: Hard tabs
Column: 1
(MD010, no-hard-tabs)
225-225: Hard tabs
Column: 1
(MD010, no-hard-tabs)
226-226: Hard tabs
Column: 1
(MD010, no-hard-tabs)
227-227: Hard tabs
Column: 1
(MD010, no-hard-tabs)
228-228: Hard tabs
Column: 1
(MD010, no-hard-tabs)
247-247: Hard tabs
Column: 1
(MD010, no-hard-tabs)
248-248: Hard tabs
Column: 1
(MD010, no-hard-tabs)
249-249: Hard tabs
Column: 1
(MD010, no-hard-tabs)
250-250: Hard tabs
Column: 1
(MD010, no-hard-tabs)
251-251: Hard tabs
Column: 1
(MD010, no-hard-tabs)
⏰ 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). (18)
- GitHub Check: lint (modules/weaviate) / lint: modules/weaviate
- GitHub Check: lint (modules/openldap) / lint: modules/openldap
- GitHub Check: lint (modules/scylladb) / lint: modules/scylladb
- GitHub Check: lint (modules/dind) / lint: modules/dind
- GitHub Check: lint (modules/dockermcpgateway) / lint: modules/dockermcpgateway
- GitHub Check: lint (modules/kafka) / lint: modules/kafka
- GitHub Check: lint (modules/ollama) / lint: modules/ollama
- GitHub Check: lint (modules/cassandra) / lint: modules/cassandra
- GitHub Check: lint (modules/milvus) / lint: modules/milvus
- GitHub Check: lint (modules/pinecone) / lint: modules/pinecone
- GitHub Check: lint (modules/gcloud) / lint: modules/gcloud
- GitHub Check: lint (modules/memcached) / lint: modules/memcached
- GitHub Check: lint (modules/vault) / lint: modules/vault
- GitHub Check: lint (examples/nginx) / lint: examples/nginx
- GitHub Check: lint (modules/mockserver) / lint: modules/mockserver
- GitHub Check: lint (modules/grafana-lgtm) / lint: modules/grafana-lgtm
- GitHub Check: lint (modules/solace) / lint: modules/solace
- GitHub Check: Analyze (go)
🔇 Additional comments (13)
wait/tls_test.go (2)
11-11: LGTM: required import addedImporting filepath is necessary for the new docker build context join.
111-116: LGTM: Run + WithDockerfile + WithWaitStrategy migrationThe example correctly uses Run with a FromDockerfile context and applies the TLS wait strategy.
Please verify that wait/testdata/http contains a valid Dockerfile so the example build context resolves at runtime.
wait/file_test.go (1)
81-97: LGTM: Run-based setup with FileStrategyClean migration to Run with a chained FileStrategy and matcher; CleanupContainer usage matches project pattern.
reaper_test.go (1)
110-113: LGTM: Use Run(...) with exposed ports in reaper testsSwitching to Run keeps semantics (started container) and simplifies setup across start/stop/terminate helpers.
Also applies to: 165-168, 188-191
wait/exec_test.go (2)
24-28: LGTM: Example uses Run + Entrypoint + Exec wait strategyConcise, correct example demonstrating ExecStrategy with startup timeout.
175-189: LGTM: Test uses Run with Exec wait strategy and custom matchersThe exit code and response matchers are correctly wired via WithWaitStrategy.
port_forwarding.go (1)
179-186: Migration to Run + options looks solidGood use of WithExposedPorts, WithEnv, and WithWaitStrategy and then delegating to Run. Error wrapping message is clear.
Also applies to: 187-195
reuse_test.go (2)
17-23: Run + WithReuseByName adoption LGTMSwitching to Run with WithExposedPorts and WithReuseByName is consistent with the new API and keeps the test intent intact.
60-63: Double-cleanup of the same reused container can be harmless, but verify idempotencyIf the second Run returns a handle to the same container, you register CleanupContainer twice. Ensure CleanupContainer/TerminateContainer are idempotent to avoid noisy errors at test teardown.
logconsumer_test.go (2)
79-92: Run + WithDockerfile + WithLogConsumerConfig migration looks correctOption set mirrors previous request fields and keeps behavior. Using filepath.Join for testdata is a good portability improvement.
238-246: dind setup via Run is correctPrivileged host config, DOCKER_TLS_CERTDIR unset, port exposure, and wait-for-port are properly captured with options.
wait/http_test.go (2)
34-37: Example migration to Run is accurateUsing WithExposedPorts and WithWaitStrategy maintains previous readiness semantics.
80-96: Option-based build-and-run flow LGTMSwitch to WithDockerfile and complex WithWaitStrategy chain reads cleanly and matches prior behavior.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/features/creating_container.md(3 hunks)
⏰ 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). (20)
- GitHub Check: lint (modules/kafka) / lint: modules/kafka
- GitHub Check: lint (modules/surrealdb) / lint: modules/surrealdb
- GitHub Check: lint (modules/redis) / lint: modules/redis
- GitHub Check: lint (modules/aerospike) / lint: modules/aerospike
- GitHub Check: lint (modules/azurite) / lint: modules/azurite
- GitHub Check: lint (modules/mysql) / lint: modules/mysql
- GitHub Check: lint (modules/scylladb) / lint: modules/scylladb
- GitHub Check: lint (modules/ollama) / lint: modules/ollama
- GitHub Check: lint (modules/localstack) / lint: modules/localstack
- GitHub Check: lint (modules/openldap) / lint: modules/openldap
- GitHub Check: lint (examples/nginx) / lint: examples/nginx
- GitHub Check: lint (modules/chroma) / lint: modules/chroma
- GitHub Check: lint (modules/qdrant) / lint: modules/qdrant
- GitHub Check: lint (modules/dynamodb) / lint: modules/dynamodb
- GitHub Check: lint (modules/databend) / lint: modules/databend
- GitHub Check: lint (modules/grafana-lgtm) / lint: modules/grafana-lgtm
- GitHub Check: lint (modules/mariadb) / lint: modules/mariadb
- GitHub Check: lint (modules/openfga) / lint: modules/openfga
- GitHub Check: lint (modules/gcloud) / lint: modules/gcloud
- GitHub Check: Analyze (go)
1 participant
What does this PR do?
This PR uses the Run function instead of GenericContainer in a reduced list of places:
Why is it important?
Continue with the migration from GenericContainer to Run
Related issues