fix: control shell test timers and bound test concurrency #28
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/ci-test-flakiness"
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?
ShellWorldfixed timestamps but inherited real timers, so a delayed fixture could exhaust the shell's ten-second call budget even though its test clock had not advanced. This could also prevent the regex-timeout test from printing its expected exit status.Base
ManualClockonFakeTimeProvider, preserving backwards file timestamps throughAdjustTime. The existing deadline and interrupted-command-record tests now advance the fixture clock used by the real DI composition, verifying that the cutoff still works. Cap the SQLite-backed integration and bUnit suites at two concurrent collections so their HTTP and renderer work can make progress on a busy runner. Parallel operations inside concurrency tests still run together.This builds on main's sequential CI workflow and backup error-modal helper, which waits for the Continue button itself. Production deadlines and test wait durations are unchanged.
Validation on the rebased branch:
🔮 fufu~ Jibril reviewed your code!
Oh, clocks and concurrency — my favorite kind of puzzle~ The
ManualClockrebase ontoFakeTimeProvideris exactly the right surgery: timestamps throughAdjustTime, timers only throughAdvance, so a slow runner can never eat a call's ten-second budget. I verified the semantics against the pinned package's own docs (AdjustTime— "updates the current time, and has no impact on outstanding timers"), and it follows the pattern your ownGateClockalready established. Lovely~ ♡But fufu... your CI is red, sweetie. And I dug into exactly why.
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
ContinueAsynchas an unguarded gap between its wait and its click, and it is what failed this PR's own CI (CI / test, run 95 job 1). The wait only requires"Something went wrong"to exist somewhere incut.Markup; the very next line requires aContinuebutton to exist. The stack trace proves the gap fired: the wait passed, thenFindAll("button").First(b => b.TextContent.Trim() == "Continue")threwInvalidOperationException: Sequence contains no matching element. I reproduced it mechanically — same test, same stack, 5/5 runs at this PR's head — so this is not runner folklore; it is a deterministic race in my sandbox and a live one on your runner. A PR whose whole purpose is stable CI cannot merge on a red head, and this helper is squarely in its scope: it's the same class of bUnit timing bug the PR set out to bury. ♡To be precise and fair: I also ran it at the merge base — it fails there too, so this PR did not introduce it. But it inherited it, and it's blocking you now.
Fix: wait for the button (or the modal container) itself, not for text that more than one element can satisfy: Both callers share this one helper, so one fix covers the
FailandFailReadsvariants.💡 Little ideas (non-blocking)~
Nowproperty whose getter silently normalizes throughGetUtcNowis fine for an internal helper (every current assignment is UTC-offset anyway); if a test ever assigns a zoned time, the getter will disagree with what was set. Not today's problem~✅ What I liked~
AdjustTime-for-timestamps /Advance-for-timers split is precisely correct forCancellationTokenSource(MaxDuration, clock), matches theGateClocksibling, and the doc comment states the contract exactly as the package defines it. Verified, not just plausible~ActivatorUtilitiesinterpreter — and they genuinely guard the cutoff: my mutation probe (11s → 9s advance) killed both tests instantly. No decorative coverage here. ♪Now-setter probe (setter no-op) collapsedFixtureReplayTests— the backwards-timestamp path throughAdjustTimeis well guarded too.-m:1 -nodeReuse:falsefinally matches what AGENTS.md has been prescribing all along, and serializing the coverage hosts while capping the two heavyweight assemblies atMaxParallelThreads = 2targets the actual contention instead of sprinklingTask.Delayfairy dust. The comments on the newTestAssembly.csfiles say exactly why they exist. This is how you fix infrastructure~Automated review by Jibril · 2026-10-22
CI: failed (test job) for head SHA
7d44043621· Mutation probes: 2 run, 0 survived · Local suite: failing test reproduced 5/5 on head and merge base; build green (0 warnings); deadline + fixture suites run via probes7d44043621d19ce45e6afix: control shell test timers and bound CI concurrencyto fix: control shell test timers and bound test concurrencySummary
Summary
Coverage
Shikigami.BlazorAdapter - 95.9%
Shikigami.Domain - 98.6%
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__Hex_0
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__Hex_2
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__MediaTypeShape_3
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__NamePattern_1
Shikigami.Infrastructure - 97.2%
on
ration
shot
234D40E1D5003482F41439E482AB211B6EAD2E13F0A9D40B0__Pattern_0
Shikigami.Kernel - 90.9%
Shikigami.Server - 100%
Shikigami.UseCases - 96.1%
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__AssignmentHead_10
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__BraceExpansion_9
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__HexColour_4
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Interval_1
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__IntervalBody_8
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__NameRule_3
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__NewerXY_6
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Number_5
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__OldCount_7
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__TimeOfDay_0
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Window_2
🔮 fufu~ Jibril reviewed your code!
Back so soon? Of course I am — I never take my eyes off a branch I've marked~ ♡ And look what I found: everything I demanded has been delivered.
Verdict: ✅ Looks good to me~
The race I blocked on is dead.
ContinueAsyncnow waits for the very button it clicks (BackupsSectionTests.cs:276-278) — no more gap where"Something went wrong"text satisfies the wait while the circuit's modal hasn't rendered itsContinueyet. And the proof is not just my word: CI is green at this head —build,test, andimageall passed ford19ce45e, including the test job that was red last time. A flakiness-fix PR merging on a green head — that's the natural order of things~ ♪I diffed your new head against the state I reviewed at
7d440436: the six files this PR carries are byte-identical to what I approved and probed, so my earlier verdicts carry over unchanged:Now→AdjustTimefor file timestamps, timers only throughAdvance. Matches theGateClocksibling; the XML-doc contract was verified against the pinned package.RunAsync+Time.Advance), not a bespoke interpreter. My earlier probe (11s → 9s advance) killed both instantly; theNow-setter no-op probe collapsedFixtureReplayTests. Nothing here changed, so nothing needed re-probing.The rest of the delta since my last pass is rebased main (PR #26 — where the
ContinueAsyncfix and the-nodeReuse:falseCI line landed) plus comment polish: the rewordedAdapterTestContextandSearchRunTestscomments are accurate, and the two newTestAssembly.csattributes say exactly why they exist, with no conflictingCollectionBehavioranywhere else in the tree.💡 Little ideas (non-blocking)~
Nowgetter that normalizes throughGetUtcNowis fine while every assignment is UTC-offset (they all are). Not today's problem~✅ What I liked~
The inline error can render before the circuit's modal) tells the next reader exactly why the naive wait was a trap. ♡Fufu~ merge it before I change my mind~ ♡
Automated review by Jibril · 2026-10-22
CI: green for head SHA
d19ce45e· Mutation probes: 0 run, 2 carried over from previous review (probed code byte-identical; both mutants were killed) · Local suite: skipped (CI green)