CI: Add basic integration test#12577
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new basic integration test for Data Migration (DM), including SQL setup scripts, source/task configurations, and expected results. The feedback suggests using single quotes for SQL string literals to follow standards and replacing the deprecated block-allow-list configuration with the modern filters syntax in the task YAML.
| INSERT INTO t1 VALUES (1, "test 1"); | ||
| INSERT INTO t1 VALUES (2, "test 2"), (3, "test 3"); | ||
| UPDATE t1 SET name="test 2 updated" WHERE id=2; |
There was a problem hiding this comment.
| block-allow-list: "test" | ||
|
|
||
| block-allow-list: | ||
| test: | ||
| do-dbs: ["test*"] |
916dd1a to
507fdaa
Compare
9ef81ba to
6fe00a2
Compare
|
/retest |
3 similar comments
|
/retest |
|
/retest |
|
/retest |
| ./bin/dmctl query-status mysql-to-tidb | ||
| - name: install mysql-tester | ||
| run: | | ||
| go install github.com/pingcap/mysql-tester/src@latest |
There was a problem hiding this comment.
Can we pin mysql-tester instead of installing @latest at runtime? Otherwise an unrelated upstream mysql-tester change can break this workflow without any TiFlow change. We can do manual updates if needed.
There was a problem hiding this comment.
Yes, I'll do that. However as there are no releases nor tags for mysql-tester this is a bit more painful than it should.
There was a problem hiding this comment.
Got it, thanks. I think my main concern was just avoiding @latest; pinning to a commit SHA is totally fine as well. It looks like the MariaDB job is pinned now, while the MySQL job is still floating. Non-blocking from my side.
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
/cc gmhdbjd D3Hunter |
| - 'dm/**' | ||
| - 'cmd/dm*/*' |
There was a problem hiding this comment.
maybe it should be triggered when go.mod file is changed.
| ./bin/dmctl query-status mariadb-to-tidb | ||
| - name: install mysql-tester | ||
| run: | | ||
| go install github.com/pingcap/mysql-tester/src@f2d90ea9522d30c9a8e8d70cc31c7f016ca2801f |
There was a problem hiding this comment.
can we manager the version in go.mod file?
There was a problem hiding this comment.
Yes, we could do this by using the tool functionality, see also: https://go.dev/doc/go1.24#tools
Do you think this is better?
diff --git a/go.mod b/go.mod
index aceb438e4..bba975342 100644
--- a/go.mod
+++ b/go.mod
@@ -180,6 +180,7 @@ require (
github.com/cncf/xds/go v0.0.0-20251022180443-0feb69152e9f // indirect
github.com/cockroachdb/fifo v0.0.0-20240606204812-0bbfbd93a7ce // indirect
github.com/cockroachdb/tokenbucket v0.0.0-20230807174530-cc333fc44b06 // indirect
+ github.com/defined2014/mysql v0.0.0-20231121061906-fcfacaa39f49 // indirect
github.com/envoyproxy/go-control-plane/envoy v1.35.0 // indirect
github.com/envoyproxy/protoc-gen-validate v1.2.1 // indirect
github.com/felixge/httpsnoop v1.0.4 // indirect
@@ -216,13 +217,14 @@ require (
github.com/oasdiff/yaml3 v0.0.0-20250309153720-d2182401db90 // indirect
github.com/perimeterx/marshmallow v1.1.5 // indirect
github.com/pingcap/metering_sdk v0.0.0-20251110022152-dac449ac5389 // indirect
+ github.com/pingcap/mysql-tester v0.0.0-20260121034350-f2d90ea9522d // indirect
github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 // indirect
github.com/qri-io/jsonpointer v0.1.1 // indirect
github.com/qri-io/jsonschema v0.2.1 // indirect
github.com/robfig/cron/v3 v3.0.1 // indirect
github.com/segmentio/asm v1.2.0 // indirect
github.com/segmentio/fasthash v1.0.3 // indirect
- github.com/sergi/go-diff v1.3.1 // indirect
+ github.com/sergi/go-diff v1.4.0 // indirect
github.com/spf13/afero v1.15.0 // indirect
github.com/spiffe/go-spiffe/v2 v2.6.0 // indirect
github.com/tidwall/btree v1.7.0 // indirect
@@ -462,3 +464,5 @@ replace golang.org/x/text => golang.org/x/text v0.28.0
// tls10server=1
godebug tlsrsakex=1
+
+tool github.com/pingcap/mysql-tester/srcAnd then:
cd dm/tests/integration_basic
go tool github.com/pingcap/mysql-tester/src
OliverS929
left a comment
There was a problem hiding this comment.
Other than these two small issues, the rest looks good to me.
| pull_request: | ||
| paths: | ||
| - 'dm/**' | ||
| - 'cmd/dm*/*' |
There was a problem hiding this comment.
Could we widen the pull_request.paths filter a bit? Changes in shared DM-related code or files like pkg/**, go.mod, go.sum, or this workflow itself can still affect make dm, but this workflow would not run for them.
There was a problem hiding this comment.
I thought dm only used dm/pkg/ and not pkg/, but that's not the case. Maybe at some point the two pkg directories should be merged...
I've update the paths.
| ./bin/dmctl query-status mysql-to-tidb | ||
| - name: install mysql-tester | ||
| run: | | ||
| go install github.com/pingcap/mysql-tester/src@latest |
There was a problem hiding this comment.
Got it, thanks. I think my main concern was just avoiding @latest; pinning to a commit SHA is totally fine as well. It looks like the MariaDB job is pinned now, while the MySQL job is still floating. Non-blocking from my side.
|
/retest |
1 similar comment
|
/retest |
|
@dveeden: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: GMHDBJD, OliverS929 The full list of commands accepted by this bot can be found here. The pull request process is described here |
[LGTM Timeline notifier]Timeline:
|
What problem does this PR solve?
Issue Number: close #12614
What is changed and how it works?
Check List
Tests
Questions
Will it cause performance regression or break compatibility?
Do you need to update user documentation, design documentation or monitoring documentation?
Release note