Github user SebastianCarroll commented on the issue:
https://github.com/apache/nifi/pull/2154
Hey @JPercivall! Thanks for the review! What I was referring to with the 3
letter change was in the test class itself not the core code. For example, I
thought about changing the 'ssl' value in
[TestConsumeMqttSSL](https://github.com/apache/nifi/blob/7923fd04c35737df8145b213536bdf333ef72713/nifi-nar-bundles/nifi-mqtt-bundle/nifi-mqtt-processors/src/test/java/org/apache/nifi/processors/mqtt/integration/TestConsumeMqttSSL.java#L74)
assuming that we would want to run all the same tests for wss as to ssl (and
similarly for ws as for tcp). Subclassing common/Test*MqttCommon as you suggest
won't remove this issue as I would essentially be reimplementing all the tests
and setup from TestConsumeMqttSSL. However I could subclass TestConsumeMqttSSL
-> TestConsumeMqttWss (for example) and pull the assigning of the broker out
into a method and just overwrite that in the subclass.
For the changes to MqttTestClient.java, they were autogenerated using
Intellij. I believe this is due to changes in the IMqttClient interface
referenced in the newer version of the Paho dependancy (1.1.1 from 1.0.2). Is
it good practice to leave these as are? It does feel unnecessary to have all
the comments there given they are only implemented to make a stub (or mock? I'm
never sure exactly which is which)
I'm still trying to get the -Pcontrib-check flag to work. I originally
ticked that box in the PR erroneously.
Hope this helps with the review and thanks again!
Seb
---
