openhab / openhab-addons

Add-ons for openHAB
https://www.openhab.org/
Eclipse Public License 2.0
1.86k stars 3.58k forks source link

[mqtt] BrokerHandlerTest tests are failing #12784

Open jlaur opened 2 years ago

jlaur commented 2 years ago

Tests are failing locally as well on build servers:

[main] INFO org.openhab.core.io.transport.mqtt.MqttBrokerConnection - Starting MQTT broker connection to '10.10.0.10' with clientid BrokerHandlerTest
Error:  Tests run: 3, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 13.014 s <<< FAILURE! - in org.openhab.binding.mqtt.handler.BrokerHandlerTest
Error:  org.openhab.binding.mqtt.handler.BrokerHandlerTest.handlerInit  Time elapsed: 10.049 s  <<< FAILURE!
java.lang.AssertionError: 

Expected: is <ONLINE>
     but: was <OFFLINE>
    at org.openhab.binding.mqtt.handler.BrokerHandlerTest.handlerInit(BrokerHandlerTest.java:104)

[INFO] Running org.openhab.binding.mqtt.internal.MQTTTopicDiscoveryServiceTest
[main] INFO org.openhab.core.io.transport.mqtt.MqttBrokerConnection - Starting MQTT broker connection to '10.10.0.10' with clientid BrokerHandlerTest
[main] INFO org.openhab.core.io.transport.mqtt.MqttBrokerConnection - Starting MQTT broker connection to '10.10.0.10' with clientid BrokerHandlerTest
[main] INFO org.openhab.core.io.transport.mqtt.MqttBrokerConnection - Starting MQTT broker connection to '10.10.0.10' with clientid BrokerHandlerTest
[main] INFO org.openhab.core.io.transport.mqtt.MqttBrokerConnection - Starting MQTT broker connection to '10.10.0.10' with clientid BrokerHandlerTest
[INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.175 s - in org.openhab.binding.mqtt.internal.MQTTTopicDiscoveryServiceTest
[INFO] Running org.openhab.binding.mqtt.internal.ssl.PinningSSLContextProviderTest
[INFO] Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.244 s - in org.openhab.binding.mqtt.internal.ssl.PinningSSLContextProviderTest
[INFO] 
[INFO] Results:
[INFO] 
Error:  Failures: 
Error:    BrokerHandlerTest.handlerInit:104 
Expected: is <ONLINE>
     but: was <OFFLINE>
[INFO] 
Error:  Tests run: 12, Failures: 1, Errors: 0, Skipped: 0

Error:  Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.0.0-M5:test (default-test) on project org.openhab.binding.mqtt: There are test failures.
Error:  
Error:  Please refer to /home/runner/work/openhab-addons/openhab-addons/bundles/org.openhab.binding.mqtt/target/surefire-reports for the individual test results.
Error:  Please refer to dump files (if any exist) [date].dump, [date]-jvmRun[N].dump and [date].dumpstream.
Error:  -> [Help 1]
Error:  
Error:  To see the full stack trace of the errors, re-run Maven with the -e switch.
Error:  Re-run Maven using the -X switch to enable full debug logging.
Error:  
Error:  For more information about the errors and possible solutions, please read the following articles:
Error:  [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
Error:  
Error:  After correcting the problems, you can resume the build with the command
Error:    mvn <args> -rf :org.openhab.binding.mqtt
Error: Process completed with exit code 1.

I'm not sure, but it seems these tests are actually starting a listener on 10.10.0.10 port 80? If this is the case, perhaps they should have been integration tests? For reference: #11448

This might also be relevant: #12431

@wborn - can you add anything to this? I guess if they cannot be fixed easily, it would be preferable to temporarily disable them as it seems they are causing all builds to fail, and they have for some time.

wborn commented 2 years ago

Maybe it's caused by https://github.com/openhab/openhab-core/pull/2951?

jlaur commented 2 years ago

@J-N-K - can you assess if this could be related?

jlaur commented 2 years ago

@davidgraeff - as you are author of the test, you might also have an idea. :)

J-N-K commented 2 years ago

Probably TestPortUtil.findFreePort() should be used instead of fixes port 80.

Also the test-design seems to be broken. ArgumentCaptor.getValue() returns the latest captured value. Maybe it would be better to use getAllValues() and check if ONLINE is part of that. Since CONNECTED was reported as connection state (which probably results in ONLINE as thing status even when disconnect() was called, it might be that the last reported state was indeed ONLINE.

In any case it is the test that needs to be fixed, because if https://github.com/openhab/openhab-core/pull/2951 is the cause, then the test relied on a bug to succeed.

wborn commented 2 years ago

I did check that it is caused by https://github.com/openhab/openhab-core/pull/2951, because the test succeeds again when those changes are reverted. Still need to find some more time to see if the test was wrong all the time and how it should be fixed. :wink: