Repository navigation
CI: measure where Build and test's 10 minutes go, and what the backend suite covers #775
Description
Activity
- added a commit that references this issue
on 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:Project Line Branch Domain 81.4% (1480/1817) 74.9% (651/869) Application 13.9% (1666/11901) 3.6% (114/3129) Integration 93% (15698/16866) 76.4% (3814/4989) AppHost 100% (30/30) 100% (2/2) Combined 94% (15890/16896) 79.5% (3968/4991) 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.
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 withoutDisableParallelization, 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.
- Split the job. The 791 fast tests (Domain + Application + AppHost, ~7s) currently report only after the 1,793 integration tests finish. Splitting them into their own job gives failure signal in seconds and deletes nothing. Edit point:
.github/workflows/ci.yml:192-193, which today is onedotnet test Cluckwork.sln. This is the highest value-per-risk item on the list. - Container reuse / fixture sharing. Per-class Testcontainers startup is the likeliest dominant cost inside the 5m20s. Measure it before assuming — that is the issue's own instruction and it has not been done.
- Parallelism. Limited by shared-state tests; the two flakes in section A are exactly the contention-sensitive ones, so fixing them and raising parallelism are the same investigation.
- Delete redundancy. See above — blocked on mutation testing, not on this issue.
Constraints that must not be traded away
- Integration tests use a real Postgres via Testcontainers and SQLite is forbidden (
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. - Generated EF migration code is excluded from coverage (feat(eggs): make cracked and dirty eggs sellable stock via condition grades (#396) #407 freezes it; every integration test executes all of it on container boot regardless of what it asserts).
- There is no coverage threshold anywhere, and adding one is a separate decision (Backend: measure test coverage, report before gating #776). A floor picked before anyone has seen a number is a guess, and a wrong guard reads as safety.
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.-
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.Run Date PR Test that failed Failure 34509778735 09-10 release 0.1.0 OtlpSubprocessExporterTests.Collector_credentials_never_appear_in_child_outputchild died: Failed to bind to address http://127.0.0.1:32869: address already in use34441172590 09-10 #744 DurableJobWorkerLeaderGateTests.Leader_PollsAssert.Truefalse,:7734046159381 09-06 release 0.1.0 StealLossConnectionReleaseTests.StealLoss_ReleasesTheConnection_AndGivesUpQuickly_WhenTheClaimVanishesNpgsqlException : The operation has timed outduring connection open33985683213 09-05 #688 FakeOtlpCollectorTests.Predicate_wait_throws_a_terminal_error_completed_at_the_timeout_catch_boundaryHttpListenerException : Address already in usethrown fromDispose()33589728196 09-02 release 0.1.0 MultiInstanceRateLimitTests.Login_budget_is_shared_across_two_instances_over_one_redisExpected: TooManyRequests, Actual: Unauthorized,:22033555425832 09-01 release 0.1.0 DurableJobWorkerLeaderGateTests.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.
- added 4 commits that reference this issue
on Sep 13, 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
- Decision 1 — measure the backend before cutting anything. Backend: measure test coverage, report before gating #776 landed backend coverage
(docs/decisions/776-backend-coverage.md), measured and deliberately not gated. Its finding
stands and still argues against deleting tests. - Decision 2, the "split the job" lever.
ci.yml'stestsjob is now a four-leg matrix with
fail-fast: true, so a five-second domain failure cancels the nine-minute integration leg
(docs/decisions/775-ci-test-matrix.md, guarded bySolutionTestProjectSplitTests). - Decision 3, two of six flakes. test(ci): fix the ephemeral-port flakes and make a failing test group cancel the rest #815 fixed the ephemeral-port check-to-use races, including the
one that failed the 0.1.0 release.
Moved out, still open
- CI: measure and cut the integration suite's wall clock (container reuse, parallelism) #839 — measure and cut the integration wall clock. The matrix shortened a red run only; a
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. - CI: root-cause the four remaining integration flakes #840 — the four remaining flakes. Characterised, not root-caused, with the dead ends already
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.- Decision 1 — measure the backend before cutting anything. Backend: measure test coverage, report before gating #776 landed backend coverage
- added a commit that references this issue
on Sep 14, 2026 - added a commit that references this issue
on Oct 3, 2026
What
Build and testruns 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
mainrun (34678673607, commit489180e):Build and testacross the last week onmain: 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:148configures@vitest/coverage-v8with a regression floor, andci.yml:179runsnpm 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, noreportgeneratoranywhere inDirectory.Packages.props, the test.csprojfiles, orci.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.mdrequires a real Postgres via Testcontainers and forbids SQLite because EF's SQL semantics differ — several shipped defects (theVersionconcurrency-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) andDistributedRateLimiterWiringTests.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:
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
DistributedRateLimiterWiringTestsfailed the release PR and needed a re-run.