From dca81cbf00f951a50c94bec37e3f61da9e432c62 Mon Sep 17 00:00:00 2001
From: Violeta Georgieva <violeta.georgieva@broadcom.com>
Date: Mon, 13 May 2024 12:13:23 +0300
Subject: [PATCH] Fix flaky
 DefaultPooledConnectionProviderTest#testDisposeInactivePoolsInBackground
 (#3237)

There are random DefaultPooledConnectionProviderTest#testDisposeInactivePoolsInBackground failures

DefaultPooledConnectionProviderTest > testDisposeInactivePoolsInBackground(boolean, boolean, boolean) > [5] enableEvictInBackground=true, isHttp2=false, isBuiltInMetrics=false FAILED
    org.opentest4j.AssertionFailedError:
    expected: true
     but was: false
        at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
        at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
        at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
        at reactor.netty.resources.DefaultPooledConnectionProviderTest.testDisposeInactivePoolsInBackground(DefaultPooledConnectionProviderTest.java:729)

DefaultPooledConnectionProviderTest > testDisposeInactivePoolsInBackground(boolean, boolean, boolean) > [8] enableEvictInBackground=true, isHttp2=true, isBuiltInMetrics=true FAILED
    org.opentest4j.AssertionFailedError:
    expected: null
     but was: io.micrometer.core.instrument.internal.DefaultGauge@78e01239
        at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
        at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
        at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
        at reactor.netty.resources.DefaultPooledConnectionProviderTest.testDisposeInactivePoolsInBackground(DefaultPooledConnectionProviderTest.java:733)
---
 .../DefaultPooledConnectionProviderTest.java  | 24 +++++++++++++++----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/reactor-netty-http/src/test/java/reactor/netty/resources/DefaultPooledConnectionProviderTest.java b/reactor-netty-http/src/test/java/reactor/netty/resources/DefaultPooledConnectionProviderTest.java
index 57930a259f..cf6175a00b 100644
--- a/reactor-netty-http/src/test/java/reactor/netty/resources/DefaultPooledConnectionProviderTest.java
+++ b/reactor-netty-http/src/test/java/reactor/netty/resources/DefaultPooledConnectionProviderTest.java
@@ -669,12 +669,20 @@ void testDisposeInactivePoolsInBackground(boolean enableEvictInBackground, boole
 		}
 
 		MeterRegistrarImpl meterRegistrar;
-		String metricsName = "";
+		CountDownLatch meterRemoved = new CountDownLatch(1);
+		String metricsName = CONNECTION_PROVIDER_PREFIX + ACTIVE_CONNECTIONS;
+		String metricsTagName = "";
 		if (isBuiltInMetrics) {
 			meterRegistrar = null;
 			builder.metrics(true);
 
-			metricsName = isHttp2 ? "http2.testDisposeInactivePoolsInBackground" : "testDisposeInactivePoolsInBackground";
+			metricsTagName = isHttp2 ? "http2.testDisposeInactivePoolsInBackground" : "testDisposeInactivePoolsInBackground";
+
+			registry.config().onMeterRemoved(meter -> {
+				if (metricsName.equals(meter.getId().getName())) {
+					meterRemoved.countDown();
+				}
+			});
 		}
 		else {
 			meterRegistrar = new MeterRegistrarImpl();
@@ -711,7 +719,7 @@ void testDisposeInactivePoolsInBackground(boolean enableEvictInBackground, boole
 				assertThat(meterRegistrar.registered.get()).isTrue();
 			}
 			else {
-				assertGauge(registry, CONNECTION_PROVIDER_PREFIX + ACTIVE_CONNECTIONS, NAME, metricsName).isNotNull();
+				assertGauge(registry, metricsName, NAME, metricsTagName).isNotNull();
 			}
 
 			if (enableEvictInBackground) {
@@ -726,14 +734,18 @@ void testDisposeInactivePoolsInBackground(boolean enableEvictInBackground, boole
 
 			assertThat(provider.isDisposed()).isEqualTo(enableEvictInBackground);
 			if (meterRegistrar != null) {
+				if (enableEvictInBackground) {
+					assertThat(meterRegistrar.latch.await(30, TimeUnit.SECONDS)).isTrue();
+				}
 				assertThat(meterRegistrar.deRegistered.get()).isEqualTo(enableEvictInBackground);
 			}
 			else {
 				if (enableEvictInBackground) {
-					assertGauge(registry, CONNECTION_PROVIDER_PREFIX + ACTIVE_CONNECTIONS, NAME, metricsName).isNull();
+					assertThat(meterRemoved.await(30, TimeUnit.SECONDS)).isTrue();
+					assertGauge(registry, metricsName, NAME, metricsTagName).isNull();
 				}
 				else {
-					assertGauge(registry, CONNECTION_PROVIDER_PREFIX + ACTIVE_CONNECTIONS, NAME, metricsName).isNotNull();
+					assertGauge(registry, metricsName, NAME, metricsTagName).isNotNull();
 				}
 			}
 		}
@@ -869,6 +881,7 @@ void testHttp2PoolAndGoAway() {
 	static final class MeterRegistrarImpl implements ConnectionProvider.MeterRegistrar {
 		AtomicBoolean registered = new AtomicBoolean();
 		AtomicBoolean deRegistered = new AtomicBoolean();
+		final CountDownLatch latch = new CountDownLatch(1);
 
 		MeterRegistrarImpl() {
 		}
@@ -881,6 +894,7 @@ public void registerMetrics(String poolName, String id, SocketAddress remoteAddr
 		@Override
 		public void deRegisterMetrics(String poolName, String id, SocketAddress remoteAddress) {
 			deRegistered.compareAndSet(false, true);
+			latch.countDown();
 		}
 	}
 }