Repository navigation
Upgrade-path tests read the preload line where the managed settings now live (#4336) - #4446
Merged
Merged
Conversation
…ow live (#4336) Three gated tests looked for shared_preload_libraries only in postgresql.conf. After #4336's Step A the line lives in darling-managed.conf, reached from postgresql.conf through its include line, so the old asserts no longer match the field shape. Adds one shared helper (ManagedPreloadAssert) used by all three: - exactly one effective shared_preload_libraries line across both files; - that line lives in the managed file, reached through the include; - its value carries timescaledb and pg_stat_statements. The two RuntimeAdvance_SameMajor... cases also read the live pg_settings row on a Pooling=false connection, and check its sourcefile ends with darling-managed.conf. The operator-line test keeps its pg_file_settings applied check, unchanged, after the new asserts. Adds one render-level pin (RenderBody_PreloadLine_IsExactlyOne_WithBothLibraries) that runs ungated on every build, so a broken merge is caught before a gated run. Test-only.
erikdarlingdata
marked this pull request as ready for review
September 26, 2026 20:16
This was referenced Sep 27, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #4336, #4405 and #4358.
Why
The 2026-09-26 nightly (3.8.0-nightly.20260926.473) failed three gated upgrade-path tests on the same shape:
DarlingStoreUpgradeTests.RuntimeAdvance_SameMajorStoreWithRealData_SwapsWithoutPgUpgradeAndKeepsEverything_Gated(bothstampedcases) assertedshared_preload_libraries = 'timescaledb'againstpostgresql.confalone.ManagedConfUpgradePathTests.UpgradeInPlace_OperatorLineBelowInclude_CarriedAndApplied_NoManagedKeyDuplicated_Gatedcounted that same literal only inpostgresql.conf, expecting exactly one; itspg_file_settings"applied" check never ran because the count assert above it failed first.After #4336's Step A, a migrated store's
shared_preload_librariesline moves intodarling-managed.conf, reached frompostgresql.confthrough aninclude 'darling-managed.conf'line. The three tests still looked for the literal string only inpostgresql.conf, so they failed against the actual field shape — the bundled managed store on the production Windows boxes builds and migrates exactly this way.What changes
Adds one shared, internal test helper,
ManagedPreloadAssert, used by all three tests:FileLevel_HasOneManagedPreloadLine_WithBothLibraries): reads bothpostgresql.confanddarling-managed.conffrom the data directory and parses every effective (non-comment)shared_preload_librariesassignment in each. Asserts exactly one such assignment across the two files (the "no managed key duplicated" rule), that it lives in the managed file, thatpostgresql.confcarries the include line reaching it, and that its value holds bothtimescaledbandpg_stat_statements(the v13 merge always keeps both, so this suite's managed stores always carry both).Live_ReportsBothLibraries_SourcedFromManagedConf): opens aPooling=falseconnection and readspg_settings.setting/pg_settings.sourcefileforshared_preload_libraries, asserting both libraries are reported and the source file ends withdarling-managed.conf— the live counterpart of the field canary.The two
RuntimeAdvance_SameMajor...cases now call both checks. The operator-line test calls the file-level check in place of its old single-file count, and keeps itspg_file_settings"applied" check afterward, unchanged.Also adds one render-level unit fact,
ManagedConfFileTests.RenderBody_PreloadLine_IsExactlyOne_WithBothLibraries, that pins the same rule directly onManagedConfFile.RenderBody's output — ungated, runs on every build. Verified with a temporary mutation: appending a secondshared_preload_librariesline to the render's output turned this fact (and the existingRenderBody_PreloadMerge_KeepsAnOperatorLibraryInForce) RED, then reverting the mutation restored GREEN.Test plan
Darling.TestsandLite.Testsboth build 0 errors / 0 warnings-that-matter with-p:EnableWindowsTargeting=true.ManagedConfFileTestsin-process on macOS: 33/33 pass, including the new render-level fact.DocCommentHygieneTestsin-process: 77/77 pass.ManagedConfUpgradePathTestsin-process: 5 total, 1 skipped (gated, no PG runtime env vars on this rig), 0 failed.DarlingStoreUpgradeTestsin-process: 146 total, 13 skipped (gated), 6 failed — all six are pre-existing macOS-path failures unrelated to this change (e.g. aC:\pg\lib\...literal-path parsing case), confirmed unchanged against the pre-fixdevtip with the same class.DARLING_TEST_PGRUNTIME_*real-runtime env vars, Windows-only bundled PostgreSQL). That nightly's failing run is the RED; the next nightly dispatch is the GREEN this PR expects.CHANGELOG entry
SECTION: None
ENTRY: None: test-only change, no user-visible effect.