Skip to content

Fix remoting/etcdv3/client test cases#375

Merged
AlexStocks merged 1 commit intoapache:developfrom
swr1bm86:improve/remoting-etcdv3-specs
Feb 29, 2020
Merged

Fix remoting/etcdv3/client test cases#375
AlexStocks merged 1 commit intoapache:developfrom
swr1bm86:improve/remoting-etcdv3-specs

Conversation

@swr1bm86
Copy link
Copy Markdown
Contributor

Add up more assertations for the etcdv3 client.

@zouyx
Copy link
Copy Markdown
Member

zouyx commented Feb 26, 2020

Thanks for your fisrt contirbution, should make travis succes @zjhmale

@swr1bm86 swr1bm86 force-pushed the improve/remoting-etcdv3-specs branch from a51bc40 to 2a15f0b Compare February 27, 2020 00:09
@swr1bm86
Copy link
Copy Markdown
Contributor Author

@zouyx pls review again :)

e1 := events[0]
e2 := events[1]

if e1.Type != mvccpb.PUT || string(e1.Kv.Key) != "name" || string(e1.Kv.Value) != "scott.wang" {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if e1.Type != mvccpb.PUT || string(e1.Kv.Key) != "name" || string(e1.Kv.Value) != "scott.wang" {
assert.Contains()

if array is not always sorting, suggest use assert.Contains()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the hint, but for the sake of event queue semantic, the watching channel should be a sequential generator which means the event created in the first place should always be the first one consumed outside the watching channel IIRC, maybe I'm wrong.


e1 := events[0]
e2 := events[1]
if e1.Type != mvccpb.PUT || string(e1.Kv.Key) != "scott/wang" || e1.Kv.Value != nil {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as above

@swr1bm86 swr1bm86 force-pushed the improve/remoting-etcdv3-specs branch from 2a15f0b to 6e92af9 Compare February 27, 2020 03:25
@codecov-io
Copy link
Copy Markdown

codecov-io commented Feb 27, 2020

Codecov Report

Merging #375 into develop will decrease coverage by 0.35%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #375      +/-   ##
===========================================
- Coverage    66.72%   66.36%   -0.36%     
===========================================
  Files          150      150              
  Lines         7963     7964       +1     
===========================================
- Hits          5313     5285      -28     
- Misses        2146     2178      +32     
+ Partials       504      501       -3
Impacted Files Coverage Δ
config_center/nacos/facade.go 35.29% <0%> (-29.42%) ⬇️
cluster/cluster_impl/base_cluster_invoker.go 62.31% <0%> (-10.15%) ⬇️
protocol/dubbo/client.go 63.75% <0%> (-5%) ⬇️
protocol/dubbo/pool.go 74.77% <0%> (-1.84%) ⬇️
config_center/nacos/client.go 55.55% <0%> (-1.71%) ⬇️
protocol/dubbo/listener.go 56.75% <0%> (-1.09%) ⬇️
registry/directory/directory.go 82.8% <0%> (+0.11%) ⬆️
protocol/dubbo/readwriter.go 70.73% <0%> (+2.43%) ⬆️
protocol/dubbo/codec.go 75.67% <0%> (+5.4%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd1a3c2...6e92af9. Read the comment docs.

@swr1bm86 swr1bm86 requested a review from zouyx February 27, 2020 16:00
Copy link
Copy Markdown
Member

@zouyx zouyx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Copy Markdown
Contributor

@fangyincheng fangyincheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Copy Markdown
Contributor

@hxmhlt hxmhlt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AlexStocks AlexStocks merged commit 44edf4f into apache:develop Feb 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants