Refactor clock-injection pool tests to remove duplication by running a single parameterized test over PoolConcurrencyPolicy and centralizing pool construction in PoolTestSupport.

Author: arturobernalg

httpcore5/src/test/java/org/apache/hc/core5/pool/PoolTestSupport.java

@@ -27,6 +27,7 @@
 package org.apache.hc.core5.pool;
 
 import java.io.IOException;
+import java.time.Clock;
 
 import org.apache.hc.core5.io.CloseMode;
 import org.apache.hc.core5.io.ModalCloseable;
@@ -59,54 +60,49 @@ boolean isClosed() {
 
     static final DisposalCallback<DummyConn> DISPOSAL = DummyConn::close;
 
-    enum PoolType {
-        STRICT,
-        LAX,
-        OFFLOCK;
-
-        ManagedConnPool<String, DummyConn> createPool(
-                final int defaultMaxPerRoute,
-                final int maxTotal) {
-
-            final TimeValue ttl = TimeValue.NEG_ONE_MILLISECOND;
-            final PoolReusePolicy reusePolicy = PoolReusePolicy.LIFO;
-
-            switch (this) {
-                case STRICT: {
-                    return new StrictConnPool<>(
-                            defaultMaxPerRoute,
-                            maxTotal,
-                            ttl,
-                            reusePolicy,
-                            DISPOSAL,
-                            null);
-                }
-                case LAX: {
-                    return new LaxConnPool<>(
-                            defaultMaxPerRoute,
-                            ttl,
-                            reusePolicy,
-                            DISPOSAL,
-                            null);
-                }
-                case OFFLOCK: {
-                    return new RouteSegmentedConnPool<>(
-                            defaultMaxPerRoute,
-                            maxTotal,
-                            ttl,
-                            reusePolicy,
-                            DISPOSAL);
-                }
-                default: {
-                    throw new IllegalStateException("Unexpected: " + this);
-                }
+    static ManagedConnPool<String, DummyConn> createPool(
+            final PoolConcurrencyPolicy policy,
+            final int defaultMaxPerRoute,
+            final int maxTotal,
+            final Clock clock) {
+
+        final TimeValue ttl = TimeValue.NEG_ONE_MILLISECOND;
+        final PoolReusePolicy reusePolicy = PoolReusePolicy.LIFO;
+
+        switch (policy) {
+            case STRICT: {
+                return new StrictConnPool<>(
+                        defaultMaxPerRoute,
+                        maxTotal,
+                        ttl,
+                        reusePolicy,
+                        DISPOSAL,
+                        null,
+                        clock);
+            }
+            case LAX: {
+                return new LaxConnPool<>(
+                        defaultMaxPerRoute,
+                        ttl,
+                        reusePolicy,
+                        DISPOSAL,
+                        null,
+                        clock);
+            }
+            case OFFLOCK: {
+                return new RouteSegmentedConnPool<>(
+                        defaultMaxPerRoute,
+                        maxTotal,
+                        ttl,
+                        reusePolicy,
+                        DISPOSAL,
+                        null,
+                        clock);
+            }
+            default: {
+                throw new IllegalStateException("Unexpected: " + policy);
             }
         }
-
-        boolean hasHardTotalLimit() {
-            return this != LAX;
-        }
-
     }
 
 }

httpcore5/src/test/java/org/apache/hc/core5/pool/TestConnPoolClockInjection.java

@@ -35,12 +35,14 @@
 import java.time.Instant;
 import java.time.ZoneId;
 import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicLong;
 
 import org.apache.hc.core5.io.CloseMode;
 import org.apache.hc.core5.util.TimeValue;
 import org.apache.hc.core5.util.Timeout;
-import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EnumSource;
 
 final class TestConnPoolClockInjection {
 
@@ -77,27 +79,20 @@ public long millis() {
         public Instant instant() {
             return Instant.ofEpochMilli(millis());
         }
+
     }
 
-    @Test
-    void strictPoolCloseIdleUsesInjectedClock() throws Exception {
+    @ParameterizedTest
+    @EnumSource(PoolConcurrencyPolicy.class)
+    void closeIdleUsesInjectedClock(final PoolConcurrencyPolicy policy) throws Exception {
         final TestClock clock = new TestClock(0L);
-
-        final StrictConnPool<String, PoolTestSupport.DummyConn> pool = new StrictConnPool<>(
-                1,
-                1,
-                TimeValue.NEG_ONE_MILLISECOND,
-                PoolReusePolicy.LIFO,
-                PoolTestSupport.DISPOSAL,
-                null,
-                clock);
+        final ManagedConnPool<String, PoolTestSupport.DummyConn> pool =
+                PoolTestSupport.createPool(policy, 1, 1, clock);
 
         try {
             final Future<PoolEntry<String, PoolTestSupport.DummyConn>> f1 =
-                    pool.lease("r1", null, Timeout.DISABLED, null);
-            assertTrue(f1.isDone());
-
-            final PoolEntry<String, PoolTestSupport.DummyConn> e1 = f1.get();
+                    pool.lease("r1", null, Timeout.ofSeconds(1), null);
+            final PoolEntry<String, PoolTestSupport.DummyConn> e1 = f1.get(1, TimeUnit.SECONDS);
             assertNotNull(e1);
 
             final PoolTestSupport.DummyConn conn = new PoolTestSupport.DummyConn();
@@ -106,100 +101,20 @@ void strictPoolCloseIdleUsesInjectedClock() throws Exception {
 
             pool.closeIdle(TimeValue.ofMilliseconds(1));
             assertFalse(conn.isClosed());
-
-            clock.advanceMillis(10_000L);
-
-            pool.closeIdle(TimeValue.ofMilliseconds(1));
-            assertTrue(conn.isClosed());
-
-            assertEquals(0, pool.getTotalStats().getAvailable());
+            assertEquals(1, pool.getTotalStats().getAvailable());
             assertEquals(0, pool.getTotalStats().getLeased());
             assertEquals(0, pool.getTotalStats().getPending());
-        } finally {
-            pool.close(CloseMode.IMMEDIATE);
-        }
-    }
-
-    @Test
-    void laxPoolCloseIdleUsesInjectedClock() throws Exception {
-        final TestClock clock = new TestClock(0L);
-
-        final LaxConnPool<String, PoolTestSupport.DummyConn> pool = new LaxConnPool<>(
-                1,
-                TimeValue.NEG_ONE_MILLISECOND,
-                PoolReusePolicy.LIFO,
-                PoolTestSupport.DISPOSAL,
-                null,
-                clock);
-
-        try {
-            final Future<PoolEntry<String, PoolTestSupport.DummyConn>> f1 =
-                    pool.lease("r1", null, Timeout.DISABLED, null);
-            assertTrue(f1.isDone());
-
-            final PoolEntry<String, PoolTestSupport.DummyConn> e1 = f1.get();
-            assertNotNull(e1);
-
-            final PoolTestSupport.DummyConn conn = new PoolTestSupport.DummyConn();
-            e1.assignConnection(conn);
-            pool.release(e1, true);
-
-            pool.closeIdle(TimeValue.ofMilliseconds(1));
-            assertFalse(conn.isClosed());
 
             clock.advanceMillis(10_000L);
 
             pool.closeIdle(TimeValue.ofMilliseconds(1));
-            assertTrue(conn.isClosed());
-
             assertEquals(0, pool.getTotalStats().getAvailable());
             assertEquals(0, pool.getTotalStats().getLeased());
             assertEquals(0, pool.getTotalStats().getPending());
-        } finally {
-            pool.close(CloseMode.IMMEDIATE);
-        }
-    }
-
-    @Test
-    void offlockPoolCloseIdleUsesInjectedClock() throws Exception {
-        final TestClock clock = new TestClock(0L);
-
-        final RouteSegmentedConnPool<String, PoolTestSupport.DummyConn> pool = new RouteSegmentedConnPool<>(
-                1,
-                1,
-                TimeValue.NEG_ONE_MILLISECOND,
-                PoolReusePolicy.LIFO,
-                PoolTestSupport.DISPOSAL,
-                null,
-                clock);
-
-        try {
-            final Future<PoolEntry<String, PoolTestSupport.DummyConn>> f1 =
-                    pool.lease("r1", null, Timeout.DISABLED, null);
-            assertTrue(f1.isDone());
-
-            final PoolEntry<String, PoolTestSupport.DummyConn> e1 = f1.get();
-            assertNotNull(e1);
-
-            final PoolTestSupport.DummyConn conn = new PoolTestSupport.DummyConn();
-            e1.assignConnection(conn);
-            pool.release(e1, true);
-
-            // Not idle yet at t=0
-            pool.closeIdle(TimeValue.ofMilliseconds(1));
-            assertFalse(conn.isClosed());
-            assertEquals(1, pool.getStats("r1").getAvailable());
-
-            // Advance injected clock: now the entry is definitely idle
-            clock.advanceMillis(10_000L);
-
-            pool.closeIdle(TimeValue.ofMilliseconds(1));
-            assertEquals(0, pool.getStats("r1").getAvailable());
 
-            final PoolStats totals = pool.getTotalStats();
-            assertEquals(0, totals.getAvailable());
-            assertEquals(0, totals.getLeased());
-            assertEquals(0, totals.getPending());
+            if (policy != PoolConcurrencyPolicy.OFFLOCK) {
+                assertTrue(conn.isClosed());
+            }
         } finally {
             pool.close(CloseMode.IMMEDIATE);
         }