From c8894c0869a42e7d436ee89fb0b21740857a522c Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sun, 25 Oct 2026 01:06:58 +0000 Subject: [PATCH 01/12] test(testhelpers): add RequireEmptyDatabase on the shared Postgres server The migrations package tests need a database with nothing in it, so they cannot use RequireMigratedDatabase and still start a container each (92 per CI run). RequireEmptyDatabase gives them a fresh database cloned from template0 on the same per-binary server, dropped when the test ends, with the same skip/fail rules and return shape as RequireMigratedDatabase. Templates are now keyed by absolute path, so a package that names the migrations directory both relatively and via runtime.Caller migrates one template instead of two. SetupPostgresContainer and RequirePostgresContainer now say to prefer the shared helpers and when a dedicated server is still right. They are not marked Deprecated: the shared server is built on them and the dedicated cases are legitimate, so SA1019 on every call would be noise. DEVELOPMENT.md shows the shared pattern instead of a per-test container. Co-Authored-By: Claude Opus 5.5 --- docs/DEVELOPMENT.md | 26 ++-- .../database/postgres/testhelpers/postgres.go | 18 +++ .../database/postgres/testhelpers/shared.go | 73 +++++++-- .../testhelpers/shared_integration_test.go | 141 +++++++++++++++++- 4 files changed, 233 insertions(+), 25 deletions(-) diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index b47c7200e..c411e69d4 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -265,26 +265,30 @@ app := &Application{Scheduler: mockScheduler} ### Integration Test Setup +Integration tests share one PostgreSQL container per test binary and each get a +fresh database of their own on it (`internal/database/postgres/testhelpers`): + ```go //go:build integration func TestWithPostgres(t *testing.T) { - if testing.Short() { - t.Skip("Skipping integration test") - } - - ctx := testutil.TestContext(t) - pg, err := testutil.SetupPostgresContainer(ctx, t) - testutil.AssertNoError(t, err) - - for k, v := range pg.Config() { - testutil.SetEnv(t, k, v) - } + ctx := t.Context() + // Fully migrated, dropped when the test ends. For a migration test that + // needs to start from nothing, use testhelpers.RequireEmptyDatabase(ctx, t). + pg := testhelpers.RequireMigratedDatabase(ctx, t, "../database/postgres/migrations") + store := config.NewPostgresStore(pg.DB) // Test with real database... } ``` +The package needs a `TestMain` (in a `//go:build integration` file) that calls +`testhelpers.TerminateSharedPostgres()` after `m.Run()`; see +`internal/config/main_integration_test.go`. Start a dedicated server with +`testhelpers.RequirePostgresContainer` only for a test that touches server-wide +state (roles, `ALTER SYSTEM`, connection limits, other sessions' backends), and +say why next to the call. + ### Coverage **Targets:** Unit >85%, Integration >80%, Critical paths 100% diff --git a/internal/database/postgres/testhelpers/postgres.go b/internal/database/postgres/testhelpers/postgres.go index 4bbac4974..68ceb7d75 100644 --- a/internal/database/postgres/testhelpers/postgres.go +++ b/internal/database/postgres/testhelpers/postgres.go @@ -23,6 +23,20 @@ type PostgresContainer struct { } // SetupPostgresContainer creates and starts a PostgreSQL test container. +// +// Prefer RequireMigratedDatabase or RequireEmptyDatabase in tests. Each call +// here starts a server of its own, about 2s before the test has even migrated +// it, where those hand out a fresh database on one server per test binary in +// milliseconds. A dedicated server is still right when a test changes or +// depends on server-wide state: roles, ALTER SYSTEM or other server settings, +// terminating other sessions' backends, connection limits, or stopping the +// server. Say why next to the call. Tests should go through +// RequirePostgresContainer, which only skips when Docker is unusable; this +// function remains for the shared server itself and for that wrapper. +// +// Not marked Deprecated: the shared server is built on it and the dedicated +// cases above are legitimate, so a staticcheck SA1019 warning on every call +// would be noise rather than guidance. func SetupPostgresContainer(ctx context.Context, t *testing.T) (*PostgresContainer, error) { t.Helper() @@ -121,6 +135,10 @@ func terminateAfterError(ctx context.Context, c testcontainers.Container, stage // present but unhealthy skips too, rather than failing. // // Callers remain responsible for Cleanup, matching SetupPostgresContainer. +// +// Prefer RequireMigratedDatabase or RequireEmptyDatabase, which share one +// server per test binary. Use this only for a test that needs a server to +// itself (see SetupPostgresContainer for when), with a comment saying why. func RequirePostgresContainer(ctx context.Context, t *testing.T) *PostgresContainer { t.Helper() diff --git a/internal/database/postgres/testhelpers/shared.go b/internal/database/postgres/testhelpers/shared.go index 3647e80d8..41aabf957 100644 --- a/internal/database/postgres/testhelpers/shared.go +++ b/internal/database/postgres/testhelpers/shared.go @@ -16,13 +16,13 @@ import ( ) // sharedServer is one PostgreSQL container shared by every test in a test -// binary that asks for a migrated database (issue #982). Starting a container -// and applying the full migration chain for each test was most of the -// internal/config integration run. Here the container starts once, the -// migrations run once into a template database, and each test gets its own -// CREATE DATABASE ... TEMPLATE copy of it. Tests stay isolated from each other -// exactly as before: every test starts from a freshly migrated database that -// nothing else writes to. +// binary that asks for a migrated or an empty database (issue #982). Starting +// a container and applying the full migration chain for each test was most of +// the integration run. Here the container starts once, the migrations run once +// into a template database, and each test gets its own CREATE DATABASE ... +// TEMPLATE copy of it (of template0, for an empty one). Tests stay isolated +// from each other exactly as before: every test starts from a fresh database +// that nothing else writes to. // // One mutex covers all of it. Holding it while the container starts and the // template migrates makes concurrent first callers wait for the one setup @@ -59,12 +59,38 @@ var shared sharedServer // job of TerminateSharedPostgres, called from the package's TestMain. func RequireMigratedDatabase(ctx context.Context, t *testing.T, migrationsPath string) *PostgresContainer { t.Helper() + require.NotEmpty(t, migrationsPath, "RequireMigratedDatabase needs a migrations path; use RequireEmptyDatabase for an unmigrated database") + return requireSharedDatabase(ctx, t, migrationsPath) +} + +// RequireEmptyDatabase gives t an empty database of its own on the same +// shared container RequireMigratedDatabase uses: no tables, no +// schema_migrations, nothing but what CREATE DATABASE ... TEMPLATE template0 +// puts there. It is for tests whose subject is the migration chain itself (up, +// down, dirty, heal), which need to start from nothing and must not see a +// template another test migrated. +// +// Cleanup, skips and the missing Container are exactly as for +// RequireMigratedDatabase. Being per database, it is no substitute for a +// dedicated server where a test touches server-wide state (roles, ALTER +// SYSTEM, connection limits, other sessions' backends); see +// RequirePostgresContainer for those. +func RequireEmptyDatabase(ctx context.Context, t *testing.T) *PostgresContainer { + t.Helper() + return requireSharedDatabase(ctx, t, "") +} + +// requireSharedDatabase creates the per-test database (cloned from the +// migrated template for migrationsPath, or from template0 when it is empty), +// registers its drop, and connects to it. +func requireSharedDatabase(ctx context.Context, t *testing.T, migrationsPath string) *PostgresContainer { + t.Helper() testcontainers.SkipIfProviderIsNotHealthy(t) name, cfg, err := shared.newDatabase(ctx, t, migrationsPath) require.NoError(t, err, - "Docker is healthy but no migrated PostgreSQL test database could be created; "+ + "Docker is healthy but no PostgreSQL test database could be created on the shared server; "+ "this is a real failure, not an environment without Docker") // Registered before connecting, so a database that was created but could not // be connected to is still dropped. @@ -95,8 +121,15 @@ func TerminateSharedPostgres() error { return err } +// emptyTemplate is what RequireEmptyDatabase clones. template0 is the pristine +// database every PostgreSQL server ships and never accepts connections, so, +// unlike template1 (which CREATE DATABASE uses by default and buildTemplate +// clones), nothing can have added objects to it or be connected to it. +const emptyTemplate = "template0" + // newDatabase returns the name and connection config of a new database cloned -// from the migrated template for migrationsPath. +// from the migrated template for migrationsPath, or from template0 when +// migrationsPath is empty. func (s *sharedServer) newDatabase(ctx context.Context, t *testing.T, migrationsPath string) (string, *database.Config, error) { s.mu.Lock() defer s.mu.Unlock() @@ -104,9 +137,12 @@ func (s *sharedServer) newDatabase(ctx context.Context, t *testing.T, migrations if err := s.start(ctx, t); err != nil { return "", nil, err } - tmpl, err := s.template(ctx, migrationsPath) - if err != nil { - return "", nil, err + tmpl := emptyTemplate + if migrationsPath != "" { + var err error + if tmpl, err = s.template(ctx, migrationsPath); err != nil { + return "", nil, err + } } s.seq++ @@ -130,10 +166,21 @@ func (s *sharedServer) start(ctx context.Context, t *testing.T) error { return s.startErr } +// templateKey identifies the migrations directory a template was built from. +// It is absolute so that "../database/postgres/migrations" and a +// runtime.Caller-derived path to the same directory share one template instead +// of each paying for a migration run. +func templateKey(migrationsPath string) string { + if abs, err := filepath.Abs(migrationsPath); err == nil { + return abs + } + return filepath.Clean(migrationsPath) +} + // template returns the template database for migrationsPath, creating and // migrating it on first use. Callers hold s.mu. func (s *sharedServer) template(ctx context.Context, migrationsPath string) (string, error) { - key := filepath.Clean(migrationsPath) + key := templateKey(migrationsPath) if tmpl, ok := s.templates[key]; ok { return tmpl.name, tmpl.err } diff --git a/internal/database/postgres/testhelpers/shared_integration_test.go b/internal/database/postgres/testhelpers/shared_integration_test.go index 16cf82f51..5b6a5a2dd 100644 --- a/internal/database/postgres/testhelpers/shared_integration_test.go +++ b/internal/database/postgres/testhelpers/shared_integration_test.go @@ -19,6 +19,7 @@ import ( "time" "github.com/LeanerCloud/cloud-commitments-platform/internal/database" + "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/jackc/pgx/v5" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -116,7 +117,7 @@ func TestRequireMigratedDatabase_TemplateRefusesConnections(t *testing.T) { db := RequireMigratedDatabase(ctx, t, sharedTestMigrationsPath()) shared.mu.Lock() - tmpl := shared.templates[filepath.Clean(sharedTestMigrationsPath())] + tmpl := shared.templates[templateKey(sharedTestMigrationsPath())] shared.mu.Unlock() require.NoError(t, tmpl.err) require.NotEmpty(t, tmpl.name) @@ -202,3 +203,141 @@ func connString(cfg *database.Config, dbName string) string { } return u.String() } + +// userRelations counts the relations (tables, views, sequences, indexes, ...) +// outside the system schemas: zero for a database nothing has touched. +func userRelations(ctx context.Context, t *testing.T, db *database.Connection) int { + t.Helper() + var n int + require.NoError(t, db.QueryRow(ctx, ` + SELECT count(*) FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE n.nspname NOT IN ('pg_catalog', 'information_schema') AND n.nspname NOT LIKE 'pg_toast%'`).Scan(&n)) + return n +} + +// requireEmpty asserts db is what CREATE DATABASE ... TEMPLATE template0 makes: +// no user relations and no extension beyond the built-in plpgsql. +func requireEmpty(ctx context.Context, t *testing.T, db *database.Connection, name string) { + t.Helper() + assert.Zero(t, userRelations(ctx, t, db), "%s must have no tables", name) + assert.False(t, tableExists(ctx, t, db, "schema_migrations"), "%s must not be migrated", name) + rows, err := db.Query(ctx, `SELECT extname FROM pg_extension ORDER BY extname`) + require.NoError(t, err) + extensions, err := pgx.CollectRows(rows, pgx.RowTo[string]) + require.NoError(t, err) + assert.Equal(t, []string{"plpgsql"}, extensions, "%s must have only the built-in extension", name) +} + +// TestRequireEmptyDatabase_IsEmpty pins the contract the migration tests rely +// on: the database starts with nothing in it, even after a migrated template +// exists on the same server, and it is the caller's alone. +func TestRequireEmptyDatabase_IsEmpty(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() + + // A migrated database first, so a template with every table exists on the + // server and an empty database cloned from the wrong template would show it. + migrated := RequireMigratedDatabase(ctx, t, sharedTestMigrationsPath()) + require.True(t, tableExists(ctx, t, migrated.DB, "schema_migrations")) + + db := RequireEmptyDatabase(ctx, t) + assert.Nil(t, db.Container, "the shared container is not the caller's to terminate") + assert.NotEqual(t, migrated.Config.Database, db.Config.Database) + assert.Equal(t, migrated.Config.Host, db.Config.Host, "empty databases live on the one shared server") + assert.Equal(t, migrated.Config.Port, db.Config.Port, "empty databases live on the one shared server") + requireEmpty(ctx, t, db.DB, "a new empty database") + + var current string + require.NoError(t, db.DB.QueryRow(ctx, `SELECT current_database()`).Scan(¤t)) + assert.Equal(t, db.Config.Database, current, "Config and DB must point at the same database") +} + +// TestRequireEmptyDatabase_IsolatedFromOtherDatabases migrates one empty +// database all the way, as a migration test does, and checks that neither a +// sibling nor a database handed out afterwards sees any of it, and that the +// migrated template is untouched by it. +func TestRequireEmptyDatabase_IsolatedFromOtherDatabases(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() + want := latestMigrationVersion(t) + + a := RequireEmptyDatabase(ctx, t) + b := RequireEmptyDatabase(ctx, t) + assert.NotEqual(t, a.Config.Database, b.Config.Database, "each caller needs a database of its own") + + require.NoError(t, migrations.RunMigrations(ctx, a.DB.Pool(), sharedTestMigrationsPath(), "", ""), + "the full chain must apply to an empty database") + requireFullyMigrated(ctx, t, a.DB, want) + _, err := a.DB.Exec(ctx, `CREATE TABLE isolation_probe (id int)`) + require.NoError(t, err) + + c := RequireEmptyDatabase(ctx, t) + requireEmpty(ctx, t, b.DB, "a sibling empty database") + requireEmpty(ctx, t, c.DB, "a later empty database") + + m := RequireMigratedDatabase(ctx, t, sharedTestMigrationsPath()) + assert.False(t, tableExists(ctx, t, m.DB, "isolation_probe"), "a migrated database sees a table an empty one created") +} + +// TestRequireEmptyDatabase_DropsDatabaseWhenTestEnds checks that an empty +// database does not outlive its test, even with a leaked connection to it. +func TestRequireEmptyDatabase_DropsDatabaseWhenTestEnds(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() + + var name string + var cfg *database.Config + t.Run("uses and leaks", func(t *testing.T) { + db := RequireEmptyDatabase(ctx, t) + name, cfg = db.Config.Database, db.Config + leaked, err := pgx.Connect(ctx, connString(cfg, name)) + require.NoError(t, err) + require.NoError(t, leaked.Ping(ctx)) // never closed, on purpose + }) + require.NotEmpty(t, name) + + admin, err := pgx.Connect(ctx, connString(cfg, "postgres")) + require.NoError(t, err) + defer func() { _ = admin.Close(ctx) }() + var exists bool + require.NoError(t, admin.QueryRow(ctx, `SELECT EXISTS (SELECT 1 FROM pg_database WHERE datname = $1)`, name).Scan(&exists)) + assert.False(t, exists, "database %s outlived its test", name) +} + +// TestRequireEmptyDatabase_ConcurrentFirstUse has parallel callers ask for an +// empty database from a cold start. They must share one container, each get a +// distinct empty database, and not cause a template to be migrated: nobody +// asked for one. +func TestRequireEmptyDatabase_ConcurrentFirstUse(t *testing.T) { + require.NoError(t, TerminateSharedPostgres(), "reset to a cold start") + + const callers = 8 + var mu sync.Mutex + names := map[string]bool{} + servers := map[string]bool{} + t.Run("group", func(t *testing.T) { + for i := 0; i < callers; i++ { + t.Run(fmt.Sprintf("caller-%d", i), func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() + db := RequireEmptyDatabase(ctx, t) + // Each caller writes to its own database; a shared one would + // make the second CREATE fail. + _, err := db.DB.Exec(ctx, `CREATE TABLE concurrency_probe (id int)`) + require.NoError(t, err) + + mu.Lock() + defer mu.Unlock() + names[db.Config.Database] = true + servers[fmt.Sprintf("%s:%d", db.Config.Host, db.Config.Port)] = true + }) + } + }) + + assert.Len(t, names, callers, "every caller needs a database of its own") + assert.Len(t, servers, 1, "concurrent first callers must share one container") + shared.mu.Lock() + defer shared.mu.Unlock() + assert.Empty(t, shared.templates, "an empty database must not migrate a template") +} From 146f1dc78f13ecf75feb700abe39d8bdc2eeaa51 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sun, 25 Oct 2026 01:07:00 +0000 Subject: [PATCH 02/12] test(api): share one Postgres server across integration tests internal/api started 151 PostgreSQL containers per integration run, each paying about 2s to start plus the full 127-migration chain. Every call site now takes a fresh database from testhelpers.RequireMigratedDatabase (or RequireEmptyDatabase for the test that walks migrations 100 -> 101 itself) on one server per test binary. Locally the package goes from 927s to 119s and from 151 containers to 1. The package already owns an untagged TestMain for the no-network guard, so main_integration_test.go registers the shared server's teardown via an afterTests hook rather than adding a second TestMain; both the tagged and untagged builds compile. The lock-wait poll in the plan concurrency test now filters pg_stat_activity to the current database, so a backend from another test's database on the shared server cannot satisfy it. Co-Authored-By: Claude Opus 5.5 --- .../api/db_rate_limiter_integration_test.go | 26 ++++++----------- ...alytics_scope_provider_integration_test.go | 6 +--- ...ler_config_propagation_integration_test.go | 6 +--- ...andler_ladder_timeline_integration_test.go | 6 +--- ...rketplace_compensation_integration_test.go | 22 ++++----------- ...dler_plans_concurrency_integration_test.go | 11 +++----- ...urchases_submit_dedupe_integration_test.go | 6 +--- ...dations_global_filters_integration_test.go | 6 +--- ...exchange_history_scope_integration_test.go | 5 +--- .../handler_ri_exchange_integration_test.go | 22 ++++----------- ...dler_users_update_race_integration_test.go | 6 +--- internal/api/main_integration_test.go | 25 +++++++++++++++++ internal/api/main_nonet_test.go | 9 ++++++ ...nullable_purchase_cost_integration_test.go | 5 ++-- ...ion_constraints_config_integration_test.go | 6 +--- ...onstraints_marketplace_integration_test.go | 6 +--- ...ecommendation_identity_integration_test.go | 3 +- ...hange_four_eyes_system_integration_test.go | 8 ++---- .../ri_utilization_cache_integration_test.go | 28 ++++++------------- 19 files changed, 82 insertions(+), 130 deletions(-) create mode 100644 internal/api/main_integration_test.go diff --git a/internal/api/db_rate_limiter_integration_test.go b/internal/api/db_rate_limiter_integration_test.go index b19567d85..3e1b6a8cb 100644 --- a/internal/api/db_rate_limiter_integration_test.go +++ b/internal/api/db_rate_limiter_integration_test.go @@ -10,24 +10,19 @@ import ( "testing" "time" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/jackc/pgx/v5/pgxpool" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -// setupRateLimitIntegration spins up a testcontainers Postgres, runs -// the full migration chain (so the rate_limits table from migration -// 000004 is present), and returns the pool + cleanup. Mirrors the -// helper in ri_utilization_cache_integration_test.go. -func setupRateLimitIntegration(ctx context.Context, t *testing.T) (*pgxpool.Pool, func()) { +// setupRateLimitIntegration returns the pool of a fully migrated database of +// the test's own (so the rate_limits table from migration 000004 is present) +// on the package's shared Postgres server. It is dropped when t ends. Mirrors +// the helper in ri_utilization_cache_integration_test.go. +func setupRateLimitIntegration(ctx context.Context, t *testing.T) *pgxpool.Pool { t.Helper() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - err = migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "") - require.NoError(t, err) - return container.DB.Pool(), func() { _ = container.Cleanup(ctx) } + return testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()).DB.Pool() } // TestDBRateLimiter_ConcurrentFirstRequest pins the contract that the @@ -41,8 +36,7 @@ func setupRateLimitIntegration(ctx context.Context, t *testing.T) (*pgxpool.Pool // allow/deny return — no INSERT collision, no spurious 500. func TestDBRateLimiter_ConcurrentFirstRequest(t *testing.T) { ctx := context.Background() - pool, cleanup := setupRateLimitIntegration(ctx, t) - defer cleanup() + pool := setupRateLimitIntegration(ctx, t) rl := NewDBRateLimiter(pool) // Set a high MaxAttempts so all N goroutines are allowed and we @@ -86,8 +80,7 @@ func TestDBRateLimiter_ConcurrentFirstRequest(t *testing.T) { // call must reset count to 1 and push reset_time forward. func TestDBRateLimiter_WindowExpiry_AtomicReset(t *testing.T) { ctx := context.Background() - pool, cleanup := setupRateLimitIntegration(ctx, t) - defer cleanup() + pool := setupRateLimitIntegration(ctx, t) rl := NewDBRateLimiter(pool) const winSecs = 60 @@ -125,8 +118,7 @@ func TestDBRateLimiter_WindowExpiry_AtomicReset(t *testing.T) { // MaxAttempts after the burst, which is documented behavior. func TestDBRateLimiter_ExceedsLimitDenies(t *testing.T) { ctx := context.Background() - pool, cleanup := setupRateLimitIntegration(ctx, t) - defer cleanup() + pool := setupRateLimitIntegration(ctx, t) rl := NewDBRateLimiter(pool) const max = 3 diff --git a/internal/api/handler_analytics_scope_provider_integration_test.go b/internal/api/handler_analytics_scope_provider_integration_test.go index 3ce59616c..70b7f0110 100644 --- a/internal/api/handler_analytics_scope_provider_integration_test.go +++ b/internal/api/handler_analytics_scope_provider_integration_test.go @@ -20,7 +20,6 @@ import ( "github.com/LeanerCloud/cloud-commitments-platform/internal/analytics" "github.com/LeanerCloud/cloud-commitments-platform/internal/auth" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" ) @@ -56,10 +55,7 @@ type analyticsScopeDB struct { func setupAnalyticsScopeDB(t *testing.T) *analyticsScopeDB { t.Helper() ctx := t.Context() - pg, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, pg.Cleanup(context.Background())) }) - require.NoError(t, migrations.RunMigrations(ctx, pg.DB.Pool(), "../database/postgres/migrations", "", "")) + pg := testhelpers.RequireMigratedDatabase(ctx, t, "../database/postgres/migrations") store := config.NewPostgresStore(pg.DB) aws := &config.CloudAccount{Name: scopeAWSAccount, Provider: "aws", ExternalID: scopeSharedID, Enabled: true} diff --git a/internal/api/handler_config_propagation_integration_test.go b/internal/api/handler_config_propagation_integration_test.go index f6821fd18..d5c034fec 100644 --- a/internal/api/handler_config_propagation_integration_test.go +++ b/internal/api/handler_config_propagation_integration_test.go @@ -8,7 +8,6 @@ import ( "testing" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/aws/aws-lambda-go/events" "github.com/jackc/pgx/v5" @@ -20,10 +19,7 @@ import ( // payment and ramp, and a failed propagation must not save the global change. func TestUpdateConfigPropagatesOnlyChangedDefaults(t *testing.T) { ctx := context.Background() - pg, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, pg.Cleanup(ctx)) }) - require.NoError(t, migrations.RunMigrations(ctx, pg.DB.Pool(), "../database/postgres/migrations", "", "")) + pg := testhelpers.RequireMigratedDatabase(ctx, t, "../database/postgres/migrations") store := config.NewPostgresStore(pg.DB) global, err := store.GetGlobalConfig(ctx) diff --git a/internal/api/handler_ladder_timeline_integration_test.go b/internal/api/handler_ladder_timeline_integration_test.go index 24b997287..68d4bcd01 100644 --- a/internal/api/handler_ladder_timeline_integration_test.go +++ b/internal/api/handler_ladder_timeline_integration_test.go @@ -11,7 +11,6 @@ import ( "github.com/LeanerCloud/cloud-commitments-go/pkg/ladder" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/google/uuid" "github.com/stretchr/testify/mock" @@ -21,10 +20,7 @@ import ( func TestLadderTimelineAmendmentRoutedPersistence(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, container.Cleanup(context.Background())) }) - require.NoError(t, migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "")) + container := testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()) store := config.NewPostgresStore(container.DB) account := &config.CloudAccount{Name: "Ladder fixture", Provider: "aws", ExternalID: "111111111111", Enabled: true} require.NoError(t, store.CreateCloudAccount(ctx, account)) diff --git a/internal/api/handler_marketplace_compensation_integration_test.go b/internal/api/handler_marketplace_compensation_integration_test.go index fdb1f1659..6a13b6e50 100644 --- a/internal/api/handler_marketplace_compensation_integration_test.go +++ b/internal/api/handler_marketplace_compensation_integration_test.go @@ -10,7 +10,6 @@ import ( ec2svc "github.com/LeanerCloud/cloud-commitments-go/providers/aws/services/ec2" "github.com/LeanerCloud/cloud-commitments-platform/internal/auth" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/aws/aws-sdk-go-v2/aws" awscreds "github.com/aws/aws-sdk-go-v2/credentials" @@ -37,10 +36,7 @@ func (s *failFirstListingWriteStore) UpdatePurchaseHistoryListing(ctx context.Co func TestMarketplaceListCancelFailureKeepsListingRecorded(t *testing.T) { ctx := t.Context() - pg, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, pg.Cleanup(context.Background())) }) - require.NoError(t, migrations.RunMigrations(ctx, pg.DB.Pool(), "../database/postgres/migrations", "", "")) + pg := testhelpers.RequireMigratedDatabase(ctx, t, "../database/postgres/migrations") store := config.NewPostgresStore(pg.DB) account := uuid.NewString() @@ -66,7 +62,7 @@ func TestMarketplaceListCancelFailureKeepsListingRecorded(t *testing.T) { marketplaceEC2Factory: func(_ aws.Config) marketplaceEC2Client { return ec2 }} h.awsCfgOnce.Do(func() { h.awsCfg = hostAWSConfig() }) - _, err = h.marketplaceList(ctx, marketplaceReq(), row.PurchaseID) + _, err := h.marketplaceList(ctx, marketplaceReq(), row.PurchaseID) require.Error(t, err) ce, ok := IsClientError(err) require.True(t, ok, "%v", err) @@ -141,10 +137,7 @@ func newCompensationAuth() *MockAuthService { func TestMarketplaceListCancelFailureAfterContextCanceledStillRecordsListing(t *testing.T) { ctx := t.Context() - pg, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, pg.Cleanup(context.Background())) }) - require.NoError(t, migrations.RunMigrations(ctx, pg.DB.Pool(), "../database/postgres/migrations", "", "")) + pg := testhelpers.RequireMigratedDatabase(ctx, t, "../database/postgres/migrations") store := config.NewPostgresStore(pg.DB) row := setupCompensationRow(t, store) @@ -157,7 +150,7 @@ func TestMarketplaceListCancelFailureAfterContextCanceledStillRecordsListing(t * marketplaceEC2Factory: func(_ aws.Config) marketplaceEC2Client { return ec2 }} h.awsCfgOnce.Do(func() { h.awsCfg = hostAWSConfig() }) - _, err = h.marketplaceList(reqCtx, marketplaceReq(), row.PurchaseID) + _, err := h.marketplaceList(reqCtx, marketplaceReq(), row.PurchaseID) require.Error(t, err) stored, err := store.GetPurchaseHistoryByPurchaseID(ctx, row.PurchaseID) @@ -168,10 +161,7 @@ func TestMarketplaceListCancelFailureAfterContextCanceledStillRecordsListing(t * func TestMarketplaceListCancelFailureRecordsEmptyAWSStateAsPending(t *testing.T) { ctx := t.Context() - pg, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, pg.Cleanup(context.Background())) }) - require.NoError(t, migrations.RunMigrations(ctx, pg.DB.Pool(), "../database/postgres/migrations", "", "")) + pg := testhelpers.RequireMigratedDatabase(ctx, t, "../database/postgres/migrations") store := config.NewPostgresStore(pg.DB) row := setupCompensationRow(t, store) @@ -187,7 +177,7 @@ func TestMarketplaceListCancelFailureRecordsEmptyAWSStateAsPending(t *testing.T) marketplaceEC2Factory: func(_ aws.Config) marketplaceEC2Client { return ec2 }} h.awsCfgOnce.Do(func() { h.awsCfg = hostAWSConfig() }) - _, err = h.marketplaceList(ctx, marketplaceReq(), row.PurchaseID) + _, err := h.marketplaceList(ctx, marketplaceReq(), row.PurchaseID) ce, ok := IsClientError(err) require.True(t, ok, "%v", err) assert.Contains(t, ce.message, "needs reconciliation") diff --git a/internal/api/handler_plans_concurrency_integration_test.go b/internal/api/handler_plans_concurrency_integration_test.go index 1cd4a0c04..928eee6de 100644 --- a/internal/api/handler_plans_concurrency_integration_test.go +++ b/internal/api/handler_plans_concurrency_integration_test.go @@ -22,7 +22,6 @@ import ( "time" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/aws/aws-lambda-go/events" "github.com/google/uuid" @@ -45,10 +44,7 @@ type createConcurrencyFixture struct { func newCreateConcurrencyFixture(ctx context.Context, t *testing.T) *createConcurrencyFixture { t.Helper() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { _ = container.Cleanup(context.Background()) }) - require.NoError(t, migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "")) + container := testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()) store := config.NewPostgresStore(container.DB) @@ -74,7 +70,7 @@ func newCreateConcurrencyFixture(ctx context.Context, t *testing.T) *createConcu // created_by_user_id, which is a foreign key, so a synthetic UUID would make // every insert fail for a reason unrelated to what these tests measure. userID := uuid.New().String() - _, err = container.DB.Pool().Exec(ctx, ` + _, err := container.DB.Pool().Exec(ctx, ` INSERT INTO users (id, email, password_hash, salt, active, group_ids, created_at, updated_at) SELECT $1, 'creator@example.com', 'x', 'y', true, ARRAY[g.id], now(), now() FROM groups g ORDER BY g.name LIMIT 1`, userID) @@ -273,7 +269,8 @@ func TestCreatePlannedPurchases_BlocksOnTheRampLockAndSeesTheWinner(t *testing.T var blocked int if qErr := f.pool.QueryRow(ctx, ` SELECT count(*) FROM pg_stat_activity - WHERE wait_event_type = 'Lock' + WHERE datname = current_database() + AND wait_event_type = 'Lock' AND state = 'active' AND pid <> pg_backend_pid() AND query ILIKE '%purchase_plans%FOR UPDATE%'`, diff --git a/internal/api/handler_purchases_submit_dedupe_integration_test.go b/internal/api/handler_purchases_submit_dedupe_integration_test.go index 5f89a65d7..7d7a32454 100644 --- a/internal/api/handler_purchases_submit_dedupe_integration_test.go +++ b/internal/api/handler_purchases_submit_dedupe_integration_test.go @@ -11,7 +11,6 @@ import ( "github.com/LeanerCloud/cloud-commitments-go/pkg/common" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/google/uuid" "github.com/jackc/pgx/v5" @@ -71,10 +70,7 @@ func countSubmitRows(t *testing.T, pg *testhelpers.PostgresContainer) int { func setupSubmitDB(t *testing.T) (*testhelpers.PostgresContainer, *config.PostgresStore) { t.Helper() ctx := context.Background() - pg, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, pg.Cleanup(ctx)) }) - require.NoError(t, migrations.RunMigrations(ctx, pg.DB.Pool(), "../database/postgres/migrations", "", "")) + pg := testhelpers.RequireMigratedDatabase(ctx, t, "../database/postgres/migrations") return pg, config.NewPostgresStore(pg.DB) } diff --git a/internal/api/handler_recommendations_global_filters_integration_test.go b/internal/api/handler_recommendations_global_filters_integration_test.go index bacbe9744..4ef76c2dd 100644 --- a/internal/api/handler_recommendations_global_filters_integration_test.go +++ b/internal/api/handler_recommendations_global_filters_integration_test.go @@ -13,7 +13,6 @@ import ( "github.com/LeanerCloud/cloud-commitments-go/pkg/provider" "github.com/LeanerCloud/cloud-commitments-platform/internal/auth" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/LeanerCloud/cloud-commitments-platform/internal/scheduler" "github.com/aws/aws-lambda-go/events" @@ -55,10 +54,7 @@ func TestGlobalFiltersCollectedRecommendationsAPI(t *testing.T) { for _, registered := range []bool{false, true} { t.Run(fmt.Sprintf("registered=%t", registered), func(t *testing.T) { ctx := t.Context() - pg, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, pg.Cleanup(context.Background())) }) - require.NoError(t, migrations.RunMigrations(ctx, pg.DB.Pool(), "../database/postgres/migrations", "", "")) + pg := testhelpers.RequireMigratedDatabase(ctx, t, "../database/postgres/migrations") store := config.NewPostgresStore(pg.DB) require.NoError(t, store.SaveGlobalConfig(ctx, &config.GlobalConfig{EnabledProviders: []string{"aws"}, DefaultTerm: 1, DefaultPayment: "all-upfront"})) account := uuid.NewString() diff --git a/internal/api/handler_ri_exchange_history_scope_integration_test.go b/internal/api/handler_ri_exchange_history_scope_integration_test.go index a02b0781b..16bd0855b 100644 --- a/internal/api/handler_ri_exchange_history_scope_integration_test.go +++ b/internal/api/handler_ri_exchange_history_scope_integration_test.go @@ -8,7 +8,6 @@ import ( "time" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -23,9 +22,7 @@ import ( func TestRIExchangeHistory_ScopedUserSeesOlderOwnRows_Integration(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() - container := testhelpers.RequirePostgresContainer(ctx, t) - t.Cleanup(func() { require.NoError(t, container.Cleanup(context.Background())) }) - require.NoError(t, migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "")) + container := testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()) store := config.NewPostgresStore(container.DB) acctA := &config.CloudAccount{Name: "exchange-a", Provider: "aws", ExternalID: "111122223333", Enabled: true} diff --git a/internal/api/handler_ri_exchange_integration_test.go b/internal/api/handler_ri_exchange_integration_test.go index 13b71e06a..11aa589bb 100644 --- a/internal/api/handler_ri_exchange_integration_test.go +++ b/internal/api/handler_ri_exchange_integration_test.go @@ -17,7 +17,6 @@ import ( "github.com/LeanerCloud/cloud-commitments-go/providers/aws/recommendations" ec2svc "github.com/LeanerCloud/cloud-commitments-go/providers/aws/services/ec2" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/stretchr/testify/require" ) @@ -47,14 +46,9 @@ func (f *fakeReshapeRecs) GetRIUtilization(_ context.Context, _ int, _ string) ( return f.utilization, nil } -func setupReshapeHandlerIntegration(ctx context.Context, t *testing.T) (*config.PostgresStore, func()) { +func setupReshapeHandlerIntegration(ctx context.Context, t *testing.T) *config.PostgresStore { t.Helper() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - err = migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "") - require.NoError(t, err) - store := config.NewPostgresStore(container.DB) - return store, func() { _ = container.Cleanup(ctx) } + return config.NewPostgresStore(testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()).DB) } // buildReshapeHandler constructs a Handler with only the fields the @@ -117,8 +111,7 @@ func seedRecsForRegion(ctx context.Context, t *testing.T, store *config.Postgres // calls. func TestReshapeRecommendations_Integration_EndToEnd(t *testing.T) { ctx := context.Background() - store, cleanup := setupReshapeHandlerIntegration(ctx, t) - defer cleanup() + store := setupReshapeHandlerIntegration(ctx, t) // Seed cross-family recs in us-east-1. Same-family seed uses a // different SIZE (m5.2xlarge) than the primary target (m5.large) @@ -191,8 +184,7 @@ func TestReshapeRecommendations_Integration_EndToEnd(t *testing.T) { // utilization fetcher is TTL-cached. func TestReshapeRecommendations_Integration_SecondCallHitsCache(t *testing.T) { ctx := context.Background() - store, cleanup := setupReshapeHandlerIntegration(ctx, t) - defer cleanup() + store := setupReshapeHandlerIntegration(ctx, t) seedRecsForRegion(ctx, t, store, []config.RecommendationRecord{ {Provider: "aws", Service: "ec2", Region: "us-east-1", ResourceType: "c5.large", Term: 1, MonthlyCost: aws.Float64(40)}, @@ -229,8 +221,7 @@ func TestReshapeRecommendations_Integration_SecondCallHitsCache(t *testing.T) { // region yet" rather than blanking the page. func TestReshapeRecommendations_Integration_NoCachedRecsReturnsPrimaryOnly(t *testing.T) { ctx := context.Background() - store, cleanup := setupReshapeHandlerIntegration(ctx, t) - defer cleanup() + store := setupReshapeHandlerIntegration(ctx, t) // Note: NO seedRecsForRegion call — table starts empty. @@ -301,8 +292,7 @@ func seedCloudAccount(ctx context.Context, t *testing.T, store *config.PostgresS // through bug) would have shipped undetected. func TestReshapeRecommendations_Integration_ScopedAccount_FiltersToAccount(t *testing.T) { ctx := context.Background() - store, cleanup := setupReshapeHandlerIntegration(ctx, t) - defer cleanup() + store := setupReshapeHandlerIntegration(ctx, t) // Two registered accounts, distinct AWS account numbers. The recs // scope filter compares against the CloudAccount UUID (not the AWS diff --git a/internal/api/handler_users_update_race_integration_test.go b/internal/api/handler_users_update_race_integration_test.go index 7d7f3b819..909b7733f 100644 --- a/internal/api/handler_users_update_race_integration_test.go +++ b/internal/api/handler_users_update_race_integration_test.go @@ -11,7 +11,6 @@ import ( "github.com/LeanerCloud/cloud-commitments-platform/internal/auth" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" "github.com/LeanerCloud/cloud-commitments-platform/internal/database" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/aws/aws-lambda-go/events" "github.com/stretchr/testify/assert" @@ -49,10 +48,7 @@ func (s *afterReadStore) GetUserByID(ctx context.Context, id string) (*auth.User // columns it never meant to change. func TestAdminUpdateUserHTTPKeepsConcurrentMFA(t *testing.T) { ctx := t.Context() - pg, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, pg.Cleanup(context.Background())) }) - require.NoError(t, migrations.RunMigrations(ctx, pg.DB.Pool(), "../database/postgres/migrations", "", "")) + pg := testhelpers.RequireMigratedDatabase(ctx, t, "../database/postgres/migrations") authStore := auth.NewPostgresStore(pg.DB) csrfKey := []byte(strings.Repeat("u", 32)) view := []auth.Permission{{Action: auth.ActionView, Resource: "recommendations"}} diff --git a/internal/api/main_integration_test.go b/internal/api/main_integration_test.go new file mode 100644 index 000000000..ab8969ec6 --- /dev/null +++ b/internal/api/main_integration_test.go @@ -0,0 +1,25 @@ +//go:build integration + +package api + +import ( + "log" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" +) + +// The integration tests in this package share one PostgreSQL container (issue +// #982): each still gets a fresh database of its own from +// testhelpers.RequireMigratedDatabase or RequireEmptyDatabase, but the server +// and its migrated template outlive any one test. TestMain, in +// main_nonet_test.go because it exists in every build, stops the server after +// the last test. +func init() { + afterTests = append(afterTests, func() { + if err := testhelpers.TerminateSharedPostgres(); err != nil { + // Ryuk reaps the container anyway, so a teardown hiccup does not + // overrule the tests' own verdict. + log.Printf("terminate shared postgres container: %v", err) + } + }) +} diff --git a/internal/api/main_nonet_test.go b/internal/api/main_nonet_test.go index 52d5e4d62..4ea2ea883 100644 --- a/internal/api/main_nonet_test.go +++ b/internal/api/main_nonet_test.go @@ -19,6 +19,12 @@ import ( // netGuard is the no-network guard TestMain installs for this package (#855). var netGuard *noNetworkGuard +// afterTests is teardown that TestMain runs once m.Run returns. A package has +// one TestMain and this file owns it in every build, so build-tagged files +// register their teardown here from init instead: main_integration_test.go +// stops the Postgres server the integration tests share. +var afterTests []func() + // TestMain keeps this package's tests hermetic (#855). Before any test runs it: // // - strips every AWS_* variable and points the shared config and @@ -45,6 +51,9 @@ func TestMain(m *testing.M) { } netGuard = g code := m.Run() + for _, teardown := range afterTests { + teardown() + } if blocked := g.stop(); len(blocked) > 0 { fmt.Fprintf(os.Stderr, "FAIL: internal/api tests attempted %d outbound network call(s); stub the client instead (#855):\n %s\n", len(blocked), strings.Join(blocked, "\n ")) diff --git a/internal/api/nullable_purchase_cost_integration_test.go b/internal/api/nullable_purchase_cost_integration_test.go index 247eb6669..16f544c20 100644 --- a/internal/api/nullable_purchase_cost_integration_test.go +++ b/internal/api/nullable_purchase_cost_integration_test.go @@ -21,9 +21,8 @@ import ( func TestNullablePurchaseCostPostgres(t *testing.T) { ctx := context.Background() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, container.Cleanup(ctx)) }) + // Empty, not migrated: the test walks the chain to 100 and then 101 itself. + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() path := filepath.Join("..", "database", "postgres", "migrations") require.NoError(t, migrations.MigrateToVersion(ctx, pool, path, 100)) diff --git a/internal/api/permission_constraints_config_integration_test.go b/internal/api/permission_constraints_config_integration_test.go index 8c92c776d..bdafcd3ad 100644 --- a/internal/api/permission_constraints_config_integration_test.go +++ b/internal/api/permission_constraints_config_integration_test.go @@ -12,7 +12,6 @@ import ( "testing" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/aws/aws-lambda-go/events" "github.com/google/uuid" @@ -21,10 +20,7 @@ import ( func TestConfigInfrastructureKeyHTTP(t *testing.T) { ctx := context.Background() - pg, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, pg.Cleanup(ctx)) }) - require.NoError(t, migrations.RunMigrations(ctx, pg.DB.Pool(), "../database/postgres/migrations", "", "")) + pg := testhelpers.RequireMigratedDatabase(ctx, t, "../database/postgres/migrations") store := config.NewPostgresStore(pg.DB) authService := new(MockAuthService) handler := NewHandler(HandlerConfig{ConfigStore: store, AuthService: authService}) diff --git a/internal/api/permission_constraints_marketplace_integration_test.go b/internal/api/permission_constraints_marketplace_integration_test.go index fd071468a..76e86f54b 100644 --- a/internal/api/permission_constraints_marketplace_integration_test.go +++ b/internal/api/permission_constraints_marketplace_integration_test.go @@ -18,7 +18,6 @@ import ( "github.com/LeanerCloud/cloud-commitments-platform/internal/auth" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/aws/aws-lambda-go/events" "github.com/aws/aws-sdk-go-v2/aws" @@ -81,10 +80,7 @@ func marketplaceSession(t *testing.T, store *auth.PostgresStore, permissions []a func TestMarketplacePermissionConstraintsHTTP(t *testing.T) { ctx := t.Context() - pg, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, pg.Cleanup(context.Background())) }) - require.NoError(t, migrations.RunMigrations(ctx, pg.DB.Pool(), "../database/postgres/migrations", "", "")) + pg := testhelpers.RequireMigratedDatabase(ctx, t, "../database/postgres/migrations") store, authStore := config.NewPostgresStore(pg.DB), auth.NewPostgresStore(pg.DB) csrfKey := []byte(strings.Repeat("m", 32)) handler := NewHandler(HandlerConfig{ConfigStore: store, AuthService: &marketplaceAuthFixture{service: auth.NewService(auth.ServiceConfig{Store: authStore, CSRFKey: csrfKey})}}) diff --git a/internal/api/recommendation_identity_integration_test.go b/internal/api/recommendation_identity_integration_test.go index adf88b18c..5f563627d 100644 --- a/internal/api/recommendation_identity_integration_test.go +++ b/internal/api/recommendation_identity_integration_test.go @@ -85,8 +85,7 @@ func (identityEmail) SendNewRecommendationsNotification(context.Context, email.N func TestRecommendationIdentity_CollectionPostgresDetailAndPricing(t *testing.T) { ctx := context.Background() - store, cleanup := setupRICacheIntegration(ctx, t) - defer cleanup() + store := setupRICacheIntegration(ctx, t) require.NoError(t, store.SaveGlobalConfig(ctx, &config.GlobalConfig{ EnabledProviders: []string{"aws"}, DefaultTerm: 1, DefaultPayment: "all-upfront", })) diff --git a/internal/api/ri_exchange_four_eyes_system_integration_test.go b/internal/api/ri_exchange_four_eyes_system_integration_test.go index 39f21037b..ae3d95568 100644 --- a/internal/api/ri_exchange_four_eyes_system_integration_test.go +++ b/internal/api/ri_exchange_four_eyes_system_integration_test.go @@ -21,7 +21,6 @@ import ( "github.com/stretchr/testify/require" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" ) @@ -56,12 +55,9 @@ func seedSystemCreatedExchange(ctx context.Context, t *testing.T, store *config. func setupSystemExchangeHandler(ctx context.Context, t *testing.T, fourEyes bool) (*Handler, *config.PostgresStore, *int) { t.Helper() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { _ = container.Cleanup(context.Background()) }) - require.NoError(t, migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "")) + container := testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()) store := config.NewPostgresStore(container.DB) - _, err = store.UpdateGlobalConfigAtomic(ctx, func(c *config.GlobalConfig) error { + _, err := store.UpdateGlobalConfigAtomic(ctx, func(c *config.GlobalConfig) error { c.RequireDifferentApprover = fourEyes return nil }) diff --git a/internal/api/ri_utilization_cache_integration_test.go b/internal/api/ri_utilization_cache_integration_test.go index c63a89c6d..09a470a8b 100644 --- a/internal/api/ri_utilization_cache_integration_test.go +++ b/internal/api/ri_utilization_cache_integration_test.go @@ -14,7 +14,6 @@ import ( "github.com/LeanerCloud/cloud-commitments-go/providers/aws/recommendations" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/stretchr/testify/require" ) @@ -28,17 +27,12 @@ func getMigrationsPath() string { return filepath.Join(filepath.Dir(filename), "..", "database", "postgres", "migrations") } -func setupRICacheIntegration(ctx context.Context, t *testing.T) (*config.PostgresStore, func()) { +// setupRICacheIntegration returns a store over a fully migrated database of +// the test's own on the package's shared Postgres server, so the test uses the +// same schema production does. The database is dropped when t ends. +func setupRICacheIntegration(ctx context.Context, t *testing.T) *config.PostgresStore { t.Helper() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - // Migration 000027 was made idempotent in commit F — the full - // migration chain now runs cleanly on fresh containers, so the - // test uses the same schema production does. - err = migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "") - require.NoError(t, err) - store := config.NewPostgresStore(container.DB) - return store, func() { _ = container.Cleanup(ctx) } + return config.NewPostgresStore(testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()).DB) } // TestRIUtilizationCache_Integration_ColdReadFetchesAndPersists verifies @@ -46,8 +40,7 @@ func setupRICacheIntegration(ctx context.Context, t *testing.T) (*config.Postgre // persists the result in ri_utilization_cache. func TestRIUtilizationCache_Integration_ColdReadFetchesAndPersists(t *testing.T) { ctx := context.Background() - store, cleanup := setupRICacheIntegration(ctx, t) - defer cleanup() + store := setupRICacheIntegration(ctx, t) cache := newRIUtilizationCache(store, false) fetched := []recommendations.RIUtilization{ @@ -81,8 +74,7 @@ func TestRIUtilizationCache_Integration_ColdReadFetchesAndPersists(t *testing.T) // invoking the fetcher. func TestRIUtilizationCache_Integration_FreshHitNoFetch(t *testing.T) { ctx := context.Background() - store, cleanup := setupRICacheIntegration(ctx, t) - defer cleanup() + store := setupRICacheIntegration(ctx, t) cache := newRIUtilizationCache(store, false) fetched := []recommendations.RIUtilization{{ReservedInstanceID: "ri-fresh-1"}} @@ -108,8 +100,7 @@ func TestRIUtilizationCache_Integration_FreshHitNoFetch(t *testing.T) { // refresh fires, Postgres row is updated within a short window. func TestRIUtilizationCache_Integration_StaleNonLambdaServesStaleAndRefreshes(t *testing.T) { ctx := context.Background() - store, cleanup := setupRICacheIntegration(ctx, t) - defer cleanup() + store := setupRICacheIntegration(ctx, t) staleData := []recommendations.RIUtilization{{ReservedInstanceID: "ri-stale"}} freshData := []recommendations.RIUtilization{{ReservedInstanceID: "ri-fresh"}} @@ -151,8 +142,7 @@ func TestRIUtilizationCache_Integration_StaleNonLambdaServesStaleAndRefreshes(t // non-Lambda: the caller blocks until fresh data arrives. func TestRIUtilizationCache_Integration_HardExpiredSyncRefetch(t *testing.T) { ctx := context.Background() - store, cleanup := setupRICacheIntegration(ctx, t) - defer cleanup() + store := setupRICacheIntegration(ctx, t) staleData := []recommendations.RIUtilization{{ReservedInstanceID: "ri-stale"}} freshData := []recommendations.RIUtilization{{ReservedInstanceID: "ri-fresh"}} From b83a3b1e8b5d32f781bdc09d3f12adb5ec5fc2d2 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sun, 25 Oct 2026 01:19:47 +0000 Subject: [PATCH 03/12] test(commitmentopts): share one Postgres server across integration tests Each of the five store tests started its own Postgres container and ran all migrations into it, about 10s of overhead per test (issue #982). setupStore now takes a fresh copy of the migrated template from testhelpers.RequireMigratedDatabase, and a TestMain stops the shared server after the last test. Assertions are unchanged. Co-Authored-By: Claude Opus 5.5 --- .../commitmentopts/main_integration_test.go | 26 +++++++++++++++++++ .../commitmentopts/store_postgres_test.go | 14 +++------- 2 files changed, 30 insertions(+), 10 deletions(-) create mode 100644 internal/commitmentopts/main_integration_test.go diff --git a/internal/commitmentopts/main_integration_test.go b/internal/commitmentopts/main_integration_test.go new file mode 100644 index 000000000..354efcac7 --- /dev/null +++ b/internal/commitmentopts/main_integration_test.go @@ -0,0 +1,26 @@ +//go:build integration +// +build integration + +package commitmentopts_test + +import ( + "log" + "os" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" +) + +// TestMain stops the PostgreSQL container that every integration test in this +// package shares (issue #982). Each test still gets a fresh database of its own +// from testhelpers.RequireMigratedDatabase or RequireEmptyDatabase; only the +// container and the migrated template it is cloned from outlive a single test. +func TestMain(m *testing.M) { + code := m.Run() + if err := testhelpers.TerminateSharedPostgres(); err != nil { + // Ryuk reaps the container anyway, so a teardown hiccup does not + // overrule the tests' own verdict. + log.Printf("terminate shared postgres container: %v", err) + } + os.Exit(code) +} diff --git a/internal/commitmentopts/store_postgres_test.go b/internal/commitmentopts/store_postgres_test.go index ef3c08f11..58f24bd62 100644 --- a/internal/commitmentopts/store_postgres_test.go +++ b/internal/commitmentopts/store_postgres_test.go @@ -11,7 +11,6 @@ import ( "testing" "github.com/LeanerCloud/cloud-commitments-platform/internal/commitmentopts" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -24,17 +23,12 @@ func getMigrationsPath() string { return filepath.Join(filepath.Dir(filename), "..", "database", "postgres", "migrations") } -// setupStore stands up a fresh Postgres container and returns a store -// bound to it. The container is torn down via t.Cleanup. +// setupStore returns a store bound to a fresh, migrated database of its own on +// the package's shared Postgres server. The helper drops the database when the +// test ends. func setupStore(ctx context.Context, t *testing.T) *commitmentopts.PostgresStore { t.Helper() - - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { container.Cleanup(ctx) }) - - require.NoError(t, migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "")) - return commitmentopts.NewPostgresStore(container.DB) + return commitmentopts.NewPostgresStore(testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()).DB) } func TestPostgresStore_HasData_EmptyIsFalse(t *testing.T) { From e0384ad17071c23536a0e0f9b77f30bfc481ad70 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sun, 25 Oct 2026 01:19:50 +0000 Subject: [PATCH 04/12] test(database): share one Postgres server across integration tests The four integration tests each started a dedicated container (issue #982). None needs a schema or server-level isolation: advisory lock keys and the pg_locks holder query are per database, the pool-floor test exhausts the client pool rather than the server's max_connections, and the query-logging test creates its own table. They now take an empty database from testhelpers.RequireEmptyDatabase on one shared server, and a TestMain stops it after the last test. Co-Authored-By: Claude Opus 5.5 --- cmd/cleanup-lambda/main_integration_test.go | 6 +---- .../advisory_lock_holder_integration_test.go | 5 ++-- .../advisory_lock_integration_test.go | 5 ++-- internal/database/main_integration_test.go | 26 +++++++++++++++++++ .../database/pool_floor_integration_test.go | 5 ++-- .../query_logging_integration_test.go | 4 +-- 6 files changed, 38 insertions(+), 13 deletions(-) create mode 100644 internal/database/main_integration_test.go diff --git a/cmd/cleanup-lambda/main_integration_test.go b/cmd/cleanup-lambda/main_integration_test.go index 4f0afbcde..282665ca2 100644 --- a/cmd/cleanup-lambda/main_integration_test.go +++ b/cmd/cleanup-lambda/main_integration_test.go @@ -17,7 +17,6 @@ import ( "github.com/LeanerCloud/cloud-commitments-platform/internal/config" "github.com/LeanerCloud/cloud-commitments-platform/internal/database" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/stretchr/testify/require" ) @@ -35,12 +34,9 @@ var ( func cleanupTestDB(ctx context.Context, t *testing.T) *database.Connection { t.Helper() - c := testhelpers.RequirePostgresContainer(ctx, t) - t.Cleanup(func() { _ = c.Cleanup(context.Background()) }) _, file, _, _ := runtime.Caller(0) path := filepath.Join(filepath.Dir(file), "..", "..", "internal", "database", "postgres", "migrations") - require.NoError(t, migrations.RunMigrations(ctx, c.DB.Pool(), path, "", "")) - return c.DB + return testhelpers.RequireMigratedDatabase(ctx, t, path).DB } // insertExecution adds a plan-less execution whose expires_at is offset from diff --git a/internal/database/advisory_lock_holder_integration_test.go b/internal/database/advisory_lock_holder_integration_test.go index 30c8ee7fb..dedf79637 100644 --- a/internal/database/advisory_lock_holder_integration_test.go +++ b/internal/database/advisory_lock_holder_integration_test.go @@ -24,8 +24,9 @@ import ( // the lock it is asking about. func TestAdvisoryLockHeldFor(t *testing.T) { ctx := context.Background() - container := testhelpers.RequirePostgresContainer(ctx, t) - t.Cleanup(func() { _ = container.Cleanup(context.Background()) }) + // Empty, not migrated: advisory locks and pg_locks need no schema, and both + // are scoped to the current database, so the shared server is fine. + container := testhelpers.RequireEmptyDatabase(ctx, t) dsn := container.Config.DSN(container.Config.Password) holder, err := pgx.Connect(ctx, dsn) diff --git a/internal/database/advisory_lock_integration_test.go b/internal/database/advisory_lock_integration_test.go index a4aba3631..dc070edad 100644 --- a/internal/database/advisory_lock_integration_test.go +++ b/internal/database/advisory_lock_integration_test.go @@ -39,8 +39,9 @@ import ( // the lock in the first place. func TestReleaseAdvisoryLock_DetachesFromCanceledCallerContext(t *testing.T) { ctx := context.Background() - container := testhelpers.RequirePostgresContainer(ctx, t) - t.Cleanup(func() { _ = container.Cleanup(context.Background()) }) + // Empty, not migrated: advisory lock keys are per database, so another + // database on the shared server cannot hold this one's lock. + container := testhelpers.RequireEmptyDatabase(ctx, t) const lockID int64 = 424242424242 diff --git a/internal/database/main_integration_test.go b/internal/database/main_integration_test.go new file mode 100644 index 000000000..9082cc456 --- /dev/null +++ b/internal/database/main_integration_test.go @@ -0,0 +1,26 @@ +//go:build integration +// +build integration + +package database_test + +import ( + "log" + "os" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" +) + +// TestMain stops the PostgreSQL container that every integration test in this +// package shares (issue #982). Each test still gets a fresh database of its own +// from testhelpers.RequireMigratedDatabase or RequireEmptyDatabase; only the +// container and the migrated template it is cloned from outlive a single test. +func TestMain(m *testing.M) { + code := m.Run() + if err := testhelpers.TerminateSharedPostgres(); err != nil { + // Ryuk reaps the container anyway, so a teardown hiccup does not + // overrule the tests' own verdict. + log.Printf("terminate shared postgres container: %v", err) + } + os.Exit(code) +} diff --git a/internal/database/pool_floor_integration_test.go b/internal/database/pool_floor_integration_test.go index 9b5b57e9f..5432c6576 100644 --- a/internal/database/pool_floor_integration_test.go +++ b/internal/database/pool_floor_integration_test.go @@ -65,8 +65,9 @@ func runLockedTasks(t *testing.T, db *database.Connection, tasks int) int { // connections and every task waits on itself until its deadline. func TestPoolSizeFloorForAdvisoryLockedTasks(t *testing.T) { ctx := context.Background() - container := testhelpers.RequirePostgresContainer(ctx, t) - t.Cleanup(func() { _ = container.Cleanup(context.Background()) }) + // Empty, not migrated. The limit under test is the client pool's size, not + // the server's max_connections, so the shared server is fine. + container := testhelpers.RequireEmptyDatabase(ctx, t) cases := []struct { name string diff --git a/internal/database/query_logging_integration_test.go b/internal/database/query_logging_integration_test.go index b6139032c..c8637a385 100644 --- a/internal/database/query_logging_integration_test.go +++ b/internal/database/query_logging_integration_test.go @@ -37,13 +37,13 @@ func (b *queryLogBuffer) take() string { func TestQueryLogging_RedactsBoundSecretsOnFailure(t *testing.T) { ctx := context.Background() - container := testhelpers.RequirePostgresContainer(ctx, t) + // Empty, not migrated: the test creates the one table it needs. + container := testhelpers.RequireEmptyDatabase(ctx, t) var output queryLogBuffer previousOutput := logging.SetOutput(&output) previousLevel := logging.GetLevel() logging.SetLevelValue(logging.LevelDebug) t.Cleanup(func() { - assert.NoError(t, container.Cleanup(context.Background())) logging.SetOutput(previousOutput) logging.SetLevelValue(previousLevel) }) From eeb922fd31776e0d2230bc0f5f89853182674cae Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sun, 25 Oct 2026 01:21:26 +0000 Subject: [PATCH 05/12] test(cleanup-lambda): stop the shared Postgres server after the tests cleanupTestDB moved to testhelpers.RequireMigratedDatabase in e0384ad1 (it went in with the internal/database commit by mistake): the four tests and subtests now clone one migrated template instead of each starting and migrating a container (issue #982). This adds the TestMain that stops the shared server after the last test. It goes in main_integration_test.go rather than a file of its own because .gitignore's "cleanup-lambda" entry, meant for the built binary, also matches cmd/cleanup-lambda/, so a new test file there would stay untracked. Co-Authored-By: Claude Opus 5.5 --- cmd/cleanup-lambda/main_integration_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/cmd/cleanup-lambda/main_integration_test.go b/cmd/cleanup-lambda/main_integration_test.go index 282665ca2..4bb7d3fb4 100644 --- a/cmd/cleanup-lambda/main_integration_test.go +++ b/cmd/cleanup-lambda/main_integration_test.go @@ -8,6 +8,8 @@ package main import ( "context" + "log" + "os" "path/filepath" "regexp" "runtime" @@ -32,6 +34,22 @@ var ( cleanupKept = []string{"pending", "notified", "approved", "running", "scheduled", "paused", "failed", "canceled", config.LegacyStatusCanceled} ) +// TestMain stops the PostgreSQL container that every test in this package shares +// (issue #982). Each test still gets a fresh database of its own from +// testhelpers.RequireMigratedDatabase; only the container and the migrated +// template it is cloned from outlive a single test. It lives here rather than in +// a file of its own because .gitignore's "cleanup-lambda" (the built binary) +// also matches this directory, so a new file would silently stay untracked. +func TestMain(m *testing.M) { + code := m.Run() + if err := testhelpers.TerminateSharedPostgres(); err != nil { + // Ryuk reaps the container anyway, so a teardown hiccup does not + // overrule the tests' own verdict. + log.Printf("terminate shared postgres container: %v", err) + } + os.Exit(code) +} + func cleanupTestDB(ctx context.Context, t *testing.T) *database.Connection { t.Helper() _, file, _, _ := runtime.Caller(0) From 451ca03322da10dbe4a499c885de63df555afcb4 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sun, 25 Oct 2026 01:27:58 +0000 Subject: [PATCH 06/12] test(analytics): share one Postgres server across integration tests Each of the 19 analytics integration tests started its own Postgres container and applied the full migration chain, which was most of the package's run time. They now take a fresh database of their own from testhelpers.RequireMigratedDatabase (a TEMPLATE clone on one container per test binary), or RequireEmptyDatabase for the two Close tests, which never migrated. A TestMain stops the shared container after the last test. None of these tests touches server-wide state, so none keeps a dedicated container. Assertions are unchanged. Co-Authored-By: Claude Opus 5.5 --- internal/analytics/main_integration_test.go | 26 ++++++ ...stgres_analytics_account_filter_db_test.go | 6 +- .../analytics/postgres_analytics_db_test.go | 88 +++++------------- .../postgres_analytics_integration_test.go | 91 ++++--------------- 4 files changed, 70 insertions(+), 141 deletions(-) create mode 100644 internal/analytics/main_integration_test.go diff --git a/internal/analytics/main_integration_test.go b/internal/analytics/main_integration_test.go new file mode 100644 index 000000000..70317a8be --- /dev/null +++ b/internal/analytics/main_integration_test.go @@ -0,0 +1,26 @@ +//go:build integration +// +build integration + +package analytics_test + +import ( + "log" + "os" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" +) + +// TestMain stops the PostgreSQL container that every integration test in this +// package shares (issue #982). Each test still gets a fresh database of its own +// from testhelpers.RequireMigratedDatabase or RequireEmptyDatabase; only the +// container and the migrated template it is cloned from outlive a single test. +func TestMain(m *testing.M) { + code := m.Run() + if err := testhelpers.TerminateSharedPostgres(); err != nil { + // Ryuk reaps the container anyway, so a teardown hiccup does not + // overrule the tests' own verdict. + log.Printf("terminate shared postgres container: %v", err) + } + os.Exit(code) +} diff --git a/internal/analytics/postgres_analytics_account_filter_db_test.go b/internal/analytics/postgres_analytics_account_filter_db_test.go index f088609fd..df95ed2ba 100644 --- a/internal/analytics/postgres_analytics_account_filter_db_test.go +++ b/internal/analytics/postgres_analytics_account_filter_db_test.go @@ -10,7 +10,6 @@ import ( "time" "github.com/LeanerCloud/cloud-commitments-platform/internal/analytics" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -66,10 +65,7 @@ func TestPostgresAnalyticsStore_AccountFilterSemantics_DB(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() - container := testhelpers.RequirePostgresContainer(ctx, t) - defer container.Cleanup(ctx) - - require.NoError(t, migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "")) + container := testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()) fx := seedAccountFilterFixture(ctx, t, container) diff --git a/internal/analytics/postgres_analytics_db_test.go b/internal/analytics/postgres_analytics_db_test.go index 549f0ff8f..4627bc192 100644 --- a/internal/analytics/postgres_analytics_db_test.go +++ b/internal/analytics/postgres_analytics_db_test.go @@ -12,7 +12,6 @@ import ( "time" "github.com/LeanerCloud/cloud-commitments-platform/internal/analytics" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -25,9 +24,9 @@ import ( // skipIfDBTestsOptedOut honors the SKIP_DB_TESTS opt-out. It was called // skipIfNoDocker, which is what a reader of #1597 would flag: the name promised // a Docker probe, so a suite that silently reported nothing looked accounted -// for. It never probed anything. Docker is probed by RequirePostgresContainer, -// which each of these tests calls, and which fails rather than skips once the -// daemon answers a health check. +// for. It never probed anything. Docker is probed by RequireMigratedDatabase +// (or RequireEmptyDatabase), which each of these tests calls, and which fails +// rather than skips once the daemon answers a health check. // // The remaining exposure is the opt-out itself: setting SKIP_DB_TESTS zeroes // this suite without failing. That is deliberate and it is the caller's @@ -57,13 +56,8 @@ func TestPostgresAnalyticsStore_SaveSnapshot_DB(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() - // Setup test container - container := testhelpers.RequirePostgresContainer(ctx, t) - defer container.Cleanup(ctx) - - // Run migrations - err := migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "") - require.NoError(t, err) + // A fresh migrated database of its own, on the package's shared server + container := testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()) // Create store store := analytics.NewPostgresAnalyticsStore(container.DB) @@ -138,13 +132,8 @@ func TestPostgresAnalyticsStore_QuerySavings_DB(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() - // Setup test container - container := testhelpers.RequirePostgresContainer(ctx, t) - defer container.Cleanup(ctx) - - // Run migrations - err := migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "") - require.NoError(t, err) + // A fresh migrated database of its own, on the package's shared server + container := testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()) // Create store store := analytics.NewPostgresAnalyticsStore(container.DB) @@ -269,13 +258,8 @@ func TestPostgresAnalyticsStore_QueryByProvider_DB(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() - // Setup test container - container := testhelpers.RequirePostgresContainer(ctx, t) - defer container.Cleanup(ctx) - - // Run migrations - err := migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "") - require.NoError(t, err) + // A fresh migrated database of its own, on the package's shared server + container := testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()) // Create store store := analytics.NewPostgresAnalyticsStore(container.DB) @@ -358,13 +342,8 @@ func TestPostgresAnalyticsStore_QueryByService_DB(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() - // Setup test container - container := testhelpers.RequirePostgresContainer(ctx, t) - defer container.Cleanup(ctx) - - // Run migrations - err := migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "") - require.NoError(t, err) + // A fresh migrated database of its own, on the package's shared server + container := testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()) // Create store store := analytics.NewPostgresAnalyticsStore(container.DB) @@ -420,13 +399,8 @@ func TestPostgresAnalyticsStore_BulkInsertSnapshots_DB(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() - // Setup test container - container := testhelpers.RequirePostgresContainer(ctx, t) - defer container.Cleanup(ctx) - - // Run migrations - err := migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "") - require.NoError(t, err) + // A fresh migrated database of its own, on the package's shared server + container := testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()) // Create store store := analytics.NewPostgresAnalyticsStore(container.DB) @@ -480,13 +454,8 @@ func TestPostgresAnalyticsStore_PartitionManagement_DB(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() - // Setup test container - container := testhelpers.RequirePostgresContainer(ctx, t) - defer container.Cleanup(ctx) - - // Run migrations - err := migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "") - require.NoError(t, err) + // A fresh migrated database of its own, on the package's shared server + container := testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()) // Create store store := analytics.NewPostgresAnalyticsStore(container.DB) @@ -523,13 +492,8 @@ func TestPostgresAnalyticsStore_QueryMonthlyTotals_DB(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() - // Setup test container - container := testhelpers.RequirePostgresContainer(ctx, t) - defer container.Cleanup(ctx) - - // Run migrations - err := migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "") - require.NoError(t, err) + // A fresh migrated database of its own, on the package's shared server + container := testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()) // Create store store := analytics.NewPostgresAnalyticsStore(container.DB) @@ -552,12 +516,12 @@ func TestPostgresAnalyticsStore_QueryMonthlyTotals_DB(t *testing.T) { } for _, snapshot := range testSnapshots { - err = store.SaveSnapshot(ctx, snapshot) + err := store.SaveSnapshot(ctx, snapshot) require.NoError(t, err) } // Refresh materialized views so data is available - err = store.RefreshMaterializedViews(ctx) + err := store.RefreshMaterializedViews(ctx) require.NoError(t, err) t.Run("query monthly totals from materialized view", func(t *testing.T) { @@ -589,13 +553,8 @@ func TestPostgresAnalyticsStore_RefreshMaterializedViews_DB(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() - // Setup test container - container := testhelpers.RequirePostgresContainer(ctx, t) - defer container.Cleanup(ctx) - - // Run migrations - err := migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "") - require.NoError(t, err) + // A fresh migrated database of its own, on the package's shared server + container := testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()) // Create store store := analytics.NewPostgresAnalyticsStore(container.DB) @@ -612,9 +571,8 @@ func TestPostgresAnalyticsStore_Close_DB(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() - // Setup test container - container := testhelpers.RequirePostgresContainer(ctx, t) - defer container.Cleanup(ctx) + // Empty database: Close needs a connection, not a schema + container := testhelpers.RequireEmptyDatabase(ctx, t) // Create store store := analytics.NewPostgresAnalyticsStore(container.DB) diff --git a/internal/analytics/postgres_analytics_integration_test.go b/internal/analytics/postgres_analytics_integration_test.go index bd207e041..070425f30 100644 --- a/internal/analytics/postgres_analytics_integration_test.go +++ b/internal/analytics/postgres_analytics_integration_test.go @@ -9,7 +9,6 @@ import ( "time" "github.com/LeanerCloud/cloud-commitments-platform/internal/analytics" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -18,14 +17,8 @@ import ( func TestPostgresAnalyticsStore_SaveSnapshot(t *testing.T) { ctx := context.Background() - // Setup test container - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) - - // Run migrations - err = migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "") - require.NoError(t, err) + // A fresh migrated database of its own, on the package's shared server + container := testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()) // Create store store := analytics.NewPostgresAnalyticsStore(container.DB) @@ -97,14 +90,8 @@ func TestPostgresAnalyticsStore_SaveSnapshot(t *testing.T) { func TestPostgresAnalyticsStore_QuerySavings(t *testing.T) { ctx := context.Background() - // Setup test container - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) - - // Run migrations - err = migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "") - require.NoError(t, err) + // A fresh migrated database of its own, on the package's shared server + container := testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()) // Create store store := analytics.NewPostgresAnalyticsStore(container.DB) @@ -226,14 +213,8 @@ func TestPostgresAnalyticsStore_QuerySavings(t *testing.T) { func TestPostgresAnalyticsStore_QueryMonthlyTotals(t *testing.T) { ctx := context.Background() - // Setup test container - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) - - // Run migrations - err = migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "") - require.NoError(t, err) + // A fresh migrated database of its own, on the package's shared server + container := testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()) // Create store store := analytics.NewPostgresAnalyticsStore(container.DB) @@ -256,12 +237,12 @@ func TestPostgresAnalyticsStore_QueryMonthlyTotals(t *testing.T) { } for _, snapshot := range testSnapshots { - err = store.SaveSnapshot(ctx, snapshot) + err := store.SaveSnapshot(ctx, snapshot) require.NoError(t, err) } // Refresh materialized views so data is available - err = store.RefreshMaterializedViews(ctx) + err := store.RefreshMaterializedViews(ctx) require.NoError(t, err) t.Run("query monthly totals from materialized view", func(t *testing.T) { @@ -290,14 +271,8 @@ func TestPostgresAnalyticsStore_QueryMonthlyTotals(t *testing.T) { func TestPostgresAnalyticsStore_QueryByProvider(t *testing.T) { ctx := context.Background() - // Setup test container - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) - - // Run migrations - err = migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "") - require.NoError(t, err) + // A fresh migrated database of its own, on the package's shared server + container := testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()) // Create store store := analytics.NewPostgresAnalyticsStore(container.DB) @@ -377,14 +352,8 @@ func TestPostgresAnalyticsStore_QueryByProvider(t *testing.T) { func TestPostgresAnalyticsStore_QueryByService(t *testing.T) { ctx := context.Background() - // Setup test container - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) - - // Run migrations - err = migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "") - require.NoError(t, err) + // A fresh migrated database of its own, on the package's shared server + container := testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()) // Create store store := analytics.NewPostgresAnalyticsStore(container.DB) @@ -437,14 +406,8 @@ func TestPostgresAnalyticsStore_QueryByService(t *testing.T) { func TestPostgresAnalyticsStore_BulkInsertSnapshots(t *testing.T) { ctx := context.Background() - // Setup test container - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) - - // Run migrations - err = migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "") - require.NoError(t, err) + // A fresh migrated database of its own, on the package's shared server + container := testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()) // Create store store := analytics.NewPostgresAnalyticsStore(container.DB) @@ -495,14 +458,8 @@ func TestPostgresAnalyticsStore_BulkInsertSnapshots(t *testing.T) { func TestPostgresAnalyticsStore_PartitionManagement(t *testing.T) { ctx := context.Background() - // Setup test container - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) - - // Run migrations - err = migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "") - require.NoError(t, err) + // A fresh migrated database of its own, on the package's shared server + container := testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()) // Create store store := analytics.NewPostgresAnalyticsStore(container.DB) @@ -536,14 +493,8 @@ func TestPostgresAnalyticsStore_PartitionManagement(t *testing.T) { func TestPostgresAnalyticsStore_RefreshMaterializedViews(t *testing.T) { ctx := context.Background() - // Setup test container - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) - - // Run migrations - err = migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "") - require.NoError(t, err) + // A fresh migrated database of its own, on the package's shared server + container := testhelpers.RequireMigratedDatabase(ctx, t, getMigrationsPath()) // Create store store := analytics.NewPostgresAnalyticsStore(container.DB) @@ -557,10 +508,8 @@ func TestPostgresAnalyticsStore_RefreshMaterializedViews(t *testing.T) { func TestPostgresAnalyticsStore_Close(t *testing.T) { ctx := context.Background() - // Setup test container - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + // Empty database: Close needs a connection, not a schema + container := testhelpers.RequireEmptyDatabase(ctx, t) // Create store store := analytics.NewPostgresAnalyticsStore(container.DB) From a89ea9fc6f91a94404c423b28b8a406ba6bd3d82 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sun, 25 Oct 2026 01:28:01 +0000 Subject: [PATCH 07/12] test(server): share one Postgres server across integration tests The 13 server integration tests that needed Postgres each started a container and migrated it. They now get a fresh database of their own from testhelpers.RequireMigratedDatabase on one container per test binary, and a TestMain stops it after the last test. TestServerIntegration used internal/testutil's separate container helper only to export connection settings; it now takes an empty database from RequireEmptyDatabase, exports the same DB_* variables and pings it instead of only logging a connection string. waitersOnLadderLock counted waiting advisory locks across the whole server. It is now scoped to the test's database, since other tests' databases share the server. That makes the check stricter, not looser. No test here touches server-wide state, so none keeps a dedicated container. Co-Authored-By: Claude Opus 5.5 --- .../server/app_islambda_integration_test.go | 6 +---- .../app_rate_limiter_integration_test.go | 8 ++---- ...ndler_ladder_immediate_integration_test.go | 5 +--- .../handler_ladder_lock_integration_test.go | 5 +++- ...i_exchange_reservation_integration_test.go | 13 +++------ internal/server/integration_test.go | 27 +++++++++++++------ internal/server/main_integration_test.go | 26 ++++++++++++++++++ ...ion_constraints_config_integration_test.go | 6 +---- 8 files changed, 58 insertions(+), 38 deletions(-) create mode 100644 internal/server/main_integration_test.go diff --git a/internal/server/app_islambda_integration_test.go b/internal/server/app_islambda_integration_test.go index f12c80a05..b1283750c 100644 --- a/internal/server/app_islambda_integration_test.go +++ b/internal/server/app_islambda_integration_test.go @@ -8,7 +8,6 @@ import ( "testing" "time" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/stretchr/testify/require" ) @@ -37,10 +36,7 @@ func TestEnsureDB_IsLambdaSingleSourceOfTruth(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() - pg, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, pg.Cleanup(context.Background())) }) - require.NoError(t, migrations.RunMigrations(ctx, pg.DB.Pool(), "../database/postgres/migrations", "", "")) + pg := testhelpers.RequireMigratedDatabase(ctx, t, "../database/postgres/migrations") for _, cfgIsLambda := range []bool{true, false} { name := "config not Lambda, environment Lambda" diff --git a/internal/server/app_rate_limiter_integration_test.go b/internal/server/app_rate_limiter_integration_test.go index a08eb69a6..2e344cfb6 100644 --- a/internal/server/app_rate_limiter_integration_test.go +++ b/internal/server/app_rate_limiter_integration_test.go @@ -12,7 +12,6 @@ import ( "testing" "time" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -38,10 +37,7 @@ func TestApplicationRateLimiter(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() - pg, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, pg.Cleanup(context.Background())) }) - require.NoError(t, migrations.RunMigrations(ctx, pg.DB.Pool(), "../database/postgres/migrations", "", "")) + pg := testhelpers.RequireMigratedDatabase(ctx, t, "../database/postgres/migrations") for mode, isLambda := range []bool{false, true} { t.Run(fmt.Sprintf("lambda=%t", isLambda), func(t *testing.T) { @@ -101,7 +97,7 @@ func TestApplicationRateLimiter(t *testing.T) { checkCount := func(sourceIP, endpoint string, want int) { t.Helper() var count int - err = pg.DB.Pool().QueryRow(ctx, "SELECT count FROM rate_limits WHERE id = $1", + err := pg.DB.Pool().QueryRow(ctx, "SELECT count FROM rate_limits WHERE id = $1", "IP#"+sourceIP+"#ENDPOINT#"+endpoint).Scan(&count) assert.NoError(t, err) assert.Equal(t, want, count) diff --git a/internal/server/handler_ladder_immediate_integration_test.go b/internal/server/handler_ladder_immediate_integration_test.go index 3580ae5c0..951b60e39 100644 --- a/internal/server/handler_ladder_immediate_integration_test.go +++ b/internal/server/handler_ladder_immediate_integration_test.go @@ -36,10 +36,7 @@ func ladderB1Conn(ctx context.Context, t *testing.T) *database.Connection { t.Helper() dsn := os.Getenv("LADDER_B1_TEST_DATABASE_URL") if dsn == "" { - c := testhelpers.RequirePostgresContainer(ctx, t) - t.Cleanup(func() { _ = c.Cleanup(context.Background()) }) - require.NoError(t, migrations.RunMigrations(ctx, c.DB.Pool(), "../database/postgres/migrations", "", "")) - return c.DB + return testhelpers.RequireMigratedDatabase(ctx, t, "../database/postgres/migrations").DB } u, err := url.Parse(dsn) require.NoError(t, err) diff --git a/internal/server/handler_ladder_lock_integration_test.go b/internal/server/handler_ladder_lock_integration_test.go index 3c20245fe..0e00aa881 100644 --- a/internal/server/handler_ladder_lock_integration_test.go +++ b/internal/server/handler_ladder_lock_integration_test.go @@ -47,13 +47,16 @@ func (h *hookedLadderStore) PlanAndSaveLadderRun(ctx context.Context, configID s } // waitersOnLadderLock counts sessions blocked on the config's ladder lock key -// (cudly:ladder-config:, split across pg_locks classid/objid). +// (cudly:ladder-config:, split across pg_locks classid/objid) in this +// test's database. pg_locks covers the whole server, which other tests' +// databases share. func waitersOnLadderLock(ctx context.Context, t *testing.T, e *ladderB1Env) int { t.Helper() var n int require.NoError(t, e.conn.QueryRow(ctx, ` SELECT count(*) FROM pg_locks l, (SELECT hashtextextended('cudly:ladder-config:' || $1::text, 0) AS k) h WHERE l.locktype = 'advisory' AND l.objsubid = 1 AND NOT l.granted + AND l.database = (SELECT oid FROM pg_database WHERE datname = current_database()) AND l.classid = ((h.k >> 32) & 4294967295)::oid AND l.objid = (h.k & 4294967295)::oid`, e.cfg.ID).Scan(&n)) return n diff --git a/internal/server/handler_ri_exchange_reservation_integration_test.go b/internal/server/handler_ri_exchange_reservation_integration_test.go index 5bb5a7609..69a50d244 100644 --- a/internal/server/handler_ri_exchange_reservation_integration_test.go +++ b/internal/server/handler_ri_exchange_reservation_integration_test.go @@ -13,23 +13,18 @@ import ( "github.com/LeanerCloud/cloud-commitments-go/providers/aws/recommendations" ec2svc "github.com/LeanerCloud/cloud-commitments-go/providers/aws/services/ec2" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/LeanerCloud/cloud-commitments-platform/internal/email" "github.com/stretchr/testify/require" ) func TestRIExchangeAutoWorkers_ConcreteStoreAndFixtureClient(t *testing.T) { - // A dedicated container, not an opt-in DSN, so this runs in CI (#724). It - // skips only when Docker is unavailable; a container or migration failure - // fails the test (#1597). + // A fresh migrated database on the package's shared container, not an + // opt-in DSN, so this runs in CI (#724). It skips only when Docker is + // unavailable; a container or migration failure fails the test (#1597). setupCtx, setupCancel := context.WithTimeout(context.Background(), 2*time.Minute) defer setupCancel() - container := testhelpers.RequirePostgresContainer(setupCtx, t) - t.Cleanup(func() { _ = container.Cleanup(context.Background()) }) - conn := container.DB - require.NoError(t, migrations.RunMigrations(setupCtx, conn.Pool(), "../database/postgres/migrations", "", ""), - "migrations failed to apply to a fresh database") + conn := testhelpers.RequireMigratedDatabase(setupCtx, t, "../database/postgres/migrations").DB ctx, cancel := context.WithTimeout(context.Background(), time.Minute) defer cancel() var testRunID string diff --git a/internal/server/integration_test.go b/internal/server/integration_test.go index 722680b1d..80f4fe7b7 100644 --- a/internal/server/integration_test.go +++ b/internal/server/integration_test.go @@ -7,9 +7,11 @@ import ( "context" "net/http" "net/http/httptest" + "strconv" "testing" "time" + "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/LeanerCloud/cloud-commitments-platform/internal/scheduler" "github.com/LeanerCloud/cloud-commitments-platform/internal/testutil" ) @@ -23,14 +25,20 @@ func TestServerIntegration(t *testing.T) { ctx := testutil.TestContext(t) - // Set up PostgreSQL container - pgContainer, err := testutil.SetupPostgresContainer(ctx, t) - if err != nil { - t.Fatalf("Failed to start postgres container: %v", err) - } + // An empty database of its own on the package's shared PostgreSQL server: + // nothing here reads the schema yet, only the connection settings. + pg := testhelpers.RequireEmptyDatabase(ctx, t) + dbCfg := pg.Config // Set environment variables for database connection - for key, value := range pgContainer.Config() { + for key, value := range map[string]string{ + "DB_HOST": dbCfg.Host, + "DB_PORT": strconv.Itoa(dbCfg.Port), + "DB_NAME": dbCfg.Database, + "DB_USER": dbCfg.User, + "DB_PASSWORD": dbCfg.Password, + "DB_SSL_MODE": dbCfg.SSLMode, + } { testutil.SetEnv(t, key, value) } @@ -41,8 +49,11 @@ func TestServerIntegration(t *testing.T) { // } // defer app.Close() - // For now, just verify container is running - t.Logf("PostgreSQL container running at: %s", pgContainer.ConnectionString()) + // For now, just verify the database is reachable + if err := pg.DB.Pool().Ping(ctx); err != nil { + t.Fatalf("Failed to reach postgres database: %v", err) + } + t.Logf("PostgreSQL database %s running at %s:%d", dbCfg.Database, dbCfg.Host, dbCfg.Port) } // TestHealthCheckIntegration tests the health check endpoint. diff --git a/internal/server/main_integration_test.go b/internal/server/main_integration_test.go new file mode 100644 index 000000000..c178b249a --- /dev/null +++ b/internal/server/main_integration_test.go @@ -0,0 +1,26 @@ +//go:build integration +// +build integration + +package server + +import ( + "log" + "os" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" +) + +// TestMain stops the PostgreSQL container that every integration test in this +// package shares (issue #982). Each test still gets a fresh database of its own +// from testhelpers.RequireMigratedDatabase or RequireEmptyDatabase; only the +// container and the migrated template it is cloned from outlive a single test. +func TestMain(m *testing.M) { + code := m.Run() + if err := testhelpers.TerminateSharedPostgres(); err != nil { + // Ryuk reaps the container anyway, so a teardown hiccup does not + // overrule the tests' own verdict. + log.Printf("terminate shared postgres container: %v", err) + } + os.Exit(code) +} diff --git a/internal/server/permission_constraints_config_integration_test.go b/internal/server/permission_constraints_config_integration_test.go index 7be1d797a..d9f7bb7b2 100644 --- a/internal/server/permission_constraints_config_integration_test.go +++ b/internal/server/permission_constraints_config_integration_test.go @@ -19,7 +19,6 @@ import ( "github.com/LeanerCloud/cloud-commitments-platform/internal/api" "github.com/LeanerCloud/cloud-commitments-platform/internal/auth" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/google/uuid" "github.com/stretchr/testify/assert" @@ -28,10 +27,7 @@ import ( func TestConfigPermissionConstraintsHTTP(t *testing.T) { ctx := context.Background() - pg, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, pg.Cleanup(ctx)) }) - require.NoError(t, migrations.RunMigrations(ctx, pg.DB.Pool(), "../database/postgres/migrations", "", "")) + pg := testhelpers.RequireMigratedDatabase(ctx, t, "../database/postgres/migrations") store, authStore := config.NewPostgresStore(pg.DB), auth.NewPostgresStore(pg.DB) csrfKey := []byte(strings.Repeat("c", 32)) authSvc := auth.NewService(auth.ServiceConfig{Store: authStore, CSRFKey: csrfKey}) From f5ee243e967e0133b481fce7d3714158b1e50531 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sun, 25 Oct 2026 01:31:51 +0000 Subject: [PATCH 08/12] test(scheduler): share one Postgres server across integration tests Every scheduler integration test started its own PostgreSQL container and applied the full migration chain, 29 times per run (the completeness table test alone does it once per subtest). Use testhelpers.RequireMigratedDatabase instead: one container per test binary, migrations applied once into a template, and each test gets a fresh CREATE DATABASE ... TEMPLATE copy that is dropped when it ends. A TestMain stops the shared container after the last test (#982). None of these tests touches server-wide state (roles, server settings, other sessions, connection limits), so none needs a dedicated server. Assertions are unchanged. Co-Authored-By: Claude Opus 5.5 --- internal/scheduler/main_integration_test.go | 26 +++++++++++++++++++ ...mendation_completeness_integration_test.go | 9 +------ ...ndation_source_account_integration_test.go | 9 +------ ...heduler_global_filters_integration_test.go | 19 +++----------- .../suppression_variant_integration_test.go | 9 ++----- 5 files changed, 34 insertions(+), 38 deletions(-) create mode 100644 internal/scheduler/main_integration_test.go diff --git a/internal/scheduler/main_integration_test.go b/internal/scheduler/main_integration_test.go new file mode 100644 index 000000000..b3831af95 --- /dev/null +++ b/internal/scheduler/main_integration_test.go @@ -0,0 +1,26 @@ +//go:build integration +// +build integration + +package scheduler + +import ( + "log" + "os" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" +) + +// TestMain stops the PostgreSQL container that every integration test in this +// package shares (issue #982). Each test still gets a freshly migrated database +// of its own from testhelpers.RequireMigratedDatabase; only the container and +// the migrated template it is cloned from outlive a single test. +func TestMain(m *testing.M) { + code := m.Run() + if err := testhelpers.TerminateSharedPostgres(); err != nil { + // Ryuk reaps the container anyway, so a teardown hiccup does not + // overrule the tests' own verdict. + log.Printf("terminate shared postgres container: %v", err) + } + os.Exit(code) +} diff --git a/internal/scheduler/recommendation_completeness_integration_test.go b/internal/scheduler/recommendation_completeness_integration_test.go index 785d73401..414bf9f02 100644 --- a/internal/scheduler/recommendation_completeness_integration_test.go +++ b/internal/scheduler/recommendation_completeness_integration_test.go @@ -9,7 +9,6 @@ import ( "fmt" "io" "net/http" - "path/filepath" "strings" "sync" "sync/atomic" @@ -20,7 +19,6 @@ import ( "github.com/LeanerCloud/cloud-commitments-go/pkg/provider" awsprovider "github.com/LeanerCloud/cloud-commitments-go/providers/aws" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/aws/aws-sdk-go-v2/aws" "github.com/google/uuid" @@ -210,12 +208,7 @@ func TestAWSRecommendationCompletenessPersistence(t *testing.T) { var diagnostics bytes.Buffer previous := logging.SetOutput(&diagnostics) t.Cleanup(func() { logging.SetOutput(previous) }) - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { container.Cleanup(ctx) }) - migrationPath, err := filepath.Abs("../database/postgres/migrations") - require.NoError(t, err) - require.NoError(t, migrations.RunMigrations(ctx, container.DB.Pool(), migrationPath, "", "")) + container := testhelpers.RequireMigratedDatabase(ctx, t, "../database/postgres/migrations") store := config.NewPostgresStore(container.DB) factory := new(MockProviderFactory) fixture := &completenessHTTP{resourceType: "db.t3.medium", details: tc.details, fallback: tc.fallback, failScope: tc.failScope, spMode: tc.spMode, spFallback: tc.spFallback} diff --git a/internal/scheduler/recommendation_source_account_integration_test.go b/internal/scheduler/recommendation_source_account_integration_test.go index d504ebd65..f3364597f 100644 --- a/internal/scheduler/recommendation_source_account_integration_test.go +++ b/internal/scheduler/recommendation_source_account_integration_test.go @@ -4,12 +4,10 @@ package scheduler import ( "context" - "path/filepath" "testing" "time" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/google/uuid" "github.com/stretchr/testify/require" @@ -17,12 +15,7 @@ import ( func TestAWSRecommendationSourceAccountPersistence(t *testing.T) { ctx := context.Background() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { container.Cleanup(ctx) }) - migrationPath, err := filepath.Abs("../database/postgres/migrations") - require.NoError(t, err) - require.NoError(t, migrations.RunMigrations(ctx, container.DB.Pool(), migrationPath, "", "")) + container := testhelpers.RequireMigratedDatabase(ctx, t, "../database/postgres/migrations") store := config.NewPostgresStore(container.DB) collector := uuid.NewString() require.NoError(t, store.CreateCloudAccount(ctx, &config.CloudAccount{ diff --git a/internal/scheduler/scheduler_global_filters_integration_test.go b/internal/scheduler/scheduler_global_filters_integration_test.go index c12703268..f04f69af2 100644 --- a/internal/scheduler/scheduler_global_filters_integration_test.go +++ b/internal/scheduler/scheduler_global_filters_integration_test.go @@ -3,7 +3,6 @@ package scheduler import ( - "context" "encoding/json" "fmt" "testing" @@ -11,7 +10,6 @@ import ( "github.com/LeanerCloud/cloud-commitments-go/pkg/common" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/google/uuid" "github.com/stretchr/testify/assert" @@ -20,10 +18,7 @@ import ( func TestGlobalFiltersPersistedAmbientAndRegistered(t *testing.T) { ctx := t.Context() - pg, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, pg.Cleanup(context.Background())) }) - require.NoError(t, migrations.RunMigrations(ctx, pg.DB.Pool(), "../database/postgres/migrations", "", "")) + pg := testhelpers.RequireMigratedDatabase(ctx, t, "../database/postgres/migrations") store := config.NewPostgresStore(pg.DB) account := uuid.NewString() require.NoError(t, store.CreateCloudAccount(ctx, &config.CloudAccount{ID: account, Name: "registered", Provider: "aws", ExternalID: "111111111111", Enabled: true})) @@ -38,7 +33,7 @@ func TestGlobalFiltersPersistedAmbientAndRegistered(t *testing.T) { } require.NoError(t, store.UpsertRecommendations(ctx, time.Now(), records, nil)) s := &Scheduler{config: store, isLambda: true} - _, err = store.GetServiceConfig(ctx, "aws", "rds") + _, err := store.GetServiceConfig(ctx, "aws", "rds") require.ErrorIs(t, err, config.ErrNotFound) all, err := s.ListRecommendations(ctx, config.RecommendationFilter{}) require.NoError(t, err) @@ -109,10 +104,7 @@ func TestGlobalFiltersPersistedAmbientAndRegistered(t *testing.T) { // the whole 500-row batch lands, siblings included. func TestPersistCollectionMultiAZInOneBatch(t *testing.T) { ctx := t.Context() - pg, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, pg.Cleanup(context.Background())) }) - require.NoError(t, migrations.RunMigrations(ctx, pg.DB.Pool(), "../database/postgres/migrations", "", "")) + pg := testhelpers.RequireMigratedDatabase(ctx, t, "../database/postgres/migrations") store := config.NewPostgresStore(pg.DB) zonal := func(az string) config.RecommendationRecord { @@ -156,10 +148,7 @@ func TestPersistCollectionMultiAZInOneBatch(t *testing.T) { // generated platform and tenancy columns is the one production writes. func TestPersistCollectionPlatformTenancyInOneBatch(t *testing.T) { ctx := t.Context() - pg, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, pg.Cleanup(context.Background())) }) - require.NoError(t, migrations.RunMigrations(ctx, pg.DB.Pool(), "../database/postgres/migrations", "", "")) + pg := testhelpers.RequireMigratedDatabase(ctx, t, "../database/postgres/migrations") store := config.NewPostgresStore(pg.DB) s := &Scheduler{config: store} diff --git a/internal/scheduler/suppression_variant_integration_test.go b/internal/scheduler/suppression_variant_integration_test.go index 6c6001e74..37728557e 100644 --- a/internal/scheduler/suppression_variant_integration_test.go +++ b/internal/scheduler/suppression_variant_integration_test.go @@ -3,12 +3,10 @@ package scheduler import ( - "context" "testing" "time" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/google/uuid" "github.com/jackc/pgx/v5" @@ -26,10 +24,7 @@ import ( // variant of its 6-tuple. func TestSuppressionsScopedToPurchasedVariant_RealPostgres(t *testing.T) { ctx := t.Context() - pg, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, pg.Cleanup(context.Background())) }) - require.NoError(t, migrations.RunMigrations(ctx, pg.DB.Pool(), "../database/postgres/migrations", "", "")) + pg := testhelpers.RequireMigratedDatabase(ctx, t, "../database/postgres/migrations") store := config.NewPostgresStore(pg.DB) rec := func(id, resourceType, az, platform, tenancy string) config.RecommendationRecord { @@ -58,7 +53,7 @@ func TestSuppressionsScopedToPurchasedVariant_RealPostgres(t *testing.T) { require.NoError(t, store.WithTx(ctx, func(tx pgx.Tx) error { return store.CreateSuppressionTx(ctx, tx, &sups[0]) })) // The insert old code issues: 6-tuple only, variant columns left NULL. - _, err = pg.DB.Pool().Exec(ctx, ` + _, err := pg.DB.Pool().Exec(ctx, ` INSERT INTO purchase_suppressions (execution_id, account_id, provider, service, region, resource_type, engine, suppressed_count, expires_at) VALUES ($1, '', 'aws', 'ec2', 'us-east-1', 'c5.large', '', 1, NOW() + INTERVAL '1 day')`, uuid.NewString()) From b626300a881241a7e93dd24979f1b9c311e6758e Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sun, 25 Oct 2026 01:31:54 +0000 Subject: [PATCH 09/12] test(auth): share one Postgres server across integration tests setupAuthTestDB started a PostgreSQL container and applied every migration for each of the 23 DB-backed auth tests. It now takes a fresh database from testhelpers.RequireMigratedDatabase on one container shared by the test binary, cloned from a template migrated once, with no bootstrap admin seeded as before. A TestMain stops the shared container after the last test (#982). A migration that will not apply still fails the test rather than skipping it (#1597). No auth test touches server-wide state, so none needs a dedicated server. Assertions are unchanged. Co-Authored-By: Claude Opus 5.5 --- internal/auth/main_integration_test.go | 26 ++++++++++++++++++++ internal/auth/store_postgres_db_test.go | 32 ++++++++++--------------- 2 files changed, 39 insertions(+), 19 deletions(-) create mode 100644 internal/auth/main_integration_test.go diff --git a/internal/auth/main_integration_test.go b/internal/auth/main_integration_test.go new file mode 100644 index 000000000..37690c970 --- /dev/null +++ b/internal/auth/main_integration_test.go @@ -0,0 +1,26 @@ +//go:build integration +// +build integration + +package auth + +import ( + "log" + "os" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" +) + +// TestMain stops the PostgreSQL container that every integration test in this +// package shares (issue #982). Each test still gets a freshly migrated database +// of its own from testhelpers.RequireMigratedDatabase; only the container and +// the migrated template it is cloned from outlive a single test. +func TestMain(m *testing.M) { + code := m.Run() + if err := testhelpers.TerminateSharedPostgres(); err != nil { + // Ryuk reaps the container anyway, so a teardown hiccup does not + // overrule the tests' own verdict. + log.Printf("terminate shared postgres container: %v", err) + } + os.Exit(code) +} diff --git a/internal/auth/store_postgres_db_test.go b/internal/auth/store_postgres_db_test.go index 8326e5ff6..b7e5ebf24 100644 --- a/internal/auth/store_postgres_db_test.go +++ b/internal/auth/store_postgres_db_test.go @@ -5,7 +5,7 @@ package auth // store_postgres_db_test.go - DB-backed integration tests for the // security-critical PostgresStore bootstrap paths (TEST-01, issue #1153). -// These run the real SQL against a migrated PostgreSQL testcontainer, +// These run the real SQL against a migrated PostgreSQL database, // proving the 19-column bootstrap INSERT, the NOT EXISTS guard and its // agreement with AdminExists (active-membership semantics), and the // insert-once guarantee under two concurrent CreateAdminIfNone calls. @@ -19,7 +19,6 @@ import ( "time" "github.com/LeanerCloud/cloud-commitments-platform/internal/database" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -31,29 +30,24 @@ func getAuthTestMigrationsPath() string { return filepath.Join(filepath.Dir(filename), "..", "database", "postgres", "migrations") } -// setupAuthTestDB starts a PostgreSQL container and runs all migrations -// without seeding a bootstrap admin (empty adminEmail), so tests start from -// the "fresh deployment, no admin yet" state CreateAdminIfNone exists for. +// setupAuthTestDB gives the test a fresh database of its own, with every +// migration applied, on the PostgreSQL server this package's integration tests +// share (issue #982). No bootstrap admin is seeded (the template is migrated +// with an empty adminEmail), so tests start from the "fresh deployment, no +// admin yet" state CreateAdminIfNone exists for. The database is dropped when +// the test ends. +// +// A migration that will not apply fails the test rather than skipping it: it +// is a broken schema, and skipping would report the suite green on exactly the +// defect it exists to catch (issue #1597). RequireMigratedDatabase skips only +// when Docker itself is unusable. func setupAuthTestDB(t *testing.T) *database.Connection { t.Helper() ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) defer cancel() - container := testhelpers.RequirePostgresContainer(ctx, t) - t.Cleanup(func() { - container.Cleanup(context.Background()) - }) - - // Migrating is the thing under test here, not a precondition of it: a - // migration that will not apply is a broken schema, and skipping on it would - // report the whole suite green on exactly the defect it exists to catch - // (issue #1597). - require.NoError(t, - migrations.RunMigrations(ctx, container.DB.Pool(), getAuthTestMigrationsPath(), "", ""), - "migrations failed to apply to a fresh database") - - return container.DB + return testhelpers.RequireMigratedDatabase(ctx, t, getAuthTestMigrationsPath()).DB } // resetUsers wipes the users table between scenarios. TRUNCATE is used From e3be7444568691911eabb155ce7f71fcc2a046dc Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sun, 25 Oct 2026 01:37:45 +0000 Subject: [PATCH 10/12] test(purchase): share one Postgres server across integration tests Every purchase integration test started its own postgres:16-alpine container and applied all migrations to it: 116 containers per run and most of the package's ~1000s integration time. rampStepStore, the helper behind nearly all of them, now takes a fresh database from testhelpers.RequireMigratedDatabase (issue #982). The two tests that walk the migration chain themselves (Azure pricing staleness and the never-started ramp ledger migration) use RequireEmptyDatabase. The binary now starts one container and clones each test's database from a template migrated once. TestMain lives in the untagged main_nonet_test.go, so it gains an afterTests hook that main_integration_test.go fills to stop the shared server, as internal/api does. The lock-wait poll in TestCompletePlanStepBlocksOnTheRowLockAndSeesTheWinner now filters pg_stat_activity to current_database(), so a backend in another test's database cannot satisfy it. That makes the check stricter. No test needed a dedicated server: none creates roles, alters system settings, terminates other sessions or counts server-wide connections. Co-Authored-By: Claude Opus 5.5 --- .../azure_pricing_integration_test.go | 9 ++----- internal/purchase/main_integration_test.go | 25 +++++++++++++++++++ internal/purchase/main_nonet_test.go | 9 +++++++ ..._step_ledger_migration_integration_test.go | 7 +++--- .../ramp_step_progress_integration_test.go | 23 ++++++----------- 5 files changed, 47 insertions(+), 26 deletions(-) create mode 100644 internal/purchase/main_integration_test.go diff --git a/internal/purchase/azure_pricing_integration_test.go b/internal/purchase/azure_pricing_integration_test.go index 0fe2d4dcf..59fb3e30c 100644 --- a/internal/purchase/azure_pricing_integration_test.go +++ b/internal/purchase/azure_pricing_integration_test.go @@ -26,13 +26,8 @@ func (s *azurePricingPostgresStore) ListStoredRecommendations(ctx context.Contex func TestApproveAzurePricingRequiresMigrationAndRecollection(t *testing.T) { ctx := context.Background() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { - if cleanupErr := container.Cleanup(ctx); cleanupErr != nil { - t.Logf("container cleanup: %v", cleanupErr) - } - }) + // Empty, not migrated: the test walks the chain to 105, 106 and 129 itself. + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() require.NoError(t, migrations.MigrateToVersion(ctx, pool, rampMigrationsPath(), 105)) pricing := config.NewPostgresStore(container.DB) diff --git a/internal/purchase/main_integration_test.go b/internal/purchase/main_integration_test.go new file mode 100644 index 000000000..74d98039f --- /dev/null +++ b/internal/purchase/main_integration_test.go @@ -0,0 +1,25 @@ +//go:build integration + +package purchase + +import ( + "log" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" +) + +// The integration tests in this package share one PostgreSQL container (issue +// #982): each still gets a fresh database of its own from +// testhelpers.RequireMigratedDatabase or RequireEmptyDatabase, but the server +// and its migrated template outlive any one test. TestMain, in +// main_nonet_test.go because it exists in every build, stops the server after +// the last test. +func init() { + afterTests = append(afterTests, func() { + if err := testhelpers.TerminateSharedPostgres(); err != nil { + // Ryuk reaps the container anyway, so a teardown hiccup does not + // overrule the tests' own verdict. + log.Printf("terminate shared postgres container: %v", err) + } + }) +} diff --git a/internal/purchase/main_nonet_test.go b/internal/purchase/main_nonet_test.go index 57f93350f..3d8896430 100644 --- a/internal/purchase/main_nonet_test.go +++ b/internal/purchase/main_nonet_test.go @@ -8,6 +8,12 @@ import ( "testing" ) +// afterTests is teardown that TestMain runs once m.Run returns. A package has +// one TestMain and this file owns it in every build, so build-tagged files +// register their teardown here from init instead: main_integration_test.go +// stops the Postgres server the integration tests share. +var afterTests []func() + // TestMain routes every proxy-aware HTTP client in this test binary (Go's // default transport and the AWS SDK clients, which honor HTTP(S)_PROXY) into a // local listener that counts and drops connections. If any non-loopback @@ -41,6 +47,9 @@ func TestMain(m *testing.M) { _ = os.Setenv(k, v) } code := m.Run() + for _, teardown := range afterTests { + teardown() + } _ = ln.Close() if n := attempts.Load(); n > 0 { fmt.Fprintf(os.Stderr, "FAIL: purchase tests attempted %d outbound network connection(s)\n", n) diff --git a/internal/purchase/plan_step_ledger_migration_integration_test.go b/internal/purchase/plan_step_ledger_migration_integration_test.go index 925195af4..3b7e6a178 100644 --- a/internal/purchase/plan_step_ledger_migration_integration_test.go +++ b/internal/purchase/plan_step_ledger_migration_integration_test.go @@ -55,9 +55,8 @@ func stagedMigrations(t *testing.T) (dir string, addTarget func()) { func TestMigration_NeverStartedLegacyRampsBecomeNew(t *testing.T) { ctx := context.Background() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { _ = container.Cleanup(ctx) }) + // Empty, not migrated: the test applies the staged chain itself, then the target. + container := testhelpers.RequireEmptyDatabase(ctx, t) dir, addTarget := stagedMigrations(t) require.NoError(t, migrations.RunMigrations(ctx, container.DB.Pool(), dir, "", "")) store, pool := config.NewPostgresStore(container.DB), container.DB.Pool() @@ -73,7 +72,7 @@ func TestMigration_NeverStartedLegacyRampsBecomeNew(t *testing.T) { return p.ID } withExec := clone("b-has-execution", 4, 0) - _, err = pool.Exec(ctx, `INSERT INTO purchase_executions + _, err := pool.Exec(ctx, `INSERT INTO purchase_executions (plan_id, execution_id, status, no_submission, step_number, scheduled_date) VALUES ($1, $2, $3, $4, $5, $6)`, withExec, uuid.NewString(), config.StatusFailed, true, 1, time.Now()) require.NoError(t, err) diff --git a/internal/purchase/ramp_step_progress_integration_test.go b/internal/purchase/ramp_step_progress_integration_test.go index 8553c6420..538aed696 100644 --- a/internal/purchase/ramp_step_progress_integration_test.go +++ b/internal/purchase/ramp_step_progress_integration_test.go @@ -23,7 +23,6 @@ import ( "github.com/LeanerCloud/cloud-commitments-go/pkg/common" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/google/uuid" "github.com/jackc/pgx/v5" @@ -39,22 +38,15 @@ func rampMigrationsPath() string { return filepath.Join(filepath.Dir(filename), "..", "database", "postgres", "migrations") } -// rampStepStore stands up a fresh Postgres container with all migrations -// applied and returns a production store bound to it, plus the pool so a test -// can drive its own transaction alongside the store's. +// rampStepStore gives the test a fresh database with all migrations applied, +// on the Postgres server the package's integration tests share, and returns a +// production store bound to it, plus the pool so a test can drive its own +// transaction alongside the store's. func rampStepStore(ctx context.Context, t *testing.T) (*config.PostgresStore, *pgxpool.Pool) { t.Helper() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { - if cleanupErr := container.Cleanup(ctx); cleanupErr != nil { - t.Logf("container cleanup: %v", cleanupErr) - } - }) - - require.NoError(t, migrations.RunMigrations(ctx, container.DB.Pool(), rampMigrationsPath(), "", "")) - return config.NewPostgresStore(container.DB), container.DB.Pool() + db := testhelpers.RequireMigratedDatabase(ctx, t, rampMigrationsPath()) + return config.NewPostgresStore(db.DB), db.DB.Pool() } // rampStepFixture is the world a ramp-step test runs against: a plan sitting at @@ -591,7 +583,8 @@ func TestCompletePlanStepBlocksOnTheRowLockAndSeesTheWinner(t *testing.T) { var blocked int if qErr := pool.QueryRow(ctx, ` SELECT count(*) FROM pg_stat_activity - WHERE wait_event_type = 'Lock' + WHERE datname = current_database() + AND wait_event_type = 'Lock' AND state = 'active' AND pid <> pg_backend_pid() AND query ILIKE '%purchase_plans%FOR UPDATE%'`, From 2ffba75ab6f910c633d3adce1f7ec5e6284f1167 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sun, 25 Oct 2026 01:44:48 +0000 Subject: [PATCH 11/12] test(migrations): share one Postgres server across chain and bootstrap tests Every integration test in the migrations package started its own PostgreSQL container (92 per run), about 2s each before the test did any work. Add a TestMain that stops the shared server from issue #982 and move the up/down, dirty-state, cancellation, ensure_admin_user and bootstrap logging tests onto it. These tests are about the migration chain itself, so they take an empty database from RequireEmptyDatabase and keep every MigrateToVersion, RunMigrations and RollbackMigrations call they made before. The cancellation tests already created a fresh database per test on a server; migrationDB now uses an empty shared-server database for that unless CUDLY_TEST_ADMIN_DSN points at an external server. TestPGPasswordAndKeywordDSN_ConnectWithHostilePassword keeps a dedicated container: ALTER ROLE ... PASSWORD changes the cluster-wide role every shared-server database logs in as. Co-Authored-By: Claude Opus 5.5 --- .../backfill_admin_group_ids_test.go | 6 ++--- .../migrations/ensure_admin_user_test.go | 22 +++++----------- .../migrations/main_integration_test.go | 26 +++++++++++++++++++ .../migrations/migrate_autoheal_test.go | 16 +++--------- .../migrate_cancel_integration_test.go | 8 +++--- .../migrations/migrate_down_all_test.go | 4 +-- .../migrations/migrate_idempotency_test.go | 4 +-- .../migrate_pgpassword_integration_test.go | 2 ++ .../migrate_security_integration_test.go | 4 +-- .../migrations/savings_snapshots_pk_test.go | 12 +++------ .../migrations/split_savingsplans_test.go | 22 +++++----------- 11 files changed, 58 insertions(+), 68 deletions(-) create mode 100644 internal/database/postgres/migrations/main_integration_test.go diff --git a/internal/database/postgres/migrations/backfill_admin_group_ids_test.go b/internal/database/postgres/migrations/backfill_admin_group_ids_test.go index 764c0b60b..8408a2015 100644 --- a/internal/database/postgres/migrations/backfill_admin_group_ids_test.go +++ b/internal/database/postgres/migrations/backfill_admin_group_ids_test.go @@ -28,9 +28,7 @@ func TestMigration_BackfillAdminGroupIDs(t *testing.T) { migrationsPath := getMigrationsPath() const adminEmail = "restore-path@test.example" - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Pin the DB at version 55 so 000056 (the SQL backfill under test) is @@ -41,7 +39,7 @@ func TestMigration_BackfillAdminGroupIDs(t *testing.T) { // Simulate a restored/manually-seeded admin whose group_ids drifted to // empty (the bug pattern from issue #351). - _, err = pool.Exec(ctx, ` + _, err := pool.Exec(ctx, ` INSERT INTO users (id, email, password_hash, salt, role, active, group_ids, created_at, updated_at) VALUES (gen_random_uuid(), $1, '', '', 'admin', false, '{}', NOW(), NOW()) `, adminEmail) diff --git a/internal/database/postgres/migrations/ensure_admin_user_test.go b/internal/database/postgres/migrations/ensure_admin_user_test.go index 49c730417..19af8105c 100644 --- a/internal/database/postgres/migrations/ensure_admin_user_test.go +++ b/internal/database/postgres/migrations/ensure_admin_user_test.go @@ -47,9 +47,7 @@ func TestEnsureAdminUser_GroupAssignment(t *testing.T) { const adminEmail = "admin@test.example" t.Run("fresh insert no password", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() require.NoError(t, migrations.RunMigrations(ctx, pool, migrationsPath, adminEmail, "")) @@ -60,9 +58,7 @@ func TestEnsureAdminUser_GroupAssignment(t *testing.T) { }) t.Run("fresh insert with password", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() require.NoError(t, migrations.RunMigrations(ctx, pool, migrationsPath, adminEmail, "TestPass!9aF")) @@ -80,9 +76,7 @@ func TestEnsureAdminUser_GroupAssignment(t *testing.T) { // re-running RunMigrations when the admin already exists with the // correct group_ids is a no-op that neither fails nor doubles the // group entry (issue #945). - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // First pass: create admin via the normal path (migrations + bootstrap). @@ -103,9 +97,7 @@ func TestEnsureAdminUser_GroupAssignment(t *testing.T) { }) t.Run("idempotency", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Two consecutive RunMigrations calls must not duplicate the @@ -121,9 +113,7 @@ func TestEnsureAdminUser_GroupAssignment(t *testing.T) { }) t.Run("operator customisation preserved", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // First pass: create the admin via the normal path. @@ -135,7 +125,7 @@ func TestEnsureAdminUser_GroupAssignment(t *testing.T) { // subsequent boots (issue body: "leave existing group_ids // alone"). const customGroupID = "11111111-1111-5111-8111-111111111111" - _, err = pool.Exec(ctx, ` + _, err := pool.Exec(ctx, ` INSERT INTO groups (id, name, description, permissions, allowed_accounts) VALUES ($1::UUID, 'OperatorCustom', 'test', '[]'::jsonb, ARRAY['*']) ON CONFLICT DO NOTHING diff --git a/internal/database/postgres/migrations/main_integration_test.go b/internal/database/postgres/migrations/main_integration_test.go new file mode 100644 index 000000000..465882546 --- /dev/null +++ b/internal/database/postgres/migrations/main_integration_test.go @@ -0,0 +1,26 @@ +//go:build integration +// +build integration + +package migrations_test + +import ( + "log" + "os" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" +) + +// TestMain stops the PostgreSQL container that every integration test in this +// package shares (issue #982). Each test still gets a fresh database of its own +// from testhelpers.RequireMigratedDatabase or RequireEmptyDatabase; only the +// container and the migrated template it is cloned from outlive a single test. +func TestMain(m *testing.M) { + code := m.Run() + if err := testhelpers.TerminateSharedPostgres(); err != nil { + // Ryuk reaps the container anyway, so a teardown hiccup does not + // overrule the tests' own verdict. + log.Printf("terminate shared postgres container: %v", err) + } + os.Exit(code) +} diff --git a/internal/database/postgres/migrations/migrate_autoheal_test.go b/internal/database/postgres/migrations/migrate_autoheal_test.go index f86152508..bb4df2e09 100644 --- a/internal/database/postgres/migrations/migrate_autoheal_test.go +++ b/internal/database/postgres/migrations/migrate_autoheal_test.go @@ -51,9 +51,7 @@ func TestMigrations_AutoHealDirty(t *testing.T) { migrationsPath := getMigrationsPath() t.Run("by default a dirty DB fails loud with recovery instructions", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Migrate to head, then simulate a dirty state by forcing the dirty @@ -87,9 +85,7 @@ func TestMigrations_AutoHealDirty(t *testing.T) { }) t.Run("with CUDLY_MIGRATION_AUTOHEAL=true a dirty DB fails loud with recovery instructions", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() require.NoError(t, migrations.RunMigrations(ctx, pool, migrationsPath, "", "")) @@ -109,9 +105,7 @@ func TestMigrations_AutoHealDirty(t *testing.T) { }) t.Run("CUDLY_MIGRATION_AUTOHEAL=false bypasses auto-heal check; dirty DB errors via Up()", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() require.NoError(t, migrations.RunMigrations(ctx, pool, migrationsPath, "", "")) @@ -145,9 +139,7 @@ func TestMigrations_AutoHealDirty(t *testing.T) { // dirty at head -- NOT Force-cleared to clean, which would permanently skip // the missing DDL. t.Run("dirty-at-N with N-effects-absent: auto-heal refuses to silently mark it applied", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Migrate to head so we know the head version number. diff --git a/internal/database/postgres/migrations/migrate_cancel_integration_test.go b/internal/database/postgres/migrations/migrate_cancel_integration_test.go index 0b259e92a..c6d9c6da0 100644 --- a/internal/database/postgres/migrations/migrate_cancel_integration_test.go +++ b/internal/database/postgres/migrations/migrate_cancel_integration_test.go @@ -22,16 +22,16 @@ import ( // migrationDB returns a pool on a fresh, empty database so each test controls // its own schema_migrations state. CUDLY_TEST_ADMIN_DSN points at an existing -// server (used where Docker is unavailable); otherwise a container is started. +// server (used where Docker is unavailable); otherwise the test gets an empty +// database on the package's shared container. func migrationDB(t *testing.T) (*pgxpool.Pool, string) { t.Helper() ctx := context.Background() adminDSN := os.Getenv("CUDLY_TEST_ADMIN_DSN") if adminDSN == "" { - c := testhelpers.RequirePostgresContainer(ctx, t) - t.Cleanup(func() { _ = c.Cleanup(ctx) }) - adminDSN = fmt.Sprintf("postgres://cudly_test:test_password@%s:%d/cudly_test?sslmode=disable", c.Config.Host, c.Config.Port) + c := testhelpers.RequireEmptyDatabase(ctx, t) + return c.DB.Pool(), c.Config.Database } admin, err := pgxpool.New(ctx, adminDSN) diff --git a/internal/database/postgres/migrations/migrate_down_all_test.go b/internal/database/postgres/migrations/migrate_down_all_test.go index c13268ac3..c47de6cec 100644 --- a/internal/database/postgres/migrations/migrate_down_all_test.go +++ b/internal/database/postgres/migrations/migrate_down_all_test.go @@ -21,9 +21,7 @@ func TestMigrations_DownAllRoundTrip(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, migrationVersionsDesc(t)[0]), diff --git a/internal/database/postgres/migrations/migrate_idempotency_test.go b/internal/database/postgres/migrations/migrate_idempotency_test.go index d2303d33b..9aa74e2e8 100644 --- a/internal/database/postgres/migrations/migrate_idempotency_test.go +++ b/internal/database/postgres/migrations/migrate_idempotency_test.go @@ -34,9 +34,7 @@ func TestMigrations_FullStackIdempotent(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // First run: migrate to head on a fresh DB. diff --git a/internal/database/postgres/migrations/migrate_pgpassword_integration_test.go b/internal/database/postgres/migrations/migrate_pgpassword_integration_test.go index 0c9add33f..9318eca1d 100644 --- a/internal/database/postgres/migrations/migrate_pgpassword_integration_test.go +++ b/internal/database/postgres/migrations/migrate_pgpassword_integration_test.go @@ -45,6 +45,8 @@ func hostilePasswords() map[string]string { // A wrong PGPASSWORD must fail, proving the server really checks it. func TestPGPasswordAndKeywordDSN_ConnectWithHostilePassword(t *testing.T) { ctx := context.Background() + // Dedicated server: ALTER ROLE ... PASSWORD changes the cluster-wide role that + // every shared-server test database logs in as. container, err := testhelpers.SetupPostgresContainer(ctx, t) require.NoError(t, err) t.Cleanup(func() { _ = container.Cleanup(ctx) }) diff --git a/internal/database/postgres/migrations/migrate_security_integration_test.go b/internal/database/postgres/migrations/migrate_security_integration_test.go index 992616ddd..d1abdf4f9 100644 --- a/internal/database/postgres/migrations/migrate_security_integration_test.go +++ b/internal/database/postgres/migrations/migrate_security_integration_test.go @@ -29,9 +29,7 @@ func TestAdminBootstrap_LogsToStderr_NotStdout(t *testing.T) { migrationsPath := getMigrationsPath() const adminEmail = "stderr-bootstrap@test.example" - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() readStdout := captureStdout(t) diff --git a/internal/database/postgres/migrations/savings_snapshots_pk_test.go b/internal/database/postgres/migrations/savings_snapshots_pk_test.go index 75a32848b..ab6f2d15d 100644 --- a/internal/database/postgres/migrations/savings_snapshots_pk_test.go +++ b/internal/database/postgres/migrations/savings_snapshots_pk_test.go @@ -26,9 +26,7 @@ func TestMigration_SavingsSnapshotsPK(t *testing.T) { migrationsPath := getMigrationsPath() t.Run("full migration from scratch succeeds (000018 + 000027 coexist)", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Running all migrations must succeed on a fresh DB — 000027's @@ -39,7 +37,7 @@ func TestMigration_SavingsSnapshotsPK(t *testing.T) { // The primary key must exist and cover (id, timestamp). var constraintExists bool - err = pool.QueryRow(ctx, ` + err := pool.QueryRow(ctx, ` SELECT EXISTS ( SELECT 1 FROM pg_constraint WHERE conname = 'savings_snapshots_pkey' @@ -51,9 +49,7 @@ func TestMigration_SavingsSnapshotsPK(t *testing.T) { }) t.Run("rollback to 000026 then re-apply 000027 succeeds", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Pin the schema at 000027 (which re-created the PK over 000018's @@ -67,7 +63,7 @@ func TestMigration_SavingsSnapshotsPK(t *testing.T) { // Verify the constraint is gone (000027 was rolled back). var constraintExists bool - err = pool.QueryRow(ctx, ` + err := pool.QueryRow(ctx, ` SELECT EXISTS ( SELECT 1 FROM pg_constraint WHERE conname = 'savings_snapshots_pkey' diff --git a/internal/database/postgres/migrations/split_savingsplans_test.go b/internal/database/postgres/migrations/split_savingsplans_test.go index 2aab02b2d..adca2c26f 100644 --- a/internal/database/postgres/migrations/split_savingsplans_test.go +++ b/internal/database/postgres/migrations/split_savingsplans_test.go @@ -54,9 +54,7 @@ func TestMigration_SplitSavingsPlans(t *testing.T) { } t.Run("scenario 1: umbrella only", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Pin the DB at 000039 (just below the migration under test) and @@ -66,7 +64,7 @@ func TestMigration_SplitSavingsPlans(t *testing.T) { require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, 39)) // Seed: a single umbrella row with non-default values. - _, err = pool.Exec(ctx, ` + _, err := pool.Exec(ctx, ` INSERT INTO service_configs (provider, service, enabled, term, payment, coverage, ramp_schedule) VALUES ('aws', 'savings-plans', true, 1, 'no-upfront', 90.50, 'weekly-25pct')`) require.NoError(t, err) @@ -113,9 +111,7 @@ func TestMigration_SplitSavingsPlans(t *testing.T) { }) t.Run("scenario 2: umbrella + PR #71 sagemaker row (different values)", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Pin the DB at 000039, just below the migration under test. @@ -123,7 +119,7 @@ func TestMigration_SplitSavingsPlans(t *testing.T) { // Seed both rows with intentionally divergent term/payment so // we can verify which value wins per slot. - _, err = pool.Exec(ctx, ` + _, err := pool.Exec(ctx, ` INSERT INTO service_configs (provider, service, enabled, term, payment, coverage) VALUES ('aws', 'savings-plans', true, 3, 'all-upfront', 80.00)`) require.NoError(t, err) @@ -162,9 +158,7 @@ func TestMigration_SplitSavingsPlans(t *testing.T) { }) t.Run("scenario 3: fresh install (no SP rows)", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Pin the DB at 000039, just below the migration under test. @@ -198,9 +192,7 @@ func TestMigration_SplitSavingsPlans_PurchasePlansJSONB(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Pin the DB at 000039, just below the migration under test. @@ -209,7 +201,7 @@ func TestMigration_SplitSavingsPlans_PurchasePlansJSONB(t *testing.T) { // Seed two plans: one with the umbrella SP key, one with only RDS // (untouched control). Both have a non-SP key (aws:rds) that must // pass through unchanged. - _, err = pool.Exec(ctx, ` + _, err := pool.Exec(ctx, ` INSERT INTO purchase_plans (id, name, services) VALUES ('11111111-1111-1111-1111-111111111111', 'sp-plan', '{"aws:savings-plans": {"term": "1yr", "payment": "all-upfront"}, "aws:rds": {"term": "3yr"}}'::jsonb), From e5ff9e5c3583f6818c6ea7be19fbc6b7ec160a5b Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sun, 25 Oct 2026 01:44:56 +0000 Subject: [PATCH 12/12] test(migrations): share one Postgres server across per-migration tests The per-migration tests (000053 through 000134) each started their own PostgreSQL container. Move them onto the package's shared server (issue #982). Tests that pin a version, seed rows, and then step up or down take an empty database from RequireEmptyDatabase and walk the chain exactly as before. Tests whose first step was a full RunMigrations purely as a fixture (000064's seed check, 000065's last-admin trigger tests, 000096's seed and idempotency checks, 000102, 000103) take a clone of the migrated template from RequireMigratedDatabase instead; the template is built by the same RunMigrations call, so their starting state is unchanged. No test here touches server-wide state. 000132's ALTER DATABASE targets current_database(), and its pg_stat_activity poll filters by pid. Co-Authored-By: Claude Opus 5.5 --- ...053_executions_account_fk_restrict_test.go | 12 ++++------ .../000057_drop_user_role_to_groups_test.go | 12 ++++------ .../000060_cleanup_universal_plans_test.go | 6 ++--- .../000064_relocate_purchaser_group_test.go | 20 +++++------------ .../000065_enforce_min_one_admin_test.go | 12 ++-------- ...0078_monthly_summary_nested_rollup_test.go | 3 +-- ...ve_approve_own_from_standard_users_test.go | 8 ++----- ...rchase_history_marketplace_listing_test.go | 3 +-- ...ant_view_config_to_nonadmin_groups_test.go | 22 +++++-------------- ...00089_rename_cancelled_to_canceled_test.go | 12 ++++------ ...093_recommendations_state_owner_id_test.go | 6 ++--- ..._purchase_history_account_id_width_test.go | 9 +++----- .../000096_seed_ri_exchanger_group_test.go | 22 +++++-------------- ...098_backfill_execution_step_number_test.go | 4 +--- .../000099_user_deactivated_at_test.go | 4 +--- ...00100_hash_approval_tokens_at_rest_test.go | 10 +++------ .../000102_purchase_plan_update_token_test.go | 7 ++---- .../000103_api_key_password_version_test.go | 7 ++---- .../000104_user_mfa_last_totp_counter_test.go | 6 ++--- ...5_purchase_history_listing_attempt_test.go | 6 ++--- ...validate_azure_recs_lookback_units_test.go | 6 ++--- .../000107_ladder_tranche_amendments_test.go | 10 +++------ ..._recommendations_availability_zone_test.go | 6 ++--- ...p_recommendations_8col_natural_key_test.go | 6 ++--- ...1_recommendations_platform_tenancy_test.go | 6 ++--- ...plan_step_cap_auto_purchase_switch_test.go | 6 ++--- ..._purchase_executions_no_submission_test.go | 4 +--- ...0114_ladder_tranches_status_reason_test.go | 4 +--- ...000115_auto_purchase_switch_stamps_test.go | 6 ++--- .../000117_ladder_tranches_settled_at_test.go | 6 ++--- ...00118_purchase_plans_ramp_baseline_test.go | 3 +-- ...p_recommendations_9col_natural_key_test.go | 6 ++--- ...120_recommendations_source_account_test.go | 8 +++---- ...1_ladder_tranche_amount_provenance_test.go | 3 +-- ..._purchase_execution_fan_out_shapes_test.go | 6 ++--- .../000125_lock_default_user_groups_test.go | 3 +-- ...purchase_history_active_expiry_idx_test.go | 3 +-- ...executions_detach_keeps_updated_at_test.go | 3 +-- ...0129_recommendations_state_partial_test.go | 6 ++--- ...chase_executions_created_by_system_test.go | 3 +-- ...0131_ri_exchange_direct_executions_test.go | 4 +--- ...00132_savings_partition_utc_bounds_test.go | 6 ++--- ...istory_reverify_ec2_offering_class_test.go | 6 ++--- ...0134_purchase_suppressions_variant_test.go | 3 +-- 44 files changed, 96 insertions(+), 218 deletions(-) diff --git a/internal/database/postgres/migrations/000053_executions_account_fk_restrict_test.go b/internal/database/postgres/migrations/000053_executions_account_fk_restrict_test.go index 919cad82d..bb48a5d61 100644 --- a/internal/database/postgres/migrations/000053_executions_account_fk_restrict_test.go +++ b/internal/database/postgres/migrations/000053_executions_account_fk_restrict_test.go @@ -21,9 +21,7 @@ func TestMigration_ExecutionsAccountFKRestrict(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Pin the schema at 000053 so the assertions exercise this migration's @@ -33,7 +31,7 @@ func TestMigration_ExecutionsAccountFKRestrict(t *testing.T) { // Verify the FK is RESTRICT after the migration. var deleteAction string - err = pool.QueryRow(ctx, ` + err := pool.QueryRow(ctx, ` SELECT confdeltype::text FROM pg_constraint WHERE conname = 'purchase_executions_cloud_account_id_fkey' @@ -104,9 +102,7 @@ func TestMigration_ExecutionsAccountFKRestrict_Rollback(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Pin the schema at 000053, then roll back exactly that migration. A @@ -116,7 +112,7 @@ func TestMigration_ExecutionsAccountFKRestrict_Rollback(t *testing.T) { require.NoError(t, migrations.RollbackMigrations(ctx, pool, migrationsPath, 1)) var deleteAction string - err = pool.QueryRow(ctx, ` + err := pool.QueryRow(ctx, ` SELECT confdeltype::text FROM pg_constraint WHERE conname = 'purchase_executions_cloud_account_id_fkey' diff --git a/internal/database/postgres/migrations/000057_drop_user_role_to_groups_test.go b/internal/database/postgres/migrations/000057_drop_user_role_to_groups_test.go index 3062bc8dd..18ceee325 100644 --- a/internal/database/postgres/migrations/000057_drop_user_role_to_groups_test.go +++ b/internal/database/postgres/migrations/000057_drop_user_role_to_groups_test.go @@ -42,9 +42,7 @@ func TestMigration_DropUserRoleToGroups(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Pin the DB at v56, where the `role` column still exists and we can @@ -70,7 +68,7 @@ func TestMigration_DropUserRoleToGroups(t *testing.T) { // the legacy {admin,user,readonly} set; this is the path the migration's // fail-safe net (step 3 in 000057.up.sql) is designed to catch -- without // it, that net is exercised by no test and could rot silently. - _, err = pool.Exec(ctx, `ALTER TABLE users DROP CONSTRAINT IF EXISTS valid_role`) + _, err := pool.Exec(ctx, `ALTER TABLE users DROP CONSTRAINT IF EXISTS valid_role`) require.NoError(t, err) seed("unknown@test.example", "legacy-custom") @@ -137,16 +135,14 @@ func TestMigration_DropUserRoleToGroups_Down(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Pin at v57 so the rollback below exercises exactly 000057's down. require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, 57)) // Seed group-only users (post-#907 shape) before rolling back. - _, err = pool.Exec(ctx, ` + _, err := pool.Exec(ctx, ` INSERT INTO users (id, email, password_hash, salt, active, group_ids, created_at, updated_at) VALUES (gen_random_uuid(), 'adm@test.example', '', '', true, ARRAY['`+defaultAdminGroupIDTest+`']::uuid[], NOW(), NOW()), diff --git a/internal/database/postgres/migrations/000060_cleanup_universal_plans_test.go b/internal/database/postgres/migrations/000060_cleanup_universal_plans_test.go index ad461038f..75cb09e0d 100644 --- a/internal/database/postgres/migrations/000060_cleanup_universal_plans_test.go +++ b/internal/database/postgres/migrations/000060_cleanup_universal_plans_test.go @@ -36,9 +36,7 @@ func TestMigration_CleanupUniversalPlans(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Pin the DB at v59, seed fixtures, then apply 000060 in isolation. @@ -47,7 +45,7 @@ func TestMigration_CleanupUniversalPlans(t *testing.T) { require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, 59)) // Seed: two cloud accounts (needed for plan_accounts FK). - _, err = pool.Exec(ctx, ` + _, err := pool.Exec(ctx, ` INSERT INTO cloud_accounts (id, name, provider, external_id) VALUES ('aaaaaaaa-aaaa-aaaa-aaaa-000000000001', 'acct-1', 'aws', '111111111111'), diff --git a/internal/database/postgres/migrations/000064_relocate_purchaser_group_test.go b/internal/database/postgres/migrations/000064_relocate_purchaser_group_test.go index 9294395ce..19092d65e 100644 --- a/internal/database/postgres/migrations/000064_relocate_purchaser_group_test.go +++ b/internal/database/postgres/migrations/000064_relocate_purchaser_group_test.go @@ -49,16 +49,12 @@ func TestMigration_RelocatePurchaserGroup(t *testing.T) { migrationsPath := getMigrationsPath() t.Run("Purchaser seeded at new UUID after full migration run", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireMigratedDatabase(ctx, t, migrationsPath) pool := container.DB.Pool() - require.NoError(t, migrations.RunMigrations(ctx, pool, migrationsPath, "", "")) - // Purchaser must exist at the new UUID. var name string - err = pool.QueryRow(ctx, `SELECT name FROM groups WHERE id = $1`, purchaserGroupIDTest).Scan(&name) + err := pool.QueryRow(ctx, `SELECT name FROM groups WHERE id = $1`, purchaserGroupIDTest).Scan(&name) require.NoError(t, err, "Purchaser group must exist at UUID %s", purchaserGroupIDTest) assert.Equal(t, "Purchaser", name, "group at new UUID must be named 'Purchaser'") @@ -70,9 +66,7 @@ func TestMigration_RelocatePurchaserGroup(t *testing.T) { }) t.Run("admin-backfill: Administrators members land in Purchaser", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Pin the DB just below 000064 so we can seed an admin and @@ -83,7 +77,7 @@ func TestMigration_RelocatePurchaserGroup(t *testing.T) { // Seed an admin user in the Administrators group but NOT in Purchaser. const adminEmail = "admin-purchaser-test@test.example" - _, err = pool.Exec(ctx, ` + _, err := pool.Exec(ctx, ` INSERT INTO users (id, email, password_hash, salt, active, group_ids, created_at, updated_at) VALUES (gen_random_uuid(), $1, '', '', true, ARRAY[$2::uuid], NOW(), NOW()) @@ -104,16 +98,14 @@ func TestMigration_RelocatePurchaserGroup(t *testing.T) { }) t.Run("idempotent: re-running migrations does not duplicate Purchaser membership", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Seed an admin before first run so 000064 backfill fires. require.NoError(t, migrations.RunMigrations(ctx, pool, migrationsPath, "", "")) const adminEmail = "idempotent-test@test.example" - _, err = pool.Exec(ctx, ` + _, err := pool.Exec(ctx, ` INSERT INTO users (id, email, password_hash, salt, active, group_ids, created_at, updated_at) VALUES (gen_random_uuid(), $1, '', '', true, ARRAY[$2::uuid, $3::uuid], NOW(), NOW()) diff --git a/internal/database/postgres/migrations/000065_enforce_min_one_admin_test.go b/internal/database/postgres/migrations/000065_enforce_min_one_admin_test.go index 5729dc73a..2cea63e68 100644 --- a/internal/database/postgres/migrations/000065_enforce_min_one_admin_test.go +++ b/internal/database/postgres/migrations/000065_enforce_min_one_admin_test.go @@ -8,7 +8,6 @@ import ( "sync" "testing" - "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/migrations" "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" "github.com/jackc/pgx/v5/pgxpool" "github.com/stretchr/testify/assert" @@ -73,13 +72,9 @@ func TestMigration_EnforceMinOneAdmin_ConcurrentRace(t *testing.T) { // arbitration. It returns how many of the two transactions committed and the // number of active admins left afterwards. runConcurrentRace := func(t *testing.T, mutateSQL string) (commits, remainingAdmins int) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireMigratedDatabase(ctx, t, migrationsPath) pool := container.DB.Pool() - require.NoError(t, migrations.RunMigrations(ctx, pool, migrationsPath, "", "")) - adminA := seedActiveAdmin(t, ctx, pool, "race-admin-a@test.example") adminB := seedActiveAdmin(t, ctx, pool, "race-admin-b@test.example") require.Equal(t, 2, countActiveAdmins(t, ctx, pool), "test setup: two active admins expected") @@ -175,11 +170,8 @@ func TestMigration_EnforceMinOneAdmin_BlocksLastAdminRemoval(t *testing.T) { // from a clean admin count. freshPool := func(t *testing.T) *pgxpool.Pool { t.Helper() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { _ = container.Cleanup(ctx) }) + container := testhelpers.RequireMigratedDatabase(ctx, t, migrationsPath) pool := container.DB.Pool() - require.NoError(t, migrations.RunMigrations(ctx, pool, migrationsPath, "", "")) return pool } diff --git a/internal/database/postgres/migrations/000078_monthly_summary_nested_rollup_test.go b/internal/database/postgres/migrations/000078_monthly_summary_nested_rollup_test.go index e2483280c..1fe4bce6a 100644 --- a/internal/database/postgres/migrations/000078_monthly_summary_nested_rollup_test.go +++ b/internal/database/postgres/migrations/000078_monthly_summary_nested_rollup_test.go @@ -35,8 +35,7 @@ func TestAnalyticsNestedRollup_COR02(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) defer cancel() - container := testhelpers.RequirePostgresContainer(ctx, t) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) require.NoError(t, migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", "")) diff --git a/internal/database/postgres/migrations/000086_remove_approve_own_from_standard_users_test.go b/internal/database/postgres/migrations/000086_remove_approve_own_from_standard_users_test.go index dea193e1f..ac24b3b17 100644 --- a/internal/database/postgres/migrations/000086_remove_approve_own_from_standard_users_test.go +++ b/internal/database/postgres/migrations/000086_remove_approve_own_from_standard_users_test.go @@ -42,9 +42,7 @@ func TestMigration_RemoveApproveOwnFromStandardUsers(t *testing.T) { migrationsPath := getMigrationsPath() t.Run("approve-own removed, sibling own-verbs retained", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Pin at 000083, the last migration before 000086. approve-own was @@ -77,9 +75,7 @@ func TestMigration_RemoveApproveOwnFromStandardUsers(t *testing.T) { }) t.Run("down migration restores approve-own", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Pin at exactly 000086 before rolling back. RunMigrations would migrate diff --git a/internal/database/postgres/migrations/000087_purchase_history_marketplace_listing_test.go b/internal/database/postgres/migrations/000087_purchase_history_marketplace_listing_test.go index b5643d1c4..810a42cd6 100644 --- a/internal/database/postgres/migrations/000087_purchase_history_marketplace_listing_test.go +++ b/internal/database/postgres/migrations/000087_purchase_history_marketplace_listing_test.go @@ -26,8 +26,7 @@ func TestMigration087_MarketplaceColumns(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) defer cancel() - container := testhelpers.RequirePostgresContainer(ctx, t) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) require.NoError(t, migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", ""), "migrations must apply cleanly through 000087") diff --git a/internal/database/postgres/migrations/000088_grant_view_config_to_nonadmin_groups_test.go b/internal/database/postgres/migrations/000088_grant_view_config_to_nonadmin_groups_test.go index 4a5d0e022..438e39dcb 100644 --- a/internal/database/postgres/migrations/000088_grant_view_config_to_nonadmin_groups_test.go +++ b/internal/database/postgres/migrations/000088_grant_view_config_to_nonadmin_groups_test.go @@ -42,9 +42,7 @@ func TestMigration_GrantViewConfigToNonAdminGroups(t *testing.T) { migrationsPath := getMigrationsPath() t.Run("view:config granted to Standard Users and Read-Only Users", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Pin at 000086, the last migration on main before this change. view:config @@ -68,9 +66,7 @@ func TestMigration_GrantViewConfigToNonAdminGroups(t *testing.T) { }) t.Run("update:config is NOT granted (write gate is unaffected)", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Pin to 000088 (feedback_migration_test_pin_version, #1436). @@ -83,9 +79,7 @@ func TestMigration_GrantViewConfigToNonAdminGroups(t *testing.T) { }) t.Run("sibling permissions on Standard Users are preserved", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Pin to 000088 (feedback_migration_test_pin_version, #1436). @@ -108,9 +102,7 @@ func TestMigration_GrantViewConfigToNonAdminGroups(t *testing.T) { }) t.Run("down migration removes view:config from both groups", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Pin to 000088 so a later migration on main cannot shift the head @@ -134,9 +126,7 @@ func TestMigration_GrantViewConfigToNonAdminGroups(t *testing.T) { }) t.Run("idempotent: applying to a group already holding view:config is safe", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Pin to 000088 (feedback_migration_test_pin_version, #1436). @@ -144,7 +134,7 @@ func TestMigration_GrantViewConfigToNonAdminGroups(t *testing.T) { // Count entries for view:config before the re-apply. var count int - err = pool.QueryRow(ctx, ` + err := pool.QueryRow(ctx, ` SELECT COUNT(*) FROM groups g, jsonb_array_elements(g.permissions) AS elem WHERE g.id = $1 diff --git a/internal/database/postgres/migrations/000089_rename_cancelled_to_canceled_test.go b/internal/database/postgres/migrations/000089_rename_cancelled_to_canceled_test.go index 90a30cfd8..fc7b8e9bf 100644 --- a/internal/database/postgres/migrations/000089_rename_cancelled_to_canceled_test.go +++ b/internal/database/postgres/migrations/000089_rename_cancelled_to_canceled_test.go @@ -50,9 +50,7 @@ func TestMigration_000089_RollbackWithCanceledRows(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Apply exactly 000089. The dual-accept constraint and canceled_by column @@ -65,7 +63,7 @@ func TestMigration_000089_RollbackWithCanceledRows(t *testing.T) { execID = "78787878-7878-7878-7878-000000000001" execActor = "canceler@test.example" ) - _, err = pool.Exec(ctx, ` + _, err := pool.Exec(ctx, ` INSERT INTO purchase_executions (id, execution_id, status, step_number, scheduled_date, canceled_by) VALUES ( @@ -169,9 +167,7 @@ func TestMigration_000089_UpIsAdditiveAndDualCompatible(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Pin just below 000089 and seed a legacy 'cancelled' row + cancelled_by so @@ -182,7 +178,7 @@ func TestMigration_000089_UpIsAdditiveAndDualCompatible(t *testing.T) { execID = "77777777-7777-7777-7777-000000000001" execActor = "legacy-canceler@test.example" ) - _, err = pool.Exec(ctx, ` + _, err := pool.Exec(ctx, ` INSERT INTO purchase_executions (id, execution_id, status, step_number, scheduled_date, cancelled_by) VALUES ( diff --git a/internal/database/postgres/migrations/000093_recommendations_state_owner_id_test.go b/internal/database/postgres/migrations/000093_recommendations_state_owner_id_test.go index cf9a6e097..38384d9ca 100644 --- a/internal/database/postgres/migrations/000093_recommendations_state_owner_id_test.go +++ b/internal/database/postgres/migrations/000093_recommendations_state_owner_id_test.go @@ -21,9 +21,7 @@ func TestMigration_RecommendationsStateOwnerID(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Pin at 000092 (pre-migration) so the assertions exercise 000093's @@ -31,7 +29,7 @@ func TestMigration_RecommendationsStateOwnerID(t *testing.T) { require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, 92)) var columnExists bool - err = pool.QueryRow(ctx, ` + err := pool.QueryRow(ctx, ` SELECT EXISTS ( SELECT 1 FROM information_schema.columns WHERE table_name = 'recommendations_state' diff --git a/internal/database/postgres/migrations/000095_purchase_history_account_id_width_test.go b/internal/database/postgres/migrations/000095_purchase_history_account_id_width_test.go index 43d9aff48..86739355f 100644 --- a/internal/database/postgres/migrations/000095_purchase_history_account_id_width_test.go +++ b/internal/database/postgres/migrations/000095_purchase_history_account_id_width_test.go @@ -109,8 +109,7 @@ func TestMigration095_PurchaseHistoryAccountIDWidth(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) defer cancel() - container := testhelpers.RequirePostgresContainer(ctx, t) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() @@ -198,8 +197,7 @@ func TestMigration095_DownNarrowsWhenSafe(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) defer cancel() - container := testhelpers.RequirePostgresContainer(ctx, t) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() @@ -234,8 +232,7 @@ func TestMigration095_DownRefusesToTruncate(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) defer cancel() - container := testhelpers.RequirePostgresContainer(ctx, t) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() diff --git a/internal/database/postgres/migrations/000096_seed_ri_exchanger_group_test.go b/internal/database/postgres/migrations/000096_seed_ri_exchanger_group_test.go index 985633de4..dd2ac81db 100644 --- a/internal/database/postgres/migrations/000096_seed_ri_exchanger_group_test.go +++ b/internal/database/postgres/migrations/000096_seed_ri_exchanger_group_test.go @@ -35,16 +35,12 @@ func TestMigration_SeedRIExchangerGroup(t *testing.T) { migrationsPath := getMigrationsPath() t.Run("RI Exchanger seeded, system-managed, granting execute:ri-exchange", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireMigratedDatabase(ctx, t, migrationsPath) pool := container.DB.Pool() - require.NoError(t, migrations.RunMigrations(ctx, pool, migrationsPath, "", "")) - var name string var systemManaged bool - err = pool.QueryRow(ctx, + err := pool.QueryRow(ctx, `SELECT name, system_managed FROM groups WHERE id = $1`, riExchangerGroupIDTest). Scan(&name, &systemManaged) require.NoError(t, err, "RI Exchanger group must exist at UUID %s", riExchangerGroupIDTest) @@ -74,9 +70,7 @@ func TestMigration_SeedRIExchangerGroup(t *testing.T) { }) t.Run("admin-backfill: Administrators members land in RI Exchanger", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Pin just below 000096 so the backfill is observed firing, rather @@ -84,7 +78,7 @@ func TestMigration_SeedRIExchangerGroup(t *testing.T) { require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, 95)) const adminEmail = "admin-riexchanger-test@test.example" - _, err = pool.Exec(ctx, ` + _, err := pool.Exec(ctx, ` INSERT INTO users (id, email, password_hash, salt, active, group_ids, created_at, updated_at) VALUES (gen_random_uuid(), $1, '', '', true, ARRAY[$2::uuid], NOW(), NOW()) `, adminEmail, adminGroupIDForPurchaserTest) @@ -104,15 +98,11 @@ func TestMigration_SeedRIExchangerGroup(t *testing.T) { }) t.Run("backfill is idempotent and leaves no duplicate entry", func(t *testing.T) { - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireMigratedDatabase(ctx, t, migrationsPath) pool := container.DB.Pool() - require.NoError(t, migrations.RunMigrations(ctx, pool, migrationsPath, "", "")) - const adminEmail = "admin-riexchanger-idempotent@test.example" - _, err = pool.Exec(ctx, ` + _, err := pool.Exec(ctx, ` INSERT INTO users (id, email, password_hash, salt, active, group_ids, created_at, updated_at) VALUES (gen_random_uuid(), $1, '', '', true, ARRAY[$2::uuid, $3::uuid], NOW(), NOW()) `, adminEmail, adminGroupIDForPurchaserTest, riExchangerGroupIDTest) diff --git a/internal/database/postgres/migrations/000098_backfill_execution_step_number_test.go b/internal/database/postgres/migrations/000098_backfill_execution_step_number_test.go index c6fd3065d..5d0f41b43 100644 --- a/internal/database/postgres/migrations/000098_backfill_execution_step_number_test.go +++ b/internal/database/postgres/migrations/000098_backfill_execution_step_number_test.go @@ -60,9 +60,7 @@ func TestMigration_BackfillExecutionStepNumber(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() // Pin just below 000098 so the assertions exercise this migration's direct diff --git a/internal/database/postgres/migrations/000099_user_deactivated_at_test.go b/internal/database/postgres/migrations/000099_user_deactivated_at_test.go index 335a0a553..98f3f182d 100644 --- a/internal/database/postgres/migrations/000099_user_deactivated_at_test.go +++ b/internal/database/postgres/migrations/000099_user_deactivated_at_test.go @@ -54,9 +54,7 @@ func TestMigration_UserDeactivatedAtBackfill(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, previousMigrationVersion(t, 99))) diff --git a/internal/database/postgres/migrations/000100_hash_approval_tokens_at_rest_test.go b/internal/database/postgres/migrations/000100_hash_approval_tokens_at_rest_test.go index effa76258..1f357493d 100644 --- a/internal/database/postgres/migrations/000100_hash_approval_tokens_at_rest_test.go +++ b/internal/database/postgres/migrations/000100_hash_approval_tokens_at_rest_test.go @@ -115,9 +115,7 @@ func TestMigration_HashApprovalTokensAtRest(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() prev := previousMigrationVersion(t, 100) @@ -156,7 +154,7 @@ func TestMigration_HashApprovalTokensAtRest(t *testing.T) { for _, tt := range tokenTables { lateWritten[tt.name] = tt.seed(ctx, t, pool, "pending", strPtr("late-"+rawToken+tt.name)) } - _, err = pool.Exec(ctx, `UPDATE purchase_executions SET approval_token = '' WHERE execution_id = $1`, resavedEmpty) + _, err := pool.Exec(ctx, `UPDATE purchase_executions SET approval_token = '' WHERE execution_id = $1`, resavedEmpty) require.NoError(t, err) _, err = pool.Exec(ctx, `UPDATE purchase_executions SET approval_token = 'old-code-fresh-token' WHERE execution_id = $1`, resavedFresh) require.NoError(t, err) @@ -189,9 +187,7 @@ func TestMigration_HashApprovalTokensAtRest_StoreReadsTokenlessRows(t *testing.T ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, previousMigrationVersion(t, 100))) diff --git a/internal/database/postgres/migrations/000102_purchase_plan_update_token_test.go b/internal/database/postgres/migrations/000102_purchase_plan_update_token_test.go index 772037da9..2959b58dc 100644 --- a/internal/database/postgres/migrations/000102_purchase_plan_update_token_test.go +++ b/internal/database/postgres/migrations/000102_purchase_plan_update_token_test.go @@ -14,12 +14,9 @@ import ( func TestMigration_PurchasePlanUpdateToken(t *testing.T) { ctx := context.Background() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, container.Cleanup(ctx)) }) - pool := container.DB.Pool() path := getMigrationsPath() - require.NoError(t, migrations.RunMigrations(ctx, pool, path, "", "")) + container := testhelpers.RequireMigratedDatabase(ctx, t, path) + pool := container.DB.Pool() for _, future := range []bool{false, true} { t.Run(map[bool]string{false: "current", true: "future"}[future], func(t *testing.T) { diff --git a/internal/database/postgres/migrations/000103_api_key_password_version_test.go b/internal/database/postgres/migrations/000103_api_key_password_version_test.go index 9b1e4b39a..321688272 100644 --- a/internal/database/postgres/migrations/000103_api_key_password_version_test.go +++ b/internal/database/postgres/migrations/000103_api_key_password_version_test.go @@ -13,12 +13,9 @@ import ( func TestMigration_APIKeyPasswordVersion(t *testing.T) { ctx := context.Background() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, container.Cleanup(ctx)) }) - pool := container.DB.Pool() path := getMigrationsPath() - require.NoError(t, migrations.RunMigrations(ctx, pool, path, "", "")) + container := testhelpers.RequireMigratedDatabase(ctx, t, path) + pool := container.DB.Pool() var id string var version int64 diff --git a/internal/database/postgres/migrations/000104_user_mfa_last_totp_counter_test.go b/internal/database/postgres/migrations/000104_user_mfa_last_totp_counter_test.go index e08aa99ac..0383a9392 100644 --- a/internal/database/postgres/migrations/000104_user_mfa_last_totp_counter_test.go +++ b/internal/database/postgres/migrations/000104_user_mfa_last_totp_counter_test.go @@ -18,9 +18,7 @@ func TestMigration_UserMFALastTOTPCounter(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, previousMigrationVersion(t, 104))) @@ -32,6 +30,6 @@ func TestMigration_UserMFALastTOTPCounter(t *testing.T) { assert.Zero(t, counter, "an existing user must start with no accepted code") require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, previousMigrationVersion(t, 104))) - err = pool.QueryRow(ctx, `SELECT mfa_last_totp_counter FROM users WHERE id = $1`, userID).Scan(&counter) + err := pool.QueryRow(ctx, `SELECT mfa_last_totp_counter FROM users WHERE id = $1`, userID).Scan(&counter) require.Error(t, err, "the down migration must drop the column") } diff --git a/internal/database/postgres/migrations/000105_purchase_history_listing_attempt_test.go b/internal/database/postgres/migrations/000105_purchase_history_listing_attempt_test.go index baee477c1..bb66bcb9e 100644 --- a/internal/database/postgres/migrations/000105_purchase_history_listing_attempt_test.go +++ b/internal/database/postgres/migrations/000105_purchase_history_listing_attempt_test.go @@ -20,9 +20,7 @@ func TestMigration_PurchaseHistoryListingAttempt(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() const insertSQL = ` @@ -32,7 +30,7 @@ func TestMigration_PurchaseHistoryListingAttempt(t *testing.T) { ) VALUES ('acct', 'ri-105', $1, 'aws', 'ec2', 'us-east-1', 't3.micro', 12, 'All Upfront')` require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, previousMigrationVersion(t, 105))) - _, err = pool.Exec(ctx, insertSQL, time.Now()) + _, err := pool.Exec(ctx, insertSQL, time.Now()) require.NoError(t, err) require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, 105)) diff --git a/internal/database/postgres/migrations/000106_invalidate_azure_recs_lookback_units_test.go b/internal/database/postgres/migrations/000106_invalidate_azure_recs_lookback_units_test.go index 50dd53bd7..43b13c608 100644 --- a/internal/database/postgres/migrations/000106_invalidate_azure_recs_lookback_units_test.go +++ b/internal/database/postgres/migrations/000106_invalidate_azure_recs_lookback_units_test.go @@ -18,9 +18,7 @@ import ( func TestMigration_InvalidateAzureRecsLookbackUnits(t *testing.T) { ctx := context.Background() path := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() require.NoError(t, migrations.MigrateToVersion(ctx, pool, path, 105)) fixtures := []string{ @@ -31,7 +29,7 @@ func TestMigration_InvalidateAzureRecsLookbackUnits(t *testing.T) { expected := make([]map[string]any, len(fixtures)) for i, payload := range fixtures { require.NoError(t, json.Unmarshal([]byte(payload), &expected[i])) - _, err = pool.Exec(ctx, `INSERT INTO recommendations + _, err := pool.Exec(ctx, `INSERT INTO recommendations (collected_at,provider,service,region,resource_type,engine,payload,upfront_cost,monthly_savings,term,payment_option) VALUES (NOW(),$1,'compute','westeurope','Standard_D4s_v5','',$2::jsonb,0,0,3,$3)`, expected[i]["provider"], payload, expected[i]["payment"]) diff --git a/internal/database/postgres/migrations/000107_ladder_tranche_amendments_test.go b/internal/database/postgres/migrations/000107_ladder_tranche_amendments_test.go index 8427d21c5..c47a927b2 100644 --- a/internal/database/postgres/migrations/000107_ladder_tranche_amendments_test.go +++ b/internal/database/postgres/migrations/000107_ladder_tranche_amendments_test.go @@ -15,9 +15,7 @@ import ( func TestMigration_LadderTrancheAmendments(t *testing.T) { ctx := context.Background() path := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() require.NoError(t, migrations.MigrateToVersion(ctx, pool, path, 106)) @@ -46,7 +44,7 @@ func TestMigration_LadderTrancheAmendments(t *testing.T) { require.NoError(t, pool.QueryRow(ctx, `SELECT revision FROM ladder_tranches WHERE id = $1`, trancheID).Scan(&revision)) assert.Equal(t, int64(0), revision, "pre-existing tranches start at revision 0") - _, err = pool.Exec(ctx, `UPDATE ladder_tranches SET revision = -1 WHERE id = $1`, trancheID) + _, err := pool.Exec(ctx, `UPDATE ladder_tranches SET revision = -1 WHERE id = $1`, trancheID) require.Error(t, err, "CHECK (revision >= 0) must reject negative revisions") _, err = pool.Exec(ctx, `INSERT INTO ladder_tranche_amendments @@ -91,9 +89,7 @@ func TestMigration_LadderTrancheAmendments(t *testing.T) { func TestMigration_LadderTrancheAmendmentsRollbackWhenEmpty(t *testing.T) { ctx := context.Background() path := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() require.NoError(t, migrations.MigrateToVersion(ctx, pool, path, 107)) diff --git a/internal/database/postgres/migrations/000109_recommendations_availability_zone_test.go b/internal/database/postgres/migrations/000109_recommendations_availability_zone_test.go index b146da37f..f9b780c3c 100644 --- a/internal/database/postgres/migrations/000109_recommendations_availability_zone_test.go +++ b/internal/database/postgres/migrations/000109_recommendations_availability_zone_test.go @@ -29,9 +29,7 @@ func TestMigration_RecommendationsAvailabilityZone(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, 108)) @@ -50,7 +48,7 @@ func TestMigration_RecommendationsAvailabilityZone(t *testing.T) { "no-details": `{"service":"ec2"}`, } for resourceType, payload := range legacy { - _, err = pool.Exec(ctx, insertRecForMigration109, resourceType, payload) + _, err := pool.Exec(ctx, insertRecForMigration109, resourceType, payload) require.NoError(t, err) } diff --git a/internal/database/postgres/migrations/000110_drop_recommendations_8col_natural_key_test.go b/internal/database/postgres/migrations/000110_drop_recommendations_8col_natural_key_test.go index bf0797061..a0e8ed062 100644 --- a/internal/database/postgres/migrations/000110_drop_recommendations_8col_natural_key_test.go +++ b/internal/database/postgres/migrations/000110_drop_recommendations_8col_natural_key_test.go @@ -28,9 +28,7 @@ func TestMigration_DropRecommendations8ColNaturalKey(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() indexExists := func(name string) bool { @@ -49,7 +47,7 @@ func TestMigration_DropRecommendations8ColNaturalKey(t *testing.T) { require.True(t, indexExists("recommendations_natural_key_idx")) // A regional row and one zonal row exist before the migration. - _, err = pool.Exec(ctx, insertRecForMigration110, "2026-01-01T00:00:00Z", "regional", `{"details":{"scope":"Region"}}`) + _, err := pool.Exec(ctx, insertRecForMigration110, "2026-01-01T00:00:00Z", "regional", `{"details":{"scope":"Region"}}`) require.NoError(t, err) _, err = pool.Exec(ctx, insertRecForMigration110, "2026-01-01T00:00:00Z", "zonal", `{"availability_zone":"us-east-1a"}`) require.NoError(t, err) diff --git a/internal/database/postgres/migrations/000111_recommendations_platform_tenancy_test.go b/internal/database/postgres/migrations/000111_recommendations_platform_tenancy_test.go index ef3009be1..940660641 100644 --- a/internal/database/postgres/migrations/000111_recommendations_platform_tenancy_test.go +++ b/internal/database/postgres/migrations/000111_recommendations_platform_tenancy_test.go @@ -30,9 +30,7 @@ func TestMigration_RecommendationsPlatformTenancy(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, 110)) @@ -53,7 +51,7 @@ func TestMigration_RecommendationsPlatformTenancy(t *testing.T) { "top-level-ignored": {`{"platform":"Windows","tenancy":"host"}`, "", ""}, } for resourceType, v := range vectors { - _, err = pool.Exec(ctx, insertRecForMigration111, resourceType, v.payload) + _, err := pool.Exec(ctx, insertRecForMigration111, resourceType, v.payload) require.NoError(t, err) } diff --git a/internal/database/postgres/migrations/000112_plan_step_cap_auto_purchase_switch_test.go b/internal/database/postgres/migrations/000112_plan_step_cap_auto_purchase_switch_test.go index c02277fc1..9e5706c2e 100644 --- a/internal/database/postgres/migrations/000112_plan_step_cap_auto_purchase_switch_test.go +++ b/internal/database/postgres/migrations/000112_plan_step_cap_auto_purchase_switch_test.go @@ -22,9 +22,7 @@ func TestMigration_PlanStepCapAndAutoPurchaseSwitch(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, previousMigrationVersion(t, 112))) @@ -34,7 +32,7 @@ func TestMigration_PlanStepCapAndAutoPurchaseSwitch(t *testing.T) { INSERT INTO purchase_plans (name, enabled, auto_purchase, notification_days_before, services, ramp_schedule) VALUES ('pre-existing', true, true, 3, '{}'::jsonb, '{}'::jsonb) RETURNING id`).Scan(&planID)) - _, err = pool.Exec(ctx, `INSERT INTO global_config (id) VALUES (1) ON CONFLICT (id) DO NOTHING`) + _, err := pool.Exec(ctx, `INSERT INTO global_config (id) VALUES (1) ON CONFLICT (id) DO NOTHING`) require.NoError(t, err) require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, 112)) diff --git a/internal/database/postgres/migrations/000113_purchase_executions_no_submission_test.go b/internal/database/postgres/migrations/000113_purchase_executions_no_submission_test.go index 83edaefb4..4f0a8ac02 100644 --- a/internal/database/postgres/migrations/000113_purchase_executions_no_submission_test.go +++ b/internal/database/postgres/migrations/000113_purchase_executions_no_submission_test.go @@ -23,9 +23,7 @@ func TestMigration_PurchaseExecutionsNoSubmission(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, previousMigrationVersion(t, 113))) diff --git a/internal/database/postgres/migrations/000114_ladder_tranches_status_reason_test.go b/internal/database/postgres/migrations/000114_ladder_tranches_status_reason_test.go index 007a37869..d3909616c 100644 --- a/internal/database/postgres/migrations/000114_ladder_tranches_status_reason_test.go +++ b/internal/database/postgres/migrations/000114_ladder_tranches_status_reason_test.go @@ -22,9 +22,7 @@ func TestMigration_LadderTranchesStatusReason(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, previousMigrationVersion(t, 114))) diff --git a/internal/database/postgres/migrations/000115_auto_purchase_switch_stamps_test.go b/internal/database/postgres/migrations/000115_auto_purchase_switch_stamps_test.go index 297247f7e..e505f769c 100644 --- a/internal/database/postgres/migrations/000115_auto_purchase_switch_stamps_test.go +++ b/internal/database/postgres/migrations/000115_auto_purchase_switch_stamps_test.go @@ -23,13 +23,11 @@ func TestMigration_AutoPurchaseSwitchStamps(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, previousMigrationVersion(t, 115))) - _, err = pool.Exec(ctx, `INSERT INTO global_config (id) VALUES (1) ON CONFLICT (id) DO NOTHING`) + _, err := pool.Exec(ctx, `INSERT INTO global_config (id) VALUES (1) ON CONFLICT (id) DO NOTHING`) require.NoError(t, err) require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, 115)) diff --git a/internal/database/postgres/migrations/000117_ladder_tranches_settled_at_test.go b/internal/database/postgres/migrations/000117_ladder_tranches_settled_at_test.go index 72d809d82..1987d43a1 100644 --- a/internal/database/postgres/migrations/000117_ladder_tranches_settled_at_test.go +++ b/internal/database/postgres/migrations/000117_ladder_tranches_settled_at_test.go @@ -24,9 +24,7 @@ import ( func TestMigration_LadderTranchesSettledAt(t *testing.T) { ctx := context.Background() path := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() require.NoError(t, migrations.MigrateToVersion(ctx, pool, path, previousMigrationVersion(t, 117))) @@ -42,7 +40,7 @@ func TestMigration_LadderTranchesSettledAt(t *testing.T) { require.NoError(t, pool.QueryRow(ctx, `SELECT convalidated FROM pg_constraint WHERE conname = 'ladder_tranches_completed_has_settled_at'`).Scan(&validated)) assert.True(t, validated) - _, err = pool.Exec(ctx, `UPDATE ladder_tranches SET status_reason = 'x' WHERE id = $1`, ids.tranches["account"]) + _, err := pool.Exec(ctx, `UPDATE ladder_tranches SET status_reason = 'x' WHERE id = $1`, ids.tranches["account"]) require.NoError(t, err, "an unrelated update of a legacy completed row must succeed") _, err = pool.Exec(ctx, `DELETE FROM cloud_accounts WHERE id = $1`, ids.account) require.NoError(t, err, "deleting the account of a legacy completed tranche") diff --git a/internal/database/postgres/migrations/000118_purchase_plans_ramp_baseline_test.go b/internal/database/postgres/migrations/000118_purchase_plans_ramp_baseline_test.go index 653408cec..87fae5bd5 100644 --- a/internal/database/postgres/migrations/000118_purchase_plans_ramp_baseline_test.go +++ b/internal/database/postgres/migrations/000118_purchase_plans_ramp_baseline_test.go @@ -21,8 +21,7 @@ import ( func TestMigration_PurchasePlansRampBaseline(t *testing.T) { ctx := context.Background() path := getMigrationsPath() - container := testhelpers.RequirePostgresContainer(ctx, t) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() require.NoError(t, migrations.MigrateToVersion(ctx, pool, path, previousMigrationVersion(t, 118))) diff --git a/internal/database/postgres/migrations/000119_drop_recommendations_9col_natural_key_test.go b/internal/database/postgres/migrations/000119_drop_recommendations_9col_natural_key_test.go index 54cef7e9a..3fceb3aab 100644 --- a/internal/database/postgres/migrations/000119_drop_recommendations_9col_natural_key_test.go +++ b/internal/database/postgres/migrations/000119_drop_recommendations_9col_natural_key_test.go @@ -28,9 +28,7 @@ func TestMigration_DropRecommendations9ColNaturalKey(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() indexExists := func(name string) bool { @@ -56,7 +54,7 @@ func TestMigration_DropRecommendations9ColNaturalKey(t *testing.T) { require.True(t, indexExists("recommendations_natural_key_az_tp_idx")) // A regional row and one EC2 row exist before the migration. - _, err = pool.Exec(ctx, insertRecForMigration119, "2026-01-01T00:00:00Z", "regional", `{"details":{"scope":"Region"}}`) + _, err := pool.Exec(ctx, insertRecForMigration119, "2026-01-01T00:00:00Z", "regional", `{"details":{"scope":"Region"}}`) require.NoError(t, err) _, err = pool.Exec(ctx, insertRecForMigration119, "2026-01-01T00:00:00Z", "ec2", linux) require.NoError(t, err) diff --git a/internal/database/postgres/migrations/000120_recommendations_source_account_test.go b/internal/database/postgres/migrations/000120_recommendations_source_account_test.go index 3dd0be7a6..e2cd6dbd3 100644 --- a/internal/database/postgres/migrations/000120_recommendations_source_account_test.go +++ b/internal/database/postgres/migrations/000120_recommendations_source_account_test.go @@ -17,9 +17,7 @@ import ( func TestMigration_RecommendationsSourceAccount(t *testing.T) { ctx := context.Background() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, container.Cleanup(ctx)) }) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() path := getMigrationsPath() require.NoError(t, migrations.MigrateToVersion(ctx, pool, path, previousMigrationVersion(t, 120))) @@ -34,7 +32,7 @@ func TestMigration_RecommendationsSourceAccount(t *testing.T) { "verbatim": {`{"source_account_id":" A|β\t "}`, " A|β\t "}, } for name, v := range vectors { - _, err = pool.Exec(ctx, insert, name, v.payload) + _, err := pool.Exec(ctx, insert, name, v.payload) require.NoError(t, err) } require.NoError(t, migrations.MigrateToVersion(ctx, pool, path, 120)) @@ -54,7 +52,7 @@ func TestMigration_RecommendationsSourceAccount(t *testing.T) { assert.Contains(t, definition, "(account_key, provider, service, region, resource_type, engine, term, payment_option, availability_zone, platform, tenancy, source_account_key)") } } - _, err = pool.Exec(ctx, insert, "known", `{"source_account_id":"OtherSource"}`) + _, err := pool.Exec(ctx, insert, "known", `{"source_account_id":"OtherSource"}`) var constraint *pgconn.PgError require.ErrorAs(t, err, &constraint) assert.Equal(t, "23505", constraint.Code) diff --git a/internal/database/postgres/migrations/000121_ladder_tranche_amount_provenance_test.go b/internal/database/postgres/migrations/000121_ladder_tranche_amount_provenance_test.go index 11691b90c..e934aea13 100644 --- a/internal/database/postgres/migrations/000121_ladder_tranche_amount_provenance_test.go +++ b/internal/database/postgres/migrations/000121_ladder_tranche_amount_provenance_test.go @@ -13,8 +13,7 @@ import ( func TestMigration_LadderTrancheAmountProvenance(t *testing.T) { ctx := context.Background() - container := testhelpers.RequirePostgresContainer(ctx, t) - t.Cleanup(func() { require.NoError(t, container.Cleanup(context.Background())) }) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool, path := container.DB.Pool(), getMigrationsPath() require.NoError(t, migrations.MigrateToVersion(ctx, pool, path, 120)) var id string diff --git a/internal/database/postgres/migrations/000123_purchase_execution_fan_out_shapes_test.go b/internal/database/postgres/migrations/000123_purchase_execution_fan_out_shapes_test.go index b233ad958..966d315ad 100644 --- a/internal/database/postgres/migrations/000123_purchase_execution_fan_out_shapes_test.go +++ b/internal/database/postgres/migrations/000123_purchase_execution_fan_out_shapes_test.go @@ -24,16 +24,14 @@ func TestMigration_PurchaseExecutionFanOutShapes(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, previousMigrationVersion(t, 123))) fannedOut, singleOnly := uuid.NewString(), uuid.NewString() for _, key := range []string{fannedOut, fannedOut + ":" + uuid.NewString(), singleOnly} { - _, err = pool.Exec(ctx, ` + _, err := pool.Exec(ctx, ` INSERT INTO purchase_executions (status, scheduled_date, idempotency_key) VALUES ('failed', NOW(), $1)`, key) require.NoError(t, err) diff --git a/internal/database/postgres/migrations/000125_lock_default_user_groups_test.go b/internal/database/postgres/migrations/000125_lock_default_user_groups_test.go index f64fc2807..4e7152bc2 100644 --- a/internal/database/postgres/migrations/000125_lock_default_user_groups_test.go +++ b/internal/database/postgres/migrations/000125_lock_default_user_groups_test.go @@ -13,8 +13,7 @@ import ( func TestMigration_LockDefaultUserGroups(t *testing.T) { ctx := context.Background() - container := testhelpers.RequirePostgresContainer(ctx, t) - t.Cleanup(func() { require.NoError(t, container.Cleanup(context.Background())) }) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool, path := container.DB.Pool(), getMigrationsPath() ids := []string{ "00000000-0000-5000-8000-000000000005", // Standard Users diff --git a/internal/database/postgres/migrations/000126_purchase_history_active_expiry_idx_test.go b/internal/database/postgres/migrations/000126_purchase_history_active_expiry_idx_test.go index e4ab9a2d1..d5d6e3b68 100644 --- a/internal/database/postgres/migrations/000126_purchase_history_active_expiry_idx_test.go +++ b/internal/database/postgres/migrations/000126_purchase_history_active_expiry_idx_test.go @@ -17,8 +17,7 @@ import ( // removes it. func TestMigration_PurchaseHistoryActiveExpiryIndex(t *testing.T) { ctx := context.Background() - container := testhelpers.RequirePostgresContainer(ctx, t) - t.Cleanup(func() { require.NoError(t, container.Cleanup(context.Background())) }) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool, path := container.DB.Pool(), getMigrationsPath() indexDef := func() string { diff --git a/internal/database/postgres/migrations/000128_executions_detach_keeps_updated_at_test.go b/internal/database/postgres/migrations/000128_executions_detach_keeps_updated_at_test.go index 206b15eac..01dbb23f5 100644 --- a/internal/database/postgres/migrations/000128_executions_detach_keeps_updated_at_test.go +++ b/internal/database/postgres/migrations/000128_executions_detach_keeps_updated_at_test.go @@ -19,8 +19,7 @@ import ( // down migration restores the stamp-everything trigger (issue #265). func TestMigration_ExecutionsDetachKeepsUpdatedAt(t *testing.T) { ctx := context.Background() - container := testhelpers.RequirePostgresContainer(ctx, t) - t.Cleanup(func() { require.NoError(t, container.Cleanup(context.Background())) }) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool, path := container.DB.Pool(), getMigrationsPath() const acct = "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb" diff --git a/internal/database/postgres/migrations/000129_recommendations_state_partial_test.go b/internal/database/postgres/migrations/000129_recommendations_state_partial_test.go index d2763d9ee..fecb4f5d2 100644 --- a/internal/database/postgres/migrations/000129_recommendations_state_partial_test.go +++ b/internal/database/postgres/migrations/000129_recommendations_state_partial_test.go @@ -22,9 +22,7 @@ func TestMigration_RecommendationsStatePartial(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() const version = 129 @@ -50,7 +48,7 @@ func TestMigration_RecommendationsStatePartial(t *testing.T) { require.NoError(t, migrations.MigrateToVersion(ctx, pool, migrationsPath, version)) var dataType, isNullable string - err = pool.QueryRow(ctx, ` + err := pool.QueryRow(ctx, ` SELECT data_type, is_nullable FROM information_schema.columns WHERE table_name = 'recommendations_state' diff --git a/internal/database/postgres/migrations/000130_purchase_executions_created_by_system_test.go b/internal/database/postgres/migrations/000130_purchase_executions_created_by_system_test.go index 89db493da..ad8e5f1d5 100644 --- a/internal/database/postgres/migrations/000130_purchase_executions_created_by_system_test.go +++ b/internal/database/postgres/migrations/000130_purchase_executions_created_by_system_test.go @@ -14,8 +14,7 @@ import ( // TestMigration_PurchaseExecutionsCreatedBySystem covers 000130 up, down and the CHECK (#233). func TestMigration_PurchaseExecutionsCreatedBySystem(t *testing.T) { ctx := context.Background() - container := testhelpers.RequirePostgresContainer(ctx, t) - t.Cleanup(func() { require.NoError(t, container.Cleanup(context.Background())) }) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool, path := container.DB.Pool(), getMigrationsPath() hasColumn := func() bool { diff --git a/internal/database/postgres/migrations/000131_ri_exchange_direct_executions_test.go b/internal/database/postgres/migrations/000131_ri_exchange_direct_executions_test.go index 7a0d1fb20..a5c7fe093 100644 --- a/internal/database/postgres/migrations/000131_ri_exchange_direct_executions_test.go +++ b/internal/database/postgres/migrations/000131_ri_exchange_direct_executions_test.go @@ -24,9 +24,7 @@ func TestMigration_RIExchangeDirectExecutions(t *testing.T) { ctx := context.Background() migrationsPath := getMigrationsPath() - container, err := testhelpers.SetupPostgresContainer(ctx, t) - require.NoError(t, err) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() tableExists := func() bool { diff --git a/internal/database/postgres/migrations/000132_savings_partition_utc_bounds_test.go b/internal/database/postgres/migrations/000132_savings_partition_utc_bounds_test.go index 1c773c0a4..fbc11fbf7 100644 --- a/internal/database/postgres/migrations/000132_savings_partition_utc_bounds_test.go +++ b/internal/database/postgres/migrations/000132_savings_partition_utc_bounds_test.go @@ -115,8 +115,7 @@ func utc(s string) time.Time { // old function cut at local midnight are rebuilt with their rows re-routed. func TestMigration_SavingsPartitionUTCBounds(t *testing.T) { ctx := context.Background() - container := testhelpers.RequirePostgresContainer(ctx, t) - t.Cleanup(func() { require.NoError(t, container.Cleanup(context.Background())) }) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool, path := container.DB.Pool(), getMigrationsPath() prev := previousMigrationVersion(t, utcPartitionBoundsVersion) @@ -208,8 +207,7 @@ func TestMigration_SavingsPartitionUTCBounds(t *testing.T) { // already exists". func TestMigration_SavingsPartitionConcurrentCreate(t *testing.T) { ctx := context.Background() - container := testhelpers.RequirePostgresContainer(ctx, t) - t.Cleanup(func() { require.NoError(t, container.Cleanup(context.Background())) }) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() require.NoError(t, migrations.MigrateToVersion(ctx, pool, getMigrationsPath(), utcPartitionBoundsVersion)) diff --git a/internal/database/postgres/migrations/000133_purchase_history_reverify_ec2_offering_class_test.go b/internal/database/postgres/migrations/000133_purchase_history_reverify_ec2_offering_class_test.go index d7d9215ac..0e6eadd7a 100644 --- a/internal/database/postgres/migrations/000133_purchase_history_reverify_ec2_offering_class_test.go +++ b/internal/database/postgres/migrations/000133_purchase_history_reverify_ec2_offering_class_test.go @@ -52,8 +52,7 @@ func TestMigration133_ClearsConvertibleEC2Stamps(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) defer cancel() - container := testhelpers.RequirePostgresContainer(ctx, t) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() migrationsPath := getMigrationsPath() @@ -104,8 +103,7 @@ func TestMigration133_ConcurrentStandardStampWins(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) defer cancel() - container := testhelpers.RequirePostgresContainer(ctx, t) - defer container.Cleanup(ctx) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool := container.DB.Pool() migrationsPath := getMigrationsPath() diff --git a/internal/database/postgres/migrations/000134_purchase_suppressions_variant_test.go b/internal/database/postgres/migrations/000134_purchase_suppressions_variant_test.go index 81860b7b4..314c48ac4 100644 --- a/internal/database/postgres/migrations/000134_purchase_suppressions_variant_test.go +++ b/internal/database/postgres/migrations/000134_purchase_suppressions_variant_test.go @@ -46,8 +46,7 @@ func requireUniqueViolation134(t *testing.T, err error, constraint string) { // merges variant rows back into one row per execution and 6-tuple. func TestMigration_PurchaseSuppressionsVariant(t *testing.T) { ctx := context.Background() - container := testhelpers.RequirePostgresContainer(ctx, t) - t.Cleanup(func() { require.NoError(t, container.Cleanup(context.Background())) }) + container := testhelpers.RequireEmptyDatabase(ctx, t) pool, path := container.DB.Pool(), getMigrationsPath() prev := previousMigrationVersion(t, 134)