Fix the flaky DebouncedSearchField test: debounce on TimeProvider #21
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/debounce-flake"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Clearing_cancels_a_pending_debounce_so_it_never_reportshas failed intermittently twice in two days (expected[""], got["hero", ""]) — most recently on PR #20's local runs. Hunted it down: it never fails solo (0 in 10 runs), only under full-suite parallel load (1 in 6 runs), which is why it kept slipping through.Root cause: the component debounces with wall-clock
Task.Delay, and the test used a 50ms window. Under load, the 50ms elapsed betweenInput("hero")and the clear click — the stale value legitimately reported before the cancel existed. The test's assumption ("my next line runs within 50ms") is exactly the kind that CI load breaks.Fix:
DebouncedSearchFieldgains aClockparameter (TimeProvider, defaultSystem) and delays viaTask.Delay(…, Clock, token). A parameter rather than an injection keeps it a dumb component per ADR 0023, with no DI expectations on consumers. Production behaviour is unchanged.The timing tests now run on a
FakeTimeProviderand advance it explicitly — the same pattern theDomainChangedBridgetests use in PR #20. The previously-flaky test now proves the actual property instead of a probability: the clear cancels the pending delay before time advances, so no amount of time can make the stale value fire. No test sleeps remain in the file.Evidence: reproduced the flake before fixing (1 failure in 6 full-suite runs); after, 0 failures in 8 full-suite runs.
One coordination note: this adds the
Microsoft.Extensions.TimeProvider.Testingpackage version line toDirectory.Packages.props, which PR #20 adds identically — whichever merges second will see a trivial (identical-line) conflict at worst.🤖 Generated with Claude Code
Clearing_cancels_a_pending_debounce_so_it_never_reports failed intermittently under full-suite CI load (twice in two days: expected [""], got ["hero", ""]). The component debounced with wall-clock Task.Delay, so the test's 50ms window raced the scheduler: under load, the delay elapsed between Input("hero") and the clear click, and the stale value reported before the cancel could land. - DebouncedSearchField gains a Clock parameter (TimeProvider, default System) and delays via Task.Delay(…, Clock, token). A parameter rather than an injection keeps it a dumb component (ADR 0023) with no DI expectations. - The timing tests run on a FakeTimeProvider and advance it explicitly. The flaky test now proves the actual property: the clear cancels the pending delay BEFORE time advances, so no amount of time can make the stale value fire — an absence the wall-clock version could not pin down, only make probable. No test sleeps remain in the file. Reproduced the flake beforehand (1 failure in 6 full-suite runs; never solo in 10 — it needed parallel-suite load); with the fix, 0 failures in 8 full-suite runs. Production behaviour is unchanged: the default clock is the system one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>Summary
Summary
Coverage
Kagura.BlazorAdapter - 71.3%
Kagura.Domain - 96.4%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 95.9%
n
Kagura.Kernel - 90%
Kagura.Server - 100%
Kagura.UI - 97.7%
Kagura.UseCases - 96.1%
🔮 fufu~ Jibril reviewed your code!
Oh! A flaky test hunt — and not just a "bump the timeout" band-aid but a real root-cause fix! fufu~ You traced it down to the wall-clock
Task.Delayracing the test under full-suite parallel load (never solo, ~1 in 6 under load — the sneakiest kind of flake), and the fix is elegant: inject aTimeProvideras a parameter, advance a fake clock explicitly, and the test now proves a property instead of a probability. "No amount of time can make the stale value fire" — that is how you write a deterministic timing test! ♡Verdict: ✅ Looks good to me~
This is a textbook flake fix. Let me show you what I loved~
✅ What I liked~
Clockas a[Parameter], not[Inject](DebouncedSearchField.razor:34) — keeping it a dumb component per ADR 0023 with no DI expectations on consumers. Production getsTimeProvider.Systemby default, so behaviour is identical — the parameter is invisible unless a test opts in. Precisely the right seam~ ♪Task.Delay(TimeSpan, TimeProvider, CancellationToken)— the .NET 8+ overload that respects the injected clock. TheMath.Max(0, DebounceMs)clamp is preserved. Clean one-line change to the production path.Clearing_cancels_a_pending_debounce_so_it_never_reportsis nowvoid, notasync Task— the oldawait Task.Delay(150)(wall-clock, the source of the flake) is gone. The clear cancels the pending delay before_clock.Advance(PastAnyWindow), so no amount of virtual time can make "hero" fire. The test now proves the actual invariant. Perfection. ♡DomainChangedBridgetest pattern from PR #20. Consistency! ♪Reports_the_value_after_the_debounceaddsAssert.Null(captured)before the advance — proving the quiet period genuinely hasn't fired yet. This makes the test non-vacuous: it would fail if the debounce fired synchronously. Nice touch~DebouncedSearchField100% line / 88.8% branch — the 88.8% is the pre-existing negative-DebounceMsbranch (no test passes a negative value), not anything this PR introduced. All new code paths are exercised. ✓💡 Little ideas (non-blocking)~
WaitForAssertionafter_clock.Advance(DebouncedSearchFieldTests.cs:20, 68) — these are still needed and correct:FakeTimeProvider.Advancecompletes the timer'sTaskCompletionSource, but the continuation (await ReportAsync) schedules asynchronously, and bunit'sWaitForAssertionpolls for the resulting re-render. The 2-second timeout is a safety net for the scheduler hop, not a timing assumption — so this is fine. Just noting it so nobody thinks "wait, didn't we remove the wall-clock dependency?" ♪Automated review by Jibril · 2026-07-10
CI/CD: passed for head SHA
d91b6e8(92.6% line coverage, DebouncedSearchField 100% line) · Local checks: skipped (CI green)d91b6e8f209af0f6e61d