[4.x] Various metrics test improvements to avoid intermittent failures

integrations/micrometer/cdi/pom.xml

@@ -115,6 +115,11 @@
             <artifactId>helidon-microprofile-tests-junit5</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>io.helidon.common.testing</groupId>
+            <artifactId>helidon-common-testing-junit5</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>

integrations/micrometer/cdi/src/test/java/io/helidon/integrations/micrometer/cdi/HelloWorldTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2021 Oracle and/or its affiliates.
+ * Copyright (c) 2018, 2022 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -31,8 +31,11 @@
 import jakarta.ws.rs.core.MediaType;
 import org.junit.jupiter.api.Test;
 
+import static io.helidon.common.testing.junit5.MatcherWithRetry.assertThatWithRetry;
+
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.notNullValue;
 
 /**
@@ -83,7 +86,7 @@ void testTimer() {
     }
 
     @Test
-    public void testSlowTimer() {
+    public void testSlowTimer() throws InterruptedException {
         int exp = 3;
         AtomicReference<String> result = new AtomicReference<>();
         IntStream.range(0, exp).forEach(
@@ -95,11 +98,11 @@ public void testSlowTimer() {
         assertThat("Returned from HTTP request", result.get(), is(HelloWorldResource.SLOW_RESPONSE));
         Timer slowTimer = registry.timer(HelloWorldResource.SLOW_MESSAGE_TIMER);
         assertThat("Slow message timer", slowTimer, is(notNullValue()));
-        assertThat("Slow message timer count", slowTimer.count(), is((long) exp));
+        assertThatWithRetry("Slow message timer count", slowTimer::count, is((long) exp));
     }
 
     @Test
-    public void testFastFailCounter() {
+    public void testFastFailCounter() throws InterruptedException {
         int exp = 8;
         IntStream.range(0, exp).forEach(
                 i -> {
@@ -116,7 +119,7 @@ public void testFastFailCounter() {
 
         Counter counter = registry.counter(HelloWorldResource.FAST_MESSAGE_COUNTER);
         assertThat("Failed message counter", counter, is(notNullValue()));
-        assertThat("Failed message counter count", counter.count(), is((double) exp / 2));
+        assertThatWithRetry("Failed message counter count", counter::count, is((double) exp / 2));
     }
 
     @Test
@@ -134,7 +137,7 @@ public void testSlowFailNoCounter() {
     }
 
     @Test
-    public void testSlowFailCounter() {
+    public void testSlowFailCounter() throws InterruptedException {
         int exp = 6;
         IntStream.range(0, exp).forEach(
                 i -> {
@@ -151,7 +154,7 @@ public void testSlowFailCounter() {
 
         Counter counter = registry.counter(HelloWorldResource.SLOW_MESSAGE_FAIL_COUNTER);
         assertThat("Failed message counter", counter, is(notNullValue()));
-        assertThat("Failed message counter count", counter.count(), is((double) 3));
+        assertThatWithRetry("Failed message counter count", counter::count, is((double) 3));
     }
 
     void checkMicrometerURL(int iterations) {

integrations/micrometer/cdi/src/test/java/io/helidon/integrations/micrometer/cdi/MeteredBeanTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021 Oracle and/or its affiliates.
+ * Copyright (c) 2021, 2022 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -24,7 +24,8 @@
 import jakarta.inject.Inject;
 import org.junit.jupiter.api.Test;
 
-import static org.hamcrest.MatcherAssert.assertThat;
+import static io.helidon.common.testing.junit5.MatcherWithRetry.assertThatWithRetry;
+
 import static org.hamcrest.Matchers.is;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
@@ -39,35 +40,40 @@ public class MeteredBeanTest {
     MeterRegistry registry;
 
     @Test
-    public void testSimpleCounted() {
+    public void testSimpleCounted() throws InterruptedException {
         int exp = 3;
         IntStream.range(0, exp).forEach(i -> meteredBean.count());
-        assertThat("Value from simple counted meter", registry.counter(MeteredBean.COUNTED).count(), is((double) exp));
+        assertThatWithRetry("Value from simple counted meter", () -> registry.counter(MeteredBean.COUNTED).count(),
+                            is((double) exp));
     }
 
     @Test
-    public void testSinglyTimed() {
+    public void testSinglyTimed() throws InterruptedException {
         int exp = 4;
         IntStream.range(0, exp).forEach(i -> meteredBean.timed());
-        assertThat("Count from singly-timed meter", registry.timer(MeteredBean.TIMED_1).count(), is((long) exp));
+        assertThatWithRetry("Count from singly-timed meter", () -> registry.timer(MeteredBean.TIMED_1).count(),
+                            is((long) exp));
     }
 
     @Test
-    public void testDoublyTimed() {
+    public void testDoublyTimed() throws InterruptedException {
         int exp = 5;
         IntStream.range(0, exp).forEach(i -> meteredBean.timedA());
-        assertThat("Count from doubly-timed meter (A)", registry.timer(MeteredBean.TIMED_A).count(), is((long) exp));
-        assertThat("Count from doubly-timed meter (B)", registry.timer(MeteredBean.TIMED_B).count(), is((long) exp));
+        assertThatWithRetry("Count from doubly-timed meter (A)", () -> registry.timer(MeteredBean.TIMED_A).count(),
+                            is((long) exp));
+        assertThatWithRetry("Count from doubly-timed meter (B)", () -> registry.timer(MeteredBean.TIMED_B).count(),
+                            is((long) exp));
     }
 
     @Test
-    public void testFailable() {
+    public void testFailable() throws InterruptedException {
         int OKCalls = 2;
         int expFailed = 3;
         IntStream.range(0, OKCalls).forEach(i -> meteredBean.failable(false));
         IntStream.range(0, expFailed).forEach(i -> assertThrows(RuntimeException.class,
                 () -> meteredBean.failable(true)));
-        assertThat("Count from failed calls", registry.counter(MeteredBean.COUNTED_ONLY_FOR_FAILURE).count(),
-                is((double) expFailed));
+        assertThatWithRetry("Count from failed calls",
+                            () -> registry.counter(MeteredBean.COUNTED_ONLY_FOR_FAILURE).count(),
+                            is((double) expFailed));
     }
 }

metrics/metrics/src/test/java/io/helidon/metrics/TestPeriodicExecutor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021 Oracle and/or its affiliates.
+ * Copyright (c) 2021, 2022 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,6 +18,8 @@
 import java.time.Duration;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.logging.Handler;
 import java.util.logging.Level;
@@ -32,21 +34,20 @@
 import static org.hamcrest.Matchers.arrayContaining;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.greaterThan;
+import static org.hamcrest.Matchers.greaterThanOrEqualTo;
 import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.fail;
 
 class TestPeriodicExecutor {
 
-    private static final int SLEEP_TIME_MS = 1500;
-    private static final int SLEEP_TIME_NO_DATA_MS = 100;
+    private static final int APPROX_TEST_DURATION_MS = 1500;
+    private static final int MAX_TEST_WAIT_TIME_MS = APPROX_TEST_DURATION_MS * 15 / 10; // 1.5 * approx test duration
 
     private static final int FAST_INTERVAL = 250;
     private static final int SLOW_INTERVAL = 400;
 
-    private static final double SLOWDOWN_FACTOR = 0.80; // for slow pipelines!
-
-    private static final double MIN_FAST_COUNT = 1500 / FAST_INTERVAL * SLOWDOWN_FACTOR;
-    private static final double MIN_SLOW_COUNT = 1500 / SLOW_INTERVAL * SLOWDOWN_FACTOR;
+    private static final int MIN_FAST_COUNT = APPROX_TEST_DURATION_MS / FAST_INTERVAL;
+    private static final int MIN_SLOW_COUNT = APPROX_TEST_DURATION_MS / SLOW_INTERVAL;
 
     private static final Logger PERIODIC_EXECUTOR_LOGGER = Logger.getLogger(PeriodicExecutor.class.getName());
 
@@ -59,13 +60,28 @@ void testWithNoDeferrals() throws InterruptedException {
             AtomicInteger countA = new AtomicInteger();
             AtomicInteger countB = new AtomicInteger();
 
-            exec.enrollRunner(() -> countA.incrementAndGet(), Duration.ofMillis(FAST_INTERVAL));
-            exec.enrollRunner(() -> countB.incrementAndGet(), Duration.ofMillis(SLOW_INTERVAL));
+            CountDownLatch latchA = new CountDownLatch(MIN_FAST_COUNT);
+            CountDownLatch latchB = new CountDownLatch(MIN_SLOW_COUNT);
+
+            long startTime = System.nanoTime();
+            exec.enrollRunner(() -> {
+                countA.incrementAndGet();
+                latchA.countDown();
+            }, Duration.ofMillis(FAST_INTERVAL));
+
+            exec.enrollRunner(() -> {
+                countB.incrementAndGet();
+                latchB.countDown();
+            }, Duration.ofMillis(SLOW_INTERVAL));
 
-            Thread.sleep(SLEEP_TIME_MS);
+            assertThat("Wait latch for fast interval", latchA.await(MAX_TEST_WAIT_TIME_MS, TimeUnit.MILLISECONDS), is(true));
+            assertThat("Wait latch for slow interval", latchB.await(MAX_TEST_WAIT_TIME_MS, TimeUnit.MILLISECONDS), is(true));
 
-            assertThat("CountA", (double) countA.get(), is(greaterThan(MIN_FAST_COUNT)));
-            assertThat("CountB", (double) countB.get(), is(greaterThan(MIN_SLOW_COUNT)));
+            Duration elapsedTime = Duration.ofNanos(System.nanoTime() - startTime);
+
+            assertThat("CountA", countA.get(), is(greaterThanOrEqualTo(MIN_FAST_COUNT)));
+            assertThat("CountB", countB.get(), is(greaterThanOrEqualTo(MIN_SLOW_COUNT)));
+            assertThat("Wait duration", elapsedTime, greaterThanOrEqualTo(Duration.ofMillis(1500)));
         } finally {
             if (exec.executorState() == PeriodicExecutor.State.STARTED) {
                 exec.stopExecutor();
@@ -80,16 +96,26 @@ void testWithDeferredEnrollments() throws InterruptedException {
             AtomicInteger countA = new AtomicInteger();
             AtomicInteger countB = new AtomicInteger();
 
-            exec.enrollRunner(() -> countA.incrementAndGet(), Duration.ofMillis(FAST_INTERVAL));
+            CountDownLatch latchA = new CountDownLatch(MIN_FAST_COUNT);
+            CountDownLatch latchB = new CountDownLatch(MIN_SLOW_COUNT);
+
+            exec.enrollRunner(() -> {
+                countA.incrementAndGet();
+                latchA.countDown();
+            }, Duration.ofMillis(FAST_INTERVAL));
 
             exec.startExecutor();
 
-            exec.enrollRunner(() -> countB.incrementAndGet(), Duration.ofMillis(SLOW_INTERVAL));
+            exec.enrollRunner(() -> {
+                countB.incrementAndGet();
+                latchB.countDown();
+            }, Duration.ofMillis(SLOW_INTERVAL));
 
-            Thread.sleep(SLEEP_TIME_MS);
+            assertThat("Wait latch for fast interval", latchA.await(MAX_TEST_WAIT_TIME_MS, TimeUnit.MILLISECONDS), is(true));
+            assertThat("Wait latch for slow interval", latchB.await(MAX_TEST_WAIT_TIME_MS, TimeUnit.MILLISECONDS), is(true));
 
-            assertThat("CountA", (double) countA.get(), is(greaterThan(MIN_FAST_COUNT)));
-            assertThat("CountB", (double) countB.get(), is(greaterThan(MIN_SLOW_COUNT)));
+            assertThat("CountA", countA.get(), is(greaterThanOrEqualTo(MIN_FAST_COUNT)));
+            assertThat("CountB", countB.get(), is(greaterThanOrEqualTo(MIN_SLOW_COUNT)));
         } finally {
             if (exec.executorState() == PeriodicExecutor.State.STARTED) {
                 exec.stopExecutor();
@@ -104,17 +130,25 @@ void testWithLateEnrollment() throws InterruptedException {
             AtomicInteger countA = new AtomicInteger();
             AtomicInteger countB = new AtomicInteger();
 
-            exec.enrollRunner(() -> countA.incrementAndGet(), Duration.ofMillis(FAST_INTERVAL));
+            CountDownLatch latchA = new CountDownLatch(MIN_FAST_COUNT);
+
+            exec.enrollRunner(() -> {
+                countA.incrementAndGet();
+                latchA.countDown();
+            }, Duration.ofMillis(FAST_INTERVAL));
 
             exec.startExecutor();
-            Thread.sleep(SLEEP_TIME_MS);
 
+            assertThat("Wait latch", latchA.await(MAX_TEST_WAIT_TIME_MS, TimeUnit.MILLISECONDS), is(true));
             exec.stopExecutor();
 
             exec.enrollRunner(() -> countB.incrementAndGet(), Duration.ofMillis(SLOW_INTERVAL));
 
-            assertThat("CountA", (double) countA.get(), is(greaterThan(MIN_FAST_COUNT))); // should be 8
-            assertThat("CountB", (double) countB.get(), is(0.0));
+            // The executor is no longer running, so we cannot use a countdown latch to know when to check countB. Use time.
+            Thread.sleep(MAX_TEST_WAIT_TIME_MS);
+
+            assertThat("CountA", countA.get(), is(greaterThanOrEqualTo(MIN_FAST_COUNT))); // should be 8
+            assertThat("CountB", countB.get(), is(0));
         } finally {
             if (exec.executorState() == PeriodicExecutor.State.STARTED) {
                 exec.stopExecutor();
@@ -130,7 +164,8 @@ void testNoWarningOnStopWhenStopped() throws InterruptedException {
         try {
             PeriodicExecutor executor = PeriodicExecutor.create();
             executor.stopExecutor();
-            Thread.sleep(SLEEP_TIME_NO_DATA_MS);
+
+            waitForExecutorState(executor, PeriodicExecutor.State.STOPPED);
             handler.clear();
 
             executor.stopExecutor();
@@ -152,7 +187,7 @@ void testFineMessageOnStopWhenStopped() throws InterruptedException {
             PERIODIC_EXECUTOR_LOGGER.setLevel(Level.FINE);
             PeriodicExecutor executor = PeriodicExecutor.create();
             executor.stopExecutor();
-            Thread.sleep(SLEEP_TIME_NO_DATA_MS);
+            waitForExecutorState(executor, PeriodicExecutor.State.STOPPED);
 
             executor.stopExecutor();
             handler.clear();
@@ -184,7 +219,7 @@ void testFineLoggingOnExpectedStop(PeriodicExecutor.State testState) throws Inte
 
             if (testState == PeriodicExecutor.State.STARTED) {
                 executor.startExecutor();
-                Thread.sleep(SLEEP_TIME_NO_DATA_MS);
+                waitForExecutorState(executor, PeriodicExecutor.State.STARTED);
             }
 
             handler.clear();
@@ -205,6 +240,15 @@ void testFineLoggingOnExpectedStop(PeriodicExecutor.State testState) throws Inte
         }
     }
 
+    private void waitForExecutorState(PeriodicExecutor executor, PeriodicExecutor.State expectedState) {
+        for (int i = MAX_TEST_WAIT_TIME_MS; i > 0; i -= 500) {
+            if (executor.executorState() == expectedState) {
+                return;
+            }
+        }
+        fail("Timed out waiting for executor in state " + executor.executorState() + " to enter state " + expectedState.name());
+    }
+
     private static class MyHandler extends Handler {
 
         private List<LogRecord> logRecords = new ArrayList<>();

microprofile/metrics/src/test/java/io/helidon/microprofile/metrics/HelloWorldRestEndpointSimpleTimerDisabledTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2021 Oracle and/or its affiliates.
+ * Copyright (c) 2018, 2022 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,11 +16,13 @@
 
 package io.helidon.microprofile.metrics;
 
+import io.helidon.microprofile.tests.junit5.AddConfig;
 import io.helidon.microprofile.tests.junit5.HelidonTest;
 
 import jakarta.inject.Inject;
 import org.eclipse.microprofile.metrics.MetricRegistry;
 import org.eclipse.microprofile.metrics.annotation.RegistryType;
+import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 
 import static org.hamcrest.CoreMatchers.is;
@@ -31,15 +33,21 @@
  * the config disables that feature.
  */
 @HelidonTest
+@AddConfig(key = "metrics." + MetricsCdiExtension.REST_ENDPOINTS_METRIC_ENABLED_PROPERTY_NAME, value = "false")
 public class HelloWorldRestEndpointSimpleTimerDisabledTest {
 
+    @BeforeAll
+    static void init() {
+        MetricsMpServiceTest.cleanUpSyntheticSimpleTimerRegistry();
+    }
+
     @Inject
     @RegistryType(type = MetricRegistry.Type.BASE)
     MetricRegistry syntheticSimpleTimerRegistry;
 
     boolean isSyntheticSimpleTimerPresent() {
         return !syntheticSimpleTimerRegistry.getSimpleTimers((metricID, metric) ->
-                metricID.equals(MetricsCdiExtension.SYNTHETIC_SIMPLE_TIMER_METRIC_NAME))
+                metricID.getName().equals(MetricsCdiExtension.SYNTHETIC_SIMPLE_TIMER_METRIC_NAME))
                 .isEmpty();
     }