Repository navigation
Darling PostgreSQL tests skips shared-library changes its two sibling jobs catch, reports green, and blames documentation #3116
Description
Darling PostgreSQL tests reports success having run nothing whenever a change lands in a shared PerformanceMonitor.* library that Darling consumes. Its two sibling jobs in the same workflow file do not have this problem — they list those libraries explicitly, and only the job that runs the tests omits them.
Three jobs, three filters, one file
.github/workflows/build.yml on dev:
| job | does its filter include PerformanceMonitor.Collectors? |
|---|---|
build (core filter, ~line 101) |
yes — PerformanceMonitor.Collectors/**/!(*.md), alongside .Alerting, .Analysis, .Common, .Notifications, .PlanAnalysis |
Darling Linux build (~line 857) |
yes — PerformanceMonitor.Collectors/**, plus PerformanceMonitor.Common/** and PerformanceMonitor.Analysis/** |
Darling PostgreSQL tests (~line 674) |
NO — Darling/**/!(*.md), build.yml, nightly.yml, and nothing else |
The Darling Linux build job already carries a deliberate list of the shared libraries that reach Darling. The PostgreSQL test job carries none of them. So this is not a judgement call about how far the dependency graph extends — the answer is already written twice in this file, and the test job disagrees with both copies.
Behavioural proof from one run
PR #3114 changes exactly two files:
PerformanceMonitor.Collectors/PgIndexBloatCollector.cs
Lite.Tests/PgIndexBloatCollectorDefinitionTests.cs
Same commit, same workflow, opposite decisions:
| check | duration | outcome |
|---|---|---|
Darling Linux build |
1 m 47 s | ran — its filter includes PerformanceMonitor.Collectors |
Darling PostgreSQL tests |
17 s | skipped — steps 5–14 all skipped, including Run Darling PG tests |
For scale, the same PG job took 4 m 5 s on a sibling PR whose only change was a comment. The PR that changes a PostgreSQL collector ran no PostgreSQL tests; the PR that changes prose ran all of them.
And Darling genuinely consumes the changed code:
PerformanceMonitor.Collectors.csprojis referenced by four Darling projects —.Service,.Viewer,.Storage,.Analysis.DarlingWorker.csregisters and runs the changed class:["pg_index_bloat"] = (r, s, ct) => r.RunAsync(PgIndexBloatCollector.Instance, s, ct).Darling/Darling.Tests/PgSchemaGeneratorTests.csasserts on that same class — so there is Darling-side coverage of the change and the gate skipped the job that would have run it.
The gate keys on where the change LIVES rather than on what the tests COVER.
The skip notice asserts a false cause
::notice title=Darling PG tests skipped::No Darling code changed - documentation-only
Darling edits do not trigger the TimescaleDB suite.
For #3114 that is wrong on both clauses: Darling code changed (transitively, via a referenced project and a collector it runs), and the edit was C# source, not documentation.
The comment directly above that step explains why it exists:
"a job that reports success having quietly run nothing looks identical to one that tested everything. Costs one step; buys a log you can point at when asking 'did this actually get tested?'"
That is the right instinct, and this is the case where the log you point at gives the wrong answer confidently. The notice does not report the gate's decision — it reports one hypothesis about why the decision came out that way, with "documentation-only" hard-coded as the only explanation for darling == false. Note the Darling Linux build job's equivalent notice says "No Darling/service code changed" and makes no documentation claim; the wrong message is specific to this job.
Scope
Every shared library Darling consumes, not just .Collectors. That is the whole PerformanceMonitor.* core set the build job enumerates — including .Alerting, .Notifications and .Analysis, all shared seams where "Lite silently reads a permanently-empty value" is a known failure mode. A change to any of them currently ships with a green TimescaleDB check that ran no TimescaleDB.
This is not the dorny predicate-quantifier bug documented at length around these filters (a bare !**/*.md acting as an independent match-all rule). The patterns here are correct for what they say. The filter is simply missing entries its siblings have.
Suggested direction
Do not hand-copy the list a third time — three copies of the same dependency knowledge drifting apart is what produced this. Prefer sharing one filter decision across the jobs (the build job already computes a shared-core signal), or at minimum a single YAML anchor both Darling jobs consume.
And make the notice print the decision rather than a guessed cause — list-files: shell is already enabled on the build job's filter for exactly this purpose.
Acceptance criteria, both arms:
- a PR touching only
PerformanceMonitor.Collectors/**runsRun Darling PG teststo completion; - a PR touching only
Darling/**/*.mdstill skips it.
A fix that makes the gate always-true removes the guard rather than correcting it, and the second arm is what catches that.
Related
- Bound pg_index_bloat by what pgstatindex is applied to, not by a join qual #3114 is the PR that exposed it and cannot be judged on its
Darling PostgreSQL testsresult. - The
nightly.ymlcluster-sizing guard (CI's throwaway PostgreSQL runs max_worker_processes = 8, so TimescaleDB background jobs mostly cannot launch — the gated-live suite tests a configuration the product deliberately avoids #1888) is already in this filter for the same class of reason — an input that lives outside the tree it affects.
Activity
[-]Darling PostgreSQL tests reports success having run nothing when a shared PerformanceMonitor.* library changes, and the skip notice blames documentation[/-][+]Darling PostgreSQL tests skips shared-library changes its two sibling jobs catch, reports green, and blames documentation[/+]erikdarlingdata commentedon Sep 7, 2026
Claude posting for Erik Darling
Fixed by #3122, merged to
dev. Closed by hand — closing keywords do not fire here.Both Darling CI jobs now read one gate file,
.github/darling-paths-filter.yml, through dorny's documentedfilters: <path>form. It replaces the Linux job's private list rather than joining it, so the repo holds the same two lists as before —build'score(what to compile and publish) and the gate (what reaches Darling) — and the job that had neither now reads the other's. YAML anchors were unavailable (GHA rejects them, and the filter body is a nested document inside a string), and aneeds: buildoutput was rejected because it serialises the PG job behind the Windows build, where a redbuildwould then skip it entirely.The requirement is derived rather than restated.
SharedLibraryClosure()walksDarling.Tests' ProjectReference graph and demands aNAME/**/!(*.md)entry per root-levelPerformanceMonitor.*project — so adding a shared library to a Darling project makes the entry mandatory rather than remembered. That property earned its keep three times during the merge rounds: it re-derives against each merged tree, so a shared-library reference introduced by someone else's PR would have reddened at the merge rather than after it. A derived requirement notices another change; an enumerated list compares a frozen list to itself.The notice now reports the decision instead of guessing a cause. It prints how many changed files are in the gate and names them from the filter's own output. The old message hard-coded "documentation-only" as the sole explanation for
darling == false, which was wrong on both clauses for the C# change that exposed this.Both acceptance arms were demonstrated at runtime, not argued. Neither is demonstrable on this PR — its diff touches
build.yml, which every filter names — so they ran on throwaway PRs branched off a base carrying the fix, making each diff a single category. APerformanceMonitor.Collectors/change ranRun Darling PG teststo completion in 80s; aDarling/**/*.mdchange skipped steps 5–14 and went green in 16s. Both probes are closed unmerged with their branches deleted.Three things worth recording from the work:
cancelledvoids a step list. On one head, step 12 readskippedwhile the job wascancelledat 123s — cancelled before reaching the step, so every downstream step reportsskippedregardless of the gate. Judging a gated job by its step list is right, and the job's own conclusion has to be read first; a naive audit of that PR's history counted two gate skips where there was one.The
list-files: jsonchange is a correctness requirement, not tidiness. The array must be self-delimiting for a filename containing a space to survive theenv:indirection.list-files: shellescapes for substitution into script text, and a runtime parameter expansion does not honour those escapes.And the review bot's severity was wrong in the underestimating direction — it dismissed that escaping issue because "no filenames in this repo currently contain spaces". Eight tracked paths under
Screenshots/do, and that directory is on the documentation allowlist, so the docs fast-path notice is the one that meets a spaced path first. Reproduced under bash: two real paths printed as nine bogus bullets. Fixed in all three filter steps, including thebuildjob's, which was outside this issue's original scope.