Skip to content

chore(tests): Cover mqtt e2e scenarios for MqttSession - #950

Merged
allenporter merged 2 commits into
Python-roborock:mainfrom
borisalekseev:chore/e2e-mqtt-tests
Oct 4, 2026
Merged

allenporter merged 2 commits into
Python-roborock:mainfrom
borisalekseev:chore/e2e-mqtt-tests

Conversation

@borisalekseev

Copy link
Copy Markdown
Contributor

I wrote only e2e tests according to the conversation at #928

@allenporter allenporter left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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:

  1. Ensuring broker tests have bounded timeouts and are tagged so standard local pytest runs stay fast and offline-friendly.
  2. Decoupling the EMQX service container into a separate parallel job or standalone workflow.

Comment thread .github/workflows/ci.yml
Comment thread pyproject.toml
Comment thread CONTRIBUTING.md Outdated

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated the guide: uv run pytest remains the standard command, with Docker Compose documented separately for uv run pytest -m mqtt_broker.

Comment thread tests/e2e/test_mqtt_broker.py

@allenporter allenporter left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

@allenporter
allenporter merged commit 840139a into Python-roborock:main Oct 4, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants