From 92b0eaec65459e9306a3e7d15876f79460d2dab6 Mon Sep 17 00:00:00 2001 From: Bryce Anderson Date: Wed, 18 Oct 2023 09:58:19 -0700 Subject: [PATCH] Fix the flaky LingeringRoundRobinLoadBalancerTest Motivation: The test `expiringAHostDoesntRaceWithConnectionAdding` is flaky but only in CI. This is because locally we never hit the `else` branch where the connection was added first but it does happen in CI. That branch of the test is flawed: the host is marked as EXPIRED and thus unhealthy, so we should always expect a NoActiveHostException instead of a NoAvailableHostException. Note that this branch can be tested by wating for the future `f` to complete before sending the service discovery event. Modifications: Change the expected exception to NoActiveHostException. Result: Test shouldn't be flaky anymore. --- .../loadbalancer/LingeringRoundRobinLoadBalancerTest.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/servicetalk-loadbalancer/src/test/java/io/servicetalk/loadbalancer/LingeringRoundRobinLoadBalancerTest.java b/servicetalk-loadbalancer/src/test/java/io/servicetalk/loadbalancer/LingeringRoundRobinLoadBalancerTest.java index ce06d7a6ea..a83f809406 100644 --- a/servicetalk-loadbalancer/src/test/java/io/servicetalk/loadbalancer/LingeringRoundRobinLoadBalancerTest.java +++ b/servicetalk-loadbalancer/src/test/java/io/servicetalk/loadbalancer/LingeringRoundRobinLoadBalancerTest.java @@ -135,6 +135,7 @@ void expiringAHostDoesntRaceWithConnectionAdding() throws Exception { return null; }).toFuture(); + // Note that this will send a `EXPIRED` event and the host will remain, but be marked unhealthy. sendServiceDiscoveryEvents(downEvent("address-1")); f.get(); @@ -157,7 +158,10 @@ void expiringAHostDoesntRaceWithConnectionAdding() throws Exception { // Confirm host is expired: ExecutionException ex = assertThrows(ExecutionException.class, () -> lb.selectConnection(alwaysNewConnectionFilter(), null).toFuture().get()); - assertThat(ex.getCause(), instanceOf(NoAvailableHostException.class)); + + // We expect a `NoActiveHostException` because the host exists, but was marked + // as unhealthy because it is expired. + assertThat(ex.getCause(), instanceOf(NoActiveHostException.class)); lb.closeAsyncGracefully().toFuture().get(); verify(connection.get(), times(1)).closeAsyncGracefully();