From f0c141b5fedd170e8f7a94e74cc74a995d11463e Mon Sep 17 00:00:00 2001 From: Pranav Saxena Date: Mon, 1 Jul 2024 00:59:31 -0700 Subject: [PATCH] test refactor --- .../TestApacheClientConnectionPool.java | 71 +++++++++++++------ 1 file changed, 51 insertions(+), 20 deletions(-) diff --git a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestApacheClientConnectionPool.java b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestApacheClientConnectionPool.java index 43c34e0e2bc21..c413698584f29 100644 --- a/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestApacheClientConnectionPool.java +++ b/hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/TestApacheClientConnectionPool.java @@ -7,7 +7,7 @@ * "License"); you may not use this file except in compliance * with the License. You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, @@ -18,10 +18,10 @@ package org.apache.hadoop.fs.azurebfs.services; +import java.io.IOException; import java.util.concurrent.atomic.AtomicBoolean; import org.assertj.core.api.Assertions; -import org.junit.Assert; import org.junit.Test; import org.mockito.Mockito; @@ -77,14 +77,46 @@ public void testPoolWithZeroSysProp() throws Exception { @Test public void testEmptySizePool() throws Exception { Configuration configuration = new Configuration(); - configuration.set(FS_AZURE_APACHE_HTTP_CLIENT_MAX_CACHE_CONNECTION_SIZE, "0"); - AbfsConfiguration abfsConfiguration = new AbfsConfiguration(configuration, EMPTY_STRING); - try (KeepAliveCache keepAliveCache = new KeepAliveCache(abfsConfiguration)) { - Assertions.assertThat(keepAliveCache.put(Mockito.mock(HttpClientConnection.class))).isFalse(); - Assertions.assertThat(keepAliveCache.get()).isNull(); + configuration.set(FS_AZURE_APACHE_HTTP_CLIENT_MAX_CACHE_CONNECTION_SIZE, + "0"); + AbfsConfiguration abfsConfiguration = new AbfsConfiguration(configuration, + EMPTY_STRING); + try (KeepAliveCache keepAliveCache = new KeepAliveCache( + abfsConfiguration)) { + assertCachePutFail(keepAliveCache, + Mockito.mock(HttpClientConnection.class)); + assertCacheGetNull(keepAliveCache); } } + private void assertCacheGetNull(final KeepAliveCache keepAliveCache) + throws IOException { + Assertions.assertThat(keepAliveCache.get()) + .describedAs("cache.get()") + .isNull(); + } + + private void assertCacheGetNonNull(final KeepAliveCache keepAliveCache) + throws IOException { + Assertions.assertThat(keepAliveCache.get()) + .describedAs("cache.get()") + .isNotNull(); + } + + private void assertCachePutFail(final KeepAliveCache keepAliveCache, + final HttpClientConnection mock) { + Assertions.assertThat(keepAliveCache.put(mock)) + .describedAs("cache.put()") + .isFalse(); + } + + private void assertCachePutSuccess(final KeepAliveCache keepAliveCache, + final HttpClientConnection connections) { + Assertions.assertThat(keepAliveCache.put(connections)) + .describedAs("cache.put()") + .isTrue(); + } + private void validatePoolSize(int size) throws Exception { try (KeepAliveCache keepAliveCache = new KeepAliveCache( new AbfsConfiguration(new Configuration(), EMPTY_STRING))) { @@ -97,20 +129,20 @@ private void validatePoolSize(int size) throws Exception { } for (int i = 0; i < size; i++) { - Assertions.assertThat(keepAliveCache.put(connections[i])).isTrue(); + assertCachePutSuccess(keepAliveCache, connections[i]); Mockito.verify(connections[i], Mockito.times(0)).close(); } for (int i = size; i < size * 2; i++) { - Assertions.assertThat(keepAliveCache.put(connections[i])).isTrue(); + assertCachePutSuccess(keepAliveCache, connections[i]); Mockito.verify(connections[i - size], Mockito.times(1)).close(); } for (int i = 0; i < size * 2; i++) { if (i < size) { - Assert.assertNotNull(keepAliveCache.get()); + assertCacheGetNonNull(keepAliveCache); } else { - Assert.assertNull(keepAliveCache.get()); + assertCacheGetNull(keepAliveCache); } } System.clearProperty(HTTP_MAX_CONN_SYS_PROP); @@ -127,8 +159,7 @@ public void testKeepAliveCache() throws Exception { keepAliveCache.put(connection); - Assert.assertNotNull(keepAliveCache.get()); - keepAliveCache.put(connection); + assertCacheGetNonNull(keepAliveCache); } } @@ -157,7 +188,7 @@ public void testKeepAliveCacheCleanup() throws Exception { } // Assert that the closed connection is removed from the cache. - Assert.assertNull(keepAliveCache.get()); + assertCacheGetNull(keepAliveCache); Mockito.verify(connection, Mockito.times(1)).close(); } } @@ -181,7 +212,7 @@ public void testKeepAliveCacheCleanupWithConnections() throws Exception { * remove the TTL-elapsed connection. */ Mockito.verify(connection, Mockito.times(0)).close(); - Assert.assertNull(keepAliveCache.get()); + assertCacheGetNull(keepAliveCache); Mockito.verify(connection, Mockito.times(1)).close(); keepAliveCache.resumeThread(); } @@ -196,9 +227,9 @@ public void testKeepAliveCacheConnectionRecache() throws Exception { HttpClientConnection.class); keepAliveCache.put(connection); - Assert.assertNotNull(keepAliveCache.get()); + assertCacheGetNonNull(keepAliveCache); keepAliveCache.put(connection); - Assert.assertNotNull(keepAliveCache.get()); + assertCacheGetNonNull(keepAliveCache); } } @@ -230,10 +261,10 @@ public void testKeepAliveCacheRemoveStaleConnection() throws Exception { i--) { // The last two connections are not stale and would be returned. if (i >= (DEFAULT_HTTP_CLIENT_CONN_MAX_CACHED_CONNECTIONS - 2)) { - Assert.assertNotNull(keepAliveCache.get()); + assertCacheGetNonNull(keepAliveCache); } else { // Stale connections are closed and removed. - Assert.assertNull(keepAliveCache.get()); + assertCacheGetNull(keepAliveCache); Mockito.verify(connections[i], Mockito.times(1)).close(); } } @@ -251,7 +282,7 @@ public void testKeepAliveCacheClosed() throws Exception { () -> keepAliveCache.get()); HttpClientConnection conn = Mockito.mock(HttpClientConnection.class); - Assertions.assertThat(keepAliveCache.put(conn)).isFalse(); + assertCachePutFail(keepAliveCache, conn); Mockito.verify(conn, Mockito.times(1)).close(); keepAliveCache.close(); Mockito.verify(keepAliveCache, Mockito.times(1)).closeInternal();