From 83ae0582c9fbde5b966a074cb143253274b226b4 Mon Sep 17 00:00:00 2001 From: Arturo Bernal Date: Fri, 20 Oct 2023 18:06:11 +0200 Subject: [PATCH] Refactor ExponentialBackoffManager Tests to Remove Thread.sleep(). This commit enhances the ExponentialBackoffManager unit tests by replacing the use of Thread.sleep() with direct manipulation of internal state to simulate the cooldown period. This change improves test reliability and ensures consistent behavior in resource-constrained environments. --- .../TestExponentialBackoffManager.java | 67 +++++++++++++------ 1 file changed, 47 insertions(+), 20 deletions(-) diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestExponentialBackoffManager.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestExponentialBackoffManager.java index c45b9f0e79..2fc2a4e70c 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestExponentialBackoffManager.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestExponentialBackoffManager.java @@ -30,12 +30,16 @@ import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.util.TimeValue; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.RepeatedTest; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.time.Instant; +import java.util.Map; + public class TestExponentialBackoffManager { private ExponentialBackoffManager impl; @@ -62,14 +66,18 @@ public void exponentialBackoffApplied() { } @Test - public void exponentialGrowthRateIsConfigurable() throws InterruptedException { + public void exponentialGrowthRateIsConfigurable() { final int customCoolDownMs = 500; connPerRoute.setMaxPerRoute(route, 4); impl.setBackoffFactor(0.5); impl.setCoolDown(TimeValue.ofMilliseconds(customCoolDownMs)); impl.backOff(route); assertEquals(2, connPerRoute.getMaxPerRoute(route)); - Thread.sleep(customCoolDownMs + 100); // Sleep for a slightly longer than the custom cooldown period + + // Manipulate lastRouteBackoffs to simulate that enough time has passed for the cooldown period + final Map lastRouteBackoffs = impl.getLastRouteBackoffs(); + lastRouteBackoffs.put(route, Instant.now().minusMillis(customCoolDownMs + 1)); + impl.backOff(route); assertEquals(1, connPerRoute.getMaxPerRoute(route)); } @@ -84,41 +92,60 @@ public void doesNotIncreaseBeyondPerHostMaxOnProbe() { } @Test - public void backoffDoesNotAdjustDuringCoolDownPeriod() throws InterruptedException { + public void backoffDoesNotAdjustDuringCoolDownPeriod() { connPerRoute.setMaxPerRoute(route, 4); impl.backOff(route); final long max = connPerRoute.getMaxPerRoute(route); - Thread.sleep(1); // Sleep for 1 ms + + // Manipulate lastRouteBackoffs to simulate that not enough time has passed for the cooldown period + final Map lastRouteBackoffs = impl.getLastRouteBackoffs(); + lastRouteBackoffs.put(route, Instant.now().minusMillis(10)); impl.backOff(route); assertEquals(max, connPerRoute.getMaxPerRoute(route)); } @Test - public void backoffStillAdjustsAfterCoolDownPeriod() throws InterruptedException { + public void backoffStillAdjustsAfterCoolDownPeriod() { connPerRoute.setMaxPerRoute(route, 8); impl.backOff(route); final long max = connPerRoute.getMaxPerRoute(route); - Thread.sleep(DEFAULT_COOL_DOWN_MS + 1); // Sleep for cooldown period + 1 ms + + // Manipulate lastRouteBackoffs to simulate that enough time has passed for the cooldown period + final Map lastRouteBackoffs = impl.getLastRouteBackoffs(); + lastRouteBackoffs.put(route, Instant.now().minusMillis(DEFAULT_COOL_DOWN_MS + 1)); + + // Perform another backoff impl.backOff(route); + + // Assert that the max connections have decreased assertTrue(max == 1 || max > connPerRoute.getMaxPerRoute(route)); } + @Test - public void probeDoesNotAdjustDuringCooldownPeriod() throws InterruptedException { + public void probeDoesNotAdjustDuringCooldownPeriod() { connPerRoute.setMaxPerRoute(route, 4); impl.probe(route); final long max = connPerRoute.getMaxPerRoute(route); - Thread.sleep(1); // Sleep for 1 ms + + // Manipulate lastRouteProbes to simulate that not enough time has passed for the cooldown period + final Map lastRouteProbes = impl.getLastRouteProbes(); + lastRouteProbes.put(route, Instant.now().minusMillis(0)); + impl.probe(route); assertEquals(max, connPerRoute.getMaxPerRoute(route)); } @Test - public void probeStillAdjustsAfterCoolDownPeriod() throws InterruptedException { + public void probeStillAdjustsAfterCoolDownPeriod() { connPerRoute.setMaxPerRoute(route, 8); impl.probe(route); final long max = connPerRoute.getMaxPerRoute(route); - Thread.sleep(DEFAULT_COOL_DOWN_MS + 1); // Sleep for cooldown period + 1 ms + + // Manipulate lastRouteProbes to simulate that enough time has passed for the cooldown period + final Map lastRouteProbes = impl.getLastRouteProbes(); + lastRouteProbes.put(route, Instant.now().minusMillis(DEFAULT_COOL_DOWN_MS + 1)); + impl.probe(route); assertTrue(max < connPerRoute.getMaxPerRoute(route)); } @@ -126,7 +153,6 @@ public void probeStillAdjustsAfterCoolDownPeriod() throws InterruptedException { @Test public void willBackoffImmediatelyEvenAfterAProbe() { connPerRoute.setMaxPerRoute(route, 8); - final long now = System.currentTimeMillis(); impl.probe(route); final long max = connPerRoute.getMaxPerRoute(route); impl.backOff(route); @@ -134,22 +160,23 @@ public void willBackoffImmediatelyEvenAfterAProbe() { } @Test - public void coolDownPeriodIsConfigurable() throws InterruptedException { + public void coolDownPeriodIsConfigurable() { final long cd = 500; // Fixed cooldown period of 500 milliseconds impl.setCoolDown(TimeValue.ofMilliseconds(cd)); - - // Sleep for a short duration before starting the test to reduce potential timing issues - Thread.sleep(100); - - // Probe and check if the connection count remains the same during the cooldown period + connPerRoute.setMaxPerRoute(route, 4); impl.probe(route); final int max0 = connPerRoute.getMaxPerRoute(route); - Thread.sleep(cd / 2); // Sleep for half the cooldown period + + // Manipulate lastRouteProbes to simulate that not enough time has passed for the cooldown period + final Map lastRouteProbes = impl.getLastRouteProbes(); + lastRouteProbes.put(route, Instant.now().minusMillis(cd / 2)); + impl.probe(route); assertEquals(max0, connPerRoute.getMaxPerRoute(route)); - // Probe and check if the connection count increases after the cooldown period - Thread.sleep(cd / 2 + 1); // Sleep for the remaining half of the cooldown period + 1 ms + // Manipulate lastRouteProbes again to simulate that enough time has passed for the cooldown period + lastRouteProbes.put(route, Instant.now().minusMillis(cd + 1)); + impl.probe(route); assertTrue(max0 < connPerRoute.getMaxPerRoute(route)); }