perf(test): per-class test partition — fixes the parallelism blocker, re-lands AI.Test at 2.11x by rbuergi · Pull Request #1047 · Systemorph/MeshWeaver

rbuergi · 2026-08-09T20:41:39Z

Fixes the root cause behind #1040 and re-lands #1035.

The defect

MonolithMeshTestBase.TestPartition was public const string = "TestData", so every test class in every project wrote into one partition — and a partition has a single owning hub that serialises every write routed through it. Harmless while classes run one at a time; a contention point the moment they do not.

That is why #1035 red on CI: waits that never emitted, in a class that is green in isolation under identical CPU pressure (ThreadTokenUsageTest: 8/8 in 5.6s alone; fails alongside 1,130 neighbours).

🚨 Not a timeout budget — the failing shape is "observable emitted nothing at all", i.e. state changed underneath. Widening those waits would have hidden it.

The fix — one line, opt-in per project

TestPartition becomes public virtual with the default unchanged ("TestData"), so no other project moves. Isolation is opted into by a project's own base class:

// AITestBase — all 39 AI.Test classes get their own partition, from one line
public override string TestPartition => $"TestData{GetType().Name}";

The cascade the compiler forced

Every one is a hard error, never a silent change:


42 static-qualified refs → instance access	15 files
3 hardcoded node paths → `$"{TestPartition}/…"`	`ThreadComposerFlowTest`
2 × `CS1736` default-parameter values	→ null-default + coalesce
2 × `CS0236` static field initialisers	→ instance properties

The other 20 "TestData" strings in AI.Test are Path.Combine(AppContext.BaseDirectory, "TestData") — on-disk directories, unrelated to the partition — and are deliberately untouched.

🚨 The CS1736 sites needed the ?? TestPartition coalesce. string? ns = null alone would have passed null where the partition used to be — a silent behaviour change the compiler could not have caught.

Verified on a representative machine

A dev Mac has 18 cores; the runner has 4 — so maxParallelThreads: 4 means something completely different on each. That is precisely why #1033 and #1035 passed locally and red on CI. DOTNET_PROCESSOR_COUNT=4 makes .NET size its thread pool and GC as though it had 4 cores:

before fix, DOTNET_PROCESSOR_COUNT=4 : 1138 tests, 2 FAILED (ThreadTokenUsageTest)
after  fix, DOTNET_PROCESSOR_COUNT=4 : 1138 tests, 0 failed, 117s

Serial baseline 198.2s → 2.11x.

Scope

Hosting.Monolith.Test (#1033, measured 2.57x) is deliberately not re-enabled here — it additionally needs GcReachabilityCollection from branch perf/share-mesh-cluster. One project at a time, each independently revertible.

No What's New entry

Test infrastructure, no user-visible effect — the pullrequest skill's step 0.5 says to skip and say so.

Fixes the root cause behind #1040 and re-lands #1035. ## The defect `MonolithMeshTestBase.TestPartition` was `public const string = "TestData"`, so EVERY test class in EVERY project wrote into one partition — and a partition has a single owning hub that serialises every write routed through it. Harmless while classes run one at a time; a contention point the moment they do not. That is why #1035 (AI.Test parallel) red on CI: waits that never emitted, in a class that is green in isolation under identical CPU pressure (ThreadTokenUsageTest: 8/8 in 5.6 s alone, failing alongside 1,130 neighbours). Not a timeout budget — 🚨 do not "fix" that shape by widening the wait. ## The fix `TestPartition` becomes `public virtual`, DEFAULT UNCHANGED ("TestData"), so no other project moves. Isolation is opt-in per project base — here, one override: // AITestBase public override string TestPartition => $"TestData{GetType().Name}"; giving all 39 AI.Test classes their own partition from a single line. ## Cascade the compiler forced (each is a hard error, never a silent change) 42 static-qualified refs (MonolithMeshTestBase.TestPartition) -> instance, 15 files 3 hardcoded node paths in ThreadComposerFlowTest -> $"{TestPartition}/…" 2 CS1736 default-parameter values -> null-default + `?? TestPartition` 2 CS0236 static field initialisers -> instance properties The 20 other "TestData" strings in AI.Test are `Path.Combine(AppContext.BaseDirectory, "TestData")` — on-disk directories, unrelated to the partition — and are untouched. 🚨 The CS1736 sites needed the `?? TestPartition` coalesce: `string? ns = null` alone would have passed null where the partition used to be — a silent behaviour change the compiler could NOT have caught. ## Verified on a representative machine, not this one A dev Mac has 18 cores; the runner has 4, so "maxParallelThreads: 4" means something completely different on each — which is exactly why #1033 and #1035 passed locally and red on CI. `DOTNET_PROCESSOR_COUNT=4` makes .NET size its thread pool and GC as if it had 4 cores: before fix, DOTNET_PROCESSOR_COUNT=4 : 1138 tests, 2 FAILED (ThreadTokenUsageTest) after fix, DOTNET_PROCESSOR_COUNT=4 : 1138 tests, 0 failed, 117 s Serial baseline for reference: 198.2 s (2.11x). Hosting.Monolith.Test (#1033, measured 2.57x) is NOT re-enabled here — it also needs GcReachabilityCollection from branch perf/share-mesh-cluster. One project at a time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR fixes cross-class test interference caused by all MonolithMeshTestBase-derived tests sharing a single "TestData" partition, enabling safe intra-project parallelism (notably re-landing MeshWeaver.AI.Test parallel execution) by making the partition name overridable and switching AI tests to per-class partitions.

Changes:

Changed MonolithMeshTestBase.TestPartition from a constant to a virtual instance property (default remains "TestData"), enabling opt-in isolation per test project.
Enabled xUnit parallel test collections for MeshWeaver.AI.Test and updated tests to use the instance TestPartition instead of static/constant partition access.
Fixed compiler-enforced fallout (static-qualified references, default parameter values, static initializers) by converting to instance access patterns.

Reviewed changes

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

Show a summary per file

File	Description
src/MeshWeaver.Hosting.Monolith.TestBase/MonolithMeshTestBase.cs	Makes `TestPartition` overridable (default unchanged) to support per-class partition isolation when opted in.
test/MeshWeaver.AI.Test/AITestBase.cs	Opts AI tests into per-class partitions by overriding `TestPartition`.
test/MeshWeaver.AI.Test/xunit.runner.json	Enables parallel test collections with a fixed max thread count for `MeshWeaver.AI.Test`.
test/MeshWeaver.AI.Test/AutoRouterDispatchTest.cs	Updates thread seeding paths to use `TestPartition`.
test/MeshWeaver.AI.Test/CancellationAckTest.cs	Updates thread paths/main node to use `TestPartition`.
test/MeshWeaver.AI.Test/ChatClientCredentialResolverTest.cs	Switches fallback user namespace from static partition to instance `TestPartition`.
test/MeshWeaver.AI.Test/ChatCommandSubmissionTest.cs	Uses `TestPartition` for thread creation and start calls.
test/MeshWeaver.AI.Test/CrossHubPatchAtomicityTest.cs	Uses `TestPartition` for per-test node paths and `MainNode`.
test/MeshWeaver.AI.Test/DocumentNodeGetTest.cs	Removes static initialization tied to `TestPartition` by using an instance property.
test/MeshWeaver.AI.Test/HarnessInstallGateTest.cs	Moves harness paths and context/namespace paths to `TestPartition`.
test/MeshWeaver.AI.Test/InboxToolIntegrationTest.cs	Uses `TestPartition` for thread seeding paths.
test/MeshWeaver.AI.Test/McpCompileInFlightTruthTest.cs	Removes static initialization tied to `TestPartition` by using an instance property.
test/MeshWeaver.AI.Test/McpFailureSurfacingTest.cs	Fixes default parameter usage by switching to nullable + `?? TestPartition`.
test/MeshWeaver.AI.Test/McpNegativeOperationsTest.cs	Fixes default parameter usage by switching to nullable + `?? TestPartition`.
test/MeshWeaver.AI.Test/ModelSubstitutionTest.cs	Uses `TestPartition` in thread seed path and `MainNode`.
test/MeshWeaver.AI.Test/ProviderRefusalRoundTest.cs	Uses `TestPartition` in thread seed path and `MainNode`.
test/MeshWeaver.AI.Test/RoundFaultTerminalStateTest.cs	Uses `TestPartition` in thread seed path and `MainNode`.
test/MeshWeaver.AI.Test/StreamingTimeoutTest.cs	Uses `TestPartition` in thread seed path and `MainNode`.
test/MeshWeaver.AI.Test/ThreadComposerFlowTest.cs	Updates composer context/namespace/attachments and thread paths to use `TestPartition`.
test/MeshWeaver.AI.Test/ThreadSubmissionIntegrationTest.cs	Updates start/thread/composer paths and seeded thread nodes to use `TestPartition`.
test/MeshWeaver.AI.Test/ThreadTokenUsageTest.cs	Updates thread paths and `MainNode` to use `TestPartition`.
test/MeshWeaver.Security.Test/ScriptTemplateExecuteAccessTest.cs	Switches to instance `TestPartition` for effective-permissions checks.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+    /// <summary>Per-class partition, so concurrently-running AI.Test classes do not contend on one
+    /// partition hub — the root cause behind the parallel failures in #1040 (a partition has ONE
+    /// owning hub that serialises every write routed through it). Overriding here covers all
+    /// AI.Test classes in one place; other projects keep the "TestData" default and are unaffected.</summary>


github-actions · 2026-08-09T20:48:54Z

Test Results

0 tests 0 ✅ 0s ⏱️
0 suites 0 💤
0 files 0 ❌

Results for commit f606d26.

rbuergi · 2026-08-09T20:57:52Z

Closing — the fix is proven but this form of it does not compile solution-wide. Full detail on #1040.

Proven: per-class partitions fix the contention. DOTNET_PROCESSOR_COUNT=4 (which reproduces the 4-vCPU runner): 1138 tests, 2 failed → 0 failed, 117s. Root cause confirmed and remedied — no timeout widened, no test skipped.

Blocked: virtualising the shared MonolithMeshTestBase.TestPartition breaks the wider solution. Full CI-parity build gives 10 errors, and while CS0236 (static initialiser → instance property) is mechanical, CS0120 is not — static methods consume these values, and in xUnit some of those must stay static ([MemberData] sources and similar). Those need per-site design decisions.

The right scope is on #1040: leave the const alone, add protected virtual string ClassPartition => TestPartition, override once in AITestBase, and switch only AI.Test's references. One assembly instead of the solution.

Branch perf/per-class-partition-ai is preserved — it already contains the solved CS1736 and CS0236 cases inside AI.Test, which the scoped version still needs.

Copilot AI lite review requested due to automatic review settings August 9, 2026 20:41

Copilot started reviewing on behalf of rbuergi August 9, 2026 20:42

View session

Copilot AI reviewed Aug 9, 2026

View reviewed changes

rbuergi closed this Aug 9, 2026

rbuergi mentioned this pull request Aug 9, 2026

perf(test): parallelise Threading.Test — 65.5s → 35.3s (1.86x) #1052

Merged

Navigation Menu

Repository navigation

perf(test): per-class test partition — fixes the parallelism blocker, re-lands AI.Test at 2.11x - #1047

perf(test): per-class test partition — fixes the parallelism blocker, re-lands AI.Test at 2.11x#1047
rbuergi wants to merge 1 commit into
mainfrom
perf/per-class-partition-ai

rbuergi commented Aug 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

github-actions Bot commented Aug 9, 2026

Uh oh!

rbuergi commented Aug 9, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

rbuergi commented Aug 9, 2026

The defect

The fix — one line, opt-in per project

The cascade the compiler forced

Verified on a representative machine

Scope

No What's New entry

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

github-actions Bot commented Aug 9, 2026

Test Results

Uh oh!

rbuergi commented Aug 9, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants