chore(tests): Cover mqtt e2e scenarios for MqttSession - #950
Conversation
There was a problem hiding this comment.
Thank you @borisalekseev for following up on the discussion from #928 and putting together this real-broker E2E test suite! (Note: I am currently experimenting with agentic code review to help review PRs, so please let me know if anything in the feedback looks off).
These tests are well-structured, keep traffic isolated with per-test UUID topics, and effectively exercise the behavioral contracts of both MqttSession and MqttChannel without modifying production dependencies.
I have left a few inline comments on:
- Ensuring broker tests have bounded timeouts and are tagged so standard local
pytestruns stay fast and offline-friendly. - Decoupling the EMQX service container into a separate parallel job or standalone workflow.
|
|
||
| We use `pytest` for testing. Please ensure all tests pass and add new tests for your changes. | ||
|
|
||
| MQTT tests require a real EMQX broker at `127.0.0.1:1888`. Start it locally with |
There was a problem hiding this comment.
We should keep pytest (or uv run pytest) as the standard instruction for general testing, and document Docker Compose specifically under the pytest -m mqtt_broker section for running the real-broker test suite.
There was a problem hiding this comment.
Updated the guide: uv run pytest remains the standard command, with Docker Compose documented separately for uv run pytest -m mqtt_broker.
5340dd4 to
233d126
Compare
allenporter
left a comment
There was a problem hiding this comment.
Looks great, thank you @borisalekseev
One of the tests appears to take 10 seconds, presumably a timeout, so something to look into speeding up in a future PR.
Thanks again.
I wrote only e2e tests according to the conversation at #928