Skip to content

Hosting.Test: end negative waits on a positive settled signal (#6817) - #6823

Merged
rbuergi merged 7 commits into
mainfrom
fix/6817-hosting-negative-waits
Oct 16, 2026
Merged

rbuergi merged 7 commits into
mainfrom
fix/6817-hosting-negative-waits

Conversation

@rbuergi

@rbuergi rbuergi commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

Closes #6817

Root cause

Eight MeshWeaver.Hosting.Test tests each ended on NotEmit(TestTimeouts.Quick). Quick is 36 s on CI (Convergence / 3, CI factor 3), and a passing NotEmit always spends its whole window. So those eight cost ~290 s of the project's ~580 s on every run. Splitting the project (#6798) only moved that cost between parts. The measured list (CI timing, CI=true, local trx) has 8 tests, not the ~16 the issue estimated:

Test before after
BuildTriggeredSyncPinsTheBuiltCommitTest × 5 36.1–36.4 s each 0.2 s each
GreenBuildSkipsASettledSourceTest.AContentVerdictAtThisCommit_… 36.3 s 0.3 s
SystemOwnedSweepRetractsTheLastAdminTest.ABijectiveSync_RetractsNothing 36.1 s 0.1 s
SaveMeshNodeRequestIsACheckedWriteTest.AParticipantPosingAsTheHubItself_WritesNothing 36.1 s 3.7 s
whole project (one process, CI=true) 9m39s (Σ 578 s) 4m49s (Σ 288 s)

Fix: each negative ends on a POSITIVE settled signal

  • ObservableAssertions.NotEmitBefore(settled) is new in MeshWeaver.Reactive.Assertions. It asserts that nothing is emitted before the settled signal emits, and it fails if the signal never arrives (silence without a settle proves nothing). A public member is added to a class, and no interface changes.
  • Sentinel through the same pipeline (BuildTriggeredSync…): a push at a sentinel sha goes through the same processor into the same Space, and nothing but the sentinel may be fetched before the sentinel's own fetch. Two tests used to run without a wired source, so a wrong fetch had nothing to fetch; they now arrange one.
  • Next legitimate step (GreenBuildSkips…): the clone of the next commit ends "no second clone of this one".
  • Response that follows the work, then one store read (ABijectiveSync_RetractsNothing): the create is answered only after its post-creation handlers have run, and that includes the sweep. The STORE is then compared with the set read before.
  • Owner's answer, then a short window (AParticipantPosingAsTheHubItself): a follower save to the same hub proves the spoofed save was judged. The raw storage write that save would start has no observable completion, so NotEmit(Quick / 10) opens only after the settle.

Negative controls (product broken locally, then reverted)

  1. Sentinel pattern. I restored the old "green build imports" behaviour in ProcessWorkflowRun, which fires TriggerSyncAtCommit while still answering 1. Result: AGreenBuild_RecordsTheBuild_ButNoLongerImports FAILED with Expected the observable not to emit before its settled signal because a green build records the build and imports nothing …, but it emitted 1111111111111111111111111111111111111111.
  2. Store read after the settled response. I made the retraction sweep run for a bijective sync. Result: ABijectiveSync_RetractsNothing FAILED with Expected collection {"…/_Access/Roland_Access"} to equal {"…/_Access/Roland_Access", "…/_Access/space-owner_Access"}.
    • 🚨 This control also exposed a weak guard. Under this break the sweep retracted one of the two grants, and the last admin's delete was refused by the last-admin invariant. The set never emptied, so the old assertion ("the set does not empty") would have passed against this regression. The test now asserts its name: nothing is retracted.
    • My first break here (bypassing only IsTwoWay) was inert, because IsSystemOwned already excludes two-way syncs.
  3. Settle, then a short window. I let a participant's stamped save through only when its sender equals its target. Result: AParticipantPosingAsTheHubItself_WritesNothing FAILED with Expected the observable not to emit within 3.6s …, but it emitted True.
    • A single store read right after the follower's refusal still showed the old name, and the overwrite landed about 0.1 s later. A pure read-after-settle would therefore have been a false pass, which is why this test keeps a short window that opens after the settle.

After the reverts: full MeshWeaver.Hosting.Test (with CI=true) is 1328 passed, 0 failed, 1 skipped (pre-existing). dotnet build -c Release -warnaserror gives 0 warnings and 0 errors for MeshWeaver.Reactive.Assertions and MeshWeaver.Hosting.Test.

Docs

  • Doc/Architecture/WritingTests gets a new section, "A negative assertion that spends a fixed window when a settled signal exists", plus a new row in the assertion table.
  • Doc/Architecture/ReactiveTestAssertions, the assertions README and the testing skill are updated to match.

Pairs-with: none — no public type or member removed; one public method added to a class.

🤖 Generated with Claude Code

rbuergi and others added 2 commits October 15, 2026 12:15
Eight MeshWeaver.Hosting.Test tests each spent a full NotEmit(TestTimeouts.Quick)
window - 36 s on CI - to pass: ~290 s of the project's ~580 s. A passing NotEmit
always spends its whole window, so the cost cannot be removed by splitting.

- MeshWeaver.Reactive.Assertions: NotEmitBefore(settled) - nothing may emit before
  a positive settled signal; fails (never passes) when the signal does not arrive.
- BuildTriggeredSyncPinsTheBuiltCommitTest (5 tests): a sentinel push through the
  same processor; nothing but the sentinel may be fetched before its own fetch.
  The two tests that ran without a wired source now arrange one, so a wrong fetch
  had something to fetch.
- GreenBuildSkipsASettledSourceTest: the next commit's clone ends "no second clone".
- SystemOwnedSweepRetractsTheLastAdminTest.ABijectiveSync_RetractsNothing: the create
  is answered after its post-creation handlers, then one STORE read compared with
  the set before. The old "set did not empty" passed while a wrongly-run sweep
  retracted one of two grants (the last admin's delete is refused).
- SaveMeshNodeRequestIsACheckedWriteTest.AParticipantPosingAsTheHubItself: a follower
  save to the same hub settles that the spoofed one was judged; the raw storage
  write it would start has no observable completion, so a Quick/10 window opens
  only after the settle.
- WritingTests.md, ReactiveTestAssertions.md, testing skill: the rule and shapes.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 15, 2026 10:22
@rbuergi
rbuergi enabled auto-merge October 15, 2026 10:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Independently scheduled imports can outlive the sentinel signal, allowing negative assertions to pass before forbidden fetches occur.

4 open findings
What changed in this PR

Reduces Hosting test runtime by replacing long negative waits with positive completion signals, addressing the cost that CI splitting only redistributed.

Changes:

  • Adds NotEmitBefore(settled) to reactive assertions.
  • Updates eight tests and strengthens grant-preservation checks.
  • Documents signal-bounded negative assertions.
File Description
test/​MeshWeaver.Hosting.Test/​SystemOwnedSweepRetractsTheLastAdminTest.cs Compares stored grants after sync creation completes.
test/​MeshWeaver.Hosting.Test/​SaveMeshNodeRequestIsACheckedWriteTest.cs Adds a follower response before a shorter observation window.
test/​MeshWeaver.Hosting.Test/​GreenBuildSkipsASettledSourceTest.cs Uses the next commit’s fetch to end the negative check.
test/​MeshWeaver.Hosting.Test/​BuildTriggeredSyncPinsTheBuiltCommitTest.cs Adds sentinel-fetch checks and wired-source fixtures.
src/​MeshWeaver.Reactive.Assertions/​README.md Lists the new assertion.
src/​MeshWeaver.Reactive.Assertions/​ObservableAssertions.cs Implements NotEmitBefore.
src/​MeshWeaver.Documentation/​Data/​Architecture/​WritingTests.md Explains completion-signal patterns.
src/​MeshWeaver.Documentation/​Data/​Architecture/​ReactiveTestAssertions.md Documents the assertion contract and usage.
.claude/​skills/​testing/​SKILL.md Updates negative-assertion guidance.

🧠 Review effort: Balanced


💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +242 to +243
public async Task<ObservableAssertions<T>> NotEmitBefore<TSettled>(IObservable<TSettled> settled,
string because = "", CancellationToken cancellationToken = default)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed — added in 4467933: test/MeshWeaver.Testing.Xunit.Test/NotEmitBeforeContractTest.cs pins every failing branch (an emission before the signal, a signal that never arrives, one that completes empty, one that faults), the subject ending first (silence; the signal still decides), synchronous arming before the triggering action, and cancellation (propagates as OperationCanceledException). Both subscriptions are asserted released on success, failure, timeout and cancellation. All 7 pass in the project's full run.

Comment thread src/MeshWeaver.Reactive.Assertions/ObservableAssertions.cs
@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

   12 files   -    14     12 suites   - 14   28m 37s ⏱️ - 32m 48s
5 503 tests  - 7 290  5 501 ✅  - 7 099  2 💤  - 191  0 ❌ ±0 
5 512 runs   - 7 297  5 510 ✅  - 7 106  2 💤  - 191  0 ❌ ±0 

Results for commit 330ede8. ± Comparison against base commit 22eb9e4.

This pull request removes 7328 and adds 12 tests. Note that renamed tests count towards both.

   --- End of inner exception stack trace ---
   --- End of inner exception stack trace ---)
   --- End of inner exception stack trace ---, expected: True)
   --- End of inner exception stack trace ---, isDenial: True)
 ---> (Inner Exception #1) MeshWeaver.Mesh.QueryProviderStalledException: Query provider(s) [pg] did not emit an Initial within the query fan-in's 16s bound for query 'nodeType:NodeType' (user 'system'). The merged Initial gates on EVERY provider, so this query has NO snapshot to answer with — it is reported as unavailable (retryable) rather than left hanging with no error. This is an availability failure, never a permission verdict: a consumer deciding access must fail CLOSED and say it could not establish the answer. Fix the stalled provider; never bump the consumer's timeout.<---
 ---> (Inner Exception #1) MeshWeaver.Messaging.Hub.Test.InfrastructureFaultTest+ProviderException (0x80004005): Failed to connect to 10.42.18.4:5432<---
 ---> (Inner Exception #1) System.ArgumentException: Value does not fall within the expected range.<---
 ---> (Inner Exception #1) System.InvalidOperationException: boom<---
 ---> (Inner Exception #1) System.InvalidOperationException: source B is misconfigured<---
…
Memex.Portal.Shared.Test.InstanceIdRulesMatchTheRegistryTest ‑ TheSetupHostAgreesWithTheRegistry(candidate: "d65250cc-397f-4b3d-ba95-adfabfbc1b3c")
Memex.Portal.Shared.Test.PublishStorageFaultIsNotAConflictTest ‑ AStorageFault_IsA500_NamingTheError(fault: System.AggregateException: One or more errors occurred. (Access to the path '/data/modules/activation.d/MeshWeaver.Markdown.Export.json' is denied.)
 ---> System.UnauthorizedAccessException: Access to the path '/data/modules/activation.d/MeshWeaver.Markdown.Export.json' is denied.
   --- End of inner exception stack trace ---)
Memex.Portal.Shared.Test.PublishStorageFaultIsNotAConflictTest ‑ AStorageFault_IsA500_NamingTheError(fault: System.InvalidOperationException: landing faulted
 ---> System.UnauthorizedAccessException: Access to the path '/data/modules/activation.d/MeshWeaver.Markdown.Export.json' is denied.
   --- End of inner exception stack trace ---)
Memex.Portal.Shared.Test.SessionDenialIsAnAnswerTest ‑ OnlyAVerdictReadsAsADenial(shape: "the same verdict nested, as a late denial dispatch"···, failure: System.InvalidOperationException: write failed
 ---> System.UnauthorizedAccessException: Access denied
   --- End of inner exception stack trace ---, isDenial: True)
MeshWeaver.Hosting.Test.BuildTriggeredSyncPinsTheBuiltCommitTest ‑ APush_ImportsAtItsCommit_ThoughThatCommitsOwnBuildIsRed
MeshWeaver.Testing.Xunit.Test.NotEmitBeforeContractTest ‑ ACancelledWait_PropagatesCancellation_AndReleasesBoth
MeshWeaver.Testing.Xunit.Test.NotEmitBeforeContractTest ‑ ASignalThatCompletesEmpty_Fails
MeshWeaver.Testing.Xunit.Test.NotEmitBeforeContractTest ‑ ASignalThatFaults_Fails
MeshWeaver.Testing.Xunit.Test.NotEmitBeforeContractTest ‑ ASignalThatNeverArrives_Fails_NeverPasses
MeshWeaver.Testing.Xunit.Test.NotEmitBeforeContractTest ‑ AnEmissionBeforeTheSignal_Fails
…

♻️ This comment has been updated with latest results.

rbuergi and others added 3 commits October 15, 2026 21:24
…#6817)

The red-build test moved to a previous commit's build so the push's own fetch
could settle the negative; that dropped the case where the pushed commit's OWN
build failed. Restore it separately, with the red build's silence ended by the
sentinel push rather than by a fetch of the same sha.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…6817)

Forbidden emission, a signal that never arrives / completes empty / faults,
a subject that ends first, and cancellation; both subscriptions released on
success, failure, timeout and cancellation.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 15, 2026 19:27
@meshweaver-cloud
meshweaver-cloud Bot disabled auto-merge October 15, 2026 19:27
@rbuergi
rbuergi enabled auto-merge October 15, 2026 19:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The assertion contracts have focused coverage, and no blocking issues remain beyond the explicitly accepted test-ordering limitations.

1 open finding
3 resolved since last review

🧠 Review effort: Balanced

rbuergi and others added 2 commits October 16, 2026 07:31
…ait — no 30 s literal

The TestTimeoutLiteralRatchetGuard counted the new NotEmitBeforeContractTest bound (381 > 380).
Every case there decides synchronously; the bound only caps a hung regression.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 16, 2026 05:33
@meshweaver-cloud
meshweaver-cloud Bot disabled auto-merge October 16, 2026 05:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No unresolved blocking findings remain beyond limitations explicitly accepted in prior feedback.

1 open finding

🧠 Review effort: Balanced

@rbuergi
rbuergi enabled auto-merge October 16, 2026 05:41
@rbuergi
rbuergi merged commit 3563c80 into main Oct 16, 2026
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MeshWeaver.Hosting.Test: ~16 tests each spend ~36 s on a negative wait — one CI part runs 426 s of a 480 s cap

2 participants