-
Notifications
You must be signed in to change notification settings - Fork 54
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
Remove paho from mqqt unit test #967
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #967 +/- ##
=======================================
Coverage 77.71% 77.71%
=======================================
Files 326 326
Lines 28543 28543
=======================================
Hits 22183 22183
Misses 6360 6360
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
It might also be an idea to remove mqtt
from line 83 of the pyproject.toml
as there now shouldn't be a dependency requiring it anymore during tests no?
tests/loaders/test_mqtt.py
Outdated
monkeypatch.setitem(sys.modules, "paho", mock_mqtt) | ||
monkeypatch.setitem(sys.modules, "paho.mqtt", mock_mqtt.mqtt) | ||
monkeypatch.setitem(sys.modules, "paho.mqtt.client", mock_mqtt.mqtt.client) |
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.
I think this part is more useful in a fixture, and that all the other tests use that fixture.
as it now changes the global state of modules and doesn't clean it up afterwards.
It will throw import errors as soon as paho.mqtt
is removed.
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.
In https://docs.pytest.org/en/stable/how-to/monkeypatch.html#how-to-monkeypatch-mock-modules-and-environments
it says:
"The modifications will be undone after the requesting test function or fixture has finished" (emphasis mine)
(There is another problem though: the broker defines the client as an instance variable)
Update: "It will throw import errors as soon as paho.mqtt is removed" is indeed a problem.
Added fixture
@Miauwkeru It is no longer required for the test, but it should still be when using the Update: Understand now, dev are the test dependencies |
We remove an import at the top, but now have to import the unit under test in the test itself.
We remove an import at the top, but now have to import the unit under test in the test itself.
Closes #926