-
-
Notifications
You must be signed in to change notification settings - Fork 588
chore(gcloud): use Run function #3410
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary by CodeRabbit
WalkthroughRefactors all gcloud emulator modules to build containers via accumulated ContainerCustomizer options and testcontainers.Run, replacing GenericContainerRequest usage. Introduces default port constants, unified wait strategies, option application via applyOptions(opts), and command/file argument wiring through WithCmd/WithCmdArgs/WithFiles. Updates error messages and port endpoint retrieval accordingly. One core signature change in newGCloudContainer and applyOptions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant T as Test/Caller
participant M as Module Run(...)
participant AO as applyOptions(opts)
participant TC as testcontainers.Run
participant C as Container
participant H as Host
T->>M: Run(ctx, img, opts...)
M->>AO: Apply module options
AO-->>M: settings (project, etc.)
M->>M: Build moduleOpts (WithExposedPorts, WithWaitStrategy, WithCmd/Args, WithFiles)
M->>TC: Run(ctx, img, moduleOpts...)
TC-->>M: ctr
M->>C: Wrap ctr, store settings/URI
M->>C: PortEndpoint(defaultPort)
C-->>M: host:port
M-->>T: GCloudContainer (URI, port, cleanup)
rect rgba(224,248,234,0.6)
note over M,TC: New flow using ContainerCustomizer options
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 (beta)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 (2)
modules/gcloud/spanner/spanner.go (1)
37-66: Restore emulator command and include user customizers.The refactor drops two essential behaviors: we no longer pass the emulator start command, and we ignore caller-provided
ContainerCustomizers. As written, the container boots with the image default entrypoint (so the emulator never starts) and any customization such astestcontainers.WithEnvorWithFilesis lost. Both are regressions from the previous implementation.Please restore the command and ensure we append
optstomoduleOptsbefore running the container. For example:@@ } + moduleOpts = append(moduleOpts, testcontainers.WithCmd( + "/bin/sh", + "-c", + "gcloud beta emulators spanner start --host-port 0.0.0.0:"+defaultPortNumber+" --project="+settings.ProjectID, + )) + + moduleOpts = append(moduleOpts, opts...) + ctr, err := testcontainers.Run(ctx, img, moduleOpts...)This keeps the emulator behavior intact and preserves user overrides.
modules/gcloud/pubsub/pubsub.go (1)
37-67: Preserve caller-supplied container options.We no longer append
optstomoduleOptsbefore callingtestcontainers.Run. Any caller-providedContainerCustomizer(env vars, bind mounts, providers, etc.) is now ignored, which is a regression from the previous behavior and breaks consumers depending on those overrides.Please append
optsback before invokingRun, e.g.:@@ moduleOpts = append(moduleOpts, testcontainers.WithCmd( "/bin/sh", "-c", "gcloud beta emulators pubsub start --host-port 0.0.0.0:"+defaultPortNumber+" --project="+settings.ProjectID, )) + moduleOpts = append(moduleOpts, opts...) + ctr, err := testcontainers.Run(ctx, img, moduleOpts...)This ensures existing customizations continue to work.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
modules/gcloud/bigquery.go(1 hunks)modules/gcloud/bigquery/bigquery.go(2 hunks)modules/gcloud/bigquery/bigquery_test.go(1 hunks)modules/gcloud/bigquery/options.go(1 hunks)modules/gcloud/bigtable.go(1 hunks)modules/gcloud/bigtable/bigtable.go(3 hunks)modules/gcloud/datastore.go(1 hunks)modules/gcloud/datastore/datastore.go(3 hunks)modules/gcloud/firestore.go(1 hunks)modules/gcloud/firestore/firestore.go(3 hunks)modules/gcloud/gcloud.go(2 hunks)modules/gcloud/pubsub.go(1 hunks)modules/gcloud/pubsub/pubsub.go(3 hunks)modules/gcloud/spanner.go(1 hunks)modules/gcloud/spanner/spanner.go(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (15)
modules/gcloud/pubsub.go (4)
options.go (4)
ContainerCustomizer(22-24)WithExposedPorts(454-459)WithWaitStrategy(366-368)WithCmd(462-467)wait/all.go (1)
ForAll(44-48)wait/host_port.go (1)
ForListeningPort(67-69)wait/log.go (1)
ForLog(118-120)
modules/gcloud/bigquery/options.go (2)
options.go (2)
WithCmdArgs(470-475)WithFiles(524-529)container.go (1)
ContainerFile(110-115)
modules/gcloud/pubsub/pubsub.go (7)
options.go (4)
ContainerCustomizer(22-24)WithExposedPorts(454-459)WithWaitStrategy(366-368)WithCmd(462-467)wait/all.go (1)
ForAll(44-48)wait/host_port.go (1)
ForListeningPort(67-69)wait/log.go (1)
ForLog(118-120)modules/gcloud/spanner/spanner.go (2)
Run(36-71)Container(19-22)modules/gcloud/firestore/firestore.go (2)
Run(36-84)Container(19-22)modules/gcloud/datastore/datastore.go (2)
Run(36-79)Container(19-22)
modules/gcloud/bigquery.go (3)
options.go (5)
ContainerCustomizer(22-24)WithExposedPorts(454-459)WithWaitStrategy(366-368)WithCmdArgs(470-475)WithFiles(524-529)wait/http.go (1)
ForHTTP(149-151)container.go (1)
ContainerFile(110-115)
modules/gcloud/gcloud.go (2)
options.go (1)
ContainerCustomizer(22-24)modules/gcloud/spanner/spanner.go (2)
Run(36-71)Container(19-22)
modules/gcloud/firestore/firestore.go (6)
options.go (4)
ContainerCustomizer(22-24)WithExposedPorts(454-459)WithWaitStrategy(366-368)WithCmd(462-467)wait/all.go (1)
ForAll(44-48)wait/host_port.go (1)
ForListeningPort(67-69)wait/log.go (1)
ForLog(118-120)modules/gcloud/spanner/spanner.go (2)
Run(36-71)Container(19-22)modules/gcloud/pubsub/pubsub.go (2)
Run(36-77)Container(19-22)
modules/gcloud/datastore/datastore.go (4)
options.go (4)
ContainerCustomizer(22-24)WithExposedPorts(454-459)WithWaitStrategy(366-368)WithCmd(462-467)wait/all.go (1)
ForAll(44-48)wait/host_port.go (1)
ForListeningPort(67-69)wait/http.go (1)
ForHTTP(149-151)
modules/gcloud/datastore.go (2)
options.go (4)
ContainerCustomizer(22-24)WithExposedPorts(454-459)WithWaitStrategy(366-368)WithCmd(462-467)wait/http.go (1)
ForHTTP(149-151)
modules/gcloud/bigtable.go (4)
options.go (4)
ContainerCustomizer(22-24)WithExposedPorts(454-459)WithWaitStrategy(366-368)WithCmd(462-467)wait/all.go (1)
ForAll(44-48)wait/host_port.go (1)
ForListeningPort(67-69)wait/log.go (1)
ForLog(118-120)
modules/gcloud/firestore.go (4)
options.go (4)
ContainerCustomizer(22-24)WithExposedPorts(454-459)WithWaitStrategy(366-368)WithCmd(462-467)wait/all.go (1)
ForAll(44-48)wait/host_port.go (1)
ForListeningPort(67-69)wait/log.go (1)
ForLog(118-120)
modules/gcloud/bigquery/bigquery_test.go (2)
modules/gcloud/bigquery/bigquery.go (1)
Run(43-84)options.go (2)
ContainerCustomizer(22-24)WithCmdArgs(470-475)
modules/gcloud/bigquery/bigquery.go (6)
options.go (4)
ContainerCustomizer(22-24)WithExposedPorts(454-459)WithWaitStrategy(366-368)WithCmdArgs(470-475)wait/all.go (1)
ForAll(44-48)wait/host_port.go (1)
ForListeningPort(67-69)wait/http.go (1)
ForHTTP(149-151)modules/gcloud/spanner/spanner.go (2)
Run(36-71)Container(19-22)modules/gcloud/pubsub/pubsub.go (2)
Run(36-77)Container(19-22)
modules/gcloud/spanner/spanner.go (5)
options.go (3)
ContainerCustomizer(22-24)WithExposedPorts(454-459)WithWaitStrategy(366-368)wait/all.go (1)
ForAll(44-48)wait/host_port.go (1)
ForListeningPort(67-69)wait/log.go (1)
ForLog(118-120)modules/gcloud/pubsub/pubsub.go (2)
Run(36-77)Container(19-22)
modules/gcloud/bigtable/bigtable.go (5)
options.go (4)
ContainerCustomizer(22-24)WithExposedPorts(454-459)WithWaitStrategy(366-368)WithCmd(462-467)wait/all.go (1)
ForAll(44-48)wait/host_port.go (1)
ForListeningPort(67-69)wait/log.go (1)
ForLog(118-120)modules/gcloud/spanner/spanner.go (2)
Run(36-71)Container(19-22)
modules/gcloud/spanner.go (4)
options.go (3)
ContainerCustomizer(22-24)WithExposedPorts(454-459)WithWaitStrategy(366-368)wait/all.go (1)
ForAll(44-48)wait/host_port.go (1)
ForListeningPort(67-69)wait/log.go (1)
ForLog(118-120)
⏰ 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). (1)
- GitHub Check: Analyze (go)
🔇 Additional comments (8)
modules/gcloud/bigquery/options.go (1)
31-47: LGTM! Modular option application is correct.The refactor correctly replaces direct mutation with wrapper calls to
testcontainers.WithCmdArgsandtestcontainers.WithFiles, propagating errors appropriately. The guard against duplicate data YAML remains in place.modules/gcloud/bigquery.go (1)
20-49: LGTM! Deprecation path correctly migrated to modular options.The deprecated
RunBigQueryfunction correctly accumulates options inmoduleOptsand delegates tonewGCloudContainer. Port constants, wait strategies, and command argument application are all correct.modules/gcloud/bigquery/bigquery_test.go (2)
79-79: LGTM! Error assertion loosened appropriately.Changing from
require.EqualErrortorequire.ErrorContainsis appropriate for the refactor, as error messages may now include additional wrapping context.
83-85: LGTM! Test helper updated to match new option type.The
noValueOptionhelper correctly returnstestcontainers.ContainerCustomizerand uses theWithCmdArgswrapper, aligning with the modular options pattern.modules/gcloud/bigquery/bigquery.go (2)
19-22: LGTM! Port constants defined correctly.The port constants are defined consistently with the pattern used across other gcloud modules.
44-84: LGTM! Run function correctly refactored to modular options pattern.The
Runfunction correctly:
- Initializes
moduleOptswith exposed ports and a composite wait strategy (ForAllwithForListeningPortandForHTTP)- Applies incoming
Option-type customizers tosettings- Accumulates all options in
moduleOpts- Calls
testcontainers.Runwith accumulated options- Wraps errors with appropriate context
- Uses port constants for endpoint retrieval
The refactor aligns with the pattern used in other gcloud modules (pubsub, spanner, datastore).
modules/gcloud/datastore/datastore.go (2)
13-15: LGTM! Port constants defined correctly.The port constants are defined consistently with the pattern used across other gcloud modules.
37-79: LGTM! Run function correctly refactored to modular options pattern.The
Runfunction correctly:
- Initializes
moduleOptswith exposed ports and a composite wait strategy (ForAllwithForListeningPortandForHTTP)- Applies incoming
Option-type customizers tosettings- Appends the
gcloud datastore startcommand viaWithCmd, referencing the new port constant and project ID- Accumulates all options in
moduleOpts- Calls
testcontainers.Runwith accumulated options- Wraps errors with appropriate context ("run datastore")
- Uses port constants for endpoint retrieval
The refactor aligns with the pattern used in other gcloud modules.
|
Closing, as this PR was sent from wrong branch |
What does this PR do?
Use Run function in the GCloud module
Why is it important?
Migrate modules to the new API
Related issues