Skip to content

Conversation

@mdelapenya
Copy link
Member

What does this PR do?

Use the Run function in pulsar

Why is it important?

Migrate modules to the new API, improving consistency and leveraging the latest testcontainers functionality.

Related issues

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@mdelapenya mdelapenya requested a review from a team as a code owner October 7, 2025 19:35
@mdelapenya mdelapenya added the chore Changes that do not impact the existing functionality label Oct 7, 2025
@netlify
Copy link

netlify bot commented Oct 7, 2025

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit cddc3a8
🔍 Latest deploy log https://app.netlify.com/projects/testcontainers-go/deploys/68e56b6e643ade0007a8cf53
😎 Deploy Preview https://deploy-preview-3426--testcontainers-go.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link

coderabbitai bot commented Oct 7, 2025

Summary by CodeRabbit

  • Refactor
    • Streamlined Pulsar container setup using option-based configuration for commands, environment variables, and wait strategies.
    • Standardized orchestration of wait strategies for transactions and worker startup, improving consistency.
    • Consolidated container launch flow and exposed ports handling for more predictable behavior.
    • Updated error messaging to provide clearer context when Pulsar fails to start (e.g., “run pulsar”).
    • No changes to public APIs; existing usage remains compatible.

Walkthrough

Refactors Pulsar module to use option-based container configuration. Replaces direct request field mutations with testcontainers.With* customizers and switches container start to testcontainers.Run using a composed options slice. Updates functions to apply command, env, exposed ports, and wait strategies via wrappers.

Changes

Cohort / File(s) Summary
Pulsar module: option-based Run refactor
modules/pulsar/pulsar.go
Replaced direct ContainerRequest mutations with testcontainers.WithCmd, WithEnv, WithWaitStrategy, and WithExposedPorts; consolidated options into moduleOpts and invoked testcontainers.Run(ctx, img, moduleOpts...); adjusted wait strategy composition and error message text.

Sequence Diagram(s)

sequenceDiagram
  actor Caller
  participant Pulsar as modules/pulsar/pulsar.go
  participant TC as testcontainers.Run
  participant Docker as Container Runtime

  Caller->>Pulsar: Run(ctx, img, opts...)
  activate Pulsar
  note over Pulsar: Build moduleOpts: WithExposedPorts, WithEnv, WithCmd, WithWaitStrategy
  Pulsar->>TC: Run(ctx, img, moduleOpts...)
  activate TC
  TC->>Docker: Create & start container
  Docker-->>TC: Container started
  TC-->>Pulsar: Container handle or error
  deactivate TC
  Pulsar-->>Caller: Result
  deactivate Pulsar
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

I twitch my nose at opts array delight,
WithEnv, WithCmd—stacked neatly and tight.
No fields to mutate, no fiddly fray,
We Run with grace, in modular way.
Thump-thump—containers wake on cue,
A Pulsar hums, and tests hop through. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly describes the main change of migrating the Pulsar module to use the Run function, matching the summary of refactoring container instantiation without unnecessary noise or generic terms.
Description Check ✅ Passed The description explains that the PR migrates the Pulsar module to use the Run function and highlights its goal of aligning with the new API and leveraging testcontainers improvements, which directly relates to the actual changes in the code.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b7e70d0 and cddc3a8.

📒 Files selected for processing (1)
  • modules/pulsar/pulsar.go (4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
modules/pulsar/pulsar.go (3)
options.go (5)
  • WithCmd (462-467)
  • WithWaitStrategy (366-368)
  • WithEnv (75-85)
  • ContainerCustomizer (22-24)
  • WithExposedPorts (454-459)
wait/log.go (1)
  • ForLog (118-120)
wait/all.go (1)
  • ForAll (44-48)
⏰ 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). (5)
  • GitHub Check: lint (modules/pulsar) / lint: modules/pulsar
  • GitHub Check: Redirect rules - testcontainers-go
  • GitHub Check: Header rules - testcontainers-go
  • GitHub Check: Pages changed - testcontainers-go
  • GitHub Check: Analyze (go)
🔇 Additional comments (4)
modules/pulsar/pulsar.go (4)

75-85: LGTM! Clean refactor to option-based configuration.

The function correctly uses testcontainers.WithCmd and testcontainers.WithWaitStrategy wrappers instead of directly mutating request fields. Error handling is appropriate, and the wait strategy composition with wait.ForAll preserves the original behavior while following the new API pattern.


106-106: LGTM! Proper delegation to the wrapper function.

The function now delegates to testcontainers.WithEnv instead of directly mutating req.Env, which aligns with the option-based configuration pattern while preserving the PULSAR_PREFIX_ variable prefixing behavior.


125-125: LGTM! Consistent with the option pattern.

The function correctly uses testcontainers.WithWaitStrategy(wait.ForAll(ss...)) to compose the transaction-specific wait strategy with default strategies, replacing direct field mutation while maintaining the same behavior.


145-161: LGTM! Well-structured refactor to the Run pattern.

The function successfully migrates to the option-based approach:

  • Default options (exposed ports, wait strategy, command) are defined first in moduleOpts
  • User-provided opts are appended, allowing proper overrides
  • The defensive nil check on line 155 ensures safe container wrapping
  • Updated error message ("run pulsar: %w") is more specific

The order of options ensures users can override defaults when needed, and the refactor maintains backward compatibility.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@mdelapenya mdelapenya merged commit eba852e into testcontainers:main Oct 7, 2025
17 checks passed
@mdelapenya mdelapenya deleted the use-run-claude-pulsar branch October 7, 2025 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Changes that do not impact the existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant