[ahawastecollection] Fixed unstable test (#13864)

* Added Constructor parameter to pass custom ScheduledExecutorService, that allows deterministic execution in test cases.
* Fixed some codestyles.

Signed-off-by: Sönke Küper <soenkekueper@gmx.de>

bundles/org.openhab.binding.ahawastecollection/src/main/java/org/openhab/binding/ahawastecollection/internal/AhaWasteCollectionHandler.java

@@ -18,6 +18,7 @@ import java.time.ZonedDateTime;
 import java.util.Collections;
 import java.util.Date;
 import java.util.Map;
+import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
 
@@ -59,20 +60,23 @@ public class AhaWasteCollectionHandler extends BaseThingHandler {
     private final TimeZoneProvider timeZoneProvider;
     private final Logger logger = LoggerFactory.getLogger(AhaWasteCollectionHandler.class);
 
-    private @Nullable AhaWasteCollectionConfiguration config;
     private @Nullable AhaCollectionSchedule collectionSchedule;
 
     private @Nullable ScheduledCompletableFuture<?> dailyJob;
 
     private final AhaCollectionScheduleFactory scheduleFactory;
 
+    private final ScheduledExecutorService executorService;
+
     public AhaWasteCollectionHandler(final Thing thing, final CronScheduler scheduler,
-            final TimeZoneProvider timeZoneProvider, final AhaCollectionScheduleFactory scheduleFactory) {
+            final TimeZoneProvider timeZoneProvider, final AhaCollectionScheduleFactory scheduleFactory,
+            @Nullable final ScheduledExecutorService executorService) {
         super(thing);
         this.cronScheduler = scheduler;
         this.timeZoneProvider = timeZoneProvider;
         this.scheduleFactory = scheduleFactory;
         this.cache = new ExpiringCache<>(Duration.ofMinutes(5), this::loadCollectionDates);
+        this.executorService = executorService == null ? this.scheduler : executorService;
     }
 
     private Map<WasteType, CollectionDate> loadCollectionDates() {
@@ -89,7 +93,7 @@ public class AhaWasteCollectionHandler extends BaseThingHandler {
     @Override
     public void handleCommand(final ChannelUID channelUID, final Command command) {
         if (command instanceof RefreshType) {
-            this.scheduler.execute(this::updateCollectionDates);
+            this.executorService.execute(this::updateCollectionDates);
         } else {
             this.logger.warn("The AHA Abfuhrkalender is a read-only binding and can not handle commands");
         }
@@ -97,13 +101,13 @@ public class AhaWasteCollectionHandler extends BaseThingHandler {
 
     @Override
     public void initialize() {
-        this.config = this.getConfigAs(AhaWasteCollectionConfiguration.class);
+        final AhaWasteCollectionConfiguration config = this.getConfigAs(AhaWasteCollectionConfiguration.class);
 
-        final String commune = this.config.commune;
+        final String commune = config.commune;
-        final String street = this.config.street;
+        final String street = config.street;
-        final String houseNumber = this.config.houseNumber;
+        final String houseNumber = config.houseNumber;
-        final String houseNumberAddon = this.config.houseNumberAddon;
+        final String houseNumberAddon = config.houseNumberAddon;
-        final String collectionPlace = this.config.collectionPlace;
+        final String collectionPlace = config.collectionPlace;
 
         if (commune.isBlank() || street.isBlank() || houseNumber.isBlank() || collectionPlace.isBlank()) {
             this.updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
@@ -116,7 +120,7 @@ public class AhaWasteCollectionHandler extends BaseThingHandler {
 
         this.updateStatus(ThingStatus.UNKNOWN);
 
-        this.scheduler.execute(() -> {
+        this.executorService.execute(() -> {
             final boolean online = this.updateCollectionDates();
             if (online) {
                 this.restartJob();

bundles/org.openhab.binding.ahawastecollection/src/main/java/org/openhab/binding/ahawastecollection/internal/AhaWasteCollectionHandlerFactory.java

@@ -59,16 +59,8 @@ public class AhaWasteCollectionHandlerFactory extends BaseThingHandlerFactory {
         final ThingTypeUID thingTypeUID = thing.getThingTypeUID();
 
         if (THING_TYPE_SCHEDULE.equals(thingTypeUID)) {
-            final AhaCollectionScheduleFactory factory = new AhaCollectionScheduleFactory() {
+            return new AhaWasteCollectionHandler(thing, this.scheduler, this.timeZoneProvider,
-
+                    AhaCollectionScheduleImpl::new, null);
-                @Override
-                public AhaCollectionScheduleImpl create(final String commune, final String street,
-                        final String houseNumber, final String houseNumberAddon, final String collectionPlace) {
-                    return new AhaCollectionScheduleImpl(commune, street, houseNumber, houseNumberAddon,
-                            collectionPlace);
-                }
-            };
-            return new AhaWasteCollectionHandler(thing, this.scheduler, this.timeZoneProvider, factory);
         }
         return null;
     }

bundles/org.openhab.binding.ahawastecollection/src/test/java/org/openhab/binding/ahawastecollection/internal/AhaWasteCollectionHandlerTest.java

@@ -20,11 +20,13 @@ import java.time.ZonedDateTime;
 import java.util.Arrays;
 import java.util.Date;
 import java.util.Map;
+import java.util.concurrent.ScheduledExecutorService;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
+import org.mockito.invocation.InvocationOnMock;
 import org.openhab.core.config.core.Configuration;
 import org.openhab.core.library.types.DateTimeType;
 import org.openhab.core.scheduler.CronJob;
@@ -77,8 +79,8 @@ public class AhaWasteCollectionHandlerTest {
         return new CronScheduler() {
 
             @Override
-            public final ScheduledCompletableFuture<Void> schedule(final CronJob cronJob,
+            public ScheduledCompletableFuture<Void> schedule(final CronJob cronJob, final Map<String, Object> config,
-                    final Map<String, Object> config, final String cronExpression) {
+                    final String cronExpression) {
                 try {
                     cronJob.run(config);
                 } catch (final Exception e) {
@@ -88,7 +90,7 @@ public class AhaWasteCollectionHandlerTest {
             }
 
             @Override
-            public final ScheduledCompletableFuture<Void> schedule(final SchedulerRunnable runnable,
+            public ScheduledCompletableFuture<Void> schedule(final SchedulerRunnable runnable,
                     final String cronExpression) {
                 try {
                     runnable.run();
@@ -100,21 +102,21 @@ public class AhaWasteCollectionHandlerTest {
         };
     }
 
-    private static Thing mockThing(final Configuration config) {
+    private static Thing mockThing() {
         final Thing thing = mock(Thing.class);
         when(thing.getUID())
                 .thenReturn(new ThingUID(AhaWasteCollectionBindingConstants.THING_TYPE_SCHEDULE, "collectionCalendar"));
-        when(thing.getConfiguration()).thenReturn(config);
+        when(thing.getConfiguration()).thenReturn(CONFIG);
 
         final Channel channelBioWaste = mockChannel(thing.getUID(), AhaWasteCollectionBindingConstants.BIOWASTE);
         final Channel channelGeneralWaste = mockChannel(thing.getUID(),
                 AhaWasteCollectionBindingConstants.GENERAL_WASTE);
         final Channel channelPaper = mockChannel(thing.getUID(), AhaWasteCollectionBindingConstants.PAPER);
-        final Channel channelLeightweightPackaging = mockChannel(thing.getUID(),
+        final Channel channelLightweightPackaging = mockChannel(thing.getUID(),
                 AhaWasteCollectionBindingConstants.LEIGHTWEIGHT_PACKAGING);
 
         when(thing.getChannels()).thenReturn(
-                Arrays.asList(channelBioWaste, channelGeneralWaste, channelLeightweightPackaging, channelPaper));
+                Arrays.asList(channelBioWaste, channelGeneralWaste, channelLightweightPackaging, channelPaper));
         return thing;
     }
 
@@ -126,8 +128,15 @@ public class AhaWasteCollectionHandlerTest {
 
     private static AhaWasteCollectionHandler createAndInitHandler(final ThingHandlerCallback callback,
             final Thing thing) {
+        // Executor that executes all commands synchronous.
+        final ScheduledExecutorService executorStub = Mockito.mock(ScheduledExecutorService.class);
+        doAnswer((InvocationOnMock invocation) -> {
+            ((Runnable) invocation.getArguments()[0]).run();
+            return null;
+        }).when(executorStub).execute(any(Runnable.class));
+
         final AhaWasteCollectionHandler handler = new AhaWasteCollectionHandler(thing, createStubScheduler(),
-                ZoneId::systemDefault, new AhaCollectionScheduleStubFactory());
+                ZoneId::systemDefault, new AhaCollectionScheduleStubFactory(), executorStub);
         handler.setCallback(callback);
         handler.initialize();
         return handler;
@@ -140,25 +149,22 @@ public class AhaWasteCollectionHandlerTest {
 
     @Test
     public void testUpdateChannels() {
-        final Thing thing = mockThing(CONFIG);
+        final Thing thing = mockThing();
         final ThingHandlerCallback callback = mock(ThingHandlerCallback.class);
         final AhaWasteCollectionHandler handler = createAndInitHandler(callback, thing);
 
         try {
             verify(callback).statusUpdated(eq(thing), argThat(arg -> arg.getStatus().equals(ThingStatus.UNKNOWN)));
-            verify(callback, timeout(1000)).statusUpdated(eq(thing),
+            verify(callback).statusUpdated(eq(thing), argThat(arg -> arg.getStatus().equals(ThingStatus.ONLINE)));
-                    argThat(arg -> arg.getStatus().equals(ThingStatus.ONLINE)));
+            verify(callback).stateUpdated(new ChannelUID(thing.getUID(), AhaWasteCollectionBindingConstants.BIOWASTE),
-            verify(callback, timeout(1000)).stateUpdated(
-                    new ChannelUID(thing.getUID(), AhaWasteCollectionBindingConstants.BIOWASTE),
                     getDateTime(AhaCollectionScheduleStub.BIO_WASTE_DATE));
-            verify(callback, timeout(1000)).stateUpdated(
+            verify(callback).stateUpdated(
                     new ChannelUID(thing.getUID(), AhaWasteCollectionBindingConstants.GENERAL_WASTE),
                     getDateTime(AhaCollectionScheduleStub.GENERAL_WASTE_DATE));
-            verify(callback, timeout(1000)).stateUpdated(
+            verify(callback).stateUpdated(
                     new ChannelUID(thing.getUID(), AhaWasteCollectionBindingConstants.LEIGHTWEIGHT_PACKAGING),
                     getDateTime(AhaCollectionScheduleStub.LEIGHTWEIGHT_PACKAGING_DATE));
-            verify(callback, timeout(1000)).stateUpdated(
+            verify(callback).stateUpdated(new ChannelUID(thing.getUID(), AhaWasteCollectionBindingConstants.PAPER),
-                    new ChannelUID(thing.getUID(), AhaWasteCollectionBindingConstants.PAPER),
                     getDateTime(AhaCollectionScheduleStub.PAPER_DATE));
         } finally {
             handler.dispose();