[mqtt.generic] Fix JUnit tests (#8921)

* Fix JUnit tests
* remove extra character in copyright comment
* Cleanup mockito warnings

Signed-off-by: Jochen Klein <git@jochen.susca.de>

bundles/org.openhab.binding.mqtt/src/test/java/org/openhab/binding/mqtt/handler/AbstractBrokerHandlerTest.java

@@ -15,7 +15,7 @@ package org.openhab.binding.mqtt.handler;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertNull;
-import static org.mockito.ArgumentMatchers.*;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.*;
 
 import java.util.Collections;
@@ -30,7 +30,6 @@ import org.mockito.quality.Strictness;
 import org.openhab.binding.mqtt.internal.MqttThingID;
 import org.openhab.core.config.core.Configuration;
 import org.openhab.core.io.transport.mqtt.MqttBrokerConnection;
-import org.openhab.core.io.transport.mqtt.MqttConnectionState;
 import org.openhab.core.io.transport.mqtt.MqttException;
 import org.openhab.core.io.transport.mqtt.MqttService;
 import org.openhab.core.thing.Bridge;
@@ -56,7 +55,6 @@ public class AbstractBrokerHandlerTest {
 
     @BeforeEach
     public void setUp() {
-        doReturn(MqttThingID.getThingUID(HOST, PORT)).when(thing).getUID();
         doReturn(new Configuration(Collections.singletonMap("brokerid", MqttThingID.getThingUID(HOST, PORT).getId())))
                 .when(thing).getConfiguration();
         handler = new SystemBrokerHandler(thing, service);
@@ -68,7 +66,6 @@ public class AbstractBrokerHandlerTest {
     @Test
     public void brokerAddedWrongID() throws ConfigurationException, MqttException {
         MqttBrokerConnection brokerConnection = mock(MqttBrokerConnection.class);
-        when(brokerConnection.connectionState()).thenReturn(MqttConnectionState.CONNECTED);
         handler.brokerAdded("nonsense_id", brokerConnection);
         assertNull(handler.connection);
         // We do not expect a status change, because brokerAdded will do nothing with invalid connections.
@@ -89,7 +86,6 @@ public class AbstractBrokerHandlerTest {
     public void brokerAdded() throws ConfigurationException, MqttException {
         MqttBrokerConnectionEx connection = spy(
                 new MqttBrokerConnectionEx("10.10.0.10", 80, false, "BrokerHandlerTest"));
-        doReturn(connection).when(service).getBrokerConnection(eq(handler.brokerID));
 
         verify(callback, times(0)).statusUpdated(any(), any());
         handler.brokerAdded(handler.brokerID, connection);

bundles/org.openhab.binding.mqtt/src/test/java/org/openhab/binding/mqtt/handler/BrokerHandlerTest.java

@@ -30,7 +30,6 @@ import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.mockito.junit.jupiter.MockitoSettings;
 import org.mockito.quality.Strictness;
-import org.openhab.binding.mqtt.internal.MqttThingID;
 import org.openhab.core.config.core.Configuration;
 import org.openhab.core.io.transport.mqtt.MqttBrokerConnection;
 import org.openhab.core.io.transport.mqtt.MqttConnectionState;
@@ -63,7 +62,6 @@ public class BrokerHandlerTest {
     @BeforeEach
     public void setUp() throws ConfigurationException, MqttException {
         scheduler = new ScheduledThreadPoolExecutor(1);
-        when(thing.getUID()).thenReturn(MqttThingID.getThingUID("10.10.0.10", 80));
         connection = spy(new MqttBrokerConnectionEx("10.10.0.10", 80, false, "BrokerHandlerTest"));
         connection.setTimeoutExecutor(scheduler, 10);
         connection.setConnectionCallback(connection);

bundles/org.openhab.binding.mqtt/src/test/java/org/openhab/binding/mqtt/handler/MqttAsyncClientWrapperEx.java

@@ -0,0 +1,88 @@
+/**
+ * Copyright (c) 2010-2020 Contributors to the openHAB project
+ *
+ * See the NOTICE file(s) distributed with this work for additional
+ * information.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+package org.openhab.binding.mqtt.handler;
+
+import java.util.concurrent.CompletableFuture;
+
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
+import org.openhab.core.io.transport.mqtt.MqttConnectionState;
+import org.openhab.core.io.transport.mqtt.MqttWillAndTestament;
+import org.openhab.core.io.transport.mqtt.internal.Subscription;
+import org.openhab.core.io.transport.mqtt.internal.client.MqttAsyncClientWrapper;
+
+import com.hivemq.client.mqtt.MqttClientState;
+
+/**
+ * We need an extended MqttAsyncClientWrapper, that will, in respect to the success flags of the connection, immediately
+ * succeed or fail with publish, subscribe, unsubscribe, connect, disconnect.
+ *
+ * @author Jochen Klein - Initial contribution
+ */
+@NonNullByDefault
+public class MqttAsyncClientWrapperEx extends MqttAsyncClientWrapper {
+
+    private final MqttBrokerConnectionEx connection;
+
+    public MqttAsyncClientWrapperEx(MqttBrokerConnectionEx connection) {
+        super();
+        this.connection = connection;
+    }
+
+    @Override
+    public CompletableFuture<?> connect(@Nullable MqttWillAndTestament lwt, int keepAliveInterval,
+            @Nullable String username, @Nullable String password) {
+        if (!connection.connectTimeout) {
+            connection.getCallback().onConnected(null);
+            connection.connectionStateOverwrite = MqttConnectionState.CONNECTED;
+            return CompletableFuture.completedFuture(null);
+        }
+        return new CompletableFuture<>();
+    }
+
+    @Override
+    public CompletableFuture<@Nullable Void> disconnect() {
+        if (connection.disconnectSuccess) {
+            connection.getCallback().onDisconnected(new Throwable("disconnect called"));
+            connection.connectionStateOverwrite = MqttConnectionState.DISCONNECTED;
+            return CompletableFuture.completedFuture(null);
+        }
+        return new CompletableFuture<>();
+    }
+
+    @Override
+    public MqttClientState getState() {
+        return MqttClientState.CONNECTED;
+    }
+
+    @Override
+    public CompletableFuture<?> publish(String topic, byte[] payload, boolean retain, int qos) {
+        return CompletableFuture.completedFuture(null);
+    }
+
+    @Override
+    public CompletableFuture<?> subscribe(String topic, int qos, Subscription subscription) {
+        if (connection.subscribeSuccess) {
+            return CompletableFuture.completedFuture(null);
+        }
+        return CompletableFuture.failedFuture(new Throwable("subscription failed"));
+    }
+
+    @Override
+    public CompletableFuture<?> unsubscribe(String topic) {
+        if (connection.unsubscribeSuccess) {
+            return CompletableFuture.completedFuture(null);
+        }
+        return CompletableFuture.failedFuture(new Throwable("unsubscription failed"));
+    }
+}

bundles/org.openhab.binding.mqtt/src/test/java/org/openhab/binding/mqtt/handler/MqttBrokerConnectionEx.java

@@ -12,11 +12,9 @@
  */
 package org.openhab.binding.mqtt.handler;
 
-import static org.mockito.ArgumentMatchers.*;
+import static org.mockito.Mockito.spy;
-import static org.mockito.Mockito.*;
 
 import java.util.Map;
-import java.util.concurrent.CompletableFuture;
 
 import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.NonNullByDefault;
@@ -26,8 +24,6 @@ import org.openhab.core.io.transport.mqtt.MqttConnectionState;
 import org.openhab.core.io.transport.mqtt.internal.Subscription;
 import org.openhab.core.io.transport.mqtt.internal.client.MqttAsyncClientWrapper;
 
-import com.hivemq.client.mqtt.MqttClientState;
-
 /**
  * We need an extended MqttBrokerConnection to overwrite the protected `connectionCallbacks` with
  * an instance that takes the mocked version of `MqttBrokerConnection` and overwrite the connection state.
@@ -59,51 +55,13 @@ public class MqttBrokerConnectionEx extends MqttBrokerConnection {
         return subscribers;
     }
 
+    public ConnectionCallback getCallback() {
+        return connectionCallback;
+    }
+
     @Override
     protected MqttAsyncClientWrapper createClient() {
-        MqttAsyncClientWrapper mockedClient = mock(MqttAsyncClientWrapper.class);
+        return new MqttAsyncClientWrapperEx(this);
-        // connect
-        doAnswer(i -> {
-            if (!connectTimeout) {
-                connectionCallback.onConnected(null);
-                connectionStateOverwrite = MqttConnectionState.CONNECTED;
-                return CompletableFuture.completedFuture(null);
-            }
-            return new CompletableFuture<>();
-        }).when(mockedClient).connect(any(), anyInt(), any(), any());
-        doAnswer(i -> {
-            if (disconnectSuccess) {
-                connectionCallback.onDisconnected(new Throwable("disconnect called"));
-                connectionStateOverwrite = MqttConnectionState.DISCONNECTED;
-                return CompletableFuture.completedFuture(null);
-            }
-            return new CompletableFuture<>();
-        }).when(mockedClient).disconnect();
-        // subscribe
-        doAnswer(i -> {
-            if (subscribeSuccess) {
-                return CompletableFuture.completedFuture(null);
-            } else {
-                CompletableFuture<Void> future = new CompletableFuture<>();
-                future.completeExceptionally(new Throwable("subscription failed"));
-                return future;
-            }
-        }).when(mockedClient).subscribe(any(), anyInt(), any());
-        // unsubscribe
-        doAnswer(i -> {
-            if (unsubscribeSuccess) {
-                return CompletableFuture.completedFuture(null);
-            } else {
-                CompletableFuture<Void> future = new CompletableFuture<>();
-                future.completeExceptionally(new Throwable("unsubscription failed"));
-                return future;
-            }
-        }).when(mockedClient).unsubscribe(any());
-        // state
-        doAnswer(i -> {
-            return MqttClientState.CONNECTED;
-        }).when(mockedClient).getState();
-        return mockedClient;
     }
 
     @Override

bundles/org.openhab.binding.mqtt/src/test/java/org/openhab/binding/mqtt/internal/MQTTTopicDiscoveryServiceTest.java

@@ -12,36 +12,39 @@
  */
 package org.openhab.binding.mqtt.internal;
 
-import static org.junit.Assert.assertTrue;
+import static org.hamcrest.CoreMatchers.hasItem;
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.*;
 
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
 
-import org.openhab.core.config.core.Configuration;
+import org.junit.jupiter.api.AfterEach;
-import org.openhab.core.thing.Bridge;
+import org.junit.jupiter.api.BeforeEach;
-import org.openhab.core.thing.binding.ThingHandlerCallback;
+import org.junit.jupiter.api.Test;
-import org.openhab.core.io.transport.mqtt.MqttException;
+import org.junit.jupiter.api.extension.ExtendWith;
-import org.openhab.core.io.transport.mqtt.MqttService;
-import org.openhab.core.io.transport.mqtt.internal.Subscription;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
 import org.mockito.Mock;
-import org.mockito.MockitoAnnotations;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.mockito.junit.jupiter.MockitoSettings;
+import org.mockito.quality.Strictness;
 import org.openhab.binding.mqtt.discovery.MQTTTopicDiscoveryParticipant;
 import org.openhab.binding.mqtt.discovery.MQTTTopicDiscoveryService;
 import org.openhab.binding.mqtt.handler.BrokerHandler;
 import org.openhab.binding.mqtt.handler.BrokerHandlerEx;
 import org.openhab.binding.mqtt.handler.MqttBrokerConnectionEx;
-import org.osgi.service.cm.ConfigurationException;
+import org.openhab.core.config.core.Configuration;
+import org.openhab.core.io.transport.mqtt.MqttService;
+import org.openhab.core.thing.Bridge;
+import org.openhab.core.thing.binding.ThingHandlerCallback;
 
 /**
  * Test cases for the {@link MQTTTopicDiscoveryService} service.
  *
  * @author David Graeff - Initial contribution
  */
+@ExtendWith(MockitoExtension.class)
+@MockitoSettings(strictness = Strictness.WARN)
 public class MQTTTopicDiscoveryServiceTest {
     private ScheduledExecutorService scheduler;
 
@@ -63,10 +66,9 @@ public class MQTTTopicDiscoveryServiceTest {
 
     private BrokerHandler handler;
 
-    @Before
+    @BeforeEach
-    public void setUp() throws ConfigurationException, MqttException {
+    public void setUp() {
         scheduler = new ScheduledThreadPoolExecutor(1);
-        MockitoAnnotations.initMocks(this);
 
         when(thing.getUID()).thenReturn(MqttThingID.getThingUID("10.10.0.10", 80));
         connection = spy(new MqttBrokerConnectionEx("10.10.0.10", 80, false, "BrokerHandlerTest"));
@@ -84,7 +86,7 @@ public class MQTTTopicDiscoveryServiceTest {
         subject = new MqttBrokerHandlerFactory(mqttService);
     }
 
-    @After
+    @AfterEach
     public void tearDown() {
         scheduler.shutdownNow();
     }
@@ -96,10 +98,10 @@ public class MQTTTopicDiscoveryServiceTest {
 
         subject.subscribe(listener, "topic");
         subject.createdHandler(handler);
-        assertTrue(subject.discoveryTopics.get("topic").contains(listener));
+        assertThat(subject.discoveryTopics.get("topic"), hasItem(listener));
         // Simulate receiving
         final byte[] bytes = "TEST".getBytes();
-        connection.getSubscribers().get("topic").forEach(s -> s.processMessage("topic", bytes));
+        connection.getSubscribers().get("topic").messageArrived("topic", bytes, false);
         verify(listener).receivedMessage(eq(thing.getUID()), eq(connection), eq("topic"), eq(bytes));
     }
 
@@ -110,11 +112,11 @@ public class MQTTTopicDiscoveryServiceTest {
 
         subject.createdHandler(handler);
         subject.subscribe(listener, "topic");
-        assertTrue(subject.discoveryTopics.get("topic").contains(listener));
+        assertThat(subject.discoveryTopics.get("topic"), hasItem(listener));
 
         // Simulate receiving
         final byte[] bytes = "TEST".getBytes();
-        connection.getSubscribers().get("topic").forEach(s -> s.processMessage("topic", bytes));
+        connection.getSubscribers().get("topic").messageArrived("topic", bytes, false);
         verify(listener).receivedMessage(eq(thing.getUID()), eq(connection), eq("topic"), eq(bytes));
     }
 
@@ -122,7 +124,7 @@ public class MQTTTopicDiscoveryServiceTest {
     public void handlerInitializeAfterSubscribe() {
         subject.createdHandler(handler);
         subject.subscribe(listener, "topic");
-        assertTrue(subject.discoveryTopics.get("topic").contains(listener));
+        assertThat(subject.discoveryTopics.get("topic"), hasItem(listener));
 
         // Init handler -> create connection
         handler.initialize();
@@ -130,9 +132,7 @@ public class MQTTTopicDiscoveryServiceTest {
 
         // Simulate receiving
         final byte[] bytes = "TEST".getBytes();
-
+        connection.getSubscribers().get("topic").messageArrived("topic", bytes, false);
-        connection.getSubscribers().getOrDefault("topic", new Subscription("topic"))
-                .forEach(s -> s.processMessage("topic", bytes));
         verify(listener).receivedMessage(eq(thing.getUID()), eq(connection), eq("topic"), eq(bytes));
     }
 
@@ -143,12 +143,11 @@ public class MQTTTopicDiscoveryServiceTest {
 
         subject.createdHandler(handler);
         subject.subscribe(listener, "topic");
-        assertTrue(subject.discoveryTopics.get("topic").contains(listener));
+        assertThat(subject.discoveryTopics.get("topic"), hasItem(listener));
 
         // Simulate receiving
         final byte[] bytes = "".getBytes();
-        connection.getSubscribers().getOrDefault("topic", new Subscription("topic"))
+        connection.getSubscribers().get("topic").messageArrived("topic", bytes, false);
-                .forEach(s -> s.processMessage("topic", bytes));
         verify(listener).topicVanished(eq(thing.getUID()), eq(connection), eq("topic"));
     }
 }

bundles/org.openhab.binding.mqtt/src/test/java/org/openhab/binding/mqtt/internal/discovery/ServiceDiscoveryServiceTest.java

@@ -21,8 +21,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.TreeMap;
 
-import javax.naming.ConfigurationException;
-
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -50,7 +48,7 @@ public class ServiceDiscoveryServiceTest {
     private @Mock DiscoveryListener discoverListener;
 
     @BeforeEach
-    public void initMocks() throws ConfigurationException {
+    public void initMocks() {
         Map<String, MqttBrokerConnection> brokers = new TreeMap<>();
         brokers.put("testname", new MqttBrokerConnection("tcp://123.123.123.123", null, false, null));
         brokers.put("textual", new MqttBrokerConnection("tcp://123.123.123.123", null, true, null));
@@ -58,7 +56,7 @@ public class ServiceDiscoveryServiceTest {
     }
 
     @Test
-    public void testDiscovery() throws ConfigurationException {
+    public void testDiscovery() {
         // Setting the MqttService will enable the background scanner
         MqttServiceDiscoveryService d = new MqttServiceDiscoveryService();
         d.addDiscoveryListener(discoverListener);