Skip to content

test: make the leader-gate tests wait on a signal, not a delay - #1207

Merged
mforce merged 7 commits into
mainfrom
test/1202-leader-flakes
Oct 11, 2026
Merged

mforce merged 7 commits into
mainfrom
test/1202-leader-flakes

Conversation

@mforce

@mforce mforce commented Oct 11, 2026 •

Copy link
Copy Markdown
Owner

Closes #1202

Cause: test timing, not a gate race

Both CI failures (run 38028393782 attempt 1 on #1172, run 38031633308 attempt 1 on #1175) are the same shape: the test started the worker with a stub lease answering Leader, slept a fixed delay, stopped the worker, and then found the first iteration had done nothing.

  • OAuthPurgeTests.Sweep_RunsOnlyOnTheLeader: the leader did not run the OAuth sweep after a 500 ms sleep.
  • DurableJobWorkerLeaderGateTests.Leader_Polls: Creates > 0 false after a 120 ms sleep. The test took 999 ms, so the runner was starved.

On .NET 10, BackgroundService.StartAsync returns before ExecuteAsync runs its first iteration. A probe asserting the scope count right after await StartAsync read 0 with the loop still pending. The first poll is therefore a thread-pool work item scheduled after the sleep starts, and a starved pool can run it after the sleep ends and StopAsync has cancelled it. The gate itself is deterministic given the stub, so there is no product change.

A second, independent race in the OAuth test: the test host runs the real DurableJobWorker under the Postgres lease, and its own poll runs OAuthPurgeSweep on the same rows. With the test's worker delayed, the follower assertion failed with a follower ran the OAuth sweep because the host's worker had deleted the row. IdempotencyRecordPurgeSweepTests already removes the host worker for the same reason; that factory is now NoHostWorkerFactory and the OAuth purge tests use it too.

Fix

SignallingLease answers with a scripted status and completes a TaskCompletionSource on the second acquisition. The loop is sequential, so the second acquisition proves the first iteration (poll, sweeps or heartbeat stamp) has finished. Each test starts the worker, awaits that signal, then stops the worker. The 30 s WaitAsync bounds a hung worker; it never paces the test.

Proof

Injected-delay mutation: await Task.Delay(1000, stoppingToken) at the top of ExecuteAsync, so the first iteration arrives late.

Tests Result
before this PR red: Leader_Polls (same assertion as CI), Follower_NeverPolls_ButStampsHeartbeat; Sweep_RunsOnlyOnTheLeader red on the follower assertion from the host worker's sweep
after this PR green, 5 of 5

Signal-timeout mutation (lease never signals, wait shortened to 2 s): every gate test fails with TimeoutException on both helpers. A scratch test asserting the worker's ExecuteTask is completed afterwards fails on the helper in 74ef602 (the worker is still running after the timeout) and passes on 64ee166, where the wait sits in try/finally with the bounded StopAsync and Dispose.

Looped runs of the fixed tests (Sweep_RunsOnlyOnTheLeader + the four gate tests, serial, each run its own dotnet test on the Testcontainers cluster):

stage runs green
CPU pressure: 24 busy loops at nice 10, test process at nice 19, load average about 44; the loops expired after 30 minutes, so about the first 6 runs were under full pressure 30 30
quiet host 30 30

Pre-fix rate under the same pressure: pending, this body will be updated.

Tests run locally

dotnet test tests/Cluckwork.Api.IntegrationTests --filter "FullyQualifiedName~OAuthPurgeTests|FullyQualifiedName~DurableJobWorkerLeaderGateTests|FullyQualifiedName~IdempotencyRecordPurgeSweepTests": 16 passed.

Not changed: DurableJobWorkerResilienceTests.HostedLoop_SurvivesContinuousFailures_AndStopsPromptly sleeps the same way but asserts only that the loop is not faulted, which starvation cannot make fail.

…e leader-gate tests

On .NET 10, BackgroundService.StartAsync returns before ExecuteAsync runs its
first iteration, so a fixed delay after StartAsync raced a starved thread pool
and the leader did nothing before the test stopped it (#1202). Both tests now
wait for the stub lease's second acquisition, which the sequential loop reaches
only after the first iteration's work is done.

The OAuth purge tests also move onto a host without its own DurableJobWorker,
whose Postgres-leased poll swept the same rows and could delete them during the
follower run. NoHostWorkerFactory replaces the identical IdempotencyPurgeSweepFactory.
@mforce mforce added this to the Platform hardening milestone Oct 11, 2026
@mforce

mforce commented Oct 11, 2026

Copy link
Copy Markdown
Owner Author

Review round 1 (Codex, read-only) at 74ef6023.

The diagnosis holds. Both leader tests inject a lease that always returns Leader, so no real lock ordering is involved. In .NET 10 ExecuteAsync runs entirely after StartAsync returns, so a fixed sleep could end before the first poll. The shared host's real worker also swept the same OAuth rows. The real lease, the sequential worker loop and the heartbeat show no hidden race. "Second acquisition" means at least one finished loop iteration. The follower tests now assert after an observed iteration, not after elapsed time. NoHostWorkerFactory adds one cloned database per class and no new container.

Finding 1 (test-only, P2): a signal timeout leaves the worker running. SignallingLease.cs:33: when WaitAsync(30 s) throws, the helper exits before StopAsync, and the worker was started with CancellationToken.None and is never disposed. If a slow run recovers later, that worker keeps polling and can sweep rows from the next OAuth test in the same fixture, which is the interference this PR removes. Fix: wrap the wait in try/finally, call the bounded StopAsync in finally, then dispose the worker. The timeout should still fail the test.

VERDICT: 0 product defects, 1 test-only finding.

A timed-out wait still fails the test, but the worker it started would otherwise
keep polling into later tests of the same fixture.
@mforce

mforce commented Oct 11, 2026

Copy link
Copy Markdown
Owner Author

Fixed in 64ee166: the signal wait sits in try/finally, the bounded StopAsync and Dispose run in finally, and the timeout still fails the test.

Shown with a mutation that never signals (acquisition threshold set to int.MaxValue, wait shortened to 2 s) and a scratch test asserting TimeoutException plus worker.ExecuteTask.IsCompleted:

helper gate tests scratch
old (74ef602) 4 of 4 fail with TimeoutException fails: the worker is still running after the timeout
fixed (64ee166) 4 of 4 fail with TimeoutException passes: worker completed

Mutation and scratch reverted; the three touched classes pass 16 of 16 at the head.

@mforce
mforce merged commit e0eee70 into main Oct 11, 2026
18 checks passed
@mforce
mforce deleted the test/1202-leader-flakes branch October 11, 2026 03:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test: find why the single-leader tests fail at random

1 participant