Skip to content

Commit e175445

Browse files
tianzhouclaude
tianzhou
and
claude
authored
test: run full suite only on latest PG, essential subset on 14-17 (#489)
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>
1 parent d6b89c2 commit e175445

2 files changed

Lines changed: 105 additions & 101 deletions

File tree

‎testutil/postgres.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"database/sql"
77
"fmt"
88
"os"
9+
"strconv"
910
"testing"
1011

1112
embeddedpostgres "github.com/fergusstrange/embedded-postgres"
@@ -150,10 +151,13 @@ func ConnectToPostgres(t testing.TB, embeddedPG *postgres.EmbeddedPostgres) (con
150151
}
151152

152153
// getPostgresVersion returns the PostgreSQL version to use for testing.
153-
// It reads from the PGSCHEMA_POSTGRES_VERSION environment variable,
154-
// defaulting to "18" if not set.
154+
// It reads from the PGSCHEMA_POSTGRES_VERSION environment variable, defaulting
155+
// to LatestPostgresVersion if not set (so the default run exercises the full suite).
155156
func getPostgresVersion() postgres.PostgresVersion {
156157
versionStr := os.Getenv("PGSCHEMA_POSTGRES_VERSION")
158+
if versionStr == "" {
159+
versionStr = strconv.Itoa(LatestPostgresVersion)
160+
}
157161
switch versionStr {
158162
case "14":
159163
return embeddedpostgres.V14
@@ -163,7 +167,7 @@ func getPostgresVersion() postgres.PostgresVersion {
163167
return embeddedpostgres.V16
164168
case "17":
165169
return embeddedpostgres.V17
166-
case "18", "":
170+
case "18":
167171
return embeddedpostgres.V18
168172
default:
169173
return embeddedpostgres.V18

‎testutil/skip_list.go‎

Lines changed: 98 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -6,57 +6,78 @@ import (
66
"testing"
77
)
88

9-
// skipListPG14_15 defines test cases that should be skipped for PostgreSQL 14-15.
9+
// LatestPostgresVersion is the most recent PostgreSQL major version pgschema
10+
// targets. It is the single point of indirection for the test strategy:
1011
//
11-
// Reason for skipping:
12-
// PostgreSQL 14-15 use pg_get_viewdef() which returns table-qualified column names (e.g., employees.id),
13-
// while PostgreSQL 16+ returns simplified column names (e.g., id). This is a non-consequential
14-
// formatting difference that does not impact correctness, but causes test comparison failures.
15-
var skipListPG14_15 = []string{
16-
// View tests - pg_get_viewdef() formatting differences
17-
"create_view/add_view",
18-
"create_view/alter_view",
19-
"create_view/drop_view",
20-
"create_view/issue_350_view_options",
21-
"dependency/table_to_view",
22-
23-
// Materialized view tests - same pg_get_viewdef() issue
24-
"create_materialized_view/add_materialized_view",
25-
"create_materialized_view/alter_materialized_view",
26-
"create_materialized_view/drop_materialized_view",
27-
"dependency/table_to_materialized_view",
28-
"dependency/issue_300_function_table_composite_type",
29-
"dependency/issue_414_function_returns_deferred_view_chain",
30-
"dependency/issue_308_view_select_star_column_reorder",
31-
"dependency/issue_444_drop_column_with_dependent_view",
32-
"dependency/issue_480_view_function_recreate",
33-
34-
// Online materialized view index tests - depend on materialized view definitions
35-
"online/add_materialized_view_index",
36-
"online/alter_materialized_view_index",
37-
38-
// Comment tests - fingerprint includes view definitions
39-
"comment/add_index_comment",
40-
"comment/add_view_comment",
41-
"comment/add_trigger_comment",
42-
43-
// Index tests - fingerprint includes view definitions
44-
"create_index/drop_index",
45-
46-
// Trigger tests - fingerprint includes view definitions
47-
"create_trigger/add_trigger",
48-
49-
// Migration tests - include views and materialized views
50-
"migrate/v4",
51-
"migrate/v5",
52-
53-
// Dump integration tests - contain views with formatting differences
54-
"TestDumpCommand_Employee",
55-
"TestDumpCommand_Issue82ViewLogicExpr",
56-
"TestDumpCommand_Issue307ViewDependencyOrder",
57-
58-
// Include integration test - uses materialized views
59-
"TestIncludeIntegration",
12+
// - The full test suite runs only against this version.
13+
// - Older versions (14 .. LatestPostgresVersion-1) run only the essential
14+
// smoke-test subset defined in essentialTests.
15+
//
16+
// When a new PostgreSQL major version ships, bump this constant (and add the new
17+
// version to the CI matrix); the full suite automatically moves to it and the
18+
// previous latest drops down to the essential subset — a single-line change.
19+
const LatestPostgresVersion = 18
20+
21+
// essentialTests is the minimal smoke-test subset run against non-latest
22+
// PostgreSQL versions (14 .. LatestPostgresVersion-1).
23+
//
24+
// Rationale: re-running the entire suite on every supported version mostly
25+
// re-tests version-agnostic logic and forces a large skip list for cosmetic
26+
// differences (e.g. pg_get_viewdef() qualifies columns differently before PG 16)
27+
// and version-gated features (NULLS NOT DISTINCT in PG 15+, temporal constraints
28+
// in PG 18+). Instead we run one or two representative cases per object type on
29+
// older versions — enough to catch gross, version-specific breakage such as a
30+
// catalog query that fails on an older release — and leave exhaustive coverage
31+
// (including views, materialized views, and dependency ordering, whose golden
32+
// files drift across versions) to the full suite on LatestPostgresVersion.
33+
//
34+
// Every entry here MUST have golden files that are byte-stable across all
35+
// supported versions: no view definitions (pg_get_viewdef() drift) and no
36+
// version-gated syntax.
37+
var essentialTests = []string{
38+
// Tables: create, alter, columns, constraints
39+
"create_table/add_table",
40+
"create_table/add_column_integer",
41+
"create_table/add_column_text",
42+
"create_table/drop_column",
43+
"create_table/remove_not_null",
44+
"create_table/add_check",
45+
"create_table/add_uk",
46+
"create_table/add_unique_constraint",
47+
48+
// Indexes
49+
"create_index/add_index_with_reloptions",
50+
51+
// Functions and procedures
52+
"create_function/add_function",
53+
"create_function/drop_function",
54+
"create_procedure/add_procedure",
55+
56+
// Sequences
57+
"create_sequence/add_sequence",
58+
59+
// Types and domains
60+
"create_type/add_type",
61+
"create_type/add_value",
62+
"create_domain/add_domain",
63+
64+
// Triggers (non-view based)
65+
"create_trigger/drop_trigger",
66+
"create_trigger/add_trigger_constraint",
67+
68+
// Policies
69+
"create_policy/add_policy",
70+
71+
// Aggregates
72+
"create_aggregate/add_aggregate",
73+
74+
// Comments
75+
"comment/add_table_comment",
76+
"comment/add_column_comments",
77+
78+
// Privileges
79+
"privilege/grant_table_select",
80+
"default_privilege/add_table_privilege",
6081
}
6182

6283
// skipListRequiresExtension defines test cases that require third-party extensions
@@ -67,69 +88,48 @@ var skipListRequiresExtension = []string{
6788
"create_table/issue_295_pgvector_typmod",
6889
}
6990

70-
// skipListPG14 defines test cases that should be skipped for PostgreSQL 14 only.
71-
//
72-
// Reason for skipping:
73-
// These tests use features not available in PostgreSQL 14 (e.g., NULLS NOT DISTINCT is PG15+).
74-
var skipListPG14 = []string{
75-
"create_index/add_index",
76-
"create_table/add_unique_constraint_nulls_not_distinct",
77-
"TestDumpCommand_Issue412UniqueNullsNotDistinct",
78-
}
79-
80-
// skipListPG14_17 defines test cases that should be skipped for PostgreSQL 14-17.
81-
//
82-
// Reason for skipping:
83-
// These tests use features not available before PostgreSQL 18
84-
// (e.g., temporal constraints with WITHOUT OVERLAPS / PERIOD require pg_constraint.conperiod).
85-
var skipListPG14_17 = []string{
86-
"create_table/add_pk",
87-
"create_table/add_fk",
88-
}
89-
90-
// skipListForVersion maps PostgreSQL major versions to their skip lists.
91-
var skipListForVersion = map[int][]string{
92-
14: append(append(append([]string(nil), skipListPG14_17...), skipListPG14_15...), skipListPG14...),
93-
15: append(append([]string(nil), skipListPG14_17...), skipListPG14_15...),
94-
16: skipListPG14_17,
95-
17: skipListPG14_17,
91+
// matchesAnyPattern reports whether testName matches any of the given patterns.
92+
// Patterns use a slash before the test name (e.g. "create_view/add_view") while
93+
// test names from subtests use underscores (e.g. "create_view_add_view"), so we
94+
// accept both forms.
95+
func matchesAnyPattern(testName string, patterns []string) bool {
96+
for _, pattern := range patterns {
97+
if testName == pattern || testName == strings.ReplaceAll(pattern, "/", "_") {
98+
return true
99+
}
100+
}
101+
return false
96102
}
97103

98104
// ShouldSkipTest checks if a test should be skipped for the given PostgreSQL major version.
99105
// If the test should be skipped, it calls t.Skipf() which stops test execution.
100106
//
107+
// Strategy:
108+
// - Tests requiring an unavailable extension are skipped on every version.
109+
// - The full suite runs only against LatestPostgresVersion.
110+
// - Older versions run only the essential smoke-test subset (essentialTests).
111+
//
101112
// Test name format examples:
102113
// - "create_view_add_view" (from TestDiffFromFiles subtests - underscores separate all parts)
103114
// - "create_view/add_view" (skip list patterns - underscores in category, slash before test)
104115
// - "TestDumpCommand_Employee" (from dump tests - starts with Test)
105-
//
106-
// Matching uses exact string match with flexible slash/underscore handling:
107-
// Pattern "create_view/add_view" matches test name "create_view_add_view" (underscores)
108116
func ShouldSkipTest(t *testing.T, testName string, majorVersion int) {
109117
t.Helper()
110118

111-
// Check extension-required skip list (applies to all versions)
112-
for _, pattern := range skipListRequiresExtension {
113-
patternNormalized := strings.ReplaceAll(pattern, "/", "_")
114-
if testName == patternNormalized || testName == pattern {
115-
t.Skipf("Skipping test %q: requires third-party extension not available in embedded-postgres", testName)
116-
}
119+
// Extension-required tests are skipped on every version (the extension isn't
120+
// bundled with embedded-postgres).
121+
if matchesAnyPattern(testName, skipListRequiresExtension) {
122+
t.Skipf("Skipping test %q: requires third-party extension not available in embedded-postgres", testName)
117123
}
118124

119-
// Get skip patterns for this version
120-
skipPatterns, exists := skipListForVersion[majorVersion]
121-
if !exists {
122-
return // No skips defined for this version
125+
// The full suite runs only against the latest version.
126+
if majorVersion >= LatestPostgresVersion {
127+
return
123128
}
124129

125-
// Check if test name matches any skip pattern (exact match)
126-
for _, pattern := range skipPatterns {
127-
// Convert pattern slashes to underscores to match test name format
128-
// e.g., "create_view/add_view" -> "create_view_add_view"
129-
patternNormalized := strings.ReplaceAll(pattern, "/", "_")
130-
131-
if testName == patternNormalized || testName == pattern {
132-
t.Skipf("Skipping test %q on PostgreSQL %d (unsupported feature or formatting differences)", testName, majorVersion)
133-
}
130+
// Older versions run only the essential smoke-test subset.
131+
if !matchesAnyPattern(testName, essentialTests) {
132+
t.Skipf("Skipping test %q on PostgreSQL %d: only the essential subset runs on non-latest versions (full suite runs on PostgreSQL %d)",
133+
testName, majorVersion, LatestPostgresVersion)
134134
}
135135
}

0 commit comments

Comments
 (0)