fix: the run monitor's tests start the engine only where they ask it to act #179
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/run-monitor-test-engine-race"
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?
This fixes the red CI test job on main and on #178.
RunMonitorTests.A_sent_back_only_run_says_so_in_the_headlinefailed there with:The cause
Every
RunMonitorTeststest started the realRunEnginein its constructor. On a slow runner, CI's coverage run among them, the engine's startup pass could run after a test had seeded its rows. It then scheduled the sent-back row, which is pending again, and ran it. With no executor wired, the row failed, and the headline counted a failure the test never seeded.The fix
The constructor no longer starts the engine. Only the two tests that ask it to act call
StartEngine(): the retry test and the live re-render test. The others read the monitor over rows that nothing will run. The test file is the only change.Verified
🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 96.3%
Orihon.Domain - 100%
Orihon.Infrastructure - 98.1%
n
ration
n
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 95.1%
Orihon.UseCases - 93.9%
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__BalloonWords_7
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__Capital_0
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__CoordinateWords_1
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__JustBefore_4
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__KindWords_5
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__OutlineWords_6
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__PositionalProse_2
D7D45893AA49E1445928C26437E2DAC34A6F93741B38DCDA0__SideOfTheBox_3
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A race condition hunt, and such a clean kill~ One constructor line quietly commanding the real scheduler for all seventeen tests — of course a 2-second tick pass landing after a seed would scoop up that sent-back row, run it with no executor, and smear "failed" across a headline the test never seeded. Silly little race~ ♡
Verdict: ✅ Looks good to me~
I read the full diff, the whole test file, the engine's loop (
RunAsync/TickAsync/QueuedAsync),WorkStates, the monitor's effects, and the sibling suites — and then I didn't believe any of it until my probes said so. Three probes, three kills:StartEngine()fromRetrying_a_failed_row_asks_the_engine_for_another_attempt→ fails (the retried row should have started again; it is still on attempt 1). The call is load-bearing: the requeue only moves because a pass picks it up.StartEngine()fromThe_strip_re_renders_live_when_the_engine_reports_a_change→ fails (no.runbarever appears). Load-bearing too.A_sent_back_only_run_says_so_in_the_headline→ fails with exactly CI's message, character for character:String: "1 of 2 stage(s) failed"/Not found: "1 stage(s) sent back for rework". The sent-back row lands in Pending, is Queued as a chain-head (WorkStates.Of: no prerequisite → startable), and the pass runs it with no executor registered → failed straight to the cap. Your root-cause story is not a plausible narrative; it is mechanically confirmed. ♪And the quiet bonus:
A_burst_of_live_changes_coalesces_into_one_re_readassertsAssert.Equal(1, loads)— an engine pass acting on its seeded running row could have dispatched a strayLoadMonitorRunand poisoned that count. Stopping the engine in the read-only tests hardens that one too~I also checked the fifteen tests that don't start the engine: the cancel path (
CancelRunAsync) takes the lock, drains, deletes — self-contained, no scheduler needed — and the rest read the monitor over rows nothing will run, matching howRunChangedBridgeTestspins engine-sourced bursts deterministically elsewhere. Sibling consistency: intact.💡 Little ideas (non-blocking)~
<summary>("The engine's scheduler, which the host's background service owns…") and the new one forStartEnginenow sit stacked as two consecutive<summary>blocks on the same method. It compiles silent (0 warnings, I checked), but the first block's "Several tests here watch the engine act" describes the old always-on world; merging them into one doc that says which tests start it (the two that act through it) would read truer. Pure polish~✅ What I liked~
WaitForAssertionre-click trap at line 78, the "where it lands afterwards is the engine's business" note) are a joy — they record exactly the traps a future editor would fall into.Automated review by Jibril · 2026-10-09
CI: green for head SHA
4cae345· Mutation probes: 3 run, 0 survived · Local suite: skipped (CI green)Review feedback (Jibril, PR #179):
c30fb73💡1
c30fb73: the two stacked<summary>blocks are now one doc onStartEngine. It says only the tests that watch the engine act start it, and why the others leave it stopped.Doc only; tests unchanged: 3,429 green and 3 skipped, 0 warnings.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Back so soon~? Fufu, and with exactly what I asked for. The two stacked
<summary>blocks are gone — one doc now sits onStartEngine, and it says precisely what the little method deserves to have said about it: who starts the engine (only the tests that watch it act), why the others leave it stopped (it must never run a seeded row while the monitor is being read), and when it ends (context disposal cancels the loop's token). Knowledge, recorded where the next editor will find it. ♪Verdict: ✅ Looks good to me~
I re-read the incremental diff (
4cae345..c30fb735) and the full current file: this push touches only the<summary>onStartEngine— not a single statement, test body, or assertion moved. The code I mutation-probed last round is bit-for-bit identical, so per my own bookkeeping those three probes still stand:StartEngine()fromRetrying_a_failed_row_asks_the_engine_for_another_attempt→ killed (fails:the retried row should have started again; it is still on attempt 1).StartEngine()fromThe_strip_re_renders_live_when_the_engine_reports_a_change→ killed (no.runbarever appears).A_sent_back_only_run_says_so_in_the_headline→ killed (reproduces CI's failure message exactly:1 of 2 stage(s) failed/Not found: "1 stage(s) sent back for rework").No probes re-run — the probed code and its tests are unchanged, so the kills carry over. Nothing new to probe: comments don't have mutants worth hunting. ♡
✅ What I liked~
Automated review by Jibril · 2026-10-09
CI: green for head SHA
c30fb735· Mutation probes: 0 run, 0 survived / skipped (code + tests unchanged since4cae345, 3 kills carried over) · Local suite: skipped (CI green)