Restore discovery tests

bundles/org.openhab.binding.boschshc/src/main/java/org/openhab/binding/boschshc/internal/discovery/ThingDiscoveryService.java

@@ -18,6 +18,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.concurrent.ScheduledExecutorService;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
@@ -117,6 +118,15 @@ public ThingDiscoveryService() {
         super(BridgeHandler.class, SUPPORTED_THING_TYPES, SEARCH_TIME);
     }
 
+    /**
+     * Constructor for tests only.
+     *
+     * @param scheduler the {@link ScheduledExecutorService} to use during testing.
+     */
+    ThingDiscoveryService(ScheduledExecutorService scheduler) {
+        super(scheduler, BridgeHandler.class, SUPPORTED_THING_TYPES, SEARCH_TIME, true, null, null);
+    }
+
     @Override
     public void initialize() {
         logger.trace("initialize");
@@ -255,7 +265,7 @@ protected void addDevice(Device device, String roomName) {
      * Translates a Bosch device ID to an openHAB-compliant thing ID.
      * <p>
      * Characters that are not allowed in thing IDs are replaced by underscores.
-     * 
+     *
      * @param deviceId the Bosch device ID
      * @return the translated openHAB-compliant thing ID
      */

bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/devices/bridge/LongPollingTest.java

@@ -21,21 +21,12 @@
 
 import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
-import java.util.Collections;
-import java.util.List;
-import java.util.concurrent.AbstractExecutorService;
-import java.util.concurrent.Callable;
-import java.util.concurrent.Delayed;
 import java.util.concurrent.ExecutionException;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.ScheduledFuture;
-import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 import java.util.function.Consumer;
 
 import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.NonNullByDefault;
-import org.eclipse.jdt.annotation.Nullable;
 import org.eclipse.jetty.client.api.Request;
 import org.eclipse.jetty.client.api.Response;
 import org.eclipse.jetty.client.api.Response.CompleteListener;
@@ -58,6 +49,7 @@
 import org.openhab.binding.boschshc.internal.exceptions.BoschSHCException;
 import org.openhab.binding.boschshc.internal.exceptions.LongPollingFailedException;
 import org.openhab.binding.boschshc.internal.tests.common.CommonTestUtils;
+import org.openhab.core.util.SameThreadExecutorService;
 
 import com.google.gson.JsonObject;
 import com.google.gson.JsonSyntaxException;
@@ -72,131 +64,6 @@
 @ExtendWith(MockitoExtension.class)
 class LongPollingTest {
 
-    /**
-     * A dummy implementation of {@link ScheduledFuture}.
-     * <p>
-     * This is required because we can not return <code>null</code> in the executor service test implementation (see
-     * below).
-     *
-     * @author David Pace - Initial contribution
-     *
-     * @param <T> The result type returned by this Future
-     */
-    private static class NullScheduledFuture<T> implements ScheduledFuture<T> {
-
-        @Override
-        public long getDelay(@Nullable TimeUnit unit) {
-            return 0;
-        }
-
-        @Override
-        public int compareTo(@Nullable Delayed o) {
-            return 0;
-        }
-
-        @Override
-        public boolean cancel(boolean mayInterruptIfRunning) {
-            return false;
-        }
-
-        @Override
-        public boolean isCancelled() {
-            return false;
-        }
-
-        @Override
-        public boolean isDone() {
-            return false;
-        }
-
-        @Override
-        public T get() throws InterruptedException, ExecutionException {
-            return null;
-        }
-
-        @Override
-        public T get(long timeout, @Nullable TimeUnit unit)
-                throws InterruptedException, ExecutionException, TimeoutException {
-            return null;
-        }
-    }
-
-    /**
-     * Executor service implementation that runs all runnables in the same thread in order to enable deterministic
-     * testing.
-     *
-     * @author David Pace - Initial contribution
-     *
-     */
-    private static class SameThreadExecutorService extends AbstractExecutorService implements ScheduledExecutorService {
-
-        private volatile boolean terminated;
-
-        @Override
-        public void shutdown() {
-            terminated = true;
-        }
-
-        @NonNullByDefault({})
-        @Override
-        public List<Runnable> shutdownNow() {
-            return Collections.emptyList();
-        }
-
-        @Override
-        public boolean isShutdown() {
-            return terminated;
-        }
-
-        @Override
-        public boolean isTerminated() {
-            return terminated;
-        }
-
-        @Override
-        public boolean awaitTermination(long timeout, @Nullable TimeUnit unit) throws InterruptedException {
-            shutdown();
-            return terminated;
-        }
-
-        @Override
-        public void execute(@Nullable Runnable command) {
-            if (command != null) {
-                // execute in the same thread in unit tests
-                command.run();
-            }
-        }
-
-        @Override
-        public ScheduledFuture<?> schedule(@Nullable Runnable command, long delay, @Nullable TimeUnit unit) {
-            // not used in this tests
-            return new NullScheduledFuture<>();
-        }
-
-        @Override
-        public <V> ScheduledFuture<V> schedule(@Nullable Callable<V> callable, long delay, @Nullable TimeUnit unit) {
-            return new NullScheduledFuture<>();
-        }
-
-        @Override
-        public ScheduledFuture<?> scheduleAtFixedRate(@Nullable Runnable command, long initialDelay, long period,
-                @Nullable TimeUnit unit) {
-            if (command != null) {
-                command.run();
-            }
-            return new NullScheduledFuture<>();
-        }
-
-        @Override
-        public ScheduledFuture<?> scheduleWithFixedDelay(@Nullable Runnable command, long initialDelay, long delay,
-                @Nullable TimeUnit unit) {
-            if (command != null) {
-                command.run();
-            }
-            return new NullScheduledFuture<>();
-        }
-    }
-
     private @NonNullByDefault({}) LongPolling fixture;
 
     private @NonNullByDefault({}) BoschHttpClient httpClient;

bundles/org.openhab.binding.boschshc/src/test/java/org/openhab/binding/boschshc/internal/discovery/ThingDiscoveryServiceTest.java

@@ -20,7 +20,6 @@
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.timeout;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -47,6 +46,7 @@
 import org.openhab.core.config.discovery.DiscoveryService;
 import org.openhab.core.thing.Bridge;
 import org.openhab.core.thing.ThingUID;
+import org.openhab.core.util.SameThreadExecutorService;
 
 /**
  * Unit tests for {@link ThingDiscoveryService}.
@@ -66,7 +66,7 @@ class ThingDiscoveryServiceTest {
 
     @BeforeEach
     void beforeEach() {
-        fixture = new ThingDiscoveryService();
+        fixture = new ThingDiscoveryService(new SameThreadExecutorService());
         fixture.addDiscoveryListener(discoveryListener);
         fixture.setThingHandler(bridgeHandler);
     }
@@ -160,7 +160,7 @@ void testAddDevices() {
         fixture.addDevices(devices, emptyRooms);
 
         // two calls for the two devices expected
-        verify(discoveryListener, timeout(1000L).times(2)).thingDiscovered(any(), any());
+        verify(discoveryListener, times(2)).thingDiscovered(any(), any());
     }
 
     @Test
@@ -186,8 +186,7 @@ void testAddDevice() {
         device.name = "Test Name";
         fixture.addDevice(device, "TestRoom");
 
-        verify(discoveryListener, timeout(1000L)).thingDiscovered(discoveryServiceCaptor.capture(),
-                discoveryResultCaptor.capture());
+        verify(discoveryListener).thingDiscovered(discoveryServiceCaptor.capture(), discoveryResultCaptor.capture());
 
         assertThat(discoveryServiceCaptor.getValue().getClass(), is(ThingDiscoveryService.class));
         DiscoveryResult result = discoveryResultCaptor.getValue();
@@ -228,8 +227,7 @@ private void assertDeviceNiceName(String deviceName, String roomName, String exp
         device.id = "testDevice:ID";
         device.name = deviceName;
         fixture.addDevice(device, roomName);
-        verify(discoveryListener, timeout(1000L)).thingDiscovered(discoveryServiceCaptor.capture(),
-                discoveryResultCaptor.capture());
+        verify(discoveryListener).thingDiscovered(discoveryServiceCaptor.capture(), discoveryResultCaptor.capture());
         assertThat(discoveryServiceCaptor.getValue().getClass(), is(ThingDiscoveryService.class));
         DiscoveryResult result = discoveryResultCaptor.getValue();
         assertThat(result.getLabel(), is(expectedNiceName));

bundles/org.openhab.binding.deconz/src/main/java/org/openhab/binding/deconz/internal/discovery/ThingDiscoveryService.java

@@ -18,6 +18,7 @@
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
@@ -72,6 +73,15 @@ public ThingDiscoveryService() {
         super(DeconzBridgeHandler.class, SUPPORTED_THING_TYPES_UIDS, 30);
     }
 
+    /**
+     * Constructor for tests only.
+     *
+     * @param scheduler the {@link ScheduledExecutorService} to use during testing.
+     */
+    public ThingDiscoveryService(ScheduledExecutorService scheduler) {
+        super(scheduler, DeconzBridgeHandler.class, SUPPORTED_THING_TYPES_UIDS, 30, true, null, null);
+    }
+
     @Override
     public void startScan() {
         thingHandler.getBridgeFullState().thenAccept(fullState -> {

bundles/org.openhab.binding.deconz/src/test/java/org/openhab/binding/deconz/DeconzTest.java

@@ -14,7 +14,7 @@
 
 import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.*;
+import static org.mockito.Mockito.times;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -51,6 +51,7 @@
 import org.openhab.core.library.types.DateTimeType;
 import org.openhab.core.thing.Bridge;
 import org.openhab.core.thing.ThingUID;
+import org.openhab.core.util.SameThreadExecutorService;
 
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
@@ -92,13 +93,13 @@ public void discoveryTest() throws IOException {
 
         Mockito.doAnswer(answer -> CompletableFuture.completedFuture(Optional.of(bridgeFullState))).when(bridgeHandler)
                 .getBridgeFullState();
-        ThingDiscoveryService discoveryService = new ThingDiscoveryService();
+        ThingDiscoveryService discoveryService = new ThingDiscoveryService(new SameThreadExecutorService());
         discoveryService.modified(Map.of(DiscoveryService.CONFIG_PROPERTY_BACKGROUND_DISCOVERY, false));
         discoveryService.setThingHandler(bridgeHandler);
         discoveryService.initialize();
         discoveryService.addDiscoveryListener(discoveryListener);
         discoveryService.startScan();
-        Mockito.verify(discoveryListener, timeout(1000L).times(20)).thingDiscovered(any(), any());
+        Mockito.verify(discoveryListener, times(20)).thingDiscovered(any(), any());
     }
 
     public static <T> T getObjectFromJson(String filename, Class<T> clazz, Gson gson) throws IOException {

bundles/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/internal/discovery/HomematicDeviceDiscoveryService.java

@@ -17,6 +17,7 @@
 import java.util.Set;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
+import java.util.concurrent.ScheduledExecutorService;
 
 import org.eclipse.jdt.annotation.NonNull;
 import org.openhab.binding.homematic.internal.common.HomematicConfig;
@@ -55,6 +56,11 @@ public HomematicDeviceDiscoveryService() {
         super(HomematicBridgeHandler.class, Set.of(new ThingTypeUID(BINDING_ID, "-")), DISCOVER_TIMEOUT_SECONDS, false);
     }
 
+    HomematicDeviceDiscoveryService(ScheduledExecutorService scheduler) {
+        super(scheduler, HomematicBridgeHandler.class, Set.of(new ThingTypeUID(BINDING_ID, "-")),
+                DISCOVER_TIMEOUT_SECONDS, false, null, null);
+    }
+
     @Override
     public void initialize() {
         thingHandler.setDiscoveryService(this);
@@ -149,7 +155,7 @@ private void waitForInstallModeFinished(int timeout) throws InterruptedException
     private void waitForLoadDevicesFinished() throws InterruptedException, ExecutionException {
         Future<?> loadFuture;
         if ((loadFuture = loadDevicesFuture) != null) {
-            loadFuture.get(); // TODO: Bug - doesn't always complete
+            loadFuture.get();
         }
     }