Add copier test, upgrade testcontainers#1567
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new copier integration test for the Migrator component, standardizes and refactors existing tests to use shared test utilities, and upgrades Testcontainers (and other dependencies) to v0.37.0.
- Introduce
getTest*helpers andnewTestMigrationContextin test_utils.go - Refactor streamer, migrator, and applier tests to use the MySQL module and shared helpers
- Add
TestCopierIntPKto validate the copier logic and bump testcontainers-go to v0.37.0
Reviewed Changes
Copilot reviewed 5 out of 473 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| go/logic/test_utils.go | Add constants and helper functions for DB/table names and connection contexts |
| go/logic/streamer_test.go | Refactor to use mysql.Run, shared utils, and table helpers |
| go/logic/migrator_test.go | Rename tests, inject newTestMigrationContext, add TestCopierIntPK |
| go/logic/applier_test.go | Refactor to use shared utils for container setup and teardown |
| go.mod | Upgrade github.com/testcontainers/testcontainers-go and related modules to v0.37.0 |
Comments suppressed due to low confidence (2)
go/logic/migrator_test.go:435
- The database name in the
SHOW TABLESquery is still hardcoded totest. UsetestMysqlDatabaseinstead of the literaltestto stay consistent with the helper constants.
default:
go/logic/migrator_test.go:397
- Using
rangedirectly on anint64(numRows) is invalid in Go and will not compile. Change to an indexed loop, e.g.for i := int64(0); i < numRows; i++ {}.
for i := range numRows {
hugodorea
approved these changes
Jun 25, 2025
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.
Description
This PR adds tests for the copier part of the
Migrator. Also upgrade testcontainers to v0.37.0 per #1457 (comment).The tests are added in migrator_test.go, the rest of the changes are tidying tests and vendoring.
The PR is low risk because the changes only affect tests.
script/cibuildreturns with no formatting errors, build errors or unit test errors.