Skip to content
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

fix(mqtt): Add mqtt.MosquittoContainer (#568) #599

Merged
merged 9 commits into from
Jun 18, 2024

Conversation

f18m
Copy link
Contributor

@f18m f18m commented Jun 5, 2024

This PR is adding a new MosquittoContainer class that helps creating integration tests for MQTT clients.
The MosquittoContainer class contains a bunch of methods to help with testing:

  • checking number of messages received
  • watching topics
  • check last payload published on a particular topic
  • etc

This PR lacks tests. I can add them if there is interest in this PR...

Copy link
Collaborator

@alexanderankin alexanderankin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interest is there, would merge a more generic and simpler/smaller version

modules/mqtt/testcontainers/mosquitto/__init__.py Outdated Show resolved Hide resolved
modules/mqtt/testcontainers/mosquitto/__init__.py Outdated Show resolved Hide resolved
modules/mqtt/testcontainers/mosquitto/__init__.py Outdated Show resolved Hide resolved
modules/mqtt/testcontainers/mosquitto/__init__.py Outdated Show resolved Hide resolved
@f18m
Copy link
Contributor Author

f18m commented Jun 6, 2024

interest is there, would merge a more generic and simpler/smaller version

ok thanks for the review!
let me try to address the various points you raised

@f18m
Copy link
Contributor Author

f18m commented Jun 6, 2024

@alexanderankin please note that I also checked in the "integration-test-mosquitto.conf" that is bind-mounted in the init ctor.
I'm not sure what's the best way to deliver this kind of resources (the conf file) inside the project... so I just dropped it in the same folder of the init.py file, which is what the init ctor uses for creating the bind-mount...

@alexanderankin
Copy link
Collaborator

alexanderankin commented Jun 6, 2024

mounting files is probably the most straight forward way to do it but I would move it into the _configure (or start if you want ultimate backwards compatibility) method rather than the constructor. there is a concept of a transferable interface which can be implemented for strings and bytes/io but it is not merged, there is a couple commits here which should eventually make it into the project - 2bd20c6...alexanderankin:testcontainers-python:TCP-393_transferable - the code in the kafka container which does the some thing is even more concise. i think there may be some chance that this transferable thing is going to make it into the project through one of the PR's that are doing database seeding (helpers around init-db.d)

I misread. yes checking the files in the same directory is what i typically do also, and reference with relative path via:

from pathlib import Path
Path(__file__).parent / 'adjacent-folder' / 'the-file.ext'

Copy link

codecov bot commented Jun 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@ec76df2). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #599   +/-   ##
=======================================
  Coverage        ?   75.18%           
=======================================
  Files           ?       10           
  Lines           ?      548           
  Branches        ?       77           
=======================================
  Hits            ?      412           
  Misses          ?      110           
  Partials        ?       26           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@f18m
Copy link
Contributor Author

f18m commented Jun 7, 2024

Path(__file__).parent

moved into start() the volume mapping bind!

@f18m
Copy link
Contributor Author

f18m commented Jun 11, 2024

@alexanderankin just take a look again when you have time, thanks

@alexanderankin
Copy link
Collaborator

this is probably what id like to merge, lmk if this works for you:

https://github.com/testcontainers/testcontainers-python/compare/feature/mosquitto

@f18m
Copy link
Contributor Author

f18m commented Jun 12, 2024

this is probably what id like to merge, lmk if this works for you:

https://github.com/testcontainers/testcontainers-python/compare/feature/mosquitto

super! I see you added support also for Mosquitto 1.x, which is something I never used. I think it's good to have.

I think it's just good. Once merged I will rework my project to make use of the MosquittoContainer class and report back any possible issue. Thanks

@alexanderankin
Copy link
Collaborator

if you want you can pip install into your project from the branch and do some testing before it is released as well

@alexanderankin alexanderankin changed the title Add mqtt.MosquittoContainer (#568) fix: Add mqtt.MosquittoContainer (#568) Jun 12, 2024
@alexanderankin alexanderankin added the community-feat feature but its a community module so we wont bump tc core for it label Jun 12, 2024
@alexanderankin alexanderankin changed the title fix: Add mqtt.MosquittoContainer (#568) fix(mqtt): Add mqtt.MosquittoContainer (#568) Jun 12, 2024
@alexanderankin
Copy link
Collaborator

ok ill merge at some point today.

@f18m
Copy link
Contributor Author

f18m commented Jun 13, 2024

ok ill merge at some point today.

yeah I don't think I will manage to adapt my project that relies on MosquittoContainer fast enough.
But next week I think I will have some time to do that and will report back

@alexanderankin
Copy link
Collaborator

gonna merge because it modifies the poetry lock file and dont wanna have conflicts, still a chance to try it before it gets released

@alexanderankin alexanderankin merged commit 59cb6fc into testcontainers:main Jun 18, 2024
15 checks passed
@f18m
Copy link
Contributor Author

f18m commented Jun 19, 2024

hi @alexanderankin ,
I'm trying to understand how to test an unreleased version of testcontainers-python.
However I see that the CI/CD step 'Release Please / publish (push) ' is Skipped...
are testcontainers-python artifacts published somewhere after every commit?

@f18m
Copy link
Contributor Author

f18m commented Jun 19, 2024

hi @alexanderankin , I'm trying to understand how to test an unreleased version of testcontainers-python. However I see that the CI/CD step 'Release Please / publish (push) ' is Skipped... are testcontainers-python artifacts published somewhere after every commit?

Never mind: I installed the main branch locally from git into my python path.
So far so good. I have tested successfully the MosquittoContainer inside my pre-existing integration tests. I actually have a MosquittoContainerEnhanced class adding on top of MosquittoContainer the ability to 'watch' topics. All good so far!
Watching for next release of testcontainers-python, thanks!

alexanderankin pushed a commit that referenced this pull request Jun 20, 2024
🤖 I have created a release *beep* *boop*
---


##
[4.6.0](testcontainers-v4.5.1...testcontainers-v4.6.0)
(2024-06-18)


### Features

* **core:** Added ServerContainer
([#595](#595))
([0768490](0768490))
* **core:** Image build (Dockerfile support)
([#585](#585))
([54c88cf](54c88cf))


### Bug Fixes

* Add Cockroach DB Module to Testcontainers
([#608](#608))
([4aff679](4aff679))
* Container for Milvus database
([#606](#606))
([ec76df2](ec76df2))
* move TESTCONTAINERS_HOST_OVERRIDE to config.py
([#603](#603))
([2a5a190](2a5a190)),
closes
[#602](#602)
* **mqtt:** Add mqtt.MosquittoContainer
([#568](#568))
([#599](#599))
([59cb6fc](59cb6fc))


### Documentation

* **main:** Private registry
([#598](#598))
([9045c0a](9045c0a))
* Update private registry instructions
([#604](#604))
([f5a019b](f5a019b))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community-feat feature but its a community module so we wont bump tc core for it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants