From 7c6694e901d21eba6d113a5631b35bf8b7b02b53 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Sun, 27 Sep 2026 16:03:08 -0400 Subject: [PATCH 1/2] CI: run the store-upgrade tests on pull requests that touch the store's schema or upgrade path The gated upgrade-in-place and same-major-swap E2Es (#1706, #3906) only ran on the nightly job: pull-request CI never built the upgrade fixture or set the three env vars those tests read, so they silently skipped on every PR. Adds a store-upgrade path filter derived from what those E2Es exercise (the store schema, the service project holding DarlingManagedPostgres and DarlingStoreUpgrade plus the managed-conf migration machinery, the two fixture scripts, and the workflow/filter files themselves). When a PR's changed files match it, build.yml's darling-pg job builds the same fixture nightly.yml builds and sets DARLING_TEST_PGRUNTIME_OLD/_NEWZIP/_PREVIOUS on the shards that carry the gated classes; other shards and non-matching PRs are unaffected. Pins the filter's existence and coverage in DarlingPathFilterGateTests. nightly.yml is unchanged. --- .github/darling-paths-filter.yml | 27 ++++++++++ .github/workflows/build.yml | 29 +++++++++++ .../DarlingPathFilterGateTests.cs | 50 +++++++++++++++++++ 3 files changed, 106 insertions(+) diff --git a/.github/darling-paths-filter.yml b/.github/darling-paths-filter.yml index 58096afbfd..4009f8a810 100644 --- a/.github/darling-paths-filter.yml +++ b/.github/darling-paths-filter.yml @@ -52,6 +52,33 @@ darling: # these two jobs add over that is a live TimescaleDB cluster and a linux-x64 publish, neither of which # a Lite XAML edit can affect, so putting Lite/** here would buy a ~4-minute cluster and no signal. +# The store-upgrade E2Es (#1706, #3906) construct DarlingManagedPostgres and DarlingStoreUpgrade over a +# real store: schema, TOAST, a continuous aggregate, a compressed chunk. Nightly builds the upgrade +# fixture and sets DARLING_TEST_PGRUNTIME_OLD/_NEWZIP/_PREVIOUS so those tests run every night; on a pull +# request they were silently skipped instead — the fixture was never built and the vars were never set — so +# no PR ever ran the only coverage of an in-place major upgrade or a same-major runtime swap. This filter is +# the paths those two E2Es actually exercise, so build.yml's darling-pg job can build the fixture and set the +# vars only when a change could move their answer, not on every PR (the fixture download and build cost real +# minutes nightly already pays and PRs should not, unless the change is in scope for them): +# - the store schema itself, which the upgrade checksum-compares before and after; +# - PerformanceMonitor.Darling.Service, which is where DarlingManagedPostgres and DarlingStoreUpgrade +# (the two types the E2Es construct) live, along with the managed-conf migration machinery +# (ManagedConfMigration/ManagedConfMigrationRunner/ManagedConfMigrationSteps/ManagedConfMigrationState/ +# ManagedConfFile/ManagedConfFileSettings) the upgrade path drives through DarlingWorker; +# - the two fixture scripts the upgrade tests are gated on, and fetch-pg-runtime.ps1, which builds the +# runtime new-upgraded-store-fixture.ps1 diffs against; +# - the two workflow files and this filter file itself, for the same self-validating reason the `darling` +# filter above lists build.yml and itself. +# Same '!(*.md)' extglob form as the `darling` filter, for the same dorny v4 quantifier reason. +store-upgrade: + - 'Darling/PerformanceMonitor.Darling.Storage/**/!(*.md)' + - 'Darling/PerformanceMonitor.Darling.Service/**/!(*.md)' + - 'Darling/tools/new-upgraded-store-fixture.ps1' + - 'Darling/tools/fetch-pg-runtime.ps1' + - '.github/workflows/build.yml' + - '.github/workflows/nightly.yml' + - '.github/darling-paths-filter.yml' + # The changed-file COUNTER the two gate-decision steps report against, so a skip notice can say how # many files it classified and name them rather than assert a cause for the classification. Never a # boolean gate: every changed file matches '**', so this filter can only ever be true — the same diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f960a1a4ac..fa380040e2 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -920,6 +920,27 @@ jobs: [System.IO.Compression.ZipFile]::ExtractToDirectory($zip, $dest) if (-not (Test-Path "$dest\pgsql\bin\pg_ctl.exe")) { throw "pg-runtime missing pgsql\bin\pg_ctl.exe" } + # store-upgrade-gated (#4326): the same fixture nightly.yml builds, only when a changed path is in + # .github/darling-paths-filter.yml's store-upgrade filter, so a PR that cannot move the upgrade E2Es' + # answer does not pay for the ~680MB of pinned archives. The four classes that read the three env vars + # this fixture feeds hash into shards 1 and 2 by the class-name-hash rule above (none into shard 0), so + # shard 0 skips this and the two fixture steps below even when store-upgrade matches. Same cache key as + # nightly.yml, so a warm cache from either means no download here. + - name: Cache upgrade-fixture downloads (previous-major runtime) + if: steps.filter.outputs.darling == 'true' && steps.filter.outputs.store-upgrade == 'true' && matrix.shard != 0 + uses: actions/cache@v6 + with: + path: Darling/artifacts/upgrade-fixture/work/downloads + key: upgrade-fixture-${{ runner.os }}-${{ hashFiles('Darling/tools/new-upgraded-store-fixture.ps1') }} + + # -SkipNew: the CURRENT runtime is already extracted above from pg-runtime.zip, which is what + # DARLING_TEST_PGRUNTIME_NEWZIP points at below. This step only needs the old sides: the previous + # major, and (#3906) the previous release on the current major. + - name: Build previous-major runtime for the upgrade fixture + if: steps.filter.outputs.darling == 'true' && steps.filter.outputs.store-upgrade == 'true' && matrix.shard != 0 + shell: pwsh + run: ./Darling/tools/new-upgraded-store-fixture.ps1 -SkipNew + - name: Restore Darling.Tests if: steps.filter.outputs.darling == 'true' run: dotnet restore Darling/Darling.Tests/Darling.Tests.csproj --locked-mode @@ -1051,6 +1072,14 @@ jobs: DARLING_TEST_PG_CSVLOG_AUTOEXPLAIN: "Host=127.0.0.1;Port=5542;Username=darling;Database=darling" # csvlog-only from its first start, with log_timezone = 'UTC': PgLogTailStderrSiblingLiveTests' #4019 case. DARLING_TEST_PG_CSVLOG_ONLY: "Host=127.0.0.1;Port=5542;Username=darling;Database=darling" + # store-upgrade-gated, exactly as nightly.yml sets them (#1706, #3906); empty when the fixture + # above did not run, which the consuming tests treat as "not set" (Environment.GetEnvironmentVariable + # returns null for an empty value read this way, and every one of them guards with + # string.IsNullOrWhiteSpace before touching the path), so they SkipWhen on a shard or PR where the + # gate did not match rather than fail on a missing fixture. + DARLING_TEST_PGRUNTIME_OLD: ${{ (steps.filter.outputs.store-upgrade == 'true' && matrix.shard != 0) && format('{0}\Darling\artifacts\upgrade-fixture\old\pg-runtime', github.workspace) || '' }} + DARLING_TEST_PGRUNTIME_NEWZIP: ${{ (steps.filter.outputs.store-upgrade == 'true' && matrix.shard != 0) && format('{0}\Darling\artifacts\pg-runtime.zip', github.workspace) || '' }} + DARLING_TEST_PGRUNTIME_PREVIOUS: ${{ (steps.filter.outputs.store-upgrade == 'true' && matrix.shard != 0) && format('{0}\Darling\artifacts\upgrade-fixture\previous\pg-runtime', github.workspace) || '' }} DARLING_TEST_PG_JSONLOG: "Host=127.0.0.1;Port=5543;Username=darling;Database=darling" DARLING_TEST_PG_AUTOEXPLAIN: "Host=127.0.0.1;Port=5544;Username=darling;Database=darling" DARLING_TEST_PG_CSVLOG_SIBLING: "Host=127.0.0.1;Port=5544;Username=darling;Database=darling" diff --git a/Darling/Darling.Tests/DarlingPathFilterGateTests.cs b/Darling/Darling.Tests/DarlingPathFilterGateTests.cs index 1609634b30..a8feb4719e 100644 --- a/Darling/Darling.Tests/DarlingPathFilterGateTests.cs +++ b/Darling/Darling.Tests/DarlingPathFilterGateTests.cs @@ -188,6 +188,56 @@ is the run's own rather than a sentence about it. */ Assert.All(skipNotices, line => Assert.Contains("${ALL_COUNT", line, StringComparison.Ordinal)); } + /// + /// The store-upgrade filter (#4326) exists, includes the store schema and the two fixture scripts the + /// gated upgrade E2Es are gated on, and covers the source of every type those E2Es construct directly. + /// + /// The type list is the closure the E2Es actually reach, read off their own new/static-call + /// sites rather than judged: and + /// (constructed in DarlingStoreUpgradeTests, DarlingManagedPostgresTests and ManagedConfUpgradePathTests), + /// both declared in PerformanceMonitor.Darling.Service, which the second pattern below already covers as a + /// whole directory rather than by file — the managed-conf migration machinery those two drive + /// (ManagedConfMigration, ManagedConfMigrationRunner, ManagedConfMigrationSteps, ManagedConfMigrationState, + /// ManagedConfFile) lives beside them in the same project, so one directory pattern is the correct + /// granularity rather than five file entries nothing keeps in sync with a class move. + /// + [Fact] + public void TheStoreUpgradeGate_CoversTheSchemaAndTheFixtureScriptsAndTheServiceProject() + { + var gateYaml = ReadRepoFileLf(s_gateSegments); + var patterns = FilterPatterns(gateYaml, "store-upgrade"); + + Assert.True(patterns.Count > 0, + $"{GatePath} has no 'store-upgrade' filter \u2014 find where it moved before editing this test"); + + Assert.Contains("Darling/PerformanceMonitor.Darling.Storage/**/!(*.md)", patterns, StringComparer.Ordinal); + Assert.Contains("Darling/PerformanceMonitor.Darling.Service/**/!(*.md)", patterns, StringComparer.Ordinal); + Assert.Contains("Darling/tools/new-upgraded-store-fixture.ps1", patterns, StringComparer.Ordinal); + Assert.Contains("Darling/tools/fetch-pg-runtime.ps1", patterns, StringComparer.Ordinal); + } + + /// + /// The coverage check reports an injected gap, the same shape as + /// above and for the same reason: a parser that + /// matched nothing would report this gate complete too, which is the failure this test would otherwise + /// share with the one it is guarding. + /// + [Fact] + public void TheStoreUpgradeGate_MissingTheStorageEntry_IsDetected() + { + var real = ReadRepoFileLf(s_gateSegments); + Assert.Contains("Darling/PerformanceMonitor.Darling.Storage/**/!(*.md)", + FilterPatterns(real, "store-upgrade"), StringComparer.Ordinal); + + var mutated = real.Replace( + " - 'Darling/PerformanceMonitor.Darling.Storage/**/!(*.md)'\n", + string.Empty, + StringComparison.Ordinal); + Assert.NotEqual(real, mutated); + Assert.DoesNotContain("Darling/PerformanceMonitor.Darling.Storage/**/!(*.md)", + FilterPatterns(mutated, "store-upgrade"), StringComparer.Ordinal); + } + // ── helpers ────────────────────────────────────────────────────────────────────────────────────── /// From 96825e4159bcbad7ac7b163b07beb1748aa3a6ae Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Sun, 27 Sep 2026 16:13:45 -0400 Subject: [PATCH 2/2] Run the upgrade-fixture steps on every shard, not just shards 1-2 The fixture steps and the three DARLING_TEST_PGRUNTIME_* env expressions were gated on matrix.shard != 0 because today's upgrade classes happen to hash into shards 1 and 2. That meant a renamed class, or a new upgrade E2E that hashes into shard 0, would silently skip the fixture with no signal. A class's shard must never decide whether it runs. --- .github/workflows/build.yml | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index fa380040e2..b875c45574 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -922,12 +922,12 @@ jobs: # store-upgrade-gated (#4326): the same fixture nightly.yml builds, only when a changed path is in # .github/darling-paths-filter.yml's store-upgrade filter, so a PR that cannot move the upgrade E2Es' - # answer does not pay for the ~680MB of pinned archives. The four classes that read the three env vars - # this fixture feeds hash into shards 1 and 2 by the class-name-hash rule above (none into shard 0), so - # shard 0 skips this and the two fixture steps below even when store-upgrade matches. Same cache key as - # nightly.yml, so a warm cache from either means no download here. + # answer does not pay for the ~680MB of pinned archives. This runs on every shard, not just the ones + # today's upgrade classes happen to hash into: a class's shard must never decide whether it runs, so a + # renamed class or a new upgrade E2E landing on a different shard still gets the fixture. Same cache + # key as nightly.yml, so a warm cache from either means no download here. - name: Cache upgrade-fixture downloads (previous-major runtime) - if: steps.filter.outputs.darling == 'true' && steps.filter.outputs.store-upgrade == 'true' && matrix.shard != 0 + if: steps.filter.outputs.darling == 'true' && steps.filter.outputs.store-upgrade == 'true' uses: actions/cache@v6 with: path: Darling/artifacts/upgrade-fixture/work/downloads @@ -937,7 +937,7 @@ jobs: # DARLING_TEST_PGRUNTIME_NEWZIP points at below. This step only needs the old sides: the previous # major, and (#3906) the previous release on the current major. - name: Build previous-major runtime for the upgrade fixture - if: steps.filter.outputs.darling == 'true' && steps.filter.outputs.store-upgrade == 'true' && matrix.shard != 0 + if: steps.filter.outputs.darling == 'true' && steps.filter.outputs.store-upgrade == 'true' shell: pwsh run: ./Darling/tools/new-upgraded-store-fixture.ps1 -SkipNew @@ -1075,11 +1075,12 @@ jobs: # store-upgrade-gated, exactly as nightly.yml sets them (#1706, #3906); empty when the fixture # above did not run, which the consuming tests treat as "not set" (Environment.GetEnvironmentVariable # returns null for an empty value read this way, and every one of them guards with - # string.IsNullOrWhiteSpace before touching the path), so they SkipWhen on a shard or PR where the - # gate did not match rather than fail on a missing fixture. - DARLING_TEST_PGRUNTIME_OLD: ${{ (steps.filter.outputs.store-upgrade == 'true' && matrix.shard != 0) && format('{0}\Darling\artifacts\upgrade-fixture\old\pg-runtime', github.workspace) || '' }} - DARLING_TEST_PGRUNTIME_NEWZIP: ${{ (steps.filter.outputs.store-upgrade == 'true' && matrix.shard != 0) && format('{0}\Darling\artifacts\pg-runtime.zip', github.workspace) || '' }} - DARLING_TEST_PGRUNTIME_PREVIOUS: ${{ (steps.filter.outputs.store-upgrade == 'true' && matrix.shard != 0) && format('{0}\Darling\artifacts\upgrade-fixture\previous\pg-runtime', github.workspace) || '' }} + # string.IsNullOrWhiteSpace before touching the path), so they SkipWhen on a PR where the gate did + # not match rather than fail on a missing fixture. Set on every shard: a class's shard must never + # decide whether it runs. + DARLING_TEST_PGRUNTIME_OLD: ${{ steps.filter.outputs.store-upgrade == 'true' && format('{0}\Darling\artifacts\upgrade-fixture\old\pg-runtime', github.workspace) || '' }} + DARLING_TEST_PGRUNTIME_NEWZIP: ${{ steps.filter.outputs.store-upgrade == 'true' && format('{0}\Darling\artifacts\pg-runtime.zip', github.workspace) || '' }} + DARLING_TEST_PGRUNTIME_PREVIOUS: ${{ steps.filter.outputs.store-upgrade == 'true' && format('{0}\Darling\artifacts\upgrade-fixture\previous\pg-runtime', github.workspace) || '' }} DARLING_TEST_PG_JSONLOG: "Host=127.0.0.1;Port=5543;Username=darling;Database=darling" DARLING_TEST_PG_AUTOEXPLAIN: "Host=127.0.0.1;Port=5544;Username=darling;Database=darling" DARLING_TEST_PG_CSVLOG_SIBLING: "Host=127.0.0.1;Port=5544;Username=darling;Database=darling"