-
Notifications
You must be signed in to change notification settings - Fork 188
dmctl: use dmctl binary directly in integration test #135
Conversation
/run-all-tests |
/run-all-tests |
/run-all-tests |
/run-all-tests |
Codecov Report
@@ Coverage Diff @@
## master #135 +/- ##
==========================================
Coverage ? 34.267%
==========================================
Files ? 121
Lines ? 13602
Branches ? 0
==========================================
Hits ? 4661
Misses ? 8412
Partials ? 529 |
/run-all-tests |
/run-all-tests |
2 similar comments
/run-all-tests |
/run-all-tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
dm/ctl/common/sql_operation.go
Outdated
// return binlog-pos, sql-pattern, sharding, error | ||
func extractBinlogPosSQLPattern(cmd *cobra.Command) (string, string, bool, error) { | ||
func ExtractBinlogPosSQLPattern(cmd *cobra.Command) (string, string, bool, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we still put this function into dm/master
, it seems we wouldn't use it somewhere else, because of cmd
argument
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have the same question, but I did not raise it.:satisfied:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed @GregoryIan PTAL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good Job! LGTM
/run-all-tests |
1 similar comment
/run-all-tests |
What problem does this PR solve?
add integration test for dmctl
DNM until #142 is merged, this pr has same fix code in #142
What is changed and how it works?
dmctl_basic
test. Other dmctl related tests will be added later in more PRsCheck List
Tests