Merge pull request #114 from vert-x3/fix-checkpoint-flagging-reporting

Fix bad reporting of excessive flagging

Author: jponge

src/main/asciidoc/index.adoc

@@ -39,6 +39,7 @@ Here is a very basic usage:
 ----
 {@link examples.Examples.BTest}
 ----
+
 <1> {@link io.vertx.junit5.VertxTestContext#succeedingThenComplete} returns an asynchronous result handler that is expected to succeed and then make the test context pass.
 <2> {@link io.vertx.junit5.VertxTestContext#awaitCompletion} has the semantics of a `java.util.concurrent.CountDownLatch`, and returns `false` if the waiting delay expired before the test passed.
 <3> If the context captures a (potentially asynchronous) error, then after completion we must throw the failure exception to make the test fail.
@@ -98,7 +99,8 @@ The Vert.x integration is primarily done using the {@link io.vertx.junit5.VertxE
 {@link examples.Examples.CTest.SomeTest}
 ----
 
-NOTE: The `Vertx` instance is not clustered and has the default configuration. If you need something else then just don't use injection on that parameter and prepare a `Vertx` object by yourself.
+NOTE: The `Vertx` instance is not clustered and has the default configuration.
+If you need something else then just don't use injection on that parameter and prepare a `Vertx` object by yourself.
 
 The test is automatically wrapped around the {@link io.vertx.junit5.VertxTestContext} instance lifecycle, so you don't need to insert {@link io.vertx.junit5.VertxTestContext#awaitCompletion} calls yourself:
 
@@ -151,8 +153,10 @@ Generally-speaking, we respect the JUnit extension scoping rules, but here are t
 
 ==== Configuring `Vertx` instances
 
-By default the `Vertx` objects get created with `Vertx.vertx()`, using the default settings for `Vertx`. However you have the ability to configure `VertxOptions` to suit your needs.
-A typical use case would be "extending blocking timeout warning for debugging". To configure the `Vertx` object you must:
+By default the `Vertx` objects get created with `Vertx.vertx()`, using the default settings for `Vertx`.
+However you have the ability to configure `VertxOptions` to suit your needs.
+A typical use case would be "extending blocking timeout warning for debugging".
+To configure the `Vertx` object you must:
 
 1. create a json file with the `VertxOptions` in https://vertx.io/docs/apidocs/io/vertx/core/VertxOptions.html#VertxOptions-io.vertx.core.json.JsonObject-[json format]
 2. create an environment variable `vertx.parameter.filename` pointing to that file
@@ -183,7 +187,8 @@ JUnit 5 allows multiple methods to exist for the same lifecycle events.
 As an example, it is possible to define 3 `@BeforeEach` methods on the same test.
 Because of asynchronous operations it is possible that the effects of these methods happen concurrently rather than sequentially, which may lead to inconsistent states.
 
-This is a problem of JUnit 5 rather than this module. In case of doubt you may always wonder why a single method can't be better than many.
+This is a problem of JUnit 5 rather than this module.
+In case of doubt you may always wonder why a single method can't be better than many.
 
 == Support for additional parameter types
 
@@ -217,15 +222,15 @@ In any case it is a good idea to have the `Vertx` parameter first, and the next
 
 == Parameterized tests with `@MethodSource`
 
-You can use parameterized tests with `@MethodSource` with vertx-junit5. Therefore you need to declare the method source parameters before the vertx test parameters in
-the method definition.
+You can use parameterized tests with `@MethodSource` with vertx-junit5. Therefore you need to declare the method source parameters before the vertx test parameters in the method definition.
 
 [source,java]
 ----
 {@link examples.Examples.PTest.SomeTest}
 ----
 
-The same holds for the other `ArgumentSources`. See the section `Formal Parameter List` in the API doc of
+The same holds for the other `ArgumentSources`.
+See the section `Formal Parameter List` in the API doc of
 https://junit.org/junit5/docs/current/api/org.junit.jupiter.params/org/junit/jupiter/params/ParameterizedTest.html[ParameterizedTest]
 
 == Running tests on a Vert.x context

src/main/java/examples/Examples.java

@@ -31,8 +31,8 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.MethodSource;
 import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Stream;
@@ -243,7 +243,7 @@ static Stream<Arguments> testData() {
 
       @ParameterizedTest
       @MethodSource("testData")
-       void test2(String obj, int count, Vertx vertx, VertxTestContext testContext) {
+      void test2(String obj, int count, Vertx vertx, VertxTestContext testContext) {
         // your test code
         testContext.completeNow();
       }

src/main/java/examples/RunTestOnContextExampleTest.java

@@ -18,12 +18,15 @@
 
 import io.vertx.core.Vertx;
 import io.vertx.junit5.RunTestOnContext;
+import io.vertx.junit5.VertxExtension;
 import io.vertx.junit5.VertxTestContext;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
+@ExtendWith(VertxExtension.class)
 class RunTestOnContextExampleTest {
 
   @RegisterExtension

src/main/java/io/vertx/junit5/Checkpoint.java

@@ -19,8 +19,8 @@
 /**
  * A test completion checkpoint, flagging it advances towards the test context completion.
  *
- * @see VertxTestContext
  * @author <a href="https://julien.ponge.org/">Julien Ponge</a>
+ * @see VertxTestContext
  */
 public interface Checkpoint {
 

src/main/java/io/vertx/junit5/VertxParameterProvider.java

@@ -16,8 +16,12 @@
 
 package io.vertx.junit5;
 
-import static io.vertx.junit5.VertxExtension.DEFAULT_TIMEOUT_DURATION;
-import static io.vertx.junit5.VertxExtension.DEFAULT_TIMEOUT_UNIT;
+import io.vertx.core.Vertx;
+import io.vertx.core.VertxException;
+import io.vertx.core.VertxOptions;
+import io.vertx.core.json.JsonObject;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.api.extension.ParameterContext;
 
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
@@ -29,13 +33,8 @@
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
 
-import org.junit.jupiter.api.extension.ExtensionContext;
-import org.junit.jupiter.api.extension.ParameterContext;
-
-import io.vertx.core.Vertx;
-import io.vertx.core.VertxException;
-import io.vertx.core.VertxOptions;
-import io.vertx.core.json.JsonObject;
+import static io.vertx.junit5.VertxExtension.DEFAULT_TIMEOUT_DURATION;
+import static io.vertx.junit5.VertxExtension.DEFAULT_TIMEOUT_UNIT;
 
 public class VertxParameterProvider implements VertxExtensionParameterProvider<Vertx> {
 

src/main/java/io/vertx/junit5/VertxTestContext.java

@@ -50,6 +50,7 @@ public interface ExecutionBlock {
   // ........................................................................................... //
 
   private Throwable throwableReference = null;
+  private boolean done = false;
   private final CountDownLatch releaseLatch = new CountDownLatch(1);
   private final HashSet<CountingCheckpoint> checkpoints = new HashSet<>();
 
@@ -79,7 +80,7 @@ public synchronized Throwable causeOfFailure() {
    * @return {@code true} if the context has completed, {@code false} otherwise.
    */
   public synchronized boolean completed() {
-    return !failed() && releaseLatch.getCount() == 0;
+    return !failed() && (done || releaseLatch.getCount() == 0);
   }
 
   /**
@@ -101,6 +102,7 @@ public Set<StackTraceElement> unsatisfiedCheckpointCallSites() {
    * Complete the test context immediately, making the corresponding test pass.
    */
   public synchronized void completeNow() {
+    done = true;
     releaseLatch.countDown();
   }
 
@@ -111,7 +113,7 @@ public synchronized void completeNow() {
    */
   public synchronized void failNow(Throwable t) {
     Objects.requireNonNull(t, "The exception cannot be null");
-    if (!completed()) {
+    if (throwableReference == null) {
       throwableReference = t;
       releaseLatch.countDown();
     }
@@ -120,7 +122,7 @@ public synchronized void failNow(Throwable t) {
   /**
    * Calls {@link #failNow(Throwable)} with the {@code message}.
    *
-   * @param message  the cause of failure
+   * @param message the cause of failure
    */
   public synchronized void failNow(String message) {
     failNow(new NoStackTraceThrowable(message));
@@ -185,8 +187,8 @@ public synchronized Checkpoint checkpoint(int requiredNumberOfPasses) {
    * @param <T> the asynchronous result type.
    * @return the handler.
    * @deprecated Use {@link #succeedingThenComplete()} or {@link #succeeding(Handler)}, for example
-   *     <code>succeeding(value -> checkpoint.flag())</code>, <code>succeeding(value -> { more testing code })</code>, or
-   *     <code>succeeding(value -> {})</code>.
+   * <code>succeeding(value -> checkpoint.flag())</code>, <code>succeeding(value -> { more testing code })</code>, or
+   * <code>succeeding(value -> {})</code>.
    */
   @Deprecated
   public <T> Handler<AsyncResult<T>> succeeding() {
@@ -225,8 +227,8 @@ public <T> Handler<AsyncResult<T>> succeeding(Handler<T> nextHandler) {
    * @param <T> the asynchronous result type.
    * @return the handler.
    * @deprecated Use {@link #failingThenComplete()} or {@link #failing(Handler)}, for example
-   *     <code>failing(e -> checkpoint.flag())</code>, <code>failing(e -> { more testing code })</code>, or
-   *     <code>failing(e -> {})</code>.
+   * <code>failing(e -> checkpoint.flag())</code>, <code>failing(e -> { more testing code })</code>, or
+   * <code>failing(e -> {})</code>.
    */
   @Deprecated
   public <T> Handler<AsyncResult<T>> failing() {

src/test/java/io/vertx/junit5/AsyncBeforeEachTest.java

@@ -19,13 +19,11 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.RepeatedTest;
-import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 
 import java.util.concurrent.atomic.AtomicInteger;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 @ExtendWith(VertxExtension.class)

src/test/java/io/vertx/junit5/VertxExtensionTest.java

@@ -214,6 +214,40 @@ void doNothing(VertxTestContext testContext) {
       }
     }
 
+    @Nested
+    @ExtendWith(VertxExtension.class)
+    @DisplayName("🚫")
+    class TooMuchFlagging {
+
+      @Test
+      @Tag("programmatic")
+      void flagTooMuch(VertxTestContext testContext) {
+        Checkpoint checkpoint = testContext.checkpoint(3);
+        for (int i = 0; i < 10; i++) {
+          checkpoint.flag();
+        }
+      }
+    }
+
+    @Test
+    @DisplayName("⚙️ Check that too much flagging fails tests")
+    void checkTooMuchFlaggingFails() {
+      LauncherDiscoveryRequest request = LauncherDiscoveryRequestBuilder.request()
+        .selectors(selectClass(EmbeddedWithARunner.TooMuchFlagging.class))
+        .build();
+      Launcher launcher = LauncherFactory.create();
+      SummaryGeneratingListener listener = new SummaryGeneratingListener();
+      launcher.registerTestExecutionListeners(listener);
+      launcher.execute(request);
+      TestExecutionSummary summary = listener.getSummary();
+      assertThat(summary.getTestsStartedCount()).isEqualTo(1);
+      assertThat(summary.getTestsFailedCount()).isEqualTo(1);
+      Throwable exception = summary.getFailures().get(0).getException();
+      assertThat(exception)
+        .isInstanceOf(IllegalStateException.class)
+        .hasMessage("Strict checkpoint flagged too many times");
+    }
+
     @Test
     @DisplayName("⚙️ Check a timeout diagnosis")
     void checkTimeoutFailureTestWithIntermediateAsyncVerifier() {

src/test/java/io/vertx/junit5/VertxParameterProviderTest.java

@@ -16,68 +16,67 @@
 
 package io.vertx.junit5;
 
-import static com.github.stefanbirkner.systemlambda.SystemLambda.withEnvironmentVariable;
-import static org.junit.jupiter.api.Assertions.assertEquals;
+import io.vertx.core.json.JsonObject;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
 
 import java.io.BufferedWriter;
 import java.io.File;
 import java.io.FileWriter;
 
-import org.junit.jupiter.api.DisplayName;
-import org.junit.jupiter.api.Test;
-
-import io.vertx.core.json.JsonObject;
+import static com.github.stefanbirkner.systemlambda.SystemLambda.withEnvironmentVariable;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 
 /**
  * @author <a href="https://wissel.net/">Stephan Wisssel</a>
  */
 @DisplayName("Test of VertxParameterProvider")
 public class VertxParameterProviderTest {
 
-    @Test
-    @DisplayName("Default case - empty VertxOptions")
-    void default_empty_options() {
-        VertxParameterProvider provider = new VertxParameterProvider();
-        JsonObject expected = new JsonObject();
-        JsonObject actual = provider.getVertxOptions();
-        assertEquals(expected.encode(), actual.encode(), "Options should be equally empty but are not");
-    }
+  @Test
+  @DisplayName("Default case - empty VertxOptions")
+  void default_empty_options() {
+    VertxParameterProvider provider = new VertxParameterProvider();
+    JsonObject expected = new JsonObject();
+    JsonObject actual = provider.getVertxOptions();
+    assertEquals(expected.encode(), actual.encode(), "Options should be equally empty but are not");
+  }
 
-    @Test
-    @DisplayName("Failed retrieval of options")
-    void failed_retrieval_of_options() throws Exception {
-        VertxParameterProvider provider = new VertxParameterProvider();
-        final JsonObject expected = new JsonObject();
-        final JsonObject actual = new JsonObject();
+  @Test
+  @DisplayName("Failed retrieval of options")
+  void failed_retrieval_of_options() throws Exception {
+    VertxParameterProvider provider = new VertxParameterProvider();
+    final JsonObject expected = new JsonObject();
+    final JsonObject actual = new JsonObject();
 
-        withEnvironmentVariable(VertxParameterProvider.VERTX_PARAMETER_FILENAME, "something.that.does.not.exist.json")
-                .execute(() -> {
-                    actual.mergeIn(provider.getVertxOptions());
-                });
+    withEnvironmentVariable(VertxParameterProvider.VERTX_PARAMETER_FILENAME, "something.that.does.not.exist.json")
+      .execute(() -> {
+        actual.mergeIn(provider.getVertxOptions());
+      });
 
-        assertEquals(expected.encode(), actual.encode(), "Options retrival failure not handled");
-    }
+    assertEquals(expected.encode(), actual.encode(), "Options retrival failure not handled");
+  }
 
-    @Test
-    @DisplayName("Retrieval of options")
-    void retrieval_of_options() throws Exception {
-        VertxParameterProvider provider = new VertxParameterProvider();
-        final JsonObject expected = new JsonObject().put("BlockedThreadCheckInterval", 120).put("MaxWorkerExecuteTime",
-                42);
-        final JsonObject actual = new JsonObject();
-        // Create a temp file and populate it with our expected values
-        File tempOptionFile = File.createTempFile("VertxOptions-", ".json");
-        tempOptionFile.deleteOnExit();
-        BufferedWriter writer = new BufferedWriter(new FileWriter(tempOptionFile.getAbsolutePath()));
-        writer.write(expected.encode());
-        writer.close();
+  @Test
+  @DisplayName("Retrieval of options")
+  void retrieval_of_options() throws Exception {
+    VertxParameterProvider provider = new VertxParameterProvider();
+    final JsonObject expected = new JsonObject().put("BlockedThreadCheckInterval", 120).put("MaxWorkerExecuteTime",
+      42);
+    final JsonObject actual = new JsonObject();
+    // Create a temp file and populate it with our expected values
+    File tempOptionFile = File.createTempFile("VertxOptions-", ".json");
+    tempOptionFile.deleteOnExit();
+    BufferedWriter writer = new BufferedWriter(new FileWriter(tempOptionFile.getAbsolutePath()));
+    writer.write(expected.encode());
+    writer.close();
 
-        withEnvironmentVariable(VertxParameterProvider.VERTX_PARAMETER_FILENAME, tempOptionFile.getAbsolutePath())
-                .execute(() -> {
-                    actual.mergeIn(provider.getVertxOptions());
-                });
+    withEnvironmentVariable(VertxParameterProvider.VERTX_PARAMETER_FILENAME, tempOptionFile.getAbsolutePath())
+      .execute(() -> {
+        actual.mergeIn(provider.getVertxOptions());
+      });
 
-        assertEquals(expected.encode(), actual.encode(), "Options retrival failed");
-    }
+    assertEquals(expected.encode(), actual.encode(), "Options retrival failed");
+  }
 
 }