Skip to content

Core test suite is flaking under parallel shards — two unrelated tests failed docs-only PRs within an hour #834

Closed

Description

@rbuergi

Observation (2026-08-06)

Two docs-only PRs — changes that cannot touch runtime behaviour — went red on two different, unrelated tests within the same hour:

PR Change Failing test Symptom
#829 skills, .gitignore, Helm chart, one doc node DynamicGraphFileSystemPersistenceTest.FileSystem_Organizations_GetsHubConfiguration_FromCompiledAssembly (shard 2) Expected the observable to emit a value within 10s, but it did not
#832 one new markdown doc node CrossPartitionRealUserQueryTests.UnpinnedStructuredQuery_AsSystem_EmitsInitial (shard 5) Expected collection to contain an item matching the predicate because System sees every partition's rows

Both passed on re-run of the failed shard (#829 went 16/16 and merged).

Why this matters more than the individual tests

A flaky suite on docs PRs trains everyone to re-run red rather than read it — which is exactly how a real red gets waved through. It also costs a full shard re-run (several minutes) per occurrence, and it makes --auto merges stall unpredictably.

Two concrete leads

  1. Timeout budgets that assume an idle machine. FileSystem_Organizations_…FromCompiledAssembly performs a real on-demand Roslyn compile, and the assertion waits 10s while the test itself declares [Fact(Timeout = 20000)] — the inner wait is half the budget the author intended, so shard contention alone can fail it. Worth auditing Should().Emit() default waits in tests that compile or boot a mesh, and aligning them with the test's declared timeout rather than raising either arbitrarily.
  2. First-emission races in cross-partition queries. UnpinnedStructuredQuery_AsSystem_EmitsInitial asserts on the initial frame of a synced query. If the assertion can observe a partial first frame under load, that is a product-relevant race, not just a test bug — worth confirming the query gates its first emission until every sub-query has produced its Initial.

Lead 2 deserves a look before being written off as flake: "the first frame was incomplete under load" is the kind of thing that also bites a real page.

🤖 Generated with Claude Code

Activity

  1. rbuergi commented on Aug 6, 2026

    @rbuergi
    ContributorAuthor

    Root cause found for lead 2 — a genuine test defect, not chance

    CrossPartitionRealUserQueryTests.RunUnpinned issues:

    .Query<MeshNode>(new MeshQueryRequest
    {
        Query = "nodeType:Markdown",
        Limit  = 10,
        UserId = userId!,
    });

    …unscoped across every partition and capped at 10 rows — and UnpinnedStructuredQuery_AsSystem_EmitsInitial then asserts its own seeded row is in the result:

    change.Items.Should().Contain(n => n.Path == $"{nsA}/doc1", "System sees every partition's rows");

    The seeded doc1 is only guaranteed to appear while fewer than 10 other Markdown nodes exist. Against a shared PostgreSQL with sibling tests creating Markdown nodes concurrently, it gets crowded out of the page — which is precisely why the failure is intermittent, why it appears under parallel-shard load, and why it lands on PRs that changed nothing.

    The System sees every partition's rows message is also slightly misleading: as written the test proves "System sees ≥1 row from partition A within the first 10 global results", which is a weaker and load-dependent claim.

    Fix (test-side, no product change): make the assertion independent of unrelated rows — scope the query to the seeded namespaces (or a marker nodeType/id unique to the test run), and keep Limit only if the test is genuinely about paging. The sibling …AsAmbientRealUser_EmitsInitial and the anonymous/other-user cases share RunUnpinned and carry the same latent instability, so fixing the helper fixes all of them.

    Worth stating for the record: this one is not a first-frame-completeness race, so the GetQuery contract (gate the first emission until every sub-query has produced its Initial) is not implicated here. Lead 1 (#829's compile-timeout: a 10s assertion inside a [Fact(Timeout = 20000)] that performs a real Roslyn compile) is still open and is a budget mismatch rather than a race.

    🤖 Generated with Claude Code

  2. added a commit that references this issue on Aug 6, 2026
  3. rbuergi commented on Aug 7, 2026

    @rbuergi
    ContributorAuthor

    Both documented defects are fixed on main:

    1. Paged-out assertion — 459b440 + 7a9b159/7f989fb41: RunUnpinned now takes a namespaceScope; the row-assert test (UnpinnedStructuredQuery_AsSystem_EmitsInitial) is scoped to its seeded namespace and folds the visibility check into a Match predicate, while the deliberately-global tests assert liveness only (converge, don't hang) and no longer claim a specific row can survive an unscoped limit:10 page.
    2. Halved compile budget — 459b440: FileSystem_Organizations_GetsHubConfiguration_FromCompiledAssembly now declares [Fact(Timeout = 60_000)] with explicit 30 s inner waits, replacing the 10 s default budget inside a 20 s Fact.

    Closing. The broader one-time-flake pattern remains tracked in #826.

    🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions