Add test helpers and infrastructure#717
Merged
Merged
Conversation
Add reusable test helpers to reduce boilerplate in migration tests: pkg/testutils/table.go: - NewTestTable: creates tables with automatic artifact cleanup - SeedRows: populates tables via INSERT...SELECT doubling - TestTable.DB: provides *sql.DB for verification queries pkg/migration/helpers_test.go: - NewTestRunner: creates Runner from table+alter with sensible defaults - NewTestRunnerFromStatement: creates Runner from full SQL statement - NewTestMigration: creates Migration struct for direct API testing - With* options: WithThreads, WithBuffered, WithStatement, etc. - Moved mkPtr, mkIniFile, waitForStatus from other test files AGENTS.md: - Documented all new helpers with examples and usage patterns
morgo
force-pushed
the
test-helpers-infrastructure
branch
from
eac436e to
4a361be
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds reusable helper utilities to simplify and standardize Spirit’s migration/integration tests as the first step of the broader migration test overhaul (#716).
Changes:
- Added
pkg/testutils.TestTablewith lifecycle management (NewTestTable) and row seeding (SeedRows). - Added
pkg/migration/helpers_test.gowith migration/runner construction helpers and functional options; movedmkPtr,mkIniFile, andwaitForStatusinto this shared helper. - Documented the preferred test patterns and new helpers in
AGENTS.md.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/testutils/table.go | New TestTable helper to create/cleanup Spirit tables and seed rows for integration tests. |
| pkg/migration/runner_test.go | Removed local waitForStatus (moved to shared test helper). |
| pkg/migration/migration_test.go | Removed local mkPtr/mkIniFile (moved to shared test helper). |
| pkg/migration/helpers_test.go | New migration test helper constructors + functional options and shared utilities. |
| AGENTS.md | Added documentation for NewTestTable/SeedRows and migration runner helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Register t.Cleanup before createSQL so DB is always closed on failure - Use context.Background() in cleanup drops (t.Context() is canceled) - Use require.NoError in SeedRows loop to fail fast on insert errors - Only ignore identifier-too-long errors in dropArtifacts, not all errors
aparajon
approved these changes
Apr 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #718
Adds reusable test helpers to reduce boilerplate in migration tests. No existing test behavior is changed — this PR only:
mkPtr,mkIniFile,waitForStatustohelpers_test.go(same package, no behavior change)New files
pkg/testutils/table.go— Table lifecycle management:NewTestTable(t, name, createSQL)— creates table, registers cleanup for all Spirit artifactsSeedRows(t, insertSelectSQL, targetRows)— populates via INSERT...SELECT doublingTestTable.DB—*sql.DBfor verification queriespkg/migration/helpers_test.go— Migration test helpers:NewTestRunner(t, table, alter, opts...)— replaces ~15 lines ofmysql.ParseDSN+NewRunner(&Migration{...})NewTestRunnerFromStatement(t, stmt, opts...)— for CREATE INDEX, full ALTER TABLE, etc.NewTestMigration(t, opts...)— for tests callingMigration.Run()directlyWith*option functions for configurationmkPtr,mkIniFile,waitForStatusfrom other test filesTracking
Part of #716. Subsequent PRs will convert existing tests to use these helpers.