-
Notifications
You must be signed in to change notification settings - Fork 3
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
✨ Allowing pytest cli arguments for host and port #8
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,10 +27,12 @@ | |
|
||
|
||
class MqttClientAdapter(threading.Thread): | ||
def __init__(self, on_message_callback: t.Optional[t.Callable] = None): | ||
def __init__(self, on_message_callback: t.Optional[t.Callable] = None, host: str = "localhost", port: int = 1883): | ||
super().__init__() | ||
self.client: mqtt.Client = mqtt.Client() | ||
self.on_message_callback = on_message_callback | ||
self.host = host | ||
self.port = int(port) | ||
self.setup() | ||
|
||
def setup(self): | ||
|
@@ -43,7 +45,7 @@ def setup(self): | |
client.on_message = self.on_message_callback | ||
|
||
logger.debug("[PYTEST] Connecting to MQTT broker") | ||
client.connect("localhost", port=1883) | ||
client.connect(host=self.host, port=self.port) | ||
client.subscribe("#") | ||
|
||
def run(self): | ||
|
@@ -75,12 +77,12 @@ def publish(self, topic: str, payload: str, **kwargs) -> mqtt.MQTTMessageInfo: | |
class MqttCaptureFixture: | ||
"""Provides access and control of log capturing.""" | ||
|
||
def __init__(self, decode_utf8: t.Optional[bool]) -> None: | ||
def __init__(self, decode_utf8: t.Optional[bool], host: str = "localhost", port: int = 1883) -> None: | ||
"""Creates a new funcarg.""" | ||
self._buffer: t.List[MqttMessage] = [] | ||
self._decode_utf8: bool = decode_utf8 | ||
|
||
self.mqtt_client = MqttClientAdapter(on_message_callback=self.on_message) | ||
self.mqtt_client = MqttClientAdapter(on_message_callback=self.on_message, host=host, port=port) | ||
self.mqtt_client.start() | ||
# time.sleep(0.1) | ||
|
||
|
@@ -119,20 +121,22 @@ def publish(self, topic: str, payload: str, **kwargs) -> mqtt.MQTTMessageInfo: | |
|
||
|
||
@pytest.fixture(scope="function") | ||
def capmqtt(request): | ||
def capmqtt(request, mqttcliargs): | ||
"""Access and control MQTT messages.""" | ||
|
||
# Configure `capmqtt` fixture, obtaining the `capmqtt_decode_utf8` setting from | ||
# either a global or module-wide setting, or from a test case marker. | ||
# https://docs.pytest.org/en/7.1.x/how-to/fixtures.html#fixtures-can-introspect-the-requesting-test-context | ||
# https://docs.pytest.org/en/7.1.x/how-to/fixtures.html#using-markers-to-pass-data-to-fixtures | ||
|
||
host, port = mqttcliargs | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead, you may already be able to use the built-in def capmqtt(request, pytestconfig):
host = pytestconfig.getoption("--host")
port = int(pytestconfig.getoption("--port")) -- https://docs.pytest.org/en/8.0.x/reference/reference.html#std-fixture-pytestconfig |
||
capmqtt_decode_utf8 = ( | ||
getattr(request.config.option, "capmqtt_decode_utf8", False) | ||
or getattr(request.module, "capmqtt_decode_utf8", False) | ||
or request.node.get_closest_marker("capmqtt_decode_utf8") is not None | ||
) | ||
Comment on lines
134
to
138
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When acquiring configuration options like this, they can be provided through three different means, see usage of Acquiring |
||
|
||
result = MqttCaptureFixture(decode_utf8=capmqtt_decode_utf8) | ||
result = MqttCaptureFixture(decode_utf8=capmqtt_decode_utf8, host=host, port=port) | ||
delay() | ||
yield result | ||
result.finalize() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
# -*- coding: utf-8 -*- | ||
|
||
# Copyright (c) 2020-2022 Andreas Motl <[email protected]> | ||
# Copyright (c) 2020-2022 Richard Pobering <[email protected]> | ||
# | ||
|
@@ -36,7 +37,10 @@ | |
class Mosquitto(BaseImage): | ||
|
||
name = "mosquitto" | ||
port = 1883 | ||
|
||
def __init__(self, host: str = "localhost", port: int = 1883) -> None: | ||
self.host = host | ||
self.port = port | ||
|
||
def check(self): | ||
# TODO: Add real implementation. | ||
|
@@ -71,16 +75,18 @@ | |
mosquitto_image = Mosquitto() | ||
|
||
|
||
def is_mosquitto_running() -> bool: | ||
return probe_tcp_connect("localhost", 1883) | ||
def is_mosquitto_running(host: str, port: int) -> bool: | ||
return probe_tcp_connect(host, port) | ||
|
||
|
||
@pytest.fixture(scope="session") | ||
def mosquitto(): | ||
def mosquitto(request, mqttcliargs): | ||
|
||
host, port = mqttcliargs | ||
|
||
# Gracefully skip spinning up the Docker container if Mosquitto is already running. | ||
if is_mosquitto_running(): | ||
yield "localhost", 1883 | ||
if is_mosquitto_running(host, port): | ||
yield host, port | ||
return | ||
|
||
# Spin up Mosquitto container. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
import typing as t | ||
|
||
import pytest | ||
|
||
|
||
@pytest.fixture(scope="session") | ||
def mqttcliargs(request) -> t.Tuple[str, int]: | ||
host = request.config.getoption("--mqtt_host", "localhost") | ||
port = int(request.config.getoption("--mqtt_port", 1883)) | ||
amotl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
yield host, port | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We think the Instead, we suggest to use |
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,3 @@ | ||||||||||||||
def pytest_addoption(parser) -> None: | ||||||||||||||
parser.addoption("--mqtt_host", action="store", default="localhost", help="mqtt host to be connected through") | ||||||||||||||
parser.addoption("--mqtt_port", action="store", default=1883, help="mqtt port to be connected through") | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a suggestion about "naming things".
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about conveying
Suggested change
|
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.
We don't think that new
mqttcliargs
fixture needs to be exported for public consumption. Now that the new {host,port} information is also available as attributes onMqttClientAdapter
, it can easily be accessed through thecapmqtt
fixture, right?For improved convenience, they could be mirrored / also accessed by additional properties on
MqttCaptureFixture
, like: