fix(test): both CI flakes at the root — a paged-out assertion and a halved compile budget (#834)

[rbuergi]

Fixes both flakes from #834. Neither was chance, and both landed on PRs that changed nothing — #829 (skills/chart/docs) and #832 (one markdown file).

1. CrossPartitionRealUserQueryTests — the row was paged out

RunUnpinned issued a global nodeType:Markdown query capped at Limit = 10, and UnpinnedStructuredQuery_AsSystem_EmitsInitial then asserted its own seeded {nsA}/doc1 was in the result. That only holds while fewer than ten other Markdown nodes exist anywhere — so against a shared PostgreSQL with sibling tests writing concurrently, the seeded row falls off the page. Hence intermittent, load-dependent, and unrelated to the diff.

The System case now scopes the query to the partition it seeded, so no unrelated writer can displace its row. The two sibling cases assert only the shape of the first emission, so they stay global deliberately. The helper carries a comment explaining which kind of test may skip the scope, since all three share it.

Also corrected the assertion's because: as written it claimed "System sees every partition's rows" while actually proving "…within the first 10 global results".

Not a first-frame-completeness race — the GetQuery contract (gate the first emission until every sub-query has produced its Initial) is not implicated.

2. DynamicGraphIntegrationTest — the budget was halved

The compile test performs a real on-demand Roslyn compile, but its stream assertions were left on ObservableAssertions.DefaultTimeout (10s) inside a [Fact(Timeout = 20000)]. The inner wait silently capped at half the budget the author had declared, and a loaded runner exceeded it.

The waits are now explicit (30s) inside a 60s Fact, matching the sibling tests in the file. This is aligning a wait with the operation it wraps, not widening a timeout to hide a defect — the existing Should(TimeSpan) overload exists for exactly this.

Tested

Both test projects build clean under CI flags (-c Release -p:CIRun=true -warnaserror). No product code changed; no gate weakened.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CsVb5f4N6KuJnvLaNrvjDB

[Copilot]

Pull request overview

This PR addresses two independent CI flakes by (1) making a cross-partition query test deterministic under paging/limit pressure from other concurrent writers, and (2) aligning reactive assertion timeouts with the actual cost of an on-demand Roslyn compile in an integration test.

Changes:

• Add an optional namespaceScope to the RunUnpinned helper and use it for the System-case assertion to avoid “paged out” results under Limit = 10.
• Increase FileSystem_Organizations_GetsHubConfiguration_FromCompiledAssembly test timeout and make stream assertion waits explicit (30s) to match the intended budget.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
test/MeshWeaver.Hosting.PostgreSql.Test/CrossPartitionRealUserQueryTests.cs	Adds optional namespace scoping to avoid nondeterministic assertions against a global capped query.
test/MeshWeaver.Hosting.Monolith.Test/DynamicGraphIntegrationTest.cs	Makes observable assertion timeouts explicit for an on-demand compile path and adjusts the fact timeout accordingly.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[rbuergi]

Reviewed against the source. Fix 1 is a genuine root cause. Fix 2's justification doesn't match its diff. Detail below, plus a coverage note.

1. CrossPartitionRealUserQueryTests — confirmed, real defect

Verified: RunUnpinned issued a global nodeType:Markdown capped at Limit = 10, and UnpinnedStructuredQuery_AsSystem_EmitsInitial was the only case asserting a specific row ({nsA}/doc1) — which is exactly why the two siblings, asserting ChangeType alone, never flaked. Against a shared PG with concurrent writers the seeded row pages out. That's a defect, not chance, and the corrected because is a real improvement: the old text claimed "System sees every partition's rows" while the assertion only ever proved one row was within the first ten global results.

One thing to settle before merge. That case is documented as "Control: the System identity (boot/background queries) — known-good on prod", and it is the only System case on the unpinned path. It now passes namespace:{nsA}. So either the Unpinned/RunUnpinned naming is no longer accurate for it, or the query still fans out and merely filters — I could not confirm which from PostgreSqlMeshQuery without a deeper read. If scoping pins the query to a single schema, the System control no longer exercises cross-partition fan-out at all, which is the behaviour it was there to cover.

Suggestion that keeps determinism and the unpinned path: seed with a unique nodeType (or per-run token) rather than Markdown, so a genuinely global query cannot be crowded out by unrelated writers. Same immunity, no loss of coverage.

2. DynamicGraphIntegrationTest — the stated rationale doesn't hold

the inner waits now match it instead of silently capping at half

They don't. The diff moves the Fact 20s → 60s and the inner wait 10s → 30s. Matching the author's declared budget would be 20s. #834's own lead asks to align "with the test's declared timeout rather than raising either arbitrarily" — this raises both, 3×.

The premise (a real Roslyn compile can exceed a 10s default) is plausible, and the 10s-inside-20s mismatch is a real inconsistency worth fixing. But no measured compile duration is offered, so 30s is a guess, and per the no-band-aid rule raising a bound wants evidence the operation genuinely needs it. One timing log from a loaded shard would settle whether this is a fitted budget or just headroom — and if it turns out the compile takes ~2s and occasionally never completes, the bound was never the problem.

3. Scope of "both flakes"

Accurate for #834, but worth stating plainly in the PR body so this isn't read as closing the flake problem: on #829's first run a third test failed — ExportImportAccessControlTest.Export_ReaderWithoutExportPermission_ReturnsEmpty (60s, shard 3). Not in #834; it is item 2 in #826 ("8 distinct tests each failed exactly once"). Fix 1 also covers #826 item 4. So this closes two of a known ~9-test cluster.

For the record, the evidence that this cluster is ambient rather than diff-induced: #829 failed twice with a diff of zero C# files (2 SKILL.md, 3 Helm YAMLs, 1 doc, .gitignore), on three distinct tests across three distinct shards, all ObservableAssertionException: expected the observable to emit within Ns.

[github-actions]

Test Results (shard 0)

718 tests   714 ✅  4m 50s ⏱️
  9 suites    4 💤
  9 files      0 ❌

Results for commit 459b440.

[github-actions]

Test Results (shard 4)

1 020 tests   1 013 ✅  5m 18s ⏱️
   11 suites      7 💤
   11 files        0 ❌

Results for commit 459b440.

[github-actions]

Test Results (shard 2)

1 636 tests   1 635 ✅  4m 56s ⏱️
   11 suites      1 💤
   11 files        0 ❌

Results for commit 459b440.

[github-actions]

Test Results (shard 3)

   11 files     11 suites   5m 49s ⏱️
1 340 tests 1 340 ✅ 0 💤 0 ❌
1 417 runs  1 417 ✅ 0 💤 0 ❌

Results for commit 459b440.

[github-actions]

Test Results (shard 5)

1 537 tests   1 433 ✅  5m 52s ⏱️
   11 suites    104 💤
   11 files        0 ❌

Results for commit 459b440.

[github-actions]

Test Results (shard 1)

1 616 tests   1 429 ✅  7m 35s ⏱️
   10 suites    187 💤
   10 files        0 ❌

Results for commit 459b440.

[github-actions]

Test Results

   63 files     63 suites   34m 23s ⏱️
7 867 tests 7 564 ✅ 303 💤 0 ❌
7 944 runs  7 641 ✅ 303 💤 0 ❌

Results for commit 459b440.