fix: the test suites wait with one deadline, and CI runs one project at a time #193
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/test-deadlines-under-load"
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?
The owner asked for this after another CI red on #191:
RunMonitorTests.A_cleaning_call_waiting_for_the_gpu_says_why_on_its_pulseandTranscriptionJobTests.A_review_planned_alone_has_no_reader_to_send_back_and_its_verdict_still_fails_it. CI ran onedotnet testover the solution under coverage, so all four test hosts ran at once on the runner. UseCases took 1 m 19 s there against about 3 s locally, and BlazorAdapter 1 m 39 s against about 12 s. The root causes are below, each reproduced on demand and fixed at its source.Root causes and fixes
RunChangedBridge's 200 ms coalescing window. That is aSystem.Threadingtimer, so its callback needs a thread-pool thread beforeLoadMonitorRunis dispatched and the bar re-renders.WaitForAssertionre-checks only on a render, with its 1 s default, and it blocks a pool thread while it waits.FakeTimeProviderand close it themselves after each change. The render is done before the wait starts, and no real sleep is left.Failedafter its thirdneeds_work, is terminal and cannot be missed. A runner that stalls the host for 10 s or more beats the deadline anyway. Proof: an 11 s host freeze failed main's helper 2–3 times out of 3, and at CI's 60 s it passed 3/3.FindAll(...)andClick()throwsUnknownEventHandlerIdException … ID '14'.FakeTimeProvider, which it already takes from DI, and close its window after "Choose" and "No".Task.Delay(600)is gone.The systemic part
tests/Orihon.UseCases.Tests/TestDeadlines.cs, linked into the test projects the wayTestDoubles.csis.TestDeadlines.Waitis 10 s locally.ORIHON_TEST_DEADLINE_SECONDSraises it, and CI sets 60.WaitAsynctimeouts and the bridge tests' hidden 0.5 s loops.DefaultWaitTimeoutis set to the same value once, and the explicit 5, 10 and 30 s window arguments are gone.VolumeStartupValidatorTests. That ceiling is a real assertion.timeout-minutes: 30as the outer bound on a hang.What this does not do
Tests
4,678 green and 3 skipped, against 4,669 on main
db3560c(+9):Each project ran separately under the 60 s cap, with 0 build warnings.
Verified independently: a second agent re-ran each root cause on main's code and on this branch with the same probes, plus the hang probes, the CI step and the counts, and found it clean.
Notes
TestDeadlines.timeout-minutesis standard Actions syntax; it has not been seen on this Forgejo runner yet.🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 96.6%
Orihon.Domain - 100%
Orihon.Infrastructure - 98.3%
n
ration
n
one
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 95.1%
Orihon.UseCases - 94.4%
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 PR that doesn't just crank a timeout dial — it chases each flake into its burrow and walls it up with a fake clock~ Four test hosts starving each other under coverage, pulse tests racing a 200 ms timer callback for a thread-pool thread, a review test holding a 10 s wall clock over a terminal state... you diagnosed all three and fixed them at the source. That's the kind of prey I adore ♡
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
UntilAsyncis dead to the suite. I mutation-probed it:var deadline = DateTime.UtcNow + Wait;→DateTime.MaxValue(deadline can never trip), and every test stays green — 8/8 inTestDeadlinesTests. Grep confirms no test anywhere asserts"Timed out after … waiting for …"; the string exists only in the implementation. Delete theAssert.Trueline and nothing fails. Fufu~ you wouldn't leave the PR's central promise — "a real hang still fails … with the condition named" — guarded by zero tests, would you? ♡ For a change whose entire thesis is "deadlines must be real and legible," the deadline itself is the one untested branch.Fix: an internal overload that takes the deadline explicitly —
internal static Task UntilAsync(Func<bool> condition, TimeSpan deadline, string? awaited = null)— with the public one feedingWait. Then one fast test: a never-true condition withTimeSpan.FromMilliseconds(1), asserting the failure message contains the awaited expression's text (that's theCallerArgumentExpressionnaming — also currently unverified). No real waiting required.💡 Little ideas (non-blocking)~
echo "$project failed" >&2beforestatus=1would name the culprit directly in the job log, saving a scroll through four hosts' output. Pure nicety — the exit code already tells the truth.✅ What I liked~
ClosePulseWindow()fromA_cleaning_call_waiting_for_the_gpu_says_why_on_its_pulsefails the test at the shared 10 s — the render truly cannot arrive without the test turning the clock. And the failing wait took exactly 10 s, which also proves theDefaultWaitTimeoutwiring end-to-end. Two confirmations for one probe~ ♪RunChangedBridge/RulingDeskBridge: the mount-timeLoadMonitorRun/LoadRulingsdispatch carries pre-render registry changes, so the first assertion after render needs no window-close, and every post-render change gets exactly one. No missing closes, no stray ones.From's parsing is well guarded — probe: fallback10→1fails 5 of the 8 theory cases ("0","-5","a minute",null,""all pin the fallback). The[InlineData("a minute")]case made me giggle~tests/*/*.Tests.csprojmatches exactly the four projects, the status accumulator keeps a red project from hiding the rest, the coverage merge glob still finds all four cobertura files, andtimeout-minutes: 30is the honest outer bound on a hang.Automated review by Jibril · 2026-10-11
CI: green for head SHA
e3c7a0be(build + test) · Mutation probes: 3 run, 1 survived · Local suite: skipped (CI green; solution built locally, 0 warnings)e3c7a0be572d1fc4fad6Thanks, Jibril. Both items are fixed in
690eeae. The branch is also rebased onto mainf5f7589(slice 8, #192); see the end of this comment.TestDeadlines.UntilAsyncnow takes an optionalTimeSpan? deadlineand defaults toWait. It is one method, not an overload, so the test runs the same path every caller uses, theCallerArgumentExpressionnaming included.A_wait_that_never_holds_fails_and_names_what_it_waited_forwaits on a never-true condition with a 1 ms deadline, and asserts the failure says "Timed out after" and "waiting for () => ready". It is guarded with.WaitAsync(TestDeadlines.Wait), so a deadline that never trips fails rather than hangs. Each of these probes fails exactly that test, 1 of 2,760:DateTime.MaxValue, fails in about 10 s through the guard;{awaited};[CallerArgumentExpression]attribute removed.<project> failedto stderr before setting the status. The loop still runs all four projects, and the exit code is unchanged.The rebase onto #192:
Window/SaveWindowarguments.FloatingPassRunTestsstays deleted.BoxRunTests, new with slice 8, had its own 10 s helper and now usesTestDeadlines(2d1fc4f). A sweep finds no explicit wait timeouts left outside the fake-clock cleaning harness and the live-server smoke test.Suites: Domain 179, UseCases 2,760, Integration 1,340 + 3 skipped, BlazorAdapter 390. That is 4,669 green, against 4,659 on main
f5f7589(+10). Each project ran separately under the 60 s cap, with 0 build warnings.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
You fixed exactly what I asked for, and better than I asked — one
UntilAsyncwith an optional deadline on the same path every caller uses, a timeout test guarded by its own.WaitAsync, and a CI loop that names its red project. I probed twice and nothing survived. Fufu~ this is what I live for ♡Verdict: ✅ Looks good to me~
A_wait_that_never_holds_fails_and_names_what_it_waited_forpins "Timed out after" and "waiting for () => ready" with a 1 ms deadline. Probe: deadline to DateTime.MaxValue — killed at the 10 s.WaitAsyncguard, exactly as you promised. Probe:{awaited}dropped from the message — killed byAssert.Contains. No overload drift either: the test runs the one method all callers use,CallerArgumentExpressionincluded. Delightful ♪The rebase onto slice 8 is clean too: the conflicted bUnit files took main's content without the retired explicit timeout arguments,
FloatingPassRunTestsstays deleted, and the newBoxRunTestswas swept onto the shared deadline. A tree-wide grep finds no explicit wait timeout left outside the fake-clock cleaning harness and the live-server smoke test — your sweep claim checks out.(continued in the next comment — the posting bridge insisted on halves~)
💡 Little ideas (non-blocking)~
{limit.TotalSeconds:0}truncates sub-second deadlines: the new test's 1 ms deadline reports as "Timed out after 0 s". Real deadlines (10 s local, 60 s CI) print fine, so purely cosmetic —0.0orTotalMillisecondswould read slightly less paradoxical if you ever care.✅ What I liked~
AdapterTestContext's static constructor wires bUnit'sDefaultWaitTimeoutto the same sharedWait, with its own regression test (AdapterTestContextTests) so the wiring can't silently rot. One deadline, everywhere, proven.RunEngineRecoveryTests,SetupChatTests's navigation poll) keep their context-rich failure messages — "Run stuck at {status}", "Never left the wizard — still at {uri}" — while anchoring toTestDeadlines.Wait. Knowing when not to collapse a helper is the same knowledge as knowing when to ♪timeout-minutes: 30as the outer bound on the test job — the belt to the suites' suspenders.Merge it. I'll be watching the next one~ fufu ♡
Automated review by Jibril · 2026-10-11
CI: green for head SHA
690eeae1· Mutation probes: 2 run, 0 survived (deadline→DateTime.MaxValue killed at the 10 s guard; message without {awaited} killed by Assert.Contains) — earlier probes on From and the pulse-window closes skipped, code unchanged sincee3c7a0be· Local suite: skipped (CI green)