HotKeysClusterTests.CanUseClusterFilter skips intermittently, hiding the assertions it exists for

[mgravell]

HotKeysClusterTests.CanUseClusterFilter skips intermittently, so it can stop testing anything without saying so.

Noticed while comparing suite skip counts across runs on an unrelated branch: the total moved between 149 and 151 for the same code. The difference is this test, and it is not environmental flakiness in the usual sense - it is a deliberate Skip on the result being empty.

Where

tests/StackExchange.Redis.Tests/HotKeysTests.cs:43

Assert.SkipWhen(result.CpuByKey.IsEmpty, "Expected at least one CPU result"); // can be weird in CI

Observed

Three consecutive local runs of just this test, against the docker cluster, no code changes between them:

Passed!  - Failed: 0, Passed: 4, Skipped: 0
Passed!  - Failed: 0, Passed: 4, Skipped: 0
  Skipped HotKeysClusterTests.CanUseClusterFilter(sample: True) (RESP3)
Passed!  - Failed: 0, Passed: 3, Skipped: 1

Only the sample: True case is affected, which fits: it runs with sampleRatio: 3 (line 25), so the server samples one request in three and can legitimately observe nothing in the window.

Why it matters

Everything the test exists to assert comes after that line - including Assert.Equal(sample, result.IsSampled) at line 62, which is the sampling behaviour the sample: true case is specifically about. So on the runs where sampling happens to catch nothing, the test reports success-by-skipping and checks none of it.

This is the same failure shape as the vacuous assertion fixed in #3226 and the gaps closed in #3235: a test that fails open is worse than no test, because the suite stays green and the count of what actually ran is not something anyone reads.

Suggested direction

Two parts, either or both:

• Make the window deterministic enough that a result is guaranteed - e.g. drive enough traffic through the sampled slot that one-in-three cannot miss, rather than skipping when it does.
• Failing that, split the assertions: the ones that do not depend on CpuByKey having entries (notably IsSampled) should run regardless, so a thin sampling window narrows what is checked rather than skipping the whole test.

If the skip really is unavoidable, it should at least be loud about which assertions were not reached.

[mgravell]

A third option, from review: retry the sample a few times before skipping. That keeps the window honest without needing the key set to be large enough to make a ratio-3 miss impossible, and it fails rather than skips if the sampler genuinely never reports.

Whichever route, the load-bearing part is that IsSampled — the assertion the sample: true case exists for — stops being reachable only when sampling happens to catch something.