[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #1172: MQTT tests added

[GitBox <gi...@apache.org>]

lordgamez commented on a change in pull request #1172:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1172#discussion_r710981237



##########
File path: docker/test/integration/features/mqtt.feature
##########
@@ -0,0 +1,66 @@
+Feature: Sending data to using MQTT streaming platform using PublishMQTT

Review comment:
       The word 'using' is not needed here

##########
File path: docker/test/integration/resources/mqtt_broker/mosquitto_test.conf
##########
@@ -0,0 +1,874 @@
+# Config file for mosquitto

Review comment:
       There seems to be only 3 lines not commented out in this configuration. Could we only leave those in this test config? I think it would be more clear at glance what is actually set.

##########
File path: docker/test/integration/minifi/core/ImageStore.py
##########
@@ -23,21 +24,28 @@ def cleanup(self):
             self.client.images.remove(image.id, force=True)
 
     def get_image(self, container_engine):
+        if container_engine in self.images:

Review comment:
       Good improvement. With this change I think we should also rename the `__get_.*_image` functions to `__build_.*image`

##########
File path: docker/test/integration/minifi/core/DockerTestCluster.py
##########
@@ -54,13 +55,16 @@ def get_app_log(self, container_name):
 
         return container.status, None
 
-    def wait_for_app_logs(self, container_name, log_entry, timeout_seconds, count=1):
+    def wait_for_app_logs(self, container_name, log_entry, timeout_seconds, count=1, use_regex=False):

Review comment:
       I would prefer having a separate function for log matching and regex matching, the surrounding code could may be extracted as a decorator.

##########
File path: docker/test/integration/minifi/core/MqttBrokerContainer.py
##########
@@ -0,0 +1,23 @@
+import logging
+from .Container import Container
+
+
+class MqttBrokerContainer(Container):
+    def __init__(self, name, vols, network, image_store):
+        super().__init__(name, 'mqtt-broker', vols, network, image_store)
+
+    def get_startup_finished_log_entry(self):
+        return "mosquitto version 2.0.11 running"

Review comment:
       Could this be changed somehow not to be version dependent?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org