chore: use Run in more tests (part 4) #3309
Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary by CodeRabbit
WalkthroughRefactors container setup across tests and example to use Run(...) with functional options instead of GenericContainer/ContainerRequest. Updates assertions and image/port accessors accordingly. Adds a new unit test file validating lifecycle hook wiring for startup and after-ready commands. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Test as Test/Example
participant Run as Run(...)
participant Opts as Options
participant Engine as Container Runtime
participant Hooks as Lifecycle Hooks
Test->>Run: Invoke with image + options
Run->>Opts: Apply With... (wait, entrypoint, startup/after-ready, logs)
Run->>Engine: Create & start container
alt Wait strategy
Engine-->>Run: Signal ready per wait strategy
end
opt Startup command
Run->>Hooks: Execute PostStart actions
end
opt After-ready command
Run->>Hooks: Execute PostReady actions
end
Run-->>Test: Return ctr (with Image, PortEndpoint)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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
🧹 Nitpick comments (2)
options_test.go (2)
84-88: Reduce flakiness by matching a shorter, stable MySQL log lineThe exact message “port: 3306 MySQL Community Server - GPL” can vary across releases/locales. Matching a shorter substring is more robust.
Apply this diff:
- c, err := testcontainers.Run( - ctx, "mysql:8.0.36", - testcontainers.WithWaitStrategy(wait.ForLog("port: 3306 MySQL Community Server - GPL")), - testcontainers.WithLogConsumers(lc), - ) + c, err := testcontainers.Run( + ctx, "mysql:8.0.36", + testcontainers.WithWaitStrategy(wait.ForLog("port: 3306")), + testcontainers.WithLogConsumers(lc), + )
155-155: Confirm AfterReady hook behavior without an explicit waitIf no WaitingFor strategy is set, verify PostReadies still execute. If not, add a no-op wait to trigger the “ready” phase immediately. Based on learnings
Apply this diff if needed:
- c, err := testcontainers.Run(context.Background(), "alpine", testcontainers.WithEntrypoint("tail", "-f", "/dev/null"), testcontainers.WithAfterReadyCommand(testExec)) + c, err := testcontainers.Run( + context.Background(), + "alpine", + testcontainers.WithEntrypoint("tail", "-f", "/dev/null"), + testcontainers.WithAfterReadyCommand(testExec), + testcontainers.WithWaitStrategy(wait.ForNop(func(_ context.Context, _ wait.StrategyTarget) error { return nil })), + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
container_test.go(6 hunks)examples/nginx/nginx.go(1 hunks)options_test.go(3 hunks)options_unit_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
examples/nginx/nginx.go (2)
options.go (2)
WithExposedPorts(464-469)WithWaitStrategy(376-378)wait/http.go (1)
ForHTTP(149-151)
options_test.go (3)
generic.go (1)
Run(122-149)options.go (5)
WithWaitStrategy(376-378)WithLogConsumers(265-274)WithEntrypoint(448-453)WithStartupCommand(330-349)WithAfterReadyCommand(354-373)wait/log.go (1)
ForLog(118-120)
options_unit_test.go (4)
generic.go (1)
GenericContainerRequest(21-27)container.go (1)
ContainerRequest(131-171)options.go (3)
NewRawCommand(313-320)WithStartupCommand(330-349)WithAfterReadyCommand(354-373)exec/processor.go (1)
WithWorkingDir(52-56)
container_test.go (5)
generic.go (1)
Run(122-149)options.go (4)
WithCmd(472-477)WithWaitStrategy(376-378)WithImageSubstitutors(256-262)WithExposedPorts(464-469)wait/log.go (1)
ForLog(118-120)testing.go (1)
CleanupContainer(91-97)wait/http.go (1)
ForHTTP(149-151)
⏰ 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/vearch) / lint: modules/vearch
- GitHub Check: lint (modules/mssql) / lint: modules/mssql
- GitHub Check: lint (modulegen) / lint: modulegen
- GitHub Check: lint (modules/inbucket) / lint: modules/inbucket
- GitHub Check: lint (modules/yugabytedb) / lint: modules/yugabytedb
- GitHub Check: lint (modules/k3s) / lint: modules/k3s
- GitHub Check: lint (modules/rabbitmq) / lint: modules/rabbitmq
- GitHub Check: lint (modules/dockermodelrunner) / lint: modules/dockermodelrunner
- GitHub Check: lint (modules/aerospike) / lint: modules/aerospike
- GitHub Check: lint (modules/consul) / lint: modules/consul
- GitHub Check: lint (modules/openldap) / lint: modules/openldap
- GitHub Check: lint (modules/memcached) / lint: modules/memcached
- GitHub Check: lint (modules/gcloud) / lint: modules/gcloud
- GitHub Check: lint (modules/etcd) / lint: modules/etcd
- GitHub Check: lint (modules/mockserver) / lint: modules/mockserver
- GitHub Check: lint (modules/pulsar) / lint: modules/pulsar
- GitHub Check: lint (modules/milvus) / lint: modules/milvus
- GitHub Check: lint (modules/compose) / lint: modules/compose
- GitHub Check: Analyze (go)
🔇 Additional comments (10)
examples/nginx/nginx.go (1)
17-21: LGTM: Run-based refactor is cleanThe switch to Run with functional options is correct; PortEndpoint usage and error handling look good.
Also applies to: 23-25, 30-30
options_test.go (1)
136-140: LGTM: Startup command via lifecycle hook worksGood use of WithEntrypoint + WithStartupCommand and validating via Exec afterward.
options_unit_test.go (2)
11-23: LGTM: Validates startup hook wiringThe unit test correctly asserts a single PostStarts hook added by WithStartupCommand.
25-37: LGTM: Validates after-ready hook wiringThe unit test correctly asserts a single PostReadies hook added by WithAfterReadyCommand.
container_test.go (6)
325-328: LGTM: Label assertion stays intact after Run migrationInspect + label equality remains correct.
365-369: LGTM: Run-based failure-path test is correctThe Run + ForLog + Cleanup pattern is sound; the test’s error expectation remains valid.
465-474: LGTM: Image substitutor test migrated to RunUsing WithImageSubstitutors with Run and asserting ctr.Image is the right approach.
487-491: LGTM: Parallel start using Run with HTTP waitGood use of WithExposedPorts and ForHTTP with a bounded startup timeout.
509-515: LGTM: Example updated to Run with image substitutorsExample remains concise and accurate post-refactor.
522-523: LGTM: Example prints substituted image correctlyAccessing ctr.Image aligns with the Run return type.
1 participant
What does this PR do?
It uses the Run function in more places from the core library (no modules yet)
Why is it important?
Consistenty and progress towards the new Run API
Related issues