Skip to content

test: run full suite only on latest PG, essential subset on 14-17#489

Merged
tianzhou merged 1 commit into
mainfrom
test-strategy-essential-subset
Jun 23, 2026
Merged

test: run full suite only on latest PG, essential subset on 14-17#489
tianzhou merged 1 commit into
mainfrom
test-strategy-essential-subset

Conversation

@tianzhou

Copy link
Copy Markdown
Contributor

Summary

Changes the cross-version test strategy: the full suite runs only against the latest PostgreSQL version (currently 18), while older versions (14–17) run only an essential smoke-test subset.

Why

Re-running the entire suite on every supported version mostly re-tests version-agnostic logic and forced a steadily growing skip list for:

  • Cosmetic differencespg_get_viewdef() qualifies view columns differently before PG 16 (tb_users.id vs id).
  • Version-gated featuresNULLS NOT DISTINCT (PG 15+), temporal constraints WITHOUT OVERLAPS (PG 18+).

How

  • LatestPostgresVersion (currently 18) is the single point of indirection. The full suite runs only against it; the older versions run only essentialTests. Bumping to a new major version is a one-line change (plus adding the new version to the CI matrix and its embedded-postgres enum case).
  • essentialTests is a representative subset — one or two cases per object type (tables, columns, constraints, indexes, functions, procedures, sequences, types, domains, triggers, policies, aggregates, comments, privileges) — chosen so their golden files are byte-stable across all supported versions (no view definitions, no version-gated syntax). Enough to catch gross version-specific breakage (e.g. a catalog query that fails on an older release) without re-running everything.
  • The default test version (no PGSCHEMA_POSTGRES_VERSION) now derives from LatestPostgresVersion, so PR CI keeps running the full suite.
  • Removes the per-version skipListPG14* tables; the extension-required skip list is unchanged.

Verification

  • ✅ Essential subset (24 cases) green on PG 14 — both TestDiffFromFiles and TestPlanAndApply.
  • ✅ Full suite on PG 18 (running locally; CI matrix covers 14–18).

🤖 Generated with Claude Code

Re-running the entire suite on every supported PostgreSQL version mostly
re-tests version-agnostic logic and forces a growing skip list for cosmetic
differences (pg_get_viewdef() column qualification before PG 16) and
version-gated features (NULLS NOT DISTINCT, temporal constraints).

New strategy:
- LatestPostgresVersion (currently 18) is the single point of indirection.
  The full suite runs only against it; bumping it is a one-line switch.
- Older versions (14..latest-1) run only essentialTests, a representative
  smoke-test subset (one or two cases per object type) whose golden files
  are byte-stable across all versions. Enough to catch gross version-specific
  breakage without re-running everything.
- The default test version (no env var) now derives from LatestPostgresVersion,
  so PR CI keeps running the full suite.

Replaces the per-version skipList* tables with this model; extension-required
skips are unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 23, 2026 03:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the cross-version CI testing strategy so the full test suite runs only on the latest supported PostgreSQL major version (via a single LatestPostgresVersion constant), while older supported versions run a curated essential smoke-test subset to reduce version-specific golden drift and skip-list maintenance.

Changes:

  • Introduces LatestPostgresVersion and an essentialTests allowlist, and rewrites ShouldSkipTest to enforce “full suite on latest, essential subset on older versions”.
  • Removes the per-version skip lists (PG14/15 viewdef drift, version-gated feature skips) in favor of the essential subset approach.
  • Makes the default embedded Postgres version (when PGSCHEMA_POSTGRES_VERSION is unset) derive from LatestPostgresVersion.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
testutil/skip_list.go Replaces version-specific skip lists with LatestPostgresVersion + essential subset gating logic.
testutil/postgres.go Defaults test Postgres version selection to LatestPostgresVersion when env var is unset.
Comments suppressed due to low confidence (1)

testutil/postgres.go:174

  • getPostgresVersion falls back to embeddedpostgres.V18 for any unrecognized PGSCHEMA_POSTGRES_VERSION. This silently ignores misconfiguration and also means bumping LatestPostgresVersion alone would not update the default version (e.g., if LatestPostgresVersion becomes 19, the empty env-var path would still return V18 via the default case). Prefer failing fast (or mapping strictly) on unknown versions instead of defaulting to V18.
	case "18":
		return embeddedpostgres.V18
	default:
		return embeddedpostgres.V18
	}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@greptile-apps

greptile-apps Bot commented Jun 23, 2026

Copy link
Copy Markdown

Greptile Summary

This PR replaces the growing per-version skip-list approach with a two-tier test strategy: the full suite runs only against LatestPostgresVersion (currently 18), while PG 14–17 run only a 24-case essentialTests smoke subset chosen to be byte-stable across all supported versions.

  • testutil/skip_list.go: Removes skipListPG14, skipListPG14_15, skipListPG14_17, and skipListForVersion; introduces the LatestPostgresVersion constant and essentialTests slice; rewrites ShouldSkipTest to apply extension skips universally, pass the full suite on >= LatestPostgresVersion, and skip everything outside essentialTests on older versions.
  • testutil/postgres.go: getPostgresVersion() now derives its default from LatestPostgresVersion via strconv.Itoa, replacing the hard-coded "18" fallback in the case expression.

Confidence Score: 4/5

Safe to merge; the new strategy is correct in its current state (LatestPostgresVersion=18, case "18" present), and the essential subset is well-chosen.

The only concern is that LatestPostgresVersion drives the default PostgreSQL version via string conversion, but the switch in getPostgresVersion() requires a manually added case for each new version. A future bump of the constant without adding the matching case would silently run tests against PG 18 instead of the intended new version.

testutil/postgres.go — the default branch of the version switch and its relationship to LatestPostgresVersion deserve a quick look on the next version bump.

Important Files Changed

Filename Overview
testutil/postgres.go Adds LatestPostgresVersion-driven default for getPostgresVersion(), replacing the hard-coded "18" fallback. The switch statement's case strings are not automatically kept in sync with the constant, creating a latent mismatch if the constant is bumped before adding a new case.
testutil/skip_list.go Replaces per-version skip lists with LatestPostgresVersion constant and essentialTests subset. Logic is clean: extension skips apply universally, the full suite runs on >= LatestPostgresVersion, and older versions run only essentialTests. Well-commented and straightforward.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[ShouldSkipTest called] --> B{In skipListRequiresExtension?}
    B -- Yes --> C[t.Skipf: extension not available]
    B -- No --> D{majorVersion >= LatestPostgresVersion?}
    D -- Yes --> E[Return: run full suite]
    D -- No --> F{In essentialTests?}
    F -- Yes --> G[Return: run test]
    F -- No --> H[t.Skipf: only essential subset on older versions]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[ShouldSkipTest called] --> B{In skipListRequiresExtension?}
    B -- Yes --> C[t.Skipf: extension not available]
    B -- No --> D{majorVersion >= LatestPostgresVersion?}
    D -- Yes --> E[Return: run full suite]
    D -- No --> F{In essentialTests?}
    F -- Yes --> G[Return: run test]
    F -- No --> H[t.Skipf: only essential subset on older versions]
Loading

Comments Outside Diff (1)

  1. testutil/postgres.go, line 156-174 (link)

    P2 Constant and switch are silently decoupled

    LatestPostgresVersion is documented as "the single point of indirection", but getPostgresVersion() derives versionStr from it via strconv.Itoa and then matches it against hard-coded case strings. When LatestPostgresVersion is bumped to 19, there is no corresponding case "19":, so the empty-env-var path falls through to default: return embeddedpostgres.V18. Tests would silently run against PG 18 instead of PG 19, defeating the purpose of the constant. A comment on the default: branch — or better, a compile-time check — would prevent a silent mismatch on the next version bump.

Reviews (1): Last reviewed commit: "test: run full suite only on latest PG, ..." | Re-trigger Greptile

@tianzhou tianzhou merged commit e175445 into main Jun 23, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants