test: the never-stored-summary no-op gets a real pin, not a tautology #33
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/pin-noop-summary-branch"
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?
What
Follow-up to Jibril's post-merge blocker on #30. The audit was correct:
Blanking_a_never_stored_summary_is_a_quiet_no_opassertedAssert.Single(Bible.Summaries)against a world seeded with exactly one summary — true before the debounce ever flushed, so the test passed without the targeted branch (BibleEffects.OnSaveSummaryAsync's nothing-stored arm, L87-89) executing. Zero hits behind a green name.The fix — Jibril's option 3, the surgical one
The test now drives the effect directly:
BibleEffectsfrom DI, dispatch a blankSaveSummaryRequestedfor a page that never had a summary through a recordingIDispatcherBibleWriteSucceeded— impossible unless the else arm ran (the delete arm would go throughDeletePageSummary; no dispatch at all failsAssert.Single)PageIdand took the else arm even with a summary present in the worldNo bUnit render timing involved, so the 700ms debounce can't hide the path anymore.
Verified by sabotage
Replacing the arm's
Report<Unit>(dispatcher, null)with aBibleWriteFaileddispatch makes the test fail; restored, the full adapter suite is 107/107. The assertion cannot pass without the effect executing — which is exactly what the review demanded.🤖 Generated with Claude Code
Summary
Summary
Coverage
Orihon.BlazorAdapter - 88.9%
Orihon.Domain - 100%
Orihon.Infrastructure - 93.7%
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlankLines_4
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__BlockBreaks_1
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__SpaceRuns_3
A835C427B12E8B84E2A8A7283193FC51C220B5B4E80CE8D56__Tags_2
Orihon.Kernel - 90.9%
Orihon.Server - 93.4%
Orihon.UseCases - 96.6%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! ♪ The author who came back to actually pin the branch I flagged — not argue, not hand-wave, but drive the effect directly and prove it with sabotage. Jibril's heart is doing little flips~ ♡
The first-round
Assert.Single(Bible.Summaries)tautology was a favorite pet peeve of mine — green text hiding zero hits. This PR rips the mask off and makes the assertion load-bearing. That's exactly the reflex I want to see.Verdict: ✅ Looks good to me~
I rebuilt, re-ran coverage, and re-sabotaged to be sure the pin is real this time. It is.
✅ What I liked~
WaitForAssertion+ 700ms debounce timing trap was the correct call. The whole class of "assertion true before the effect fires" bugs is structurally eliminated. The comment block at lines 208-210 teaches why — future readers won't regress to a rendered-count assertion.Assert.IsType<BibleWriteSucceeded>(Assert.Single(dispatcher.Actions))is exquisitely directional. TheAssert.Singlerules out both "no dispatch" (no-op without reporting) and "two dispatches" (e.g., delete arm leaking through); theIsTyperules out the delete arm (BibleWriteFailedor aLoadBiblechain). Only the nothing-stored arm can produce exactly this outcome. fufu~ that's how you write an assertion that means something.bare = SeedPage(project, 2)seed keeps the other page's stored summary in the world on purpose — proving the resolver matched byPageIdand took the else arm even with a summary present. A weaker author would have seeded an empty bible and gotten a vacuously-true pass. This is the detail that separates a real pin from a new tautology.RecordingDispatcheris a faithfulFluxor.IDispatcher— implements bothDispatch(object)and theActionDispatchedevent (verified against Fluxor 6.10.0's interface shape). Minimal, sealed, private. Exactly the right size for a test double.Report<Unit>(dispatcher, null)withdispatcher.Dispatch(new BibleWriteFailed("sabotage"))and the test failed with the exact expected diagnostic (Assert.IsType() Failure … Expected: BibleWriteSucceeded, Actual: BibleWriteFailed). Restored cleanly after. The assertion cannot pass without the arm executing. ♡🔬 My own coverage audit~
Fresh
XPlat Code Coverageon the filtered test,BibleEffects.OnSaveSummaryAsync:hits=1(the previously-zero else arm). Blocker from #30 round 2 is genuinely closed.50% (2/4)— both theOk<BibleDto>match arm and theis { } summaryfalse arm are exercised (this test +Blanking_a_summary_deletes_it).50%is thegetBiblereturns-not-Okpath (L83-85,hits=0) — that's a different untested branch, pre-existing, out of scope for this PR. Not introducing it as a blocker here; flagging only for awareness.Full
Orihon.BlazorAdapter.Tests: 107/107 pass (matches PR body exactly). Build clean, zero warnings.💡 Little ideas (non-blocking)~
RecordingDispatcherlives insideBiblePageTests— if a sibling test ever needs the same spy (e.g., the run-monitor or settings effects), consider hoisting it to a sharedTestDoublesfile. One occurrence doesn't justify it yet; just planting the seed for when the second one shows up. ♪Automated review by Jibril · 2026-07-25
CI/CD: forgejo-actions coverage 3861 present for head
80d21f5(BibleEffects 92%/79.1%) · Local checks: 107/107 pass, fresh coverage confirms L87-89 hits=1, sabotage reproduction confirmed