Skip to content

Six live Darling.Tests classes run outside the live-postgres collection, producing moving cross-class flakes on a shared store #1776

Description

@erikdarlingdata

Six Darling.Tests classes connect to the shared DARLING_TEST_PG store without [Collection("live-postgres")], so they run in parallel with the 63 classes that have it - and with each other. On a local rig where several full runs share one database, this produces intermittent failures that land on a different unrelated class each run, which is expensive in exactly the way a flake is worst: it looks like the change under test broke something it never touched.

Measured, not inferred

Three consecutive full-suite runs against one long-lived database during #1775:

Run Failures
1 DarlingEndpointToggleCliTests.EnableThenDisableMcp_FlipsFlagAndSelfBumpsConfigVersion_AgainstDevPostgres + StoreConfigProviderTests.ConfigVersion_BumpTriggers_FireOnConfigWrites_AgainstDevPostgres
2 none - 3583 passed / 0 failed
3 ExcludedDatabasesStoreLiveTests.GetCollectedDatabaseNames_ReturnsDistinctUserDatabases_AcrossBothStores_AgainstDevPostgres

Every class that failed is a class without the attribute. That is the whole explanation, and it is a complete one - the failures are not random, they are confined to the set that is allowed to run concurrently. dropdb && createdb then re-running comes back clean, which is the one-step diagnostic.

The set

Classes with tests that reference DARLING_TEST_PG and do NOT carry [Collection("live-postgres")]:

Class Shares the DARLING_TEST_PG database?
StoreConfigProviderTests yes - and it writes singleton config rows
DarlingEndpointToggleCliTests yes
ExcludedDatabasesStoreLiveTests (in ExcludedDatabasesTests.cs) yes
DarlingManagedPostgresTests needs a look - some tests spin their OWN cluster
DarlingStoreUpgradeTests needs a look - the _Gated tests initdb their own cluster
DarlingPgRuntimeVersionPinTests needs a look - probes the bundle rather than the store

The first three are proven aggressors: each one failed. The last three need a case-by-case check rather than a blanket attribute - a class that stands up its own throwaway cluster does not race the shared store, and serializing it into the live collection would slow the suite for no safety gain. So this is not a six-line change; it is three lines plus three judgements.

AlertEngineTests also lacks the attribute but references DARLING_TEST_PG zero times, so it is out of scope.

Why CI never sees it

CI creates a throwaway cluster per run, so the Darling PostgreSQL tests job is green regardless. This costs only local development - which is precisely why it has survived: it is invisible to the gate and shows up as someone else's problem in someone else's PR.

Note on scope

Deliberately filed separately rather than folded into #1774/#1775: those are release-gating store fixes, and a test-infrastructure change that alters which classes run concurrently is not something to smuggle into a release branch on the strength of a hunch about parallelism. It also wants the three judgement calls above made deliberately, not in a hurry.

A correction to how this was first described to me: it is not "the one live class missing the attribute". StoreConfigProviderTests was the first one caught, but the sweep above found six, and two of the three classes that actually failed were the others.

Activity

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

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions