Stabilize Windows CI (#3168)
* Tests: probe for absent servers instead of discovering them by timeout Cluster tests set connectTimeout=10000 and only skip once a connection attempt has failed, so with no cluster running each of ~50 tests burned 10 seconds before reporting a 1ms skip. Sentinel was the same shape via SentinelBase.InitializeAsync, which polls for 15 seconds per test and then reports a failure rather than a skip. Add Skip.IfNoServer, backed by a single short TCP connect per endpoint cached for the run, and route the six cluster configuration overrides through TestBase.GetClusterConfiguration so the probe happens before a configuration is handed out. This only reports whether anything is listening: a server that is up but unreachable still fails, as it must. Also, while in SentinelBase: wait on the primary connection rather than only the sentinel connection (the retry loop already used the former as its success condition, but the assert checked the latter, so it could pass having never reached the primary), and give the assert a message. Full suite against a primary/replica/secure/failover-only topology goes from 10m34s with 27 failures to 49s with none attributable to the missing servers; against the full topology the skip count is unchanged at 150, confirming nothing is newly suppressed. CI: the Windows job now starts that smaller topology by default, since the cluster and sentinel instances gossip continuously and that job is a fractional vCPU running Windows running WSL running redis. Dispatch with windows-topology=full for everything. Also allows pinning the apt redis version instead of silently testing whatever is GA, and polls for readiness rather than sleeping a fixed 5 seconds. * CI: don't let the readiness probe kill its own step The wsl-bash shell runs with -euo pipefail, so `out=$(redis-cli ... PING)` aborts the entire step the first time a server is not yet accepting connections - which is the exact state the poll exists to wait out. The previous code never hit this because every redis-cli call was guarded with `|| echo ...`. Effect on the last run: the step died 41ms after launching the servers, continue-on-error painted it green, and the suite then ran against a Redis that wasn't there - thousands of "not possible to connect" failures across MultiPrimaryTests, RespProtocolTests and friends, ten minutes later and nowhere near the cause. So: tolerate the expected probe failure, and drop continue-on-error in favour of failing this step explicitly when a required port never comes up. One clear "no server on: 6379" beats several thousand connection errors in a later step. * Attack the actual Windows CI failures, not the topology Three post-change runs said the reduced topology was not the lever: each failed on a different set of tests, none needing the servers it omits, and the job got no faster. So windows-topology defaults back to full - this job is our only coverage of .NET Framework and of Windows' own socket/TLS stacks, and that is worth keeping complete. `basic` stays available via workflow_dispatch as a bisection tool. What the failures actually show is a machine that cannot honour the library's defaults, so: - Raise the thread pool floor (TestConfig static ctor) to max(64, cores*8). The pool grows ~1-2 threads/sec past its minimum, and the suite opens many connections at once; that ramp is what turns a healthy server into "Timeout performing PING (5000ms)" when a synchronous caller is parked waiting for a completion that cannot get a thread. Free on a fast machine. - Add a timeout floor, REDIS_TESTS_MIN_TIMEOUT_MS, set to 20s on the Windows job only, applied to SyncTimeout and AsyncTimeout and only where the test did not ask for a specific timeout - a test choosing a short timeout is testing timeout behaviour. Not applied to ConnectTimeout: bisection showed that flooring it breaks tests that simulate a failure and then allow a fixed window for the heartbeat to reconnect, because a stalled attempt can no longer be retried inside that window. SyncTimeout breaks that same test, so ConnectFailTimeoutTests.NoticesConnectFail now states its dependency explicitly. - Fix two genuinely fragile tests. SelectByLatency compared injected latencies 10ms and 15ms apart and waited a fixed 100ms to "settle"; scheduling noise on a contended machine exceeds that margin and inverts the ordering, so the margins are now 150/300ms and it polls until selection converges. And SelectByWeight's failure was really Assert.True(conn.IsConnected) immediately after ConnectGroupAsync, which races health-check probes that need a round trip; that pattern appeared at six sites, all now waiting via GroupWait. Also drops a duplicate endpoint probe in favour of caching the existing TestConfig.IsServerRunning. Verified: full suite green both with and without the floor (0 failed, 5731 passed, 150 skipped - the same skip count as before any of this), and the whole traversal builds across every TFM. * Tests: wait for the subscription connection before asserting on it PubSubGetAllAnyOrder asserts sub.IsConnected() immediately after connecting, but the subscription connection is separate from the interactive one and can still be coming up at that point - so on a slow or contended machine the assert loses the race. This was the only remaining failure on the Windows CI job, reported as the rather opaque "IsConnected" (the nameof used as the assert message). Wait via the existing UntilConditionAsync helper first. Same treatment for the identical assert in FailoverTests. Note this cannot be demonstrated locally: the race needs a machine slow enough to lose it, so a local run proves only that nothing regressed.
M
Marc Gravell committed
91ac27c8018839ad447a5c725a774d770df4ce08
Parent: 14f624b
Committed by GitHub <noreply@github.com>
on 8/10/2026, 11:40:22 AM