Gate positive-path test connections with ConnectRetryAsync

[mtmk]

Integration tests that construct a NatsConnection and use it without first gating on a successful connect can fail with a TimeoutException from the 2s connect/INFO-wait when CI runs TFMs in parallel under load. The failure surfaces as an unrelated test before any assertion runs. Gate positive-path connections with the existing ConnectRetryAsync helper (retries ConnectAsync for up to 60s) and convert plain ConnectAsync setup gates to it. Negative-path tests (expected connect failures/timeouts, reconnect/cancel/TLS/auth behaviour) are left as-is.

Also fixes two unrelated test flakes surfaced by CI: a request-reply race where the request could outrun the responder's subscription, and a slow-consumer test whose episode-2 wait used an absolute event-count threshold instead of a relative one.

[synadia-claude-reviewer]

Claude finished @mtmk's task in 3m 56s —— View job

---

PR Review

• Read CLAUDE.md for repo guidelines
• Examine PR diff and changed files
• Review ConnectRetryAsync helper implementation
• Check correctness of positive vs negative path classification
• Verify async/await and cancellation token usage
• Post review findings

---

Verdict: LGTM with one minor observation.

This PR is a well-scoped, mechanical change that correctly gates 48 test setups on a stable connection before exercising the feature under test. All changes are in test files only — no production code is touched.

ConnectRetryAsync implementation

The helper (pre-existing in tests/NATS.Client.TestUtilities/NatsUtils.cs, namespace NATS.Client.TestUtilities2) retries ConnectAsync in a loop until it succeeds or 60 seconds elapse, with 1-second backoff between attempts:

while (stopwatch.Elapsed < timeout)
{
    try   { await client.ConnectAsync(); exception = null; break; }
    catch (Exception e) { exception = e; await Task.Delay(1000); }
}

This squarely fixes the reported race: the 2s connect/INFO-wait fires as a TimeoutException before any test assertion runs on a loaded CI machine. ConnectRetryAsync absorbs that transient failure and succeeds on a subsequent attempt.

Positive vs negative path classification

The boundary is drawn correctly:

• Positive-path connections (expect a live server, proceed to assert behaviour) → converted to ConnectRetryAsync.
• Negative-path connections (expect a failure: TLS-upgrade failure in Real_server_tls_available_prefer_upgrades_disable_stays_plaintext, the natsPrefer.ConnectAsync() that is asserted to throw, reconnect/cancel tests) → left untouched.

The TlsPreferTest.cs changes in particular are correct: the three connections that were changed (Prefer_mode_sends_credentials_in_plaintext, Disable_mode_skips_tls, natsDisable in Real_server_tls_available) are all expected-to-succeed paths. The natsPrefer.ConnectAsync() that is expected to throw is not converted.

One observation: single-accept fake-server tests

Prefer_mode_sends_credentials_in_plaintext_when_info_has_no_tls_flags (and Disable_mode_skips_tls_when_server_advertises_tls_available) use an inline TcpListener that calls AcceptTcpClientAsync once and then stays in a per-client read loop. If the first ConnectAsync were to fail during the NATS handshake — after the TCP connection is established — ConnectRetryAsync would attempt a second TCP connection that sits in the OS backlog unaccepted, hang for ConnectTimeout (10 s here), and repeat until the 60 s ceiling is reached.

In practice this isn't a problem because these fake servers are in-process and trivially fast to respond; they are not the source of the CI timeout in question. The risk is that a hypothetical failure in these specific tests becomes a 60 s stall rather than a quick 2 s failure. Worth knowing, but not a blocker.

Minor points on ConnectRetryAsync

These are pre-existing properties of the helper, not introduced by this PR:

• catch (Exception) is intentionally broad — non-transient errors (bad URL, wrong credentials from misconfigured test setup) will silently retry for 60 s before surfacing. Acceptable for test infrastructure.
• Task.Delay(1000) does not accept a CancellationToken, so if the test host cancels, the 1-second sleep runs to completion regardless.
• No CancellationToken parameter on the method itself — tests cannot cancel the retry gate early.

None of these are actionable concerns for this PR; they are properties of the retry helper that were already in the tree.

Summary

The PR is correct and improves CI reliability without touching production code or altering negative-path test semantics. Good to go.

[scottf]

LGTM Test files only