chore(socat): use Run function #3312
Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary by CodeRabbit
WalkthroughRefactors socat module container creation from GenericContainer/ContainerRequest to the Run + functional options API, building options for entrypoint, exposed ports, and optional command. Updates error messaging and preserves readiness checks with the new flow. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor T as Test/Caller
participant S as Socat Module
participant TC as testcontainers.Run
participant C as Container
participant W as Wait Strategy
T->>S: Start(ctx, settings, opts...)
S->>S: Build module options (entrypoint, exposed ports, cmd)
S->>TC: Run(ctx, image, moduleOpts..., opts...)
TC-->>C: Create and start container
S->>W: Apply readiness checks
W-->>S: Ready or error
S-->>T: Container handle or error ("run socat")
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
🧹 Nitpick comments (3)
modules/socat/socat.go (3)
42-44: Gate entrypoint with command to avoid starting an idle shell and unintended overridesOnly set /bin/sh when you also set "-c ". This prevents starting a shell with no args when there are no targets and avoids unnecessarily overriding a user-provided entrypoint.
-moduleOpts := []testcontainers.ContainerCustomizer{ - testcontainers.WithEntrypoint("/bin/sh"), -} +moduleOpts := []testcontainers.ContainerCustomizer{} @@ -if settings.targetsCmd != "" { - moduleOpts = append(moduleOpts, testcontainers.WithCmdArgs("-c", settings.targetsCmd)) -} +if settings.targetsCmd != "" { + moduleOpts = append(moduleOpts, + testcontainers.WithEntrypoint("/bin/sh"), + testcontainers.WithCmdArgs("-c", settings.targetsCmd), + ) +}Also applies to: 54-54
46-47: Make exposed ports deterministic and skip no-op customizerPreallocate and sort for determinism; avoid appending a WithExposedPorts option when there are no ports.
-exposedPorts := []string{} - -for k := range settings.targets { - exposedPorts = append(exposedPorts, fmt.Sprintf("%d/tcp", k)) -} -moduleOpts = append(moduleOpts, testcontainers.WithExposedPorts(exposedPorts...)) +exposedPorts := make([]string, 0, len(settings.targets)) +ports := make([]int, 0, len(settings.targets)) +for k := range settings.targets { + ports = append(ports, k) +} +sort.Ints(ports) +for _, p := range ports { + exposedPorts = append(exposedPorts, fmt.Sprintf("%d/tcp", p)) +} +if len(exposedPorts) > 0 { + moduleOpts = append(moduleOpts, testcontainers.WithExposedPorts(exposedPorts...)) +}Add import:
-import ( +import ( "context" "fmt" "net/url" + "sort"Also applies to: 49-51
66-66: Align error prefix with other modulesFor consistency with dockermodelrunner ("socat run: %w"), prefer the same prefix ordering.
- return c, fmt.Errorf("run socat: %w", err) + return c, fmt.Errorf("socat run: %w", err)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
modules/socat/socat.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
modules/socat/socat.go (2)
options.go (4)
ContainerCustomizer(22-24)WithEntrypoint(448-453)WithExposedPorts(464-469)WithCmdArgs(480-485)modules/dockermodelrunner/dockermodelrunner.go (2)
Run(28-71)Container(20-25)
⏰ 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: test (1.24.x, modules/socat) / test: modules/socat/1.24.x
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
modules/socat/socat.go (2)
59-60: Migration to Run API looks goodSwitching to testcontainers.Run with functional options is clean and aligns with the new API.
57-60: No override risk from socat.Option
Option implements ContainerCustomizer but its Customize method is a no-op, so appending opts after defaults won’t double-apply or override Entrypoint/Cmd/ExposedPorts.
1 participant
What does this PR do?
Use Run function in the socat module
Why is it important?
Migrate modules to the new API
Related issues