[thc1006]

dbtest.Start keeps the caller's context and uses it again in the cleanup it returns. A test that passes t.Context() gets a context that is canceled just before the t.Cleanup callbacks run, so Terminate fails and the container keeps running:

warning: failed to terminate postgres container: stop: container stop: Post "http://%2Frun%2Fuser%2F1000%2Fpodman%2Fpodman.sock/v1.41/containers/aca6308c635b.../stop?t=10": context canceled

The database package's TestMain doesn't terminate its container at all. It drops the cleanup func from dbtest.Start and then calls os.Exit.

Locally, Ryuk removes these containers after the test run, so this is easy to miss. #2554 turned Ryuk off in CI because of reaper timeouts, noting that dbtest "already terminates its containers via t.Cleanup". For these callers it doesn't, so in CI the containers stay up until the runner goes away. On main, internal/database plus the controller and grpcserver tests that pass t.Context() leave 23 Postgres containers running: 1 from TestMain, 14 from the controller tests, and 8 from TestScheduledRun* and TestSessionDeletionForceThroughGeneratedClient. The sandbox and task store fixtures avoid it by passing context.WithoutCancel(t.Context()).

This PR terminates with testcontainers.TerminateContainer, which doesn't use the caller's context. It also terminates the container when postgres.Run fails, since Run returns the container it created together with the error. TestMain now runs the tests from a helper that returns the exit code, so the deferred cleanup runs before os.Exit. Fixtures that already pass context.WithoutCancel(t.Context()) keep working as before.

If a database test panics, the shared container from TestMain is still left behind, and -timeout skips all cleanup.

Testing

With rootless podman and TESTCONTAINERS_RYUK_DISABLED=true, I counted podman ps -a --filter label=org.testcontainers=true after:

go test -race ./core/internal/database/
go test ./core/internal/controller/ -run 'TestUnresolvedPoolReleasesAbandonedRevision|TestRuntimeRevisionGC|TestReferencePolicyFailurePreservesLastSuccessfulRevision'
go test -race ./core/internal/grpcserver/ -run 'TestScheduledRun|TestSessionDeletion'

Main leaves 23 running. This branch leaves none running. go test -race also passes for the other packages that use dbtest and for pkg/migrations, with envtest set up as in CI, and make -C go lint is clean.

TestStartTTerminatesContainerAfterTestContextIsCanceled fails on main and passes here. It adds one container start per run, about 5s. If you'd rather not have a test for a test helper, I'll drop it.

pkg/migrations/runner_test.go has its own copy of the startup code, startTestDB, which also leaks the container when Run fails. It can't call dbtest (dbtest imports migrations), so it needs its own fix, which I left for a separate PR.

[github-actions]

This pull request has been marked as stale because of no activity in the last 15 days. It will be closed in the next 5 days unless it is tagged "no stalebot" or other activity occurs.

[thc1006]

Dear @EItanya, this is still needed on main (92ac08a): the cleanup from dbtest.Start still terminates with the caller's context (dbtest.go#L49), and TestMain in internal/database still drops it (testhelpers_test.go#L26). With the reaper off in CI since #2554, those containers stay up until the runner goes away.

The branch now conflicts with main in dbtest.go only. Happy to rebase when you have time to review. Thanks!

[github-actions]

This pull request has been marked as stale because of no activity in the last 15 days. It will be closed in the next 5 days unless it is tagged "no stalebot" or other activity occurs.