test: share one Postgres server per package across integration tests

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

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%

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)
+}

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)
 

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)