From b6ed68bfa1cc22693e5c794b74d61cec40eaa71c Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Mon, 5 Oct 2026 23:46:30 +0800 Subject: [PATCH 1/2] test(dbtest): terminate containers after the caller's context is canceled Start captured the caller's ctx in the cleanup closure. Callers that pass t.Context() get a context that is canceled before t.Cleanup callbacks run, so Terminate failed with "context canceled" and the container kept running. With Ryuk disabled, as in CI since #2554, nothing removes it. Terminate with testcontainers.TerminateContainer, which does not use the caller's context. Also terminate the container when startup fails, since postgres.Run returns the created container together with the error. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- go/core/internal/dbtest/dbtest.go | 10 ++++++---- go/core/internal/dbtest/dbtest_test.go | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/go/core/internal/dbtest/dbtest.go b/go/core/internal/dbtest/dbtest.go index 492b9d0a2c..7b2f447c24 100644 --- a/go/core/internal/dbtest/dbtest.go +++ b/go/core/internal/dbtest/dbtest.go @@ -4,6 +4,7 @@ package dbtest import ( "context" "database/sql" + "errors" "fmt" "net/url" "strings" @@ -40,17 +41,18 @@ func Start(ctx context.Context) (connStr string, cleanup func(), err error) { ), ) if err != nil { - return "", nil, fmt.Errorf("starting postgres container: %w", err) + // Run returns the created container when it fails to start. + return "", nil, errors.Join(fmt.Errorf("starting postgres container: %w", err), testcontainers.TerminateContainer(pgContainer)) } connStr, err = pgContainer.ConnectionString(ctx, "sslmode=disable") if err != nil { - _ = pgContainer.Terminate(ctx) - return "", nil, fmt.Errorf("getting connection string: %w", err) + return "", nil, errors.Join(fmt.Errorf("getting connection string: %w", err), testcontainers.TerminateContainer(pgContainer)) } cleanup = func() { - if err := pgContainer.Terminate(ctx); err != nil { + // Not ctx: a test context is canceled before cleanup runs. + if err := testcontainers.TerminateContainer(pgContainer); err != nil { fmt.Printf("warning: failed to terminate postgres container: %v\n", err) } } diff --git a/go/core/internal/dbtest/dbtest_test.go b/go/core/internal/dbtest/dbtest_test.go index 614f5fdf80..8fd24fec19 100644 --- a/go/core/internal/dbtest/dbtest_test.go +++ b/go/core/internal/dbtest/dbtest_test.go @@ -3,6 +3,7 @@ package dbtest_test import ( "context" "testing" + "time" "github.com/jackc/pgx/v5" "github.com/stretchr/testify/require" @@ -57,3 +58,22 @@ func TestClonedDatabasesAreIsolatedAndRemoved(t *testing.T) { require.NoError(t, template.QueryRow(ctx, "SELECT count(*) FROM pg_database WHERE datname = ANY($1)", databaseNames).Scan(&count)) require.Zero(t, count, "test cleanup must remove every clone") } + +func TestStartTTerminatesContainerAfterTestContextIsCanceled(t *testing.T) { + if testing.Short() { + t.Skip("requires PostgreSQL") + } + var connStr string + require.True(t, t.Run("fixture", func(t *testing.T) { + // t.Context is canceled before the fixture's cleanup runs. + connStr = dbtest.StartT(t.Context(), t) + conn, err := pgx.Connect(t.Context(), connStr) + require.NoError(t, err) + require.NoError(t, conn.Close(t.Context())) + })) + + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + _, err := pgx.Connect(ctx, connStr) + require.Error(t, err, "postgres container still accepts connections after cleanup") +} From 4c6665e7577ef038dca6ecd7f6148cd145526226 Mon Sep 17 00:00:00 2001 From: thc1006 <84045975+thc1006@users.noreply.github.com> Date: Mon, 5 Oct 2026 23:46:30 +0800 Subject: [PATCH 2/2] test(database): terminate the shared Postgres container after TestMain TestMain discarded the cleanup returned by dbtest.Start, so the shared container was never terminated. Deferring it in TestMain would not help because os.Exit skips deferred calls. Run the tests from a helper that returns the exit code instead. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com> --- go/core/internal/database/testhelpers_test.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/go/core/internal/database/testhelpers_test.go b/go/core/internal/database/testhelpers_test.go index 574b92ce50..0c6e775783 100644 --- a/go/core/internal/database/testhelpers_test.go +++ b/go/core/internal/database/testhelpers_test.go @@ -22,25 +22,30 @@ func TestMain(m *testing.M) { if testing.Short() { os.Exit(m.Run()) } + os.Exit(runWithDatabase(m)) +} - connStr, _, err := dbtest.Start(context.Background()) +// runWithDatabase returns the exit code so deferred cleanup runs before os.Exit. +func runWithDatabase(m *testing.M) int { + connStr, cleanup, err := dbtest.Start(context.Background()) if err != nil { fmt.Fprintf(os.Stderr, "failed to start postgres container: %v\n", err) - os.Exit(1) + return 1 } + defer cleanup() sharedConnStr = connStr if err := dbtest.Migrate(connStr, true); err != nil { fmt.Fprintf(os.Stderr, "failed to migrate test database: %v\n", err) - os.Exit(1) + return 1 } db, err := Connect(context.Background(), &PostgresConfig{Pool: dbtest.PoolConfig(connStr), VectorEnabled: true, Schema: "public", VectorSchema: "public"}) if err != nil { fmt.Fprintf(os.Stderr, "failed to connect to test database: %v\n", err) - os.Exit(1) + return 1 } sharedDB = db - os.Exit(m.Run()) + return m.Run() }