Extract e2e tests into e2e_test.go (#716 subtask 3)#728
Merged
Conversation
714e797 to
2812bcb
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR continues the migration test-suite reorganization by extracting a set of general “happy-path” end-to-end Runner lifecycle tests out of runner_test.go into a dedicated e2e_test.go, while also refactoring them to use the newer test helpers (NewTestTable, NewTestRunner, NewTestMigration, SeedRows, etc.).
Changes:
- Moved a set of general E2E tests from
pkg/migration/runner_test.gointo the newpkg/migration/e2e_test.go. - Refactored extracted tests to use the new test helper APIs and fail-fast assertions (
require.*). - Reduced
runner_test.gosize by removing the extracted test cases and related unused imports.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/migration/runner_test.go | Removes extracted E2E tests and cleans up imports accordingly. |
| pkg/migration/e2e_test.go | Adds a dedicated E2E test file with refactored tests using NewTestTable/NewTestRunner/SeedRows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Move 16 general E2E tests from runner_test.go into e2e_test.go: - TestVarcharNonBinaryComparable - TestPartitioningSyntax - TestVarbinary - TestDataFromBadSqlMode - TestOnline (7 sub-scenarios) - TestTableLength - TestCreateTableNameLength - TestAddUniqueIndexChecksumEnabled - TestChangeNonIntPK - TestDropColumn - TestPartitionedTable - TestVarcharE2E - TestChunkerPrefetching - TestPreRunChecksE2E - TestPreventConcurrentRuns - TestMigrationCancelledFromTableModification Conversions: - All table setup converted to NewTestTable (removes manual DROP/RunSQL blocks) - All NewRunner calls converted to NewTestRunner/NewTestMigration - Data seeding converted to SeedRows where applicable - assert.NoError converted to require.NoError (fail-fast) - mysql.ParseDSN eliminated from all converted tests - TestOnline restructured with numbered sub-scenarios and comments runner_test.go: 2399 -> 1639 lines (-32%) e2e_test.go: 451 lines (new, 42% smaller than pure move would be) Fixes block#720 Part of block#716 (test reorganization)
2812bcb to
cbbfdaf
Compare
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
Moves 16 general E2E tests from
runner_test.gointo a focusede2e_test.go.These are happy-path end-to-end tests that exercise the full
Runner.Run()lifecycle:TestVarcharNonBinaryComparableTestPartitioningSyntaxTestVarbinaryTestDataFromBadSqlModeTestOnline(7 sub-scenarios for DDL algorithm detection)TestTableLengthTestCreateTableNameLengthTestAddUniqueIndexChecksumEnabledTestChangeNonIntPKTestDropColumnTestPartitionedTableTestVarcharE2ETestChunkerPrefetchingTestPreRunChecksE2ETestPreventConcurrentRunsTestMigrationCancelledFromTableModificationConversions
NewTestTable(removes manualDROP TABLE+RunSQLblocks)NewRunnercalls converted toNewTestRunner/NewTestMigration/NewTestRunnerFromStatementSeedRowswhere applicableassert.NoErrorconverted torequire.NoError(fail-fast)mysql.ParseDSNeliminated from all converted testsTestOnlinerestructured with numbered sub-scenarios and commentsImpact
runner_test.go: 2,399 → 1,639 lines (-32%)e2e_test.go: 451 lines (new — 42% smaller than a pure move would be)Fixes #720
Part of #716 (test reorganization)