From 5cc0326a016b49c9f1bf14219ede67c3b389c146 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Thu, 22 Oct 2026 15:44:03 +0100 Subject: [PATCH] Tests: don't race the connect's own reconfigures in AllClusterCommandsRefusedIsQuiet A freshly connected multiplexer fires fire-and-forget reconfigures of its own (connection-restored per physical connection, autoconfigure field changes), and ReconfigureAsync returns false while one is in flight. The test asserted on that return value directly, so it lost the race on the Windows runner (main @ 74c52bf8, net10.0/RESP3). Hoist the retry loop HelloClusterModeUnitTests already used for this into Poll.ReconfigureAsync and use it in both tests; dump the captured log after the assertions so a future failure shows what was actually logged. --- .../ClusterCommandsRefusedUnitTests.cs | 33 ++++++++++++------- .../HelloClusterModeUnitTests.cs | 7 +--- .../StackExchange.Redis.Tests/Helpers/Poll.cs | 20 +++++++++++ 3 files changed, 42 insertions(+), 18 deletions(-) diff --git a/tests/StackExchange.Redis.Tests/ClusterCommandsRefusedUnitTests.cs b/tests/StackExchange.Redis.Tests/ClusterCommandsRefusedUnitTests.cs index 1932824dc..303714560 100644 --- a/tests/StackExchange.Redis.Tests/ClusterCommandsRefusedUnitTests.cs +++ b/tests/StackExchange.Redis.Tests/ClusterCommandsRefusedUnitTests.cs @@ -65,20 +65,29 @@ public async Task AllClusterCommandsRefusedIsQuiet() config.LoggerFactory = logger; await using var conn = await ConnectionMultiplexer.ConnectAsync(config); - log.WriteLine(logger.All); - Assert.Empty(logger.Matching("Encountered error while updating cluster config")); - Assert.NotEmpty(logger.Matching("CLUSTER NODES was refused")); + try + { + Assert.Empty(logger.Matching("Encountered error while updating cluster config")); + Assert.NotEmpty(logger.Matching("CLUSTER NODES was refused")); - await conn.GetDatabase().PingAsync(); + await conn.GetDatabase().PingAsync(); - // ...and a later explicit reconfigure stays quiet too - Assert.True(await conn.ReconfigureAsync("test")); - Assert.Empty(logger.Matching("Encountered error while updating cluster config")); + // ...and a later explicit reconfigure stays quiet too; the connect's own follow-on reconfigures + // (connection restored, autoconfigure changes) may still be in flight, and ReconfigureAsync declines + // while one is, so insist on a pass that actually ran + await Poll.ReconfigureAsync(conn, "test"); + Assert.Empty(logger.Matching("Encountered error while updating cluster config")); - // with no slot map there is nothing to route by, so we must not *route* as a cluster - in particular, - // cross-slot commands go to the proxy rather than being refused client-side (#3284) - Assert.NotEmpty(logger.Matching("no slot map is available; routing as standalone")); - Assert.Equal(ServerType.Standalone, conn.ServerSelectionStrategy.ServerType); - await conn.GetDatabase().StringGetAsync(["a", "b"]); + // with no slot map there is nothing to route by, so we must not *route* as a cluster - in particular, + // cross-slot commands go to the proxy rather than being refused client-side (#3284) + Assert.NotEmpty(logger.Matching("no slot map is available; routing as standalone")); + Assert.Equal(ServerType.Standalone, conn.ServerSelectionStrategy.ServerType); + await conn.GetDatabase().StringGetAsync(["a", "b"]); + } + finally + { + // after the assertions, so a failure shows what the multiplexer actually logged + log.WriteLine(logger.All); + } } } diff --git a/tests/StackExchange.Redis.Tests/HelloClusterModeUnitTests.cs b/tests/StackExchange.Redis.Tests/HelloClusterModeUnitTests.cs index c5b827969..50336b9ab 100644 --- a/tests/StackExchange.Redis.Tests/HelloClusterModeUnitTests.cs +++ b/tests/StackExchange.Redis.Tests/HelloClusterModeUnitTests.cs @@ -63,12 +63,7 @@ public async Task CrossSlotSurvivesSubscriptionReconnect(RedisProtocol protocol) // the restored connection triggers a fire-and-forget reconfigure (and so can a server-type change); // ReconfigureAsync declines while one is in flight, so insist on one that actually ran - var deadline = DateTime.UtcNow.AddSeconds(10); - while (!await conn.ReconfigureAsync("after subscription reconnect")) - { - if (DateTime.UtcNow > deadline) throw new TimeoutException("Timed out waiting for a reconfigure to run"); - await Task.Delay(50); - } + await Poll.ReconfigureAsync(conn, "after subscription reconnect"); try { diff --git a/tests/StackExchange.Redis.Tests/Helpers/Poll.cs b/tests/StackExchange.Redis.Tests/Helpers/Poll.cs index a5a0dc4ff..dd55d1d08 100644 --- a/tests/StackExchange.Redis.Tests/Helpers/Poll.cs +++ b/tests/StackExchange.Redis.Tests/Helpers/Poll.cs @@ -37,4 +37,24 @@ public static async Task UntilAsync(Func predicate, int timeoutMilli return false; } + + /// + /// Runs a reconfigure that is guaranteed to have actually executed, rather than been declined. + /// + /// + /// A freshly connected multiplexer fires its own fire-and-forget reconfigures (connection-restored for each + /// physical connection, autoconfigure field changes), and + /// returns false while one is in flight. A test that asserts on that return value races those, and + /// loses on a slow runner; this keeps asking until a pass runs under the caller's cause. + /// + public static async Task ReconfigureAsync(ConnectionMultiplexer conn, string cause, int timeoutMilliseconds = 10000, int pollMilliseconds = 50) + { + var cancellationToken = TestContext.Current.CancellationToken; + for (int i = 0; i <= timeoutMilliseconds / pollMilliseconds; i++) + { + if (await conn.ReconfigureAsync(cause).ConfigureAwait(false)) return; + await Task.Delay(pollMilliseconds, cancellationToken).ConfigureAwait(false); + } + throw new TimeoutException("Timed out waiting for a reconfigure to run: " + cause); + } }