streamer: refactor and add test cases #140

csuzhangxc · 2019-05-08T10:33:50Z

What problem does this PR solve?

tiny refactor to make relay reader (streamer) clearer
improve test coverage for pkg/streamer, 31.4% --> 92.9%
ignore io.EOF returned from BinlogParser.ParseFile

What is changed and how it works?

refactor some code
add test cases for streamer pkg

Check List

Tests

Unit test
Integration test

pkg/streamer/reader_test.go

Co-Authored-By: kennytm <[email protected]>

codecov · 2019-05-14T10:20:28Z

Codecov Report

Merging #140 into master will increase coverage by 2.4499%.
The diff coverage is 89.2857%.

@@               Coverage Diff                @@
##             master       #140        +/-   ##
================================================
+ Coverage   50.8692%   53.3191%   +2.4499%     
================================================
  Files           121        121                
  Lines         13633      13648        +15     
================================================
+ Hits           6935       7277       +342     
+ Misses         5977       5644       -333     
- Partials        721        727         +6

csuzhangxc · 2019-05-20T09:31:35Z

/run-all-tests

csuzhangxc · 2019-05-21T02:01:31Z

/run-all-tests

amyangfei · 2019-06-04T09:33:49Z

/run-all-tests

pkg/streamer/reader.go

pkg/streamer/file.go

pkg/streamer/hub_test.go

pkg/binlog/filename.go

pkg/streamer/reader_test.go

csuzhangxc · 2019-06-05T11:48:18Z

/run-all-tests

csuzhangxc · 2019-06-05T11:58:03Z

/run-all-tests

csuzhangxc · 2019-06-05T12:02:46Z

/run-all-tests

IANTHEREAL · 2019-06-05T14:04:41Z

pkg/binlog/filename.go

+// Filename represents a binlog filename.
+type Filename struct {
+	BaseName string
+	Seq      string


why not float64?

maybe we want to keep the number format like 000001,
we can add one more float64 or int filed to keep the number value, something like space-time tradeoff?

I added another int64 field in d7f60d5

amyangfei · 2019-06-06T02:09:57Z

/run-all-tests

IANTHEREAL · 2019-06-10T02:22:20Z

pkg/binlog/filename.go

+}
+
+// GetFilenameIndex returns a float64 index value (seq number) of the filename.
+func GetFilenameIndex(filename string) (float64, error) {


why float64, not int64?

changed to int64 in d7f60d5.

WangXiangUSTC

rest LGTM

WangXiangUSTC · 2019-06-10T01:50:39Z

pkg/binlog/filename.go

+
+// VerifyFilename verifies whether is a valid MySQL/MariaDB binlog filename.
+// valid format is `base + '.' + seq`.
+func VerifyFilename(filename string) error {


seems this function return bool type is better

done in d7f60d5.

WangXiangUSTC · 2019-06-10T02:33:18Z

pkg/streamer/reader_test.go

+	c.Assert(errors.Cause(err), Equals, ErrReaderRunning)
+	c.Assert(s, IsNil)
+
+	c.Assert(ev, IsNil)


how about move this line after L626?

👍 done in d7f60d5.

csuzhangxc · 2019-06-10T08:02:02Z

@amyangfei @WangXiangUSTC PTAL again

amyangfei

LGTM

WangXiangUSTC · 2019-06-10T09:18:06Z

LGTM

csuzhangxc added 12 commits January 20, 2019 10:18

*: merge from upstream

87370b5

Merge pull request #1 from pingcap/master

a432881

Merge branch 'master' of github.com:csuzhangxc/dm

a4a0a57

Merge branch 'master' of github.com:csuzhangxc/dm

fb45294

Merge branch 'master' of github.com:csuzhangxc/dm

be9e6d1

Merge branch 'master' of github.com:csuzhangxc/dm

82e2b33

Merge branch 'master' of github.com:csuzhangxc/dm

4ab6e96

Merge branch 'master' of github.com:csuzhangxc/dm

3faccf1

Merge branch 'master' of github.com:csuzhangxc/dm

04dfc47

Merge branch 'master' of github.com:csuzhangxc/dm

ff6931f

Merge branch 'master' of github.com:csuzhangxc/dm

c1c3d0d

streamer: add a base test for parseFile

0776b8a

csuzhangxc added priority/normal Minor change, requires approval from ≥1 primary reviewer status/WIP This PR is still work in progress type/enhancement Performance improvement or refactoring type/qa relate to quality assurance labels May 8, 2019

csuzhangxc added 2 commits May 8, 2019 19:49

streamer: add test for getNextUUID

1a940a2

streamer: add test for getFirstBinlogName

d551dc9

kennytm reviewed May 8, 2019

View reviewed changes

pkg/streamer/reader_test.go Outdated Show resolved Hide resolved

csuzhangxc and others added 4 commits May 9, 2019 10:05

Update pkg/streamer/reader_test.go

92566de

Co-Authored-By: kennytm <[email protected]>

streamer: fix test

0d3ce15

Merge branch 'master' of github.com:csuzhangxc/dm

cb6cd86

Merge remote-tracking branch 'remotes/origin/master' into relay-reader

b919f85

Merge remote-tracking branch 'remotes/origin/master' into relay-reader

c7f0234

reader: try fix CI

fe93b01

csuzhangxc added 2 commits May 21, 2019 11:24

streamer: add test case for CollectBinlogFilesCmp

00ff83d

streamer: add test cases for file util functions

37aea93

WangXiangUSTC reviewed Jun 5, 2019

View reviewed changes

pkg/streamer/reader.go Outdated Show resolved Hide resolved

WangXiangUSTC reviewed Jun 5, 2019

View reviewed changes

pkg/streamer/file.go Show resolved Hide resolved

pkg/streamer/file.go Show resolved Hide resolved

pkg/streamer/file.go Show resolved Hide resolved

pkg/streamer/hub_test.go Show resolved Hide resolved

streamer: address comments

dc204ee

amyangfei reviewed Jun 5, 2019

View reviewed changes

csuzhangxc added 2 commits June 5, 2019 19:27

*: address comments

ec91143

worker: increase timeout in tests

f49e726

worker: increase timeout in tests

3828b09

IANTHEREAL reviewed Jun 5, 2019

View reviewed changes

IANTHEREAL reviewed Jun 10, 2019

View reviewed changes

WangXiangUSTC reviewed Jun 10, 2019

View reviewed changes

csuzhangxc added 4 commits June 10, 2019 15:10

*: address comments

d7f60d5

Merge remote-tracking branch 'remotes/origin/master' into relay-reader

8035ed5

writer: fix CI

01660d2

worker: fix CI

5766918

amyangfei reviewed Jun 10, 2019

View reviewed changes

amyangfei added status/LGT1 One reviewer already commented LGTM and removed status/PTAL This PR is ready for review. Add this label back after committing new changes labels Jun 10, 2019

WangXiangUSTC added status/LGT2 Two reviewers already commented LGTM, ready for merge and removed status/LGT1 One reviewer already commented LGTM labels Jun 10, 2019

WangXiangUSTC approved these changes Jun 10, 2019

View reviewed changes

csuzhangxc merged commit 7d427b9 into pingcap:master Jun 10, 2019

csuzhangxc deleted the relay-reader branch June 10, 2019 09:51

lichunzhu pushed a commit to lichunzhu/dm that referenced this pull request Apr 6, 2020

streamer: refactor and add test cases (pingcap#140)

047c32d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

streamer: refactor and add test cases #140

streamer: refactor and add test cases #140

csuzhangxc commented May 8, 2019 •

edited

Loading

codecov bot commented May 14, 2019 •

edited

Loading

csuzhangxc commented May 20, 2019

csuzhangxc commented May 21, 2019

amyangfei commented Jun 4, 2019

csuzhangxc commented Jun 5, 2019

csuzhangxc commented Jun 5, 2019

csuzhangxc commented Jun 5, 2019

IANTHEREAL Jun 5, 2019

amyangfei Jun 5, 2019

csuzhangxc Jun 10, 2019

amyangfei commented Jun 6, 2019

IANTHEREAL Jun 10, 2019

csuzhangxc Jun 10, 2019

WangXiangUSTC left a comment

WangXiangUSTC Jun 10, 2019

csuzhangxc Jun 10, 2019

csuzhangxc Jun 10, 2019

WangXiangUSTC Jun 10, 2019

csuzhangxc Jun 10, 2019

csuzhangxc commented Jun 10, 2019

amyangfei left a comment

WangXiangUSTC commented Jun 10, 2019

streamer: refactor and add test cases #140

streamer: refactor and add test cases #140

Conversation

csuzhangxc commented May 8, 2019 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

codecov bot commented May 14, 2019 • edited Loading

Codecov Report

csuzhangxc commented May 20, 2019

csuzhangxc commented May 21, 2019

amyangfei commented Jun 4, 2019

csuzhangxc commented Jun 5, 2019

csuzhangxc commented Jun 5, 2019

csuzhangxc commented Jun 5, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amyangfei commented Jun 6, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WangXiangUSTC left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

csuzhangxc commented Jun 10, 2019

amyangfei left a comment

Choose a reason for hiding this comment

WangXiangUSTC commented Jun 10, 2019

csuzhangxc commented May 8, 2019 •

edited

Loading

codecov bot commented May 14, 2019 •

edited

Loading