ci: fix NeonBeeTestBaseTest on GitHub

After many CI related fixes, the NeonBeeTestBaseTest was the last test that did fail on the GitHub workflow, for a long thought inexplicable reason. The root cause finally identfied, revealed a series of yet unfound issues.

The reason why the NeonBeeTestBaseTest failed in the first place, is that if a custom provideUserPrincipal method was provided in a test, the NeonBeeTestBase attempted to replace the ServerVerticle with a custom ServerVerticle that was invoking provideUserPrincipal to retrieve the user principal it should return. However due to an issue causing the port to already be blocked for the second deployment of ServerVerticle, the setUp of NeonBeeTestBase failed. Due to a bug in the setUp code, the CountDownLatch used to stop JUnit from invoking more `@BeforeEach` methods, before NeonBee was initialized, the thread was blocked forever and JUnit (as well as any built-in logic in the `@Timeout` annotation) was not able to finish the test and the execution stalled.

This commit fixes all issues related to this behaviour by:

- check all latches and make sure they ALWAYS use timeouts
- have NeonBee not close Vert.x that had been supplied from the outside
- make injecting a custom user principal more reslient to failure

src/main/java/io/neonbee/NeonBee.java

@@ -195,7 +195,7 @@ public static Future<NeonBee> create() {
      * @return the future to a new NeonBee instance initialized with default options and a new Vert.x instance
      */
     public static Future<NeonBee> create(NeonBeeOptions options) {
-        return create(() -> newVertx(options), options);
+        return create((OwnVertxSupplier) () -> newVertx(options), options);
     }
 
     @VisibleForTesting
@@ -222,9 +222,16 @@ static Future<NeonBee> create(Supplier<Future<Vertx>> vertxFutureSupplier, NeonB
             // at this point at any failure that occurs, it is in our responsibility to properly close down the created
             // Vert.x instance again. we have to be vigilant the fact that a runtime exception could happen anytime!
             Function<Throwable, Future<Void>> closeVertx = throwable -> {
+                if (!(vertxFutureSupplier instanceof OwnVertxSupplier)) {
+                    // the Vert.x instance is *not* owned by us, thus don't close it either
+                    logger.error("Failure during bootstrap phase.", throwable); // NOPMD slf4j
+                    return failedFuture(throwable);
+                }
+
                 // the instance has been created, but after initialization some post-initialization
                 // tasks went wrong, stop Vert.x again. This will also call the close hook and clean up.
                 logger.error("Failure during bootstrap phase. Shutting down Vert.x instance.", throwable);
+
                 // we wait for Vert.x to close, before we propagate the reason why booting failed
                 return vertx.close().transform(closeResult -> failedFuture(throwable));
             };
@@ -583,4 +590,11 @@ public void registerLocalConsumer(String verticleAddress) {
     public void unregisterLocalConsumer(String verticleAddress) {
         localConsumers.remove(verticleAddress);
     }
+
+    /**
+     * Hidden marker supplier interface, that indicates to the boot-stage that an own Vert.x instance was created and we
+     * must be held responsible responsible to close it again.
+     */
+    @VisibleForTesting
+    interface OwnVertxSupplier extends Supplier<Future<Vertx>> {}
 }

src/test/java/io/neonbee/NeonBeeExtension.java

@@ -45,11 +45,11 @@
 public class NeonBeeExtension implements ParameterResolver, BeforeTestExecutionCallback, AfterTestExecutionCallback,
         BeforeEachCallback, AfterEachCallback, BeforeAllCallback, AfterAllCallback {
 
-    private static final Logger LOGGER = LoggerFactory.getLogger(NeonBeeExtension.class);
+    public static final int DEFAULT_TIMEOUT_DURATION = 60;
 
-    private static final int DEFAULT_TIMEOUT_DURATION = 60;
+    public static final TimeUnit DEFAULT_TIMEOUT_UNIT = TimeUnit.SECONDS;
 
-    private static final TimeUnit DEFAULT_TIMEOUT_UNIT = TimeUnit.SECONDS;
+    private static final Logger LOGGER = LoggerFactory.getLogger(NeonBeeExtension.class);
 
     private static class ContextList extends ArrayList<VertxTestContext> {
 
@@ -225,34 +225,39 @@ public T get() {
         }
     }
 
-    private NeonBee createNeonBee(NeonBeeOptions neonBeeOptions) {
+    private NeonBee createNeonBee(NeonBeeOptions options) {
         CountDownLatch latch = new CountDownLatch(1);
-        AtomicReference<NeonBee> neonBeeHolder = new AtomicReference<>();
-        LOGGER.info("Before NeonBee init.");
-        NeonBee.create(neonBeeOptions).onComplete(ar -> {
-            LOGGER.info("NeonBee AsyncResult Handler. {}", ar.succeeded());
+        AtomicReference<NeonBee> neonBeeBox = new AtomicReference<>();
+        AtomicReference<Throwable> errorBox = new AtomicReference<>();
+
+        NeonBee.create(options).onComplete(ar -> {
             if (ar.succeeded()) {
-                neonBeeHolder.set(ar.result());
+                neonBeeBox.set(ar.result());
             } else {
-                LOGGER.error("Error while initializing NeonBee.", ar.cause()); // NOPMD slf4j
+                errorBox.set(ar.cause());
             }
+
             latch.countDown();
         });
 
         try {
-            LOGGER.info("Before CountDownLatch wait.");
-            if (latch.await(DEFAULT_TIMEOUT_DURATION, DEFAULT_TIMEOUT_UNIT)) {
-                LOGGER.info("After CountDownLatch wait.");
-                NeonBee neonBee = neonBeeHolder.get();
-                LOGGER.info("NeonBee Result {}.", neonBee);
-                if (neonBee != null) {
-                    return neonBee;
-                }
+            if (!latch.await(DEFAULT_TIMEOUT_DURATION, DEFAULT_TIMEOUT_UNIT)) {
+                throw new VertxException(new TimeoutException("Failed to initialize NeonBee in time"));
             }
         } catch (InterruptedException e) {
-            LOGGER.error("NeonBee initialization failed.", e);
+            throw new VertxException("Got interrupted when initializing NeonBee", e);
         }
-        throw new VertxException("NeonBee initialization failed.");
+
+        Throwable throwable = errorBox.get();
+        if (throwable != null) {
+            if (throwable instanceof RuntimeException) {
+                throw (RuntimeException) throwable;
+            } else {
+                throw new VertxException("Could not create NeonBee", throwable);
+            }
+        }
+
+        return neonBeeBox.get();
     }
 
     @SuppressWarnings("FutureReturnValueIgnored")
@@ -289,9 +294,11 @@ private ThrowingConsumer<NeonBee> closeNeonBee() {
                 }
                 latch.countDown();
             });
+
             if (!latch.await(DEFAULT_TIMEOUT_DURATION, DEFAULT_TIMEOUT_UNIT)) {
                 throw new TimeoutException("Closing the Vertx context timed out");
             }
+
             Throwable throwable = errorBox.get();
             if (throwable != null) {
                 if (throwable instanceof Exception) {

src/test/java/io/neonbee/NeonBeeTest.java

@@ -1,17 +1,27 @@
 package io.neonbee;
 
 import static com.google.common.truth.Truth.assertThat;
+import static io.neonbee.NeonBeeMockHelper.defaultVertxMock;
+import static io.neonbee.NeonBeeMockHelper.registerNeonBeeMock;
 import static io.neonbee.NeonBeeProfile.ALL;
 import static io.neonbee.NeonBeeProfile.CORE;
 import static io.neonbee.NeonBeeProfile.INCUBATOR;
 import static io.neonbee.NeonBeeProfile.STABLE;
 import static io.neonbee.internal.helper.StringHelper.EMPTY;
 import static io.neonbee.test.helper.OptionsHelper.defaultOptions;
+import static io.vertx.core.Future.failedFuture;
+import static io.vertx.core.Future.succeededFuture;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 import java.lang.reflect.Method;
+import java.nio.file.Files;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
+import java.util.function.BiConsumer;
+import java.util.function.Supplier;
 
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Disabled;
@@ -21,18 +31,21 @@
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mockito;
 
+import io.neonbee.NeonBee.OwnVertxSupplier;
 import io.neonbee.config.NeonBeeConfig;
 import io.neonbee.internal.tracking.MessageDirection;
 import io.neonbee.internal.tracking.TrackingDataLoggingStrategy;
 import io.neonbee.internal.tracking.TrackingInterceptor;
 import io.neonbee.test.base.NeonBeeTestBase;
 import io.neonbee.test.helper.WorkingDirectoryBuilder;
 import io.vertx.core.AbstractVerticle;
+import io.vertx.core.Future;
 import io.vertx.core.Handler;
 import io.vertx.core.Vertx;
 import io.vertx.core.eventbus.DeliveryContext;
 import io.vertx.core.eventbus.EventBus;
 import io.vertx.core.json.JsonObject;
+import io.vertx.junit5.Checkpoint;
 import io.vertx.junit5.Timeout;
 import io.vertx.junit5.VertxTestContext;
 
@@ -126,8 +139,8 @@ void testRegisterAndUnregisterLocalConsumer() {
     @Test
     @DisplayName("Vert.x should add eventbus interceptors.")
     void testDecorateEventbus() throws Exception {
-        Vertx vertx = NeonBeeMockHelper.defaultVertxMock();
-        NeonBee neonBee = NeonBeeMockHelper.registerNeonBeeMock(vertx,
+        Vertx vertx = defaultVertxMock();
+        NeonBee neonBee = registerNeonBeeMock(vertx,
                 new NeonBeeConfig(new JsonObject().put("trackingDataHandlingStrategy", "wrongvalue")));
         EventBus eventBus = mock(EventBus.class);
         when(vertx.eventBus()).thenReturn(eventBus);
@@ -158,6 +171,44 @@ void testFilterByProfile() {
         assertThat(NeonBee.filterByAutoDeployAndProfiles(SystemVerticle.class, List.of(ALL))).isFalse();
     }
 
+    @Test
+    @Timeout(value = 10, timeUnit = TimeUnit.SECONDS)
+    @DisplayName("NeonBee should close only self-owned Vert.x instances if boot fails")
+    void testCloseVertxOnError(VertxTestContext testContext) {
+        Checkpoint checkpoint = testContext.checkpoint(3);
+
+        BiConsumer<Boolean, Boolean> check = (ownVertx, closeFails) -> {
+            Vertx failingVertxMock = mock(Vertx.class);
+            when(failingVertxMock.fileSystem()).thenThrow(new RuntimeException("Failing Vert.x!"));
+            when(failingVertxMock.close()).thenReturn(closeFails ? failedFuture("ANY FAILURE!!") : succeededFuture());
+
+            Supplier<Future<Vertx>> vertxSupplier;
+            if (ownVertx) {
+                vertxSupplier = (OwnVertxSupplier) () -> succeededFuture(failingVertxMock);
+            } else {
+                vertxSupplier = () -> succeededFuture(failingVertxMock);
+            }
+
+            NeonBee.create(vertxSupplier, defaultOptions()).onComplete(testContext.failing(throwable -> {
+                testContext.verify(() -> {
+                    // assert hat it is always
+                    assertThat(throwable.getMessage()).isEqualTo("Failing Vert.x!");
+                    verify(failingVertxMock, times(ownVertx ? 1 : 0)).close();
+                    checkpoint.flag();
+                });
+            }));
+        };
+
+        // fail the boot, but close Vert.x fine and ensure a Vert.x that is NOT owned by the outside is closed
+        check.accept(true, false);
+
+        // fail the boot and assure that Vert.x is not closed for an instance that is provided from the outside
+        check.accept(false, false);
+
+        // fail the boot and also the Vert.x close
+        check.accept(true, true);
+    }
+
     @NeonBeeDeployable(profile = CORE)
     private static class CoreVerticle extends AbstractVerticle {
         // empty class (comment needed as spotless formatter works different on windows)

src/test/java/io/neonbee/test/base/NeonBeeTestBase.java

@@ -1,10 +1,14 @@
 package io.neonbee.test.base;
 
+import static io.neonbee.NeonBeeExtension.DEFAULT_TIMEOUT_DURATION;
+import static io.neonbee.NeonBeeExtension.DEFAULT_TIMEOUT_UNIT;
+import static io.neonbee.NeonBeeProfile.NO_WEB;
+import static io.neonbee.NeonBeeProfile.WEB;
 import static io.neonbee.internal.helper.ConfigHelper.readConfig;
 import static io.neonbee.test.helper.OptionsHelper.defaultOptions;
 import static io.neonbee.test.helper.WorkingDirectoryBuilder.readDeploymentOptions;
+import static io.neonbee.test.helper.WorkingDirectoryBuilder.writeDeploymentOptions;
 import static io.vertx.core.Future.failedFuture;
-import static io.vertx.core.Future.succeededFuture;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -17,6 +21,8 @@
 import java.util.Optional;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import org.apache.olingo.commons.api.edm.FullQualifiedName;
 import org.junit.jupiter.api.AfterEach;
@@ -26,6 +32,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.io.Resources;
 
 import io.neonbee.NeonBee;
@@ -46,6 +53,7 @@
 import io.neonbee.test.helper.DummyVerticleHelper.DummyDataVerticleFactory;
 import io.neonbee.test.helper.DummyVerticleHelper.DummyEntityVerticleFactory;
 import io.neonbee.test.helper.FileSystemHelper;
+import io.neonbee.test.helper.SystemHelper;
 import io.neonbee.test.helper.WorkingDirectoryBuilder;
 import io.neonbee.test.listeners.StaleVertxChecker;
 import io.vertx.core.DeploymentOptions;
@@ -71,6 +79,12 @@
 public class NeonBeeTestBase {
     private static final Logger LOGGER = LoggerFactory.getLogger(NeonBeeTestBase.class);
 
+    // create a dummy JsonObject indicating that the provideUserPrincipal method was not overridden. depending on the
+    // test case the provideUserPrincipal method maybe should return null in some cases, thus we need another way to
+    // check if we should deploy the dummy verticle. this private object can never be returned by anyone overriding the
+    // method, thus if any other value than this is returned, we can be sure a sub-class overrode the method
+    private static final JsonObject NO_USER_PRINCIPAL = new JsonObject();
+
     private Path workingDirPath;
 
     private NeonBee neonBee;
@@ -79,6 +93,7 @@ public class NeonBeeTestBase {
 
     @BeforeEach
     @Timeout(value = 5, timeUnit = TimeUnit.SECONDS)
+    @SuppressWarnings("ReferenceEquality")
     public void setUp(Vertx vertx, VertxTestContext testContext, TestInfo testInfo) throws Exception {
         // associate the Vert.x instance to the current test (unfortunately the only "identifier" that is shared between
         // TestInfo and TestIdentifier is the display name)
@@ -93,6 +108,22 @@ public void setUp(Vertx vertx, VertxTestContext testContext, TestInfo testInfo)
         adaptOptions(testInfo, options);
         options.setWorkingDirectory(workingDirPath);
 
+        // probe for a custom user principal
+        AtomicBoolean customUserPrincipal = new AtomicBoolean(false);
+        if (provideUserPrincipal(testInfo) != NO_USER_PRINCIPAL) { // do NOT use equals here! compare references!
+            if (!WEB.isActive(options.getActiveProfiles())) {
+                testContext.failNow(new IllegalStateException(
+                        "A custom user principal can only be set, if the WEB profile is active!"));
+                return;
+            }
+
+            // add the NO_WEB profile to the active profiles list, this way we won't have to undeploy the ServerVerticle
+            // again later on and can deploy our dummy ServerVerticle right away
+            options.setActiveProfiles(new ImmutableList.Builder<NeonBeeProfile>().add(NO_WEB)
+                    .addAll(options.getActiveProfiles()).build());
+            customUserPrincipal.set(true);
+        }
+
         URL defaultLogbackConfig = Resources.getResource(NeonBeeTestBase.class, "NeonBeeTestBase-Logback.xml");
         try (InputStream is = Resources.asByteSource(defaultLogbackConfig).openStream()) {
             Files.copy(is, options.getConfigDirectory().resolve("logback.xml"));
@@ -113,26 +144,43 @@ public void setUp(Vertx vertx, VertxTestContext testContext, TestInfo testInfo)
             } else {
                 neonBee = asyncNeonBee.result();
 
-                DeploymentOptions serverVerticleOptions = readDeploymentOptions(ServerVerticle.class, workingDirPath);
+                if (customUserPrincipal.get()) {
+                    // use a dummy ServerVerticle that returns the principal by calling the provideUserPrincipal method
+                    DeploymentOptions serverVerticleOptions =
+                            readDeploymentOptions(ServerVerticle.class, workingDirPath);
 
-                Optional.ofNullable(provideUserPrincipal(testInfo)).map(userPrincipal -> {
-                    // Replace current ServerVerticle with a dummy ServerVerticle that also has a dummy AuthHandler to
-                    // provide the user principal specified in the provideUserPrincipal method
-                    ServerVerticle dummyServerVerticle = createDummyServerVerticle(testInfo);
+                    try {
+                        // just to be sure, try to fetch a new port to use
+                        options.setServerPort(SystemHelper.getFreePort());
+                    } catch (IOException e) { // NOPMD
+                        // let's continue with the old port which should be still free (hopefully)
+                    }
 
+                    ServerVerticle dummyServerVerticle = createDummyServerVerticle(testInfo);
+                    serverVerticleOptions.getConfig().put("port", options.getServerPort());
                     serverVerticleOptions.getConfig().put("authenticationChain", new JsonArray());
-
-                    isDummyServerVerticleDeployed = true;
-                    return undeployVerticles(ServerVerticle.class)
-                            .compose(nothing -> deployVerticle(dummyServerVerticle, serverVerticleOptions));
-                }).orElse(succeededFuture()).onComplete(testContext.succeeding(v -> {
-                    latch.countDown();
+                    writeDeploymentOptions(ServerVerticle.class, serverVerticleOptions, workingDirPath);
+
+                    undeployVerticles(ServerVerticle.class) // just to be sure, NO_WEB should not deploy a server
+                            .compose(nothing -> deployVerticle(dummyServerVerticle, serverVerticleOptions))
+                            .onComplete(result -> {
+                                if (result.succeeded()) {
+                                    isDummyServerVerticleDeployed = true;
+                                }
+
+                                testContext.succeedingThenComplete().handle(result.mapEmpty());
+                                latch.countDown();
+                            });
+                } else {
                     testContext.completeNow();
-                }));
+                    latch.countDown();
+                }
             }
         });
 
-        latch.await();
+        if (!latch.await(DEFAULT_TIMEOUT_DURATION, DEFAULT_TIMEOUT_UNIT)) {
+            throw new TimeoutException("Preparing NeonBee timed out");
+        }
     }
 
     @AfterEach
@@ -183,7 +231,7 @@ protected void adaptOptions(TestInfo testInfo, NeonBeeOptions.Mutable options) {
      */
     @SuppressWarnings("PMD.UnusedFormalParameter")
     protected JsonObject provideUserPrincipal(TestInfo testInfo) {
-        return null;
+        return NO_USER_PRINCIPAL;
     }
 
     /**