-
Notifications
You must be signed in to change notification settings - Fork 188
release mysql dependence in syncer test (1) #209
Conversation
/run-all-tests |
/run-all-tests |
1 similar comment
/run-all-tests |
Codecov Report
@@ Coverage Diff @@
## master #209 +/- ##
==============================================
- Coverage 58.9954% 58.89% -0.1054%
==============================================
Files 123 123
Lines 14335 14055 -280
==============================================
- Hits 8457 8277 -180
+ Misses 5008 4948 -60
+ Partials 870 830 -40 |
8b137ef
to
6cbeb0f
Compare
/run-all-tests |
3 similar comments
/run-all-tests |
/run-all-tests |
/run-all-tests |
ff10fe9
to
9aa3a1c
Compare
/run-all-tests |
9aa3a1c
to
ea258de
Compare
2eac360
to
34fb516
Compare
syncer/syncer_test.go
Outdated
previousGTIDSetStr := "3ccc475b-2343-11e7-be21-6c0b84d59f30:1-14,406a3f61-690d-11e7-87c5-6c92bf46f384:1-94321383" | ||
previousGTIDSet, err := gtid.ParserGTID(s.cfg.Flavor, previousGTIDSetStr) | ||
if err != nil { | ||
log.L().Fatal("", zap.Error(err)) |
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.
it's better to use check.FailNow/Fatalf
to stop unit test
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.
Done
@@ -80,11 +84,26 @@ func (s *testSyncerSuite) SetUpSuite(c *C) { | |||
|
|||
s.resetMaster() |
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.
still need to leave them?
s.resetMaster()
s.resetBinlogSyncer()
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.
some tests still use them, but I will remove them eventually in other PRs
syncer/syncer_test.go
Outdated
@@ -222,33 +243,117 @@ func (s *testSyncerSuite) TestSelectTable(c *C) { | |||
{Schema: "~^ptest*", Name: "~^t.*"}, | |||
}, | |||
} | |||
s.resetEventsGenerator() |
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.
We can define a more generic object to represent the test case, like
type binlogEvents []binlogEvent
type binlogEvent struct {
typ string
arguments []interface{}
}
and an events generation function
function testGenerateEvent(binlogEvents) []*replication.BinlogEvent
they would save many codes
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.
Done
Rest LGTM |
/run-all-tests |
/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
Good Job!LGTM |
What problem does this PR solve?
Release mysql dependencies in syncer test, this is the first part change, there are 4 syncer tests remain mysql dependencies need to realease, and I'll finish them in other PR.
What is changed and how it works?
use sql mock and mock binlog events in
pkg/binlog/event
Check List
Tests