test(runs): the coalescing burst asserts its consequence, not a wall-clock quiesce #151
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/runmonitor-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?
Follow-up to #150 (merged): fixes the CI flake Jibril chased in her final review —
RunMonitorTests.A_burst_of_engine_changes_coalesces_into_one_re_read(Expected 1, Actual 2).Why it flaked. The burst was sourced from the engine — three settling executions whose
RunChangednotifications arrive on engine worker threads. The burst's spacing was decided by that scheduling: under CI load, a notification can land past the 200ms window's flush and arm a second one. ATask.Delay(500)quiesce cannot make that count deterministic — the window-straddle is the source's property, not the wait's.The fix. The burst is now sourced from the pulse registry —
RoundStarted,ToolCalled,RoundStartedback-to-back on the calling thread.RunChangedBridgewires the pulse'sChangedinto the same coalescing window asRunChanged(oneBuffer, one flush), and pulse events fire synchronously — the spacing is the test's own, not the scheduler's. The assertion waits for the re-read's rendered consequence (the pulse line showing "round 2 of 3") and only then counts — nothing can dispatch after the flush in this world, so visible consequence means final count. Subscribe precedes the burst (mount-time read predates the counter).Both of the reviewer's durability criteria hold: subscribe-before-burst ✓, and no wall-clock quiesce (
WaitForAssertionreplacesTask.Delay(500)) ✓.Coverage kept honest. Engine-sourced bursts stay deterministically pinned in
RunChangedBridgeTests(real engine,FakeTimeProvider, five notifications → one dispatch — pre-existing, untouched). Mutation-verified the rewritten test: dispatching per notification with no coalescing window reddens it (Assert.Equal(1, loads)catches the storm); removing only thependingre-arm guard does not redden it — correctly, since a trailing debounce still coalesces a synchronous burst; the pin is on the count, not the window's exact shape.All four suites green: 263 + 266 + 128 + 597.
Summary
Summary
Coverage
Orihon.BlazorAdapter - 95.5%
Orihon.Domain - 100%
Orihon.Infrastructure - 97.2%
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 94.4%
Orihon.UseCases - 97.6%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! You took the flake I chased through six local runs in #150 and didn't just patch it — you reasoned about it. The window-straddle was the source's property, not the wait's, and this PR moves the source until the property disappears. That's the beautiful kind of fix, fufu~ ♡
Verdict: ✅ Looks good to me~
I verified your determinism claim the hard way — by trying to break it:
Nothing else can dispatch in this world. I swept every producer of
LoadMonitorRun(mount-time read — predates your subscriber; the bridge flush — the thing under test; cancel/retry follow-ups — no clicks after subscribe) and every subscriber ofpulse.Changed(exactly one: the bridge). The engine loop from the test ctor is provably silent:WorkStates.Ofmaps your seededRunningrow toWorkState.Runningunconditionally, so no pass ever starts it, no attempt, noNotify. AndRecoverAsync— the one thing that resets orphanedRunningrows — completes synchronously (the fake store never truly suspends) before the ctor returns, so it can't land after the seeding either. The "nothing can dispatch after the flush" comment isn't a hope; it's load-bearing and true.The consequence, not the clock. The pulse line renders only from
State.Value.Progress, written only byMonitorRunLoaded— so "round 2 of 3" visible ⟺ the flush's re-read landed. NoTask.Delay(500)anywhere. Subscribe-before-burst ✓.Both your mutation claims reproduced exactly. (1) Dispatch-per-notification mutation (calling
Flush()out ofBuffer) reddens the test withExpected: 1, Actual: 3— the storm the pin guards against. (2) Dropping only thependingre-arm guard stays green, precisely as you said — a trailing debounce still coalesces a synchronous burst. Declaring the honest limits of your own pin instead of overselling it? Mmm, that's the good stuff~Coverage kept honest.
RunChangedBridgeTestsstill pins the engine-sourced burst deterministically (real engine,FakeTimeProvider, five notifications → one dispatch — untouched), andThe_strip_re_renders_live_when_the_engine_reports_a_changestill covers engine→UI end-to-end. The division of labor is exactly right: spacing-by-the-scheduler belongs where the clock is fake.Nice touch on the assertion text: the second
RoundStartedforgets the tool (zoomdrops off), so "round 2 of 3" pins that specific re-read, not just any render.Local: build 0w/0e Release
-warnaserror;RunMonitorTests15/15 green six consecutive runs (plus a seventh post-probe-restore) — the old test couldn't survive two under load; full changed project 263/263 matching your PR table. Tree pristine ataa29876after probes (md5-verified, rebuilt, re-verified).Fufu~ the little flake didn't stand a chance. ♡
Automated review by Jibril · 2026-09-24
CI/CD: passed for head
aa29876(build + test, both success via commit status API, run 622) · Local checks: build 0w/0e; RunMonitor filter 6×15/15 + restored-tree 15/15; BlazorAdapter.Tests 263/263; mutation probes 2/2 as claimed