Skip to content

Overhaul test.CreateTestDb to use testing.T idioms#78

Merged
neekolas merged 2 commits intoxmtp:mainfrom
xmtp-coder-agent:fix/issue-74
Mar 25, 2026
Merged

Overhaul test.CreateTestDb to use testing.T idioms#78
neekolas merged 2 commits intoxmtp:mainfrom
xmtp-coder-agent:fix/issue-74

Conversation

@xmtp-coder-agent
Copy link
Copy Markdown
Collaborator

@xmtp-coder-agent xmtp-coder-agent commented Mar 25, 2026

Resolves #74

Summary

  • test/helpers.go: CreateTestDb now accepts *testing.T, uses t.Context() for context, registers cleanup via t.Cleanup(), and calls t.Fatal() on migration errors. Returns only *bun.DB.
  • All callers updated: Replaced db, cleanup := test.CreateTestDb() / defer cleanup() with db := test.CreateTestDb(t) across installations_test.go, subscriptions_test.go, and listener_test.go.
  • Fixed Test_Get: Previously had cleanup commented out — now handled automatically by t.Cleanup.

Test plan

  • go vet ./... passes (verified locally)
  • go test -p 1 ./... passes with Docker services running
  • All existing test behavior is preserved — cleanup now runs automatically via t.Cleanup

🤖 Generated with Claude Code

Note

Overhaul test.CreateTestDb to use testing.T idioms

  • Changes CreateTestDb signature from () (*bun.DB, func()) to (t *testing.T) *bun.DB in test/helpers.go, eliminating the manual cleanup return value.
  • The helper now uses t.Cleanup to truncate DB tables automatically and calls t.Fatal if migration fails, rather than silently ignoring errors.
  • All callers in installations_test.go, subscriptions_test.go, and listener_test.go are updated to drop their defer cleanup() calls.
  • Behavioral Change: migration errors now immediately fail the test instead of being silently ignored.

Macroscope summarized bc8539c.

- Accept *testing.T parameter instead of returning cleanup function
- Use t.Context() for context instead of context.Background()
- Register cleanup via t.Cleanup() instead of returning a closure
- Use t.Fatal() for migration error handling
- Update all callers across 3 test files to use new signature

Resolves xmtp#74

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@xmtp-coder-agent xmtp-coder-agent requested a review from a team as a code owner March 25, 2026 06:44
t.Context() is cancelled when the test ends, but t.Cleanup runs after
that — so truncation queries were failing with "context canceled",
causing data to leak between tests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
}

t.Cleanup(func() {
cleanupCtx := context.Background()
Copy link
Copy Markdown
Collaborator

@neekolas neekolas Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you double check that t.Context() isn't available in cleanup scripts? Point me to the relevant docs.

@xmtp-coder-agent
Copy link
Copy Markdown
Collaborator Author

Re: comment — Yes, confirmed. t.Context() is intentionally cancelled before cleanup functions run. From the official docs:

Context returns a context that is canceled just before Cleanup-registered functions are called.

This is also in the Go 1.24 release notes:

The new T.Context and B.Context methods return a context that's canceled after the test completes and before test cleanup functions run.

The CI failure confirmed this — all four TRUNCATE TABLE queries failed with context canceled, causing data to leak between tests (Test_UnsubscribeResubscribe saw 4 rows instead of 1).

Using context.Background() in the cleanup function is the correct approach here.

@neekolas neekolas merged commit 2486aaa into xmtp:main Mar 25, 2026
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Overhaul test.CreateTestDb

2 participants