Repository navigation
Flakes: give hub tests a budget the CI fixture can actually meet - #688
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adjusts test timeouts to better reflect the real cost of bringing up the mesh fixture under CI load, reducing “runner budget” flakes where the fixture startup consumes most of the per-test timeout.
Changes:
- Increased explicit timeouts in
MeshNodeVersionSyncTestfrom 10s to 30s. - Updated
HubFactAttribute(non-DEBUG) timeout from 5s to 30s and expanded its XML documentation to explain the rationale.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/MeshWeaver.Persistence.Test/MeshNodeVersionSyncTest.cs | Bumps per-test [Fact(Timeout=…)] budgets to 30s to avoid CI fixture-startup timeouts. |
| src/MeshWeaver.Fixture/HubFactAttribute.cs | Raises the default Release/CI hub-test timeout to 30s and documents why the prior 5s budget was not achievable on CI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// <para>30s matches the runner-wide <c>methodTimeout</c> and still fails a genuine wedge well | ||
| /// before the run hangs — a wedged read surfaces its own <c>GetMeshNode … timed out after 60.0s</c> | ||
| /// first, so nothing is masked by the larger budget.</para> |
Test Results (shard 3) 11 files - 1 11 suites - 1 2m 35s ⏱️ - 1m 59s Results for commit 5cdf109. ± Comparison against base commit f359385. This pull request removes 59 tests.♻️ This comment has been updated with latest results. |
Test Results 60 files - 1 60 suites - 1 32m 0s ⏱️ - 1m 43s Results for commit 5cdf109. ± Comparison against base commit f359385. This pull request removes 59 tests.♻️ This comment has been updated with latest results. |
Correction to my own framing: raising budgets is a treadmill, not a cureThis PR's CI failed on a test I did not touch and had not catalogued — It already carried That sharpens the conclusion in the commit message, and I want it recorded plainly rather than buried:
I am deliberately not raising Re-ran the failed shard to confirm this PR's change is not implicated. |
Catalogued every red main run of the last two days from the check annotations. Four
named flakes, and none of them is a bad assertion — all four are timeouts, and at
least two are timeouts a correct test could never have avoided:
MeshNodeVersionSyncTest.MeshNode_InitialVersion_IsZero
"Test execution timed out after 10000 milliseconds", and the same log shows
class init 19:59:22.595 → dispose 19:59:30.720 — 8.1s of MESH FIXTURE inside a
10s budget, leaving 1.9s for the body. On a quiet runner it fits; under an
8-shard load it does not. The test measures the runner, not the code.
[HubFact] carried the same trap globally: 5s in Release (CI), which the fixture
alone can exceed. It bit a heartbeat test written the same night — green locally,
where no budget is enforced, and impossible on CI.
So both budgets move to 30s, matching the runner-wide methodTimeout. Nothing is
masked: a genuine wedge surfaces its own "GetMeshNode … timed out after 60.0s"
first, so a hang still fails — just not before the fixture has finished starting.
Not fixed here, and NOT the same class — recorded so they are not mistaken for
budget noise:
TwoSiloRecycleConvergenceTest.PostRecycleUpdate_NonOwnerSiloMirror_Converges (2 runs)
TwoWaySyncTest.TwoWay_Update_KeepsServerAddition_UnlessForced
ThreadAgentIntegrationTest.FullFlow_CreateThread_SendMessage_NonStreaming
The last one is the most interesting: it fails with
"GetMeshNode('ACME/ProductLaunch') timed out after 60.0s — the owning per-node hub
never answered … NO LOCAL HUB … it never activated here". That is the SAME
cold-hub-activation defect measured in production this week (issue #686: 0ms source
discovery on a fresh mesh, 45.20s on memex). These flakes are that bug reproducing
under CI load, not test debt.
409/409 Persistence, 290/290 Data.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The catalogue first
Pulled every red
mainrun of the last two days from check annotations. Four named flakes, and not one is a bad assertion — all four are timeouts:MeshNodeVersionSyncTest.MeshNode_InitialVersion_IsZeroTest execution timed out after 10000 millisecondsTwoSiloRecycleConvergenceTest.PostRecycleUpdate_NonOwnerSiloMirror_ConvergesSystem.TimeoutExceptionTwoWaySyncTest.TwoWay_Update_KeepsServerAddition_UnlessForcedSystem.TimeoutExceptionThreadAgentIntegrationTest.FullFlow_CreateThread_SendMessage_NonStreamingGetMeshNode(…) timed out after 60.0sWhat this PR fixes: budgets a correct test could never meet
The same CI log for
MeshNodeVersionSyncTestshows:8.1 s of mesh fixture inside a 10 s budget — 1.9 s left for the body. On a quiet runner it fits; under an 8-shard load it cannot. The test was measuring the runner.
[HubFact]carried the same trap globally: 5 s in Release (i.e. CI), which the fixture alone can exceed. It bit a heartbeat test written the same night — green locally, where no budget is enforced, and impossible on CI.Both move to 30 s, matching the runner-wide
methodTimeout.Nothing is masked. A genuine wedge surfaces its own
GetMeshNode … timed out after 60.0sfirst, so a hang still fails the test — just not before the fixture has finished starting.Not fixed here — and NOT budget noise
The other three are recorded in the commit so nobody dismisses them as timing:
ThreadAgentIntegrationTestfails withThat is the same cold-hub-activation defect measured in production this week (#686: 0 ms source discovery on a fresh mesh, 45.20 s on memex). These flakes are that bug reproducing under CI load — real signal, and they should stay red until it is fixed rather than be timed away.
Testing
409/409 Persistence, 290/290 Data.
🤖 Generated with Claude Code