-
-
Notifications
You must be signed in to change notification settings - Fork 588
feat: add Solace pubsub+ module #3230
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. |
mdelapenya
left a 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.
@unicod3 thanks for submitting this PR. I did an initial review and found some issues that need to be addressed, mostly related to the consistent patterns we apply when building the modules.
TL;DR;
- no constructor for the Container type
- options must be exposed at the package level, not added to the container type
- Run function using functional opts
In any case, we can work with you in making it consistent, and merge it into the project. Thank you so much for your contribution
@mdelapenya I've addressed them all, let me know if I missed anything, thank you for the review. |
mdelapenya
left a 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.
Looks pretty good, thanks for your hard work! Just a few comments, and we are done 😊
You're welcome, enjoyed the collaboration. In my last commit, I also addressed linter issues, there is a CodeQL check failing however I am not really sure if that's because of my branch. |
|
@mdelapenya just fixed the failing test issue fyi |
|
@unicod3 I added a commit on top of yours, just to refine the docs. PLMK what you think. We can merge this PR right after that. |
LGTM. |
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.
Pull Request Overview
This PR adds a new Testcontainers module for Solace Pubsub+, providing a ready-to-use container interface for testing applications that use Solace messaging services. The module supports configuration of multiple service types (AMQP, MQTT, REST, Management, SMF), queue/topic setup, and authentication options.
Key changes:
- Implements core Solace container functionality with configurable options for credentials, VPN, services, and message queues
- Provides service-specific URL generation for different Solace protocols (AMQP, MQTT, REST, SMF)
- Includes comprehensive test coverage and examples demonstrating message publishing and consumption
Reviewed Changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| modules/solace/solace.go | Core container implementation with Run function, service configuration, and CLI script execution |
| modules/solace/service.go | Service definitions for AMQP, MQTT, REST, Management, and SMF protocols |
| modules/solace/options.go | Configuration options for credentials, VPN, queues, services, and shared memory |
| modules/solace/solace_test.go | Basic integration test validating container startup and service accessibility |
| modules/solace/options_test.go | Unit tests for all configuration options and their combinations |
| modules/solace/examples_test.go | Example usage with Solace SDK integration for message publishing/consuming |
| modules/solace/mounts/solace.script.tpl | CLI script template for queue and topic configuration |
| docs/modules/solace.md | Module documentation with usage examples and API reference |
Comments suppressed due to low confidence (1)
docs/modules/solace.md:54
- The function name should be 'WithVPN' (all caps) to match the actual function name in the code, not 'WithVpn'.
#### WithVpn
mdelapenya
left a 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.
LGTM, thanks!
|
@unicod3 thanks for your contribution! I'm adding it to the modules catalog right now: testcontainers/community-module-registry#147 |
What does this PR do?
Adds a new module for Solace pubsub+
Why is it important?
Exposes a ready-to-use module for Solace Pubsub+
Related issues