Rename and convert runner_resume_test.go to resume_test.go (#716 subtask 8)#732
Merged
Conversation
…subtask 8) Rename runner_resume_test.go to resume_test.go and convert all 16 tests to use the new test helpers where applicable. Conversions: - E2E tests use NewTestTable + SeedRows + NewTestRunner - Manual-stepping tests use NewTestTable for setup, keep NewRunner for internals - assert converted to require for setup/teardown - Goroutine synchronization improved: done channels ensure goroutines finish before test cleanup (fixes potential races) - mysql.ParseDSN reduced from 16 to 5 (only in manual-stepping tests) - CreateUniqueTestDatabase tests use WithDBName option runner_resume_test.go: DELETED (1400 lines) resume_test.go: 1135 lines (new, -19%) Fixes block#725 Part of block#716 (test reorganization)
morgo
force-pushed
the
extract-resume-tests
branch
from
6df0cbf to
7356a51
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the checkpoint/resume migration test suite by consolidating it under resume_test.go and updating tests to use the newer table/runner test helpers, with improved goroutine lifecycle synchronization to reduce race potential during cleanup.
Changes:
- Renamed/reorganized the checkpoint/resume tests into
resume_test.goand removed the old runner-focused test file. - Converted many tests to use
testutils.NewTestTable+SeedRowsandNewTestRunner/RunnerOptionhelpers to reduce boilerplate. - Added
donechannels in async test flows soRun()goroutines finish before test cleanup proceeds.
Reviewed changes
Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/migration/resume_test.go | Converts/updates checkpoint & resume tests to use shared helpers and adds goroutine synchronization. |
| pkg/migration/runner_resume_test.go | Deleted as part of the rename/reorganization. |
Comments suppressed due to low confidence (2)
pkg/migration/resume_test.go:70
- The comment says resources are freed "between cancel and Close()", but the code calls Close() before cancel(). Please update the comment to match the actual ordering (or switch the ordering if the intent is cancel-then-Close).
pkg/migration/resume_test.go:958 - This information_schema query hard-codes TABLE_SCHEMA = 'test'. If MYSQL_DSN points at a different database, the test will fail even though the tables were created in cfg.DBName. Prefer using the parsed DSN DB name (cfg.DBName) or querying TABLE_SCHEMA = DATABASE() for the current connection.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
morgo
force-pushed
the
extract-resume-tests
branch
from
b1a3666 to
8054bae
Compare
morgo
force-pushed
the
extract-resume-tests
branch
from
8054bae to
dfb64bb
Compare
aparajon
approved these changes
Apr 30, 2026
morgo
added a commit
to morgo/spirit
that referenced
this pull request
Apr 30, 2026
- Merge alter_pk_datatype_test.go (483 lines) into datatype_test.go (+205 lines) - Rewrite check_constraint_test.go: 384 -> 223 lines (-42%) - Rewrite lint_test.go: 462 -> 224 lines (-51%) - Convert change_test.go: 374 -> 341 lines (-9%) - Convert rename_column_test.go: 514 -> 468 lines (-9%) - Convert migration_test.go: 1269 -> 957 lines (-25%) - Add WithLint() and WithLintOnly() helper options mysql.ParseDSN eliminated from all files except: - migration_test.go (2 remaining: TestBadOptions + TestDSN test config infra) - runner_resume_test.go (handled by PR block#732) - cutover_test.go (4: CreateUniqueTestDatabase tests) - replica_tls_*_test.go (9: TLS-specific) - helpers_test.go (1: the helper itself) All assert converted to require. TABLE_SCHEMA='test' replaced with DATABASE(). Part of block#716 (test reorganization)
morgo
added a commit
to morgo/spirit
that referenced
this pull request
Apr 30, 2026
- Merge alter_pk_datatype_test.go (483 lines) into datatype_test.go (+205 lines) - Rewrite check_constraint_test.go: 384 -> 223 lines (-42%) - Rewrite lint_test.go: 462 -> 224 lines (-51%) - Convert change_test.go: 374 -> 341 lines (-9%) - Convert rename_column_test.go: 514 -> 468 lines (-9%) - Convert migration_test.go: 1269 -> 957 lines (-25%) - Add WithLint() and WithLintOnly() helper options mysql.ParseDSN eliminated from all files except: - migration_test.go (2 remaining: TestBadOptions + TestDSN test config infra) - runner_resume_test.go (handled by PR block#732) - cutover_test.go (4: CreateUniqueTestDatabase tests) - replica_tls_*_test.go (9: TLS-specific) - helpers_test.go (1: the helper itself) All assert converted to require. TABLE_SCHEMA='test' replaced with DATABASE(). Part of block#716 (test reorganization)
morgo
added a commit
to morgo/spirit
that referenced
this pull request
Apr 30, 2026
- Merge alter_pk_datatype_test.go (483 lines) into datatype_test.go (+205 lines) - Rewrite check_constraint_test.go: 384 -> 223 lines (-42%) - Rewrite lint_test.go: 462 -> 224 lines (-51%) - Convert change_test.go: 374 -> 341 lines (-9%) - Convert rename_column_test.go: 514 -> 468 lines (-9%) - Convert migration_test.go: 1269 -> 957 lines (-25%) - Add WithLint() and WithLintOnly() helper options mysql.ParseDSN eliminated from all files except: - migration_test.go (2 remaining: TestBadOptions + TestDSN test config infra) - runner_resume_test.go (handled by PR block#732) - cutover_test.go (4: CreateUniqueTestDatabase tests) - replica_tls_*_test.go (9: TLS-specific) - helpers_test.go (1: the helper itself) All assert converted to require. TABLE_SCHEMA='test' replaced with DATABASE(). Part of block#716 (test reorganization)
morgo
added a commit
to morgo/spirit
that referenced
this pull request
Apr 30, 2026
- Merge alter_pk_datatype_test.go (483 lines) into datatype_test.go (+205 lines) - Rewrite check_constraint_test.go: 384 -> 223 lines (-42%) - Rewrite lint_test.go: 462 -> 224 lines (-51%) - Convert change_test.go: 374 -> 341 lines (-9%) - Convert rename_column_test.go: 514 -> 468 lines (-9%) - Convert migration_test.go: 1269 -> 957 lines (-25%) - Add WithLint() and WithLintOnly() helper options mysql.ParseDSN eliminated from all files except: - migration_test.go (2 remaining: TestBadOptions + TestDSN test config infra) - runner_resume_test.go (handled by PR block#732) - cutover_test.go (4: CreateUniqueTestDatabase tests) - replica_tls_*_test.go (9: TLS-specific) - helpers_test.go (1: the helper itself) All assert converted to require. TABLE_SCHEMA='test' replaced with DATABASE(). Part of block#716 (test reorganization)
morgo
added a commit
to morgo/spirit
that referenced
this pull request
Apr 30, 2026
- Merge alter_pk_datatype_test.go (483 lines) into datatype_test.go (+205 lines) - Rewrite check_constraint_test.go: 384 -> 223 lines (-42%) - Rewrite lint_test.go: 462 -> 224 lines (-51%) - Convert change_test.go: 374 -> 341 lines (-9%) - Convert rename_column_test.go: 514 -> 468 lines (-9%) - Convert migration_test.go: 1269 -> 957 lines (-25%) - Add WithLint() and WithLintOnly() helper options mysql.ParseDSN eliminated from all files except: - migration_test.go (2 remaining: TestBadOptions + TestDSN test config infra) - runner_resume_test.go (handled by PR block#732) - cutover_test.go (4: CreateUniqueTestDatabase tests) - replica_tls_*_test.go (9: TLS-specific) - helpers_test.go (1: the helper itself) All assert converted to require. TABLE_SCHEMA='test' replaced with DATABASE(). Part of block#716 (test reorganization)
morgo
added a commit
to morgo/spirit
that referenced
this pull request
Apr 30, 2026
- Merge alter_pk_datatype_test.go (483 lines) into datatype_test.go (+205 lines) - Rewrite check_constraint_test.go: 384 -> 223 lines (-42%) - Rewrite lint_test.go: 462 -> 224 lines (-51%) - Convert change_test.go: 374 -> 341 lines (-9%) - Convert rename_column_test.go: 514 -> 468 lines (-9%) - Convert migration_test.go: 1269 -> 957 lines (-25%) - Add WithLint() and WithLintOnly() helper options mysql.ParseDSN eliminated from all files except: - migration_test.go (2 remaining: TestBadOptions + TestDSN test config infra) - runner_resume_test.go (handled by PR block#732) - cutover_test.go (4: CreateUniqueTestDatabase tests) - replica_tls_*_test.go (9: TLS-specific) - helpers_test.go (1: the helper itself) All assert converted to require. TABLE_SCHEMA='test' replaced with DATABASE(). Part of block#716 (test reorganization)
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
Rename
runner_resume_test.gotoresume_test.goand convert all 16 checkpoint/resume tests to use the new test helpers where applicable.Conversions:
NewTestTable+SeedRows+NewTestRunnerNewTestTablefor setup, keepNewRunnerfor internalsassertconverted torequirefor setup/teardowndonechannels ensure goroutines finish before test cleanup (fixes potential races)mysql.ParseDSNreduced from 16 to 5 (only in manual-stepping tests)CreateUniqueTestDatabasetests useWithDBNameoptionImpact:
runner_resume_test.go: DELETED (1,400 lines)resume_test.go: 1,135 lines (new, -19%)mysql.ParseDSN: 16 → 5Fixes #725
Part of #716 (test reorganization)