CI: measure where Build and test's 10 minutes go, and what the backend suite covers #775
Description
Closed 2026-09-14 — split into #839 (wall clock) and #840 (the four remaining flakes).
Decision 1 (#776), the "split the job" lever (the CI matrix) and two of six flakes (#815) are
done. The record below stays as written; see the closing comment for what moved where.
What
Build and test runs 6.5–11.5 minutes and is the critical path of every PR. Before trading away tests to shrink it, measure where the time actually goes and what the tests are worth. Backend coverage is not measured at all today, so "cut down on tests" cannot currently be an informed decision.
Measured, 2026-09-12
Per-job wall clock on a recent green main run (34678673607, commit 489180e):
| Job | Duration |
|---|---|
| Build and test | 568s |
| Web typecheck, test, and build | 213s |
| Image build + Trivy scan | 129s |
| Publish the commit image | 23s |
Build and test across the last week on main: 695s, 655s, 616s (09-10), 399s, 479s (09-11), 672s, 568s (09-12). No upward trend — this is the steady state, and the spread is ~300s.
Test counts: Domain 491, Application 290, AppHost 10, Integration 1793 = 2584 backend; web 2916.
Locally the integration project alone takes ~5m20s on an unloaded machine, so the suite — not the build — is the cost, and the CI spread above is runner contention on top of it.
Coverage, as it actually stands
Web is measured and gated. web/vite.config.ts:148 configures @vitest/coverage-v8 with a regression floor, and ci.yml:179 runs npm run test:coverage. Current: 91.2% statements, 87.64% branch, 86.56% functions, 94.16% lines. The config is explicit that the thresholds are a floor near current numbers, not a target, and that ~14 SPA screens remain untested.
Backend is not measured. No coverlet, no --collect, no reportgenerator anywhere in Directory.Packages.props, the test .csproj files, or ci.yml. So for the 1793 integration tests that dominate CI, nobody can say which lines they cover, which are redundant, or what deleting any of them would cost.
Why "cut tests" is the wrong first lever
The integration suite is slow by design, and the design is load-bearing. AGENTS.md requires a real Postgres via Testcontainers and forbids SQLite because EF's SQL semantics differ — several shipped defects (the Version concurrency-token races, #562's detached tenant writes, #769's paging) are only observable against real SQL. Those tests are slow because they are the ones that catch the expensive bugs.
Two flakes surfaced in one session on 2026-09-12, both timing-sensitive and neither caused by the PR under test: IdempotencyRecordPurgeSweepTests.Sweep_DoesNotDeleteAnExpiredClaimStolenWhileTheDeleteIsBlocked (fails closed when its interleaving does not occur) and DistributedRateLimiterWiringTests.No_attacker_supplied_dimension_in_key_or_log (expected 429, got 401). Both passed locally on repeat and both passed on re-run. Contention is already costing real time in re-runs, which is evidence for parallelism and isolation work rather than for deletion.
Decide before doing anything
1 — measure the backend first — now tracked as #776, which blocks this. Add coverage collection so the "what is untested" question has data. Whether it becomes a gate is a separate decision; per AGENTS.md, a wrong gate is worse than none. Note #776's own caveat: coverage answers "what is untested", NOT "what is redundant" — the tool for redundancy is mutation testing, and that is a further follow-up.
2 — then pick a lever, in rough order of expected value:
- Container reuse / fixture sharing. Per-class Testcontainers startup is the likeliest dominant cost. Measure it before assuming.
- Parallelism. xUnit collection parallelism is limited by shared-state tests; the two flakes above show which ones are contention-sensitive.
- Split the job. Domain + Application + AppHost (791 tests, ~7s combined) could report in seconds instead of waiting behind the integration suite, giving faster failure signal without deleting anything.
- Delete redundancy — last, and only with coverage data. Duplicate coverage is real, but identifying it by eye is how a guard that catches a shipped defect gets removed.
3 — fix the two flakes regardless. They cost a full re-run each (~10 min) and they erode trust in red.
Non-goal
Reducing the count for its own sake. The suite's size is not the problem; the wall clock and the flakes are.
Found while cutting the 0.1.0 release, where a flake in DistributedRateLimiterWiringTests failed the release PR and needed a re-run.
Activity
chore(ci): measure backend test coverage, report only (#779)
mforce commentedon Sep 12, 2026
Unblocked. #776 merged as
879041b, so the coverage data this issue's decision 1 was waiting on now exists.First measurement, 2026-09-12, from
tools/coverage/collect.sh:One number bears directly on this issue's "delete redundancy" lever, and it argues against it rather than for it. Domain and Application together add 162 covered lines beyond what Integration and AppHost already reach, over an identical 16,896-line denominator. Read naively that says the 781 unit tests are nearly free to delete. They are not: coverage cannot tell a test that asserts a domain invariant from one that happens to execute the same line while asserting an HTTP status, and #771 already found three fully-covered guards that survived the exact mutations they were named for. So the measurement this issue asked for has come back saying the lever it was meant to inform is still the wrong one, and the redundancy question needs mutation testing rather than this data.
docs/decisions/776-backend-coverage.mdrecords both readings.The remaining levers in this issue's decision 2 are untouched by that and are where the value now is: container reuse and fixture sharing, parallelism, and splitting the job so the 791 fast tests report in seconds instead of behind the 1,793 integration tests. The local full run measured today was 5m54s for the integration project alone against ~7s combined for the other three, which is consistent with this issue's own reading that the suite and not the build is the cost.
Also filed while here: #782 skips
Image build + Trivy scanandWeb typecheck, test, and buildon PRs that change no code. That is a separate, smaller saving that takes nothing off the critical path this issue is about, so the two do not interact.Decision 3 of this issue, the two flakes, is still open and unaddressed.
mforce commentedon Sep 13, 2026
Handoff — #775, CI wall clock
Written 2026-09-13 during the issue-cleanup sweep, so this can be picked up cold. Everything below is verified against the repo and the run history, not recalled. Nothing in this issue is started.
Where it stands
Decision 1 is DONE and closed the lever it was meant to open. #776 shipped (
879041b),tools/coverage/collect.shexists, and the first measurement is in the comment above. The result argued against cutting tests: Domain + Application together add only 162 covered lines beyond what Integration and AppHost already reach over an identical 16,896-line denominator. Read naively that says the 781 unit tests are nearly free to delete. They are not, and this is the single most important thing not to re-derive — coverage cannot distinguish a test that asserts a domain invariant from one that happens to execute the same line while asserting an HTTP status, and #771 already found three fully-covered guards that survived the exact mutations they were named for. The redundancy question needs mutation testing;docs/decisions/776-backend-coverage.mdrecords both readings.So "delete redundant tests" is off the table until somebody does mutation testing, which is not this issue.
What is actually left
A. Two flakes — the concrete, bounded work. Start here.
Both surfaced in one session on 2026-09-12, both timing-sensitive, neither caused by the PR under test. Each hit costs a full ~10-minute re-run, and one of them failed a release PR.
IdempotencyRecordPurgeSweepTests.Sweep_DoesNotDeleteAnExpiredClaimStolenWhileTheDeleteIsBlocked—
tests/Cluckwork.Api.IntegrationTests/IdempotencyRecordPurgeSweepTests.cs:120.It fails closed when its interleaving does not occur: the test needs a delete to be blocked while a claim is stolen, and when the schedule does not happen it reports failure rather than inconclusive. That is the right instinct (a test that silently skips proves nothing) implemented in a way that produces red on a green build. Read it before changing it — the interleaving is the point of the test, so the fix is to make the schedule deterministic, not to relax the assertion.
DistributedRateLimiterWiringTests.No_attacker_supplied_dimension_in_key_or_log—
tests/Cluckwork.Api.IntegrationTests/DistributedRateLimiterWiringTests.cs, collectiondistributed-rate-limiter-wiring(:246).Observed expected 429, got 401. That smells like the limiter's window being shared with another test in the same collection, or the request being rejected on auth before it reaches the limiter. Note the collection is declared without
DisableParallelization, unlikeAppDbContextDesignTimeFactoryTests(:293) which sets it explicitly — worth checking whether this one needs it too.Both passed locally on repeat and both passed on CI re-run, so reproduction needs load or repetition, not a single run.
B. The wall-clock levers, in the order the issue argues for them
Measured baseline (run
34678673607, commit489180e):Build and test568s, web 213s, image + Trivy 129s, publish 23s.Build and testacross a week onmain: 695 / 655 / 616 / 399 / 479 / 672 / 568 — no upward trend, spread ~300s, so this is steady state plus runner contention.Test counts: Domain 491, Application 290, AppHost 10, Integration 1793 = 2584 backend. Locally the integration project alone is ~5m20s–5m54s against ~7s for the other three combined. The suite is the cost, not the build.
.github/workflows/ci.yml:192-193, which today is onedotnet test Cluckwork.sln. This is the highest value-per-risk item on the list.Constraints that must not be traded away
AGENTS.md), because EF's SQL semantics differ. Several shipped defects — theVersionconcurrency-token races, Write guard trusts OriginalValue as DB provenance; detached Update/Remove can bypass the tenant theft check #562's detached tenant writes, Sales: the Orders list cannot show which orders are unpaid #769's paging — are only observable against real SQL. The suite is slow because it catches the expensive bugs.Related, and deliberately separate
#782 shipped and skips the
imageandwebjobs on documentation-only PRs. That is a different saving and takes nothing off this issue's critical path —build-and-testis never gated, because the #508 tracked-file pins, the tenancy-docs sweep, GitGuardian andschema-docs/generate.sh --checkare all load-bearing on a markdown-only change.Suggested first PR
Fix the two flakes (section A). They are bounded, they are real defects rather than an investigation, they unblock the parallelism work, and they stop costing a re-run every time they hit. The job split (B1) is a good second PR and touches one line of
ci.yml.mforce commentedon Sep 13, 2026
What CI actually flakes on — a census, and a correction to the handoff above
The handoff names two flakes and calls them "the concrete, bounded work". I went to the CI record before fixing them, and the record does not agree. Six distinct tests failed across six failed
ci.ymlruns in the last two weeks, and neither of the two named tests is among them.OtlpSubprocessExporterTests.Collector_credentials_never_appear_in_child_outputFailed to bind to address http://127.0.0.1:32869: address already in useDurableJobWorkerLeaderGateTests.Leader_PollsAssert.Truefalse,:77StealLossConnectionReleaseTests.StealLoss_ReleasesTheConnection_AndGivesUpQuickly_WhenTheClaimVanishesNpgsqlException : The operation has timed outduring connection openFakeOtlpCollectorTests.Predicate_wait_throws_a_terminal_error_completed_at_the_timeout_catch_boundaryHttpListenerException : Address already in usethrown fromDispose()MultiInstanceRateLimitTests.Login_budget_is_shared_across_two_instances_over_one_redisExpected: TooManyRequests, Actual: Unauthorized,:220DurableJobWorkerLeaderGateTests.Follower_NeverPolls_ButStampsHeartbeatAssert.NotNullonLastSuccessfulPoll,:67Four of the six failed a release PR. That is the cost this issue is really about, and it is higher than "two flakes" suggested.
Two corrections to the handoff, both worth having before anyone plans against it. The
expected 429, got 401symptom it attributes toDistributedRateLimiterWiringTestsis in the CI record underMultiInstanceRateLimitTests— a different class, a different counter backend (real Redis, not the in-process fallback double). And the release-PR failure it credits toDistributedRateLimiterWiringTestsis in the record asOtlpSubprocessExporterTests. The handoff was not wrong thatDistributedRateLimiterWiringTestsflakes — see below — only about which failures were which.Grouped by root cause
A. Ephemeral-port check-to-use races — FIXED in #815. Two of the six, including the one that failed the 0.1.0 release. Three harnesses picked a port by binding a probe on port 0 and closing it, then let something else take that port before the real binder arrived.
FakeOtlpCollector.Dispose()had an independent instance of the same shape: it calledStop()beforeDispose(), and on the managedHttpListenerthe second prefix removal re-binds the port. Both reproduced with runtime evidence, both fixed, guard included. Details in the PR.B.
DurableJobWorkerLeaderGateTests— observed twice, NOT root-caused, and the obvious explanation is wrong. Both cases wait a fixed 120 ms afterStartAsyncand then assert the worker already polled. That reads like a classic sleep-and-hope, which is the story I started with — and it does not survive checking.BackgroundService.StartAsyncrunsExecuteAsyncinline until its first genuinely incomplete await,StubLease.TryAcquireAsyncreturnsTask.FromResultand so completes synchronously, and the firstCreateScope()/MarkSuccessfulPoll()therefore happens beforeStartAsyncreturns. Shrinking the window to 1 ms still passes five runs in a row; 25 runs under load produced no failure. So something else stopsExecuteAsyncreaching that point on a loaded runner, and I did not find it. Do not "fix" this by lengthening the delay — that would be a change with no mechanism behind it, and if the mechanism is not the window, a longer window fixes nothing while making the suite slower. Start from the two stack lines above.C.
MultiInstanceRateLimitTests/DistributedRateLimiterWiringTests— reproduced locally, not explained. Same symptom in both (expected 429, got 401past the permit limit), different backends. I reproducedDistributedRateLimiterWiringTests.Primary_down_fallback_serves_and_enforces_and_alarmsonce in 36 local runs under parallel load. Then I instrumented the counter path — recording the backend, key, count and window for every increment — and got 0 failures in 80 further runs, so I have the reproduction and not the trace. Two things worth not re-deriving: the window is 900 s and wall-clock aligned ([floor(epoch/w)*w, +w)), so a rollover mid-test would produce exactly this symptom, but the crossing probability for a 218 ms test is ~0.02% against an observed ~1-3%, which is two orders of magnitude off and argues the rollover is not it; and the tests in that class are in one xUnit collection, so the_downtoggle cannot flip mid-test and the primary/fallback counters cannot interleave that way either. The next step is instrumentation that survives longer, or a frozenTimeProvideron the counter to eliminate the rollover as a variable outright.D.
StealLossConnectionReleaseTests— one Npgsql connect timeout. Contention, not logic: the failure is insideNpgsqlConnector.Authenticateduring connection open, not in anything the test asserts. Evidence for this issue's parallelism and container-reuse levers rather than a bug of its own.What this does not change
Decision 1 is still done and still argues against cutting tests. Decision 2's levers — splitting the job so the 791 fast tests report in seconds, container reuse, parallelism — are untouched by this and are still where the wall-clock value is. Decision 3 is now two of six fixed, four characterised, which is further than it was and less finished than "fix the two flakes" implied.
test(ci): dispose a child whose readiness fails for any reason
test(ci): run the test projects as a fail-fast matrix
test(ci): fix two guards this branch got wrong
test(ci): stop the integration suite losing ephemeral ports to itself (…
mforce commentedon Sep 14, 2026
Closing — split into #839 and #840
This issue held three separate jobs and two of them finished, which made it read as further
along than it was. Splitting so the pending work is visible on its own.
Done, and staying done
(
docs/decisions/776-backend-coverage.md), measured and deliberately not gated. Its findingstands and still argues against deleting tests.
ci.yml'stestsjob is now a four-leg matrix withfail-fast: true, so a five-second domain failure cancels the nine-minute integration leg(
docs/decisions/775-ci-test-matrix.md, guarded bySolutionTestProjectSplitTests).one that failed the 0.1.0 release.
Moved out, still open
green run is still bounded by the ~9m40s integration leg. Container reuse and parallelism are
untouched, and per-class Testcontainers startup is suspected but never measured. This is where
the remaining value is.
ruled out in the census above.
Four of the six original failures hit a release PR.
Nothing is dropped: the measurements, the census and the "do not cut tests" argument all survive on
this issue as the record, and both successors link back here.
mforce commentedon Sep 14, 2026
Split into #839 and #840 — see the comment above for what is done and what moved.
test(ci): probe the shared login counter key behind the 429/401 flake (…
docs: keep cross-directory rules in root; route every trigger path in…