From 5439f17ff6dc161bb92aae6071580b569b8f4383 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Mon, 2 Sep 2024 19:34:05 +0200 Subject: [PATCH] Rework leak tracking assertions to use a common awaitility pattern (#12226) Rework leak tracking assertions to use a common awaitility pattern Signed-off-by: Ludovic Orban --- .../server/AbstractHttpClientServerTest.java | 22 +++++-------------- .../jetty/server/AbstractHttpTest.java | 4 +++- .../jetty/server/HttpServerTestFixture.java | 12 ++++------ .../jetty/server/ThreadStarvationTest.java | 4 ++-- .../server/handler/DebugHandlerTest.java | 7 +++--- .../ssl/ServerConnectorSslServerTest.java | 4 +++- .../test/client/transport/AbstractTest.java | 7 +++--- .../ee10/test/HttpInputIntegrationTest.java | 4 ++-- .../test/HttpInputTransientErrorTest.java | 3 ++- .../ee9/test/HttpInputIntegrationTest.java | 4 ++-- .../ee9/test/HttpInputTransientErrorTest.java | 3 ++- 11 files changed, 33 insertions(+), 41 deletions(-) diff --git a/jetty-core/jetty-fcgi/jetty-fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/AbstractHttpClientServerTest.java b/jetty-core/jetty-fcgi/jetty-fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/AbstractHttpClientServerTest.java index b9869c2b6c9..b0001266fc2 100644 --- a/jetty-core/jetty-fcgi/jetty-fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/AbstractHttpClientServerTest.java +++ b/jetty-core/jetty-fcgi/jetty-fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/AbstractHttpClientServerTest.java @@ -15,7 +15,6 @@ package org.eclipse.jetty.fcgi.server; import java.util.concurrent.TimeUnit; -import org.awaitility.Awaitility; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.HttpClientTransport; import org.eclipse.jetty.fcgi.client.transport.HttpClientTransportOverFCGI; @@ -29,10 +28,11 @@ import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.util.ProcessorUtils; import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.thread.QueuedThreadPool; -import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterEach; -import static org.junit.jupiter.api.Assertions.fail; +import static org.awaitility.Awaitility.await; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; public abstract class AbstractHttpClientServerTest { @@ -75,9 +75,9 @@ public abstract class AbstractHttpClientServerTest try { if (serverBufferPool != null) - assertNoLeaks(serverBufferPool, "\n---\nServer Leaks: " + serverBufferPool.dumpLeaks() + "---\n"); + await().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> assertThat("Server Leaks: " + serverBufferPool.dumpLeaks(), serverBufferPool.getLeaks().size(), is(0))); if (clientBufferPool != null) - assertNoLeaks(clientBufferPool, "\n---\nClient Leaks: " + clientBufferPool.dumpLeaks() + "---\n"); + await().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> assertThat("Client Leaks: " + clientBufferPool.dumpLeaks(), clientBufferPool.getLeaks().size(), is(0))); } finally { @@ -85,16 +85,4 @@ public abstract class AbstractHttpClientServerTest LifeCycle.stop(server); } } - - private void assertNoLeaks(ArrayByteBufferPool.Tracking bufferPool, String msg) - { - try - { - Awaitility.await().atMost(3, TimeUnit.SECONDS).until(() -> bufferPool.getLeaks().size(), Matchers.is(0)); - } - catch (Exception e) - { - fail(e.getMessage() + msg); - } - } } diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/AbstractHttpTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/AbstractHttpTest.java index fb0f077c18b..0f002eb7d92 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/AbstractHttpTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/AbstractHttpTest.java @@ -21,6 +21,7 @@ import java.net.URISyntaxException; import java.util.Arrays; import java.util.HashSet; import java.util.Set; +import java.util.concurrent.TimeUnit; import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.http.HttpVersion; @@ -33,6 +34,7 @@ import org.eclipse.jetty.util.component.LifeCycle; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import static org.awaitility.Awaitility.await; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -63,7 +65,7 @@ public abstract class AbstractHttpTest { try { - assertThat("Server leaks: " + bufferPool.dumpLeaks(), bufferPool.getLeaks().size(), is(0)); + await().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> assertThat("Server leaks: " + bufferPool.dumpLeaks(), bufferPool.getLeaks().size(), is(0))); } finally { diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestFixture.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestFixture.java index 8c263eb3567..082bde1a00f 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestFixture.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpServerTestFixture.java @@ -20,7 +20,6 @@ import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.concurrent.TimeUnit; -import org.awaitility.Awaitility; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.io.ArrayByteBufferPool; import org.eclipse.jetty.io.Content; @@ -31,11 +30,12 @@ import org.eclipse.jetty.util.Fields; import org.eclipse.jetty.util.Promise; import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler; -import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -import static org.junit.jupiter.api.Assertions.fail; +import static org.awaitility.Awaitility.await; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; // @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck public class HttpServerTestFixture @@ -81,11 +81,7 @@ public class HttpServerTestFixture { try { - Awaitility.await().atMost(5, TimeUnit.SECONDS).until(() -> _bufferPool.getLeaks().size(), Matchers.is(0)); - } - catch (Exception e) - { - fail(e.getMessage() + "\n---\nServer Leaks: " + _bufferPool.dumpLeaks() + "---\n"); + await().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> assertThat("Server leaks: " + _bufferPool.dumpLeaks(), _bufferPool.getLeaks().size(), is(0))); } finally { diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ThreadStarvationTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ThreadStarvationTest.java index 109fd1c74bd..db42bfea6a4 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ThreadStarvationTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ThreadStarvationTest.java @@ -38,13 +38,13 @@ import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.util.thread.QueuedThreadPool; -import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import static org.awaitility.Awaitility.await; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; @@ -153,7 +153,7 @@ public class ThreadStarvationTest ArrayByteBufferPool.Tracking byteBufferPool = (ArrayByteBufferPool.Tracking)_server.getConnectors()[0].getByteBufferPool(); try { - assertThat("Server Leaks: " + byteBufferPool.dumpLeaks(), byteBufferPool.getLeaks().size(), Matchers.is(0)); + await().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> assertThat("Server leaks: " + byteBufferPool.dumpLeaks(), byteBufferPool.getLeaks().size(), is(0))); } finally { diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/DebugHandlerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/DebugHandlerTest.java index afb4663a9b1..cacb7ac44f3 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/DebugHandlerTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/DebugHandlerTest.java @@ -21,6 +21,7 @@ import java.net.URI; import java.nio.charset.StandardCharsets; import java.nio.file.Path; import java.security.KeyStore; +import java.util.concurrent.TimeUnit; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.SSLContext; @@ -35,12 +36,12 @@ import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.ssl.SslContextFactory; -import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import static org.awaitility.Awaitility.await; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; @@ -135,8 +136,8 @@ public class DebugHandlerTest { try { - assertThat("Server HTTP Leaks: " + httpTrackingBufferPool.dumpLeaks(), httpTrackingBufferPool.getLeaks().size(), Matchers.is(0)); - assertThat("Server SSL Leaks: " + sslTrackingBufferPool.dumpLeaks(), sslTrackingBufferPool.getLeaks().size(), Matchers.is(0)); + await().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> assertThat("Server HTTP Leaks: " + httpTrackingBufferPool.dumpLeaks(), httpTrackingBufferPool.getLeaks().size(), is(0))); + await().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> assertThat("Server SSL Leaks: " + sslTrackingBufferPool.dumpLeaks(), sslTrackingBufferPool.getLeaks().size(), is(0))); } finally { diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/ServerConnectorSslServerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/ServerConnectorSslServerTest.java index 1114f93d786..290da4239af 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/ServerConnectorSslServerTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ssl/ServerConnectorSslServerTest.java @@ -20,6 +20,7 @@ import java.net.URI; import java.nio.charset.StandardCharsets; import java.security.KeyStore; import java.util.Arrays; +import java.util.concurrent.TimeUnit; import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.net.ssl.HttpsURLConnection; @@ -50,6 +51,7 @@ import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import static org.awaitility.Awaitility.await; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; @@ -115,7 +117,7 @@ public class ServerConnectorSslServerTest extends HttpServerTestBase @AfterEach public void dispose() throws Exception { - assertThat("Server Leaks: " + _trackingBufferPool.dumpLeaks(), _trackingBufferPool.getLeaks().size(), Matchers.is(0)); + await().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> assertThat("Server leaks: " + _trackingBufferPool.dumpLeaks(), _trackingBufferPool.getLeaks().size(), is(0))); } @Override diff --git a/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/AbstractTest.java b/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/AbstractTest.java index 8f4932f39fa..3567787976c 100644 --- a/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/AbstractTest.java +++ b/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/AbstractTest.java @@ -26,7 +26,6 @@ import java.util.concurrent.TimeUnit; import java.util.stream.Stream; import javax.management.MBeanServer; -import org.awaitility.Awaitility; import org.eclipse.jetty.alpn.server.ALPNServerConnectionFactory; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.HttpClientTransport; @@ -66,7 +65,6 @@ import org.eclipse.jetty.util.SocketAddressResolver; import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.util.thread.QueuedThreadPool; -import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Tags; @@ -75,6 +73,9 @@ import org.junit.jupiter.api.extension.BeforeTestExecutionCallback; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.RegisterExtension; +import static org.awaitility.Awaitility.await; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.fail; @ExtendWith(WorkDirExtension.class) @@ -149,7 +150,7 @@ public class AbstractTest { try { - Awaitility.await().atMost(3, TimeUnit.SECONDS).until(() -> bufferPool.getLeaks().size(), Matchers.is(0)); + await().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> assertThat("Leaks: " + bufferPool.dumpLeaks(), bufferPool.getLeaks().size(), is(0))); } catch (Exception e) { diff --git a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/HttpInputIntegrationTest.java b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/HttpInputIntegrationTest.java index ffb025e2e2f..a26a99ffac7 100644 --- a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/HttpInputIntegrationTest.java +++ b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/HttpInputIntegrationTest.java @@ -56,7 +56,6 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.ssl.SslContextFactory; -import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Tag; @@ -66,6 +65,7 @@ import org.junit.jupiter.params.provider.MethodSource; import static org.awaitility.Awaitility.await; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -141,7 +141,7 @@ public class HttpInputIntegrationTest { try { - assertThat("Server leaks: " + __bufferPool.dumpLeaks(), __bufferPool.getLeaks().size(), Matchers.is(0)); + await().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> assertThat("Server leaks: " + __bufferPool.dumpLeaks(), __bufferPool.getLeaks().size(), is(0))); } finally { diff --git a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/HttpInputTransientErrorTest.java b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/HttpInputTransientErrorTest.java index ca7658ce2d5..a767e2edea3 100644 --- a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/HttpInputTransientErrorTest.java +++ b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/HttpInputTransientErrorTest.java @@ -42,6 +42,7 @@ import org.eclipse.jetty.util.component.LifeCycle; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import static org.awaitility.Awaitility.await; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsString; @@ -64,7 +65,7 @@ public class HttpInputTransientErrorTest try { if (bufferPool != null) - assertThat("Server leaks: " + bufferPool.dumpLeaks(), bufferPool.getLeaks().size(), is(0)); + await().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> assertThat("Server leaks: " + bufferPool.dumpLeaks(), bufferPool.getLeaks().size(), is(0))); } finally { diff --git a/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/src/test/java/org/eclipse/jetty/ee9/test/HttpInputIntegrationTest.java b/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/src/test/java/org/eclipse/jetty/ee9/test/HttpInputIntegrationTest.java index 759af89dd66..c16a1f611d7 100644 --- a/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/src/test/java/org/eclipse/jetty/ee9/test/HttpInputIntegrationTest.java +++ b/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/src/test/java/org/eclipse/jetty/ee9/test/HttpInputIntegrationTest.java @@ -55,7 +55,6 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.ssl.SslContextFactory; -import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Tag; @@ -65,6 +64,7 @@ import org.junit.jupiter.params.provider.MethodSource; import static org.awaitility.Awaitility.await; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -139,7 +139,7 @@ public class HttpInputIntegrationTest { try { - assertThat("Server leaks: " + __bufferPool.dumpLeaks(), __bufferPool.getLeaks().size(), Matchers.is(0)); + await().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> assertThat("Server leaks: " + __bufferPool.dumpLeaks(), __bufferPool.getLeaks().size(), is(0))); } finally { diff --git a/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/src/test/java/org/eclipse/jetty/ee9/test/HttpInputTransientErrorTest.java b/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/src/test/java/org/eclipse/jetty/ee9/test/HttpInputTransientErrorTest.java index 950eb29e31f..2d040c460bf 100644 --- a/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/src/test/java/org/eclipse/jetty/ee9/test/HttpInputTransientErrorTest.java +++ b/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/src/test/java/org/eclipse/jetty/ee9/test/HttpInputTransientErrorTest.java @@ -38,6 +38,7 @@ import org.eclipse.jetty.util.component.LifeCycle; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import static org.awaitility.Awaitility.await; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.sameInstance; @@ -58,7 +59,7 @@ public class HttpInputTransientErrorTest try { if (bufferPool != null) - assertThat("Server leaks: " + bufferPool.dumpLeaks(), bufferPool.getLeaks().size(), is(0)); + await().atMost(5, TimeUnit.SECONDS).untilAsserted(() -> assertThat("Server leaks: " + bufferPool.dumpLeaks(), bufferPool.getLeaks().size(), is(0))); } finally {