diff --git a/cmd/cleanup-lambda/main_integration_test.go b/cmd/cleanup-lambda/main_integration_test.go index 4f0afbcde..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" @@ -17,7 +19,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" ) @@ -33,14 +34,27 @@ 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() - 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/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/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) 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"}} 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 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) { 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/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) 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), 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") +} 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) }) 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%'`, 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()) 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})