chore: use the Run funcion in tests and docs (part 1) #3304
Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary by CodeRabbit
WalkthroughTests and docs replaced GenericContainer/ContainerRequest usage with a new Run(ctx, image, ...options) API. Container configuration (Dockerfile builds, files, env, ports, mounts, wait strategies, labels, lifecycle hooks, auth, networks) is supplied via functional options. Tests adjusted error expectations to include the new error prefix. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor T as Test/Caller
participant R as Run(ctx,image,opts...)
participant O as Options parser
participant B as Builder/Engine
participant W as WaitStrategy
participant C as Container
T->>R: Invoke Run(ctx,image,opts...)
R->>O: Apply options (Dockerfile, files, env, ports, mounts, hooks)
alt Dockerfile present
O->>B: Build image (context, Dockerfile, modifiers)
B-->>R: Image ID or error
else image provided
O-->>R: Use provided image
end
R->>B: Create & start container
B-->>R: Container started or error
alt WaitStrategy configured
R->>W: Execute wait
W-->>R: Ready or timeout
end
alt Success
R-->>T: Return Container handle
else Error
R-->>T: Return error ("generic container: ...")
end
sequenceDiagram
autonumber
actor T as Test/Caller
participant R as Run(...)
participant H as Lifecycle Hooks
participant B as Builder/Engine
participant W as WaitStrategy
participant C as Container
T->>R: Run(..., WithLifecycleHooks/WithAdditionalLifecycleHooks)
R->>H: Execute PreCreates
R->>B: Create container
R->>H: Execute PostCreates
R->>H: Execute PreStarts
R->>B: Start container
R->>H: Execute PostStarts
alt WaitStrategy
R->>W: Wait for readiness
W-->>R: Ready
R->>H: Execute PostReadies
end
note over R,H: On cleanup
R->>H: Execute PreStops / PreTerminates
R->>B: Stop/Terminate
R->>H: Execute PostStops / PostTerminates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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: 1
🧹 Nitpick comments (9)
docs/quickstart.md (1)
33-43: Fix indentation formatting in code block.The code block contains hard tabs which should be replaced with spaces for consistent formatting.
- redisC, err := testcontainers.Run( - ctx, "redis:latest", - testcontainers.WithExposedPorts("6379/tcp"), - testcontainers.WithWaitStrategy( - wait.ForListeningPort("6379/tcp"), - wait.ForLog("Ready to accept connections"), - ), - ) - testcontainers.CleanupContainer(t, redisC) - require.NoError(t, err) + redisC, err := testcontainers.Run( + ctx, "redis:latest", + testcontainers.WithExposedPorts("6379/tcp"), + testcontainers.WithWaitStrategy( + wait.ForListeningPort("6379/tcp"), + wait.ForLog("Ready to accept connections"), + ), + ) + testcontainers.CleanupContainer(t, redisC) + require.NoError(t, err)docs/features/creating_container.md (4)
136-136: Tighten wording (“the first argument”)Minor grammar polish for clarity.
-_Testcontainers for Go_ allows you to define your own lifecycle hooks for better control over your containers. You just need to define functions that return an error and receive the Go context as first argument. +_Testcontainers for Go_ allows you to define your own lifecycle hooks for better control over your containers. You just need to define functions that return an error and receive the Go context as the first argument.
166-167: Clarify phrasing: “important to note”Slightly clearer phrasing; no behavior change.
-It's important to notice that the `Readiness` of a container is defined by the wait strategies defined for the container. **This hook will be executed right after the `PostStarts` hook**. If you want to add your own readiness checks, you can do it by adding a `PostReadies` hook, which will execute your own readiness check after the default ones. That said, the `PostStarts` hooks don't warrant that the container is ready, so you should not rely on that. +It's important to note that the `Readiness` of a container is defined by the wait strategies configured for the container. **This hook is executed right after the `PostStarts` hook**. If you want additional readiness checks, add a `PostReadies` hook, which runs after the default ones. The `PostStarts` hooks do not imply readiness; don't rely on them for that.
188-190: Fix “advance” → “advanced” (twice) and minor grammarImproves readability; no semantic changes.
-The aforementioned `Run` function represents a straightforward manner to configure the containers, but you could need to create your containers with more advance settings regarding the config, host config and endpoint settings Docker types. For those more advance settings, _Testcontainers for Go_ offers a way to fully customize the container and those internal Docker types. These customisations, called _modifiers_, will be applied just before the internal call to the Docker client to create the container. +The aforementioned `Run` function represents a straightforward way to configure containers, but you may need more advanced settings regarding the Docker config, host config, and endpoint settings types. For those advanced settings, _Testcontainers for Go_ offers a way to fully customize the container and those internal Docker types. These customisations, called _modifiers_, are applied just before the internal call to the Docker client to create the container.
195-195: Replace hard tab with spaces (markdownlint MD010)Use spaces for indentation in admonitions to satisfy markdownlint.
- The only special case where the modifiers are not applied last, is when there are no exposed ports and the container does not use a network mode from a container (e.g. `req.NetworkMode = container.NetworkMode("container:$CONTAINER_ID")`). In that case, _Testcontainers for Go_ will extract the ports from the underlying Docker image and export them. + The only special case where the modifiers are not applied last, is when there are no exposed ports and the container does not use a network mode from a container (e.g. `req.NetworkMode = container.NetworkMode("container:$CONTAINER_ID")`). In that case, _Testcontainers for Go_ will extract the ports from the underlying Docker image and export them.lifecycle_test.go (2)
595-596: Reuse by name option applied conditionally is correctWithReuseByName sets both Name and Reuse; good for the reuse branch.
You could omit WithName when reuse is enabled, since WithReuseByName already sets the name.
835-842: Log-on-error test is effective; consider shorter timeoutThe failure is intentional; a shorter startup timeout (e.g., 2s) reduces test duration without changing intent.
- WithWaitStrategy(wait.ForLog("I was expecting that").WithStartupTimeout(5 * time.Second)), + WithWaitStrategy(wait.ForLog("I was expecting that").WithStartupTimeout(2 * time.Second)),generic_test.go (2)
76-87: Avoid redundant WithName when using WithReuseByNameWithReuseByName sets both the name and reuse flag; adding WithName as well is redundant.
- opts := []ContainerCustomizer{ - WithExposedPorts(nginxDefaultPort), - WithWaitStrategy(wait.ForListeningPort(nginxDefaultPort)), - WithName(tc.containerName), - } - if tc.reuseOption { - opts = append(opts, WithReuseByName(tc.containerName)) - } + opts := []ContainerCustomizer{ + WithExposedPorts(nginxDefaultPort), + WithWaitStrategy(wait.ForListeningPort(nginxDefaultPort)), + } + if tc.reuseOption { + opts = append(opts, WithReuseByName(tc.containerName)) + } else { + opts = append(opts, WithName(tc.containerName)) + }
25-26: Reduce shadowing for readabilityShadowing the package‑level reusableContainerName with a local variable of the same name is legal but confusing. Use a different local name.
- reusableContainerName := reusableContainerName + "_" + time.Now().Format("20060102150405") + name := reusableContainerName + "_" + time.Now().Format("20060102150405")And update references in this function from reusableContainerName to name accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
container_test.go(4 hunks)docker_auth_test.go(5 hunks)docker_exec_test.go(3 hunks)docker_files_test.go(5 hunks)docs/features/creating_container.md(2 hunks)docs/quickstart.md(1 hunks)from_dockerfile_test.go(3 hunks)generic_test.go(4 hunks)image_substitutors_test.go(1 hunks)lifecycle_test.go(5 hunks)mounts_test.go(3 hunks)port_forwarding_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (11)
container_test.go (4)
generic.go (1)
Run(122-149)options.go (3)
WithDockerfile(47-53)WithWaitStrategy(376-378)WithLabels(488-496)container.go (1)
FromDockerfile(90-108)testing.go (1)
CleanupContainer(91-97)
docker_files_test.go (2)
options.go (3)
WithFiles(534-539)WithCmd(472-477)WithWaitStrategy(376-378)container.go (1)
ContainerFile(110-115)
port_forwarding_test.go (3)
options.go (3)
ContainerCustomizer(22-24)WithHostPortAccess(97-106)WithCmd(472-477)network/network.go (2)
New(22-56)WithNetwork(139-141)testing.go (1)
CleanupNetwork(103-111)
mounts_test.go (3)
generic.go (1)
Run(122-149)options.go (1)
WithMounts(515-520)mounts.go (3)
ContainerMount(133-140)GenericVolumeMountSource(65-69)GenericVolumeMountSource(75-77)
docs/quickstart.md (3)
reaper.go (1)
RunContainer(44-47)modules/localstack/localstack.go (1)
RunContainer(71-122)docker_test.go (2)
ExampleContainer_Start(1035-1061)TestContainerWithHostNetworkOptions(49-102)
generic_test.go (5)
generic.go (1)
Run(122-149)options.go (5)
WithExposedPorts(464-469)WithWaitStrategy(376-378)WithName(109-117)ContainerCustomizer(22-24)WithReuseByName(129-138)wait/host_port.go (1)
ForListeningPort(67-69)testing.go (1)
CleanupContainer(91-97)wait/log.go (1)
ForLog(118-120)
image_substitutors_test.go (3)
modules/redis/redis.go (1)
Run(58-156)options.go (2)
WithDockerfile(47-53)WithImageSubstitutors(256-262)container.go (2)
FromDockerfile(90-108)FromDockerfile(123-163)
from_dockerfile_test.go (4)
generic.go (1)
Run(122-149)options.go (1)
WithDockerfile(47-53)container.go (1)
FromDockerfile(90-108)testing.go (1)
CleanupContainer(91-97)
docker_auth_test.go (3)
generic.go (1)
Run(122-149)options.go (6)
WithDockerfile(47-53)WithAlwaysPull(432-437)WithExposedPorts(464-469)WithWaitStrategy(376-378)WithEnv(75-85)WithFiles(534-539)container.go (5)
FromDockerfile(90-108)ContainerFile(110-115)BuildOptions(69-79)FromDockerfile(123-163)c(345-404)
docker_exec_test.go (2)
reaper_test.go (3)
TestContainerTerminationWithoutReaper(270-309)TestContainerTerminationWithReaper(230-268)TestContainerStartsWithoutTheReaper(113-145)docker_test.go (2)
TestContainerInspect_RawInspectIsCleanedOnStop(1325-1341)TestContainerTerminationRemovesDockerImage(334-406)
lifecycle_test.go (5)
options.go (6)
ContainerCustomizer(22-24)WithAdditionalLifecycleHooks(507-512)WithReuseByName(129-138)WithLifecycleHooks(499-504)WithCmd(472-477)WithWaitStrategy(376-378)lifecycle.go (4)
ContainerLifecycleHooks(43-55)ContainerRequestHook(24-24)ContainerHook(38-38)DefaultLoggingHook(58-131)container.go (2)
ContainerRequest(131-171)Container(41-73)modules/pulsar/pulsar.go (2)
Container(33-36)Run(148-179)generic.go (2)
Run(122-149)ContainerRequest(18-24)
🪛 markdownlint-cli2 (0.18.1)
docs/quickstart.md
33-33: Hard tabs
Column: 1
(MD010, no-hard-tabs)
34-34: Hard tabs
Column: 1
(MD010, no-hard-tabs)
35-35: Hard tabs
Column: 1
(MD010, no-hard-tabs)
36-36: Hard tabs
Column: 1
(MD010, no-hard-tabs)
37-37: Hard tabs
Column: 1
(MD010, no-hard-tabs)
38-38: Hard tabs
Column: 1
(MD010, no-hard-tabs)
39-39: Hard tabs
Column: 1
(MD010, no-hard-tabs)
40-40: Hard tabs
Column: 1
(MD010, no-hard-tabs)
41-41: Hard tabs
Column: 1
(MD010, no-hard-tabs)
42-42: Hard tabs
Column: 1
(MD010, no-hard-tabs)
68-68: Hard tabs
Column: 59
(MD010, no-hard-tabs)
docs/features/creating_container.md
169-169: Hard tabs
Column: 1
(MD010, no-hard-tabs)
195-195: 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). (20)
- GitHub Check: lint (modules/dolt) / lint: modules/dolt
- GitHub Check: lint (modules/chroma) / lint: modules/chroma
- GitHub Check: lint (modules/kafka) / lint: modules/kafka
- GitHub Check: lint (modules/mssql) / lint: modules/mssql
- GitHub Check: lint (modules/toxiproxy) / lint: modules/toxiproxy
- GitHub Check: lint (modules/postgres) / lint: modules/postgres
- GitHub Check: lint (modules/scylladb) / lint: modules/scylladb
- GitHub Check: lint (modules/mysql) / lint: modules/mysql
- GitHub Check: lint (modules/etcd) / lint: modules/etcd
- GitHub Check: lint (modules/weaviate) / lint: modules/weaviate
- GitHub Check: lint (modules/valkey) / lint: modules/valkey
- GitHub Check: lint (modules/dockermcpgateway) / lint: modules/dockermcpgateway
- GitHub Check: lint (modules/grafana-lgtm) / lint: modules/grafana-lgtm
- GitHub Check: lint (modules/cockroachdb) / lint: modules/cockroachdb
- GitHub Check: lint (modules/nats) / lint: modules/nats
- GitHub Check: lint (modules/couchbase) / lint: modules/couchbase
- GitHub Check: lint (modules/redpanda) / lint: modules/redpanda
- GitHub Check: lint (modules/yugabytedb) / lint: modules/yugabytedb
- GitHub Check: lint (modules/aerospike) / lint: modules/aerospike
- GitHub Check: Analyze (go)
🔇 Additional comments (33)
image_substitutors_test.go (1)
104-109: Migration to Run API successfully demonstrates new testcontainers pattern.The test successfully migrates from
GenericContainerto the newRunAPI, using functional options for Dockerfile configuration and image substitutors. This aligns with the PR's objective to adopt the Run-based API across the codebase.docker_exec_test.go (1)
51-51: LGTM - Clean migration to Run API.The migration from
GenericContainertoRun(ctx, nginxAlpineImage)is clean and consistent across all test functions. The simplified container creation eliminates the need for explicitGenericContainerRequestandStarted: trueconfiguration.Also applies to: 77-77, 98-98
from_dockerfile_test.go (4)
87-92: Migration to Run API with proper error message update.The test correctly migrates to use
RunwithWithDockerfileoption. The updated error message expectation correctly includes the new "generic container: " prefix that theRunfunction adds as a wrapper around the underlying error.
132-141: LGTM - Dockerfile target build with Run API.The migration successfully uses the Run API with
WithDockerfileto handle multi-target Dockerfile builds. TheBuildOptionsModifierfunction properly sets the target through the options pattern.
201-210: LGTM - Error handling test correctly updated.The test correctly uses Run API and maintains the same error checking behavior for invalid build targets. The
BuildOptionsModifierfunction is properly used to set the invalid target.
158-167: Example function migrated to Run API pattern.The example function correctly demonstrates the new Run API with
WithDockerfileoption. This provides good documentation for users on how to use the new API for Dockerfile-based builds.mounts_test.go (2)
226-236: LGTM - Mount configuration migrated to functional options.The test successfully migrates to use the Run API with
WithMountsoption. The mount configuration usingContainerMountwithGenericVolumeMountSourceis properly structured and maintains the same functionality.
264-271: LGTM - Consistent mount pattern with explicit context.The second test follows the same pattern with proper context usage and mount configuration. The explicit context declaration improves readability and aligns with Go best practices.
docs/quickstart.md (2)
33-40: Documentation updated to reflect new Run API.The quickstart documentation correctly demonstrates the new
testcontainers.Runfunction with functional options likeWithExposedPortsandWithWaitStrategy. This provides clear guidance for users adopting the new API.
46-76: Comprehensive documentation updates for new API patterns.The documentation properly explains the new functional options pattern and the benefits of the Run API. The explanations about parallelization, automatic lifecycle management, and cleanup strategies are well-written and informative.
port_forwarding_test.go (2)
81-84: LGTM - Host port access configured via functional options.The migration to use
WithHostPortAccessandWithCmdoptions provides a clean way to configure container behavior. The variadic approach allows for flexible host port specification.
92-92: Network integration properly handled with Run API.The network configuration is correctly integrated by appending the network option to the existing options slice. This demonstrates good composition of the functional options pattern.
Also applies to: 96-96
container_test.go (3)
293-301: LGTM - Build context test migrated with proper error handling.The test correctly migrates to use
RunwithWithDockerfileandWithWaitStrategy. The error message expectation is properly updated to include the "generic container: " prefix added by the Run function wrapper.
322-322: LGTM - Custom labels test simplified with functional options.The test successfully uses the Run API with
WithLabelsoption, significantly simplifying the container creation code compared to the previousGenericContainerRequestapproach.
341-353: LGTM - Complex Dockerfile build with labels and build modifier.The test demonstrates advanced usage of the Run API by combining
WithDockerfile(includingBuildOptionsModifier) andWithLabels. This shows how multiple functional options can be composed together effectively.docker_files_test.go (4)
29-38: LGTM - File copying test migrated to functional options.The test successfully migrates to use
RunwithWithFiles,WithCmd, andWithWaitStrategyoptions. The file copying configuration is properly structured usingContainerFilewith reader, host path, container path, and file mode.
55-62: LGTM - Runtime file copying pattern maintained.The test correctly uses the new API for initial file setup while preserving the runtime file copying functionality through
CopyFileToContainer. The separation of concerns between container creation and runtime operations is well maintained.
85-96: LGTM - Directory copying with proper path handling.The test correctly handles directory copying with the Run API. The comments about parent directory creation provide helpful context for users about the limitations and best practices.
112-119: LGTM - Consistent file/directory copying patterns.Both tests maintain consistency in using the Run API with file copying options. The combination of initial file setup via options and runtime copying via container methods provides comprehensive file management capabilities.
Also applies to: 144-151
docker_auth_test.go (4)
163-170: Run + Dockerfile migration looks goodEmpty image with WithDockerfile is the right pattern. Options composition is clear and idiomatic.
203-215: Auth build via Run is correctPassing BuildArgs and setting Repo for the build tag is appropriate. Looks consistent with BuildOptions().
228-239: Negative-auth path correctly exercises failureGood use of Run to assert a build-time auth failure.
252-252: Pulling from private registry via Run is correctOption set is minimal and sufficient.
docs/features/creating_container.md (4)
138-141: Lifecycle hooks API doc is clearAccurately reflects WithLifecycleHooks vs WithAdditionalLifecycleHooks behavior.
174-174: Codeinclude reference looks correctMatches inside_block tag in lifecycle_test.go.
179-179: Default logging hook example reference is correctExamples align with DefaultLoggingHook + WithLifecycleHooks usage.
183-183: Custom logger include is consistentBlock tag matches lifecycle_test.go.
lifecycle_test.go (3)
498-591: Hooks via WithAdditionalLifecycleHooks are well‑structuredComprehensive coverage of all lifecycle stages; closures correctly capture the shared prints slice.
632-641: Default logging hook test is clearUsing an in‑memory logger to assert hook invocations is solid.
797-799: Multiple lifecycle hooks scenario is validPassing DefaultLoggingHook twice to verify combined hooks behavior is appropriate.
generic_test.go (3)
27-31: Run migration looks goodPorts and wait strategy via options are correct. Naming via WithName is clear.
105-109: Good: keep container reference on errorTest asserts the desired contract of Run returning a container handle when startup fails.
173-177: Subprocess reuse test aligns with new APIOption set for delayed nginx + reuse is correct and mirrors top‑level test.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
lifecycle_test.go (2)
595-596: Avoid fixed reuse name to prevent cross-test collisionsUsing a static name risks clashes/flakes when tests run in parallel or leave residue. Generate a unique name from t.Name().
Apply this diff:
- opts = append(opts, WithReuseByName("reuse-container")) + opts = append(opts, WithReuseByName("reuse-" + strings.ReplaceAll(t.Name(), "/", "-")))
835-842: Guard against nil container on error pathIf Run fails before container creation, ctr can be nil; add a quick guard before using Logs to avoid a nil deref.
Apply this diff:
ctr, err := Run(ctx, "alpine", opts...) CleanupContainer(t, ctr) // it should fail because the waiting for condition is not met require.Error(t, err) +require.NotNil(t, ctr, "expected a non-nil container to fetch logs")docker_auth_test.go (1)
291-294: Use net.JoinHostPort for IPv6-safetyConcatenating ip + ":" + port breaks for IPv6 literals. Join using net.JoinHostPort.
Apply this diff:
- ip := localAddress(t) - mp := mappedPort.Port() - addr := ip + ":" + mp + addr := net.JoinHostPort(localAddress(t), mappedPort.Port())docs/features/creating_container.md (5)
136-136: Clarify hook function signatureRecommend spelling out the expected signature to avoid ambiguity, e.g. “functions of the form: func(ctx context.Context) error”.
Apply this diff:
-_Testcontainers for Go_ allows you to define your own lifecycle hooks for better control over your containers. You just need to define functions that return an error and receive the Go context as the first argument. +_Testcontainers for Go_ allows you to define your own lifecycle hooks for better control over your containers. Define functions of the form `func(ctx context.Context) error`.
167-167: Tighten readiness ordering wordingMake it explicit the default readiness check runs as part of post-start and that PostReadies run after that.
Apply this diff:
-It's important to note that the `Readiness` of a container is defined by the wait strategies configured for the container. **This hook is executed right after the `PostStarts` hook**. If you want additional readiness checks, add a `PostReadies` hook, which runs after the default ones. The `PostStarts` hooks do not imply readiness; don't rely on them for that. +It's important to note that container readiness is defined by the configured wait strategies. The default readiness check runs immediately after `PostStarts`. For additional checks, add a `PostReadies` hook, which runs after the default readiness. `PostStarts` does not imply readiness.
169-169: Fix markdownlint MD010: hard tabLine 169 contains a leading tab. Replace with spaces to satisfy markdownlint.
Apply this diff (replace the leading tab with four spaces):
- Up to `v0.37.0`, the readiness hook included checks for all the exposed ports to be ready. This is not the case anymore, and the readiness hook only uses the wait strategies defined for the container to determine if the container is ready. + Up to `v0.37.0`, the readiness hook included checks for all the exposed ports to be ready. This is not the case anymore, and the readiness hook only uses the wait strategies defined for the container to determine if the container is ready.
195-196: Fix markdownlint MD010 and Docker terminology
- Replace the leading tab with spaces.
- Prefer “expose” (or “publish”) over “export” for ports.
Apply this diff:
- The only special case where the modifiers are not applied last, is when there are no exposed ports and the container does not use a network mode from a container (e.g. `req.NetworkMode = container.NetworkMode("container:$CONTAINER_ID")`). In that case, _Testcontainers for Go_ will extract the ports from the underlying Docker image and export them. + The only special case where the modifiers are not applied last is when there are no exposed ports and the container does not use a network mode from a container (e.g. `req.NetworkMode = container.NetworkMode("container:$CONTAINER_ID")`). In that case, _Testcontainers for Go_ will extract the ports from the underlying Docker image and expose them.
138-141: Use Go terminology (“slice” not “array”) in docs snippet
Replace “array” with “slice” in the two sentences in docs/features/creating_container.md.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docker_auth_test.go(5 hunks)docs/features/creating_container.md(2 hunks)lifecycle_test.go(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
lifecycle_test.go (6)
options.go (6)
ContainerCustomizer(22-24)WithAdditionalLifecycleHooks(507-512)WithReuseByName(129-138)WithLifecycleHooks(499-504)WithCmd(472-477)WithWaitStrategy(376-378)lifecycle.go (4)
ContainerLifecycleHooks(43-55)ContainerRequestHook(24-24)ContainerHook(38-38)DefaultLoggingHook(58-131)modules/pulsar/pulsar.go (2)
Container(33-36)Run(148-179)modules/artemis/artemis.go (2)
Container(19-23)Run(88-124)generic.go (2)
Run(122-149)ContainerRequest(18-24)modules/valkey/valkey.go (1)
Run(63-161)
docker_auth_test.go (4)
modules/registry/registry.go (1)
Run(275-316)generic.go (1)
Run(122-149)options.go (6)
WithDockerfile(47-53)WithAlwaysPull(432-437)WithExposedPorts(464-469)WithWaitStrategy(376-378)WithEnv(75-85)WithFiles(534-539)container.go (3)
FromDockerfile(90-108)ContainerFile(110-115)FromDockerfile(123-163)
docs/features/creating_container.md (4)
container.go (3)
FromDockerfile(123-163)BuildOptions(69-79)Context(83-100)docker_test.go (1)
TestContainerCapAdd(1758-1791)modules/localstack/localstack_test.go (1)
TestRunContainer(110-146)docker.go (1)
ID(58-91)
🪛 markdownlint-cli2 (0.18.1)
docs/features/creating_container.md
169-169: Hard tabs
Column: 1
(MD010, no-hard-tabs)
195-195: 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). (19)
- GitHub Check: lint (modules/influxdb) / lint: modules/influxdb
- GitHub Check: lint (modules/opensearch) / lint: modules/opensearch
- GitHub Check: lint (modules/inbucket) / lint: modules/inbucket
- GitHub Check: lint (modules/couchbase) / lint: modules/couchbase
- GitHub Check: lint (modules/chroma) / lint: modules/chroma
- GitHub Check: lint (modules/solace) / lint: modules/solace
- GitHub Check: lint (modules/weaviate) / lint: modules/weaviate
- GitHub Check: lint (modules/grafana-lgtm) / lint: modules/grafana-lgtm
- GitHub Check: lint (modules/meilisearch) / lint: modules/meilisearch
- GitHub Check: lint (modules/scylladb) / lint: modules/scylladb
- GitHub Check: lint (modules/dockermcpgateway) / lint: modules/dockermcpgateway
- GitHub Check: lint (modules/redpanda) / lint: modules/redpanda
- GitHub Check: lint (modules/dolt) / lint: modules/dolt
- GitHub Check: lint (modules/rabbitmq) / lint: modules/rabbitmq
- GitHub Check: lint (modules/vault) / lint: modules/vault
- GitHub Check: lint (modules/mssql) / lint: modules/mssql
- GitHub Check: lint (modules/minio) / lint: modules/minio
- GitHub Check: lint / lint:
- GitHub Check: Analyze (go)
🔇 Additional comments (15)
lifecycle_test.go (7)
484-491: Renamed tests align with Run-based API — LGTMClearer names; aligns with the new Run(...) flow.
497-591: Lifecycle hooks via options — LGTMGood coverage of all phases with concise hook wiring through WithAdditionalLifecycleHooks.
598-601: Run(...) adoption — LGTMCorrect construction and cleanup ordering around Run.
632-637: Default logging hook via WithLifecycleHooks — LGTMAppropriate use of DefaultLoggingHook.
640-657: Run(...) with default logger — LGTMAssertions match expected hook emissions.
797-799: Multiple lifecycle hook sets — LGTMValidates composition behavior correctly.
801-818: Run(...) with duplicate hooks — LGTMCounts reflect doubled logging as intended.
docker_auth_test.go (5)
163-170: Build from Dockerfile via Run(...) — LGTMOptions map cleanly to the previous request fields.
203-215: Private-registry build auth test — LGTMCorrectly passes BuildArgs and uses WithAlwaysPull to exercise auth.
228-239: Negative auth test — LGTMProperly expects failure with wrong creds and cleans up.
252-253: Pull from private registry via Run(...) — LGTMAppropriate use of WithAlwaysPull/Wait to ensure auth is required.
263-283: Use filepath.Join for host paths — resolvedPrevious suggestion to avoid string concatenation on host paths is now implemented.
docs/features/creating_container.md (3)
189-193: Advanced settings section reads wellClear explanation of modifiers and when they apply. No issues from my side.
174-175: codeinclude anchors verified — no doc changes requiredAll three inside_block anchors referenced in docs/features/creating_container.md are present in lifecycle_test.go: optsWithLifecycleHooks (line 497), customLoggerImplementation (line 618), optsWithDefaultLoggingHook (line 632).
179-184: DefaultLoggingHook is the correct public symbol — no docs change neededConfirmed: lifecycle.go defines DefaultLoggingHook and the code uses WithLifecycleHooks(DefaultLoggingHook(...)) (see options_test.go / lifecycle_test.go). Leave the docs as-is.
1 participant
What does this PR do?
This is the first chunk of many PRs trying to make progress with #3174
The idea behind this PR, and the upcoming ones, is to replace GenericContainer with Run. The last PR would mean the deprecation of the GenericContainer function.
Why is it important?
Start adopting the new APIs for running containers using functional options, like in the modules
Related issues