You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by GitBox <gi...@apache.org> on 2020/05/31 07:49:58 UTC

[GitHub] [dubbo-go] Patrick0308 opened a new pull request #549: Improve code coverage of zookeeper config center

Patrick0308 opened a new pull request #549:
URL: https://github.com/apache/dubbo-go/pull/549


   Improve code coverage of zookeeper config center
   
   relates #518 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org

[GitHub] [dubbo-go] Patrick0308 merged pull request #549: Improve code coverage of zookeeper config center

Posted by GitBox <gi...@apache.org>.
Patrick0308 merged pull request #549:
URL: https://github.com/apache/dubbo-go/pull/549


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org

[GitHub] [dubbo-go] codecov-commenter commented on pull request #549: Improve code coverage of zookeeper config center

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #549:
URL: https://github.com/apache/dubbo-go/pull/549#issuecomment-636437128


   # [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/549?src=pr&el=h1) Report
   > Merging [#549](https://codecov.io/gh/apache/dubbo-go/pull/549?src=pr&el=desc) into [develop](https://codecov.io/gh/apache/dubbo-go/commit/20fbbb52ea2e088c59cc25d3cacbf708594ffedd&el=desc) will **decrease** coverage by `0.17%`.
   > The diff coverage is `61.94%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/dubbo-go/pull/549/graphs/tree.svg?width=650&height=150&src=pr&token=dcPE6RyFAL)](https://codecov.io/gh/apache/dubbo-go/pull/549?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##           develop     #549      +/-   ##
   ===========================================
   - Coverage    66.92%   66.74%   -0.18%     
   ===========================================
     Files          174      184      +10     
     Lines         9261     9693     +432     
   ===========================================
   + Hits          6198     6470     +272     
   - Misses        2455     2581     +126     
   - Partials       608      642      +34     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/549?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [cluster/cluster\_impl/base\_cluster\_invoker.go](https://codecov.io/gh/apache/dubbo-go/pull/549/diff?src=pr&el=tree#diff-Y2x1c3Rlci9jbHVzdGVyX2ltcGwvYmFzZV9jbHVzdGVyX2ludm9rZXIuZ28=) | `62.31% <ø> (-10.15%)` | :arrow_down: |
   | [cluster/directory/base\_directory.go](https://codecov.io/gh/apache/dubbo-go/pull/549/diff?src=pr&el=tree#diff-Y2x1c3Rlci9kaXJlY3RvcnkvYmFzZV9kaXJlY3RvcnkuZ28=) | `65.90% <0.00%> (ø)` | |
   | [cluster/loadbalance/consistent\_hash.go](https://codecov.io/gh/apache/dubbo-go/pull/549/diff?src=pr&el=tree#diff-Y2x1c3Rlci9sb2FkYmFsYW5jZS9jb25zaXN0ZW50X2hhc2guZ28=) | `90.32% <ø> (-0.86%)` | :arrow_down: |
   | [common/config/environment.go](https://codecov.io/gh/apache/dubbo-go/pull/549/diff?src=pr&el=tree#diff-Y29tbW9uL2NvbmZpZy9lbnZpcm9ubWVudC5nbw==) | `52.38% <ø> (ø)` | |
   | [common/extension/auth.go](https://codecov.io/gh/apache/dubbo-go/pull/549/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9hdXRoLmdv) | `0.00% <ø> (ø)` | |
   | [common/extension/cluster.go](https://codecov.io/gh/apache/dubbo-go/pull/549/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jbHVzdGVyLmdv) | `0.00% <ø> (ø)` | |
   | [common/extension/config\_center.go](https://codecov.io/gh/apache/dubbo-go/pull/549/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25maWdfY2VudGVyLmdv) | `0.00% <ø> (ø)` | |
   | [common/extension/config\_center\_factory.go](https://codecov.io/gh/apache/dubbo-go/pull/549/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25maWdfY2VudGVyX2ZhY3RvcnkuZ28=) | `0.00% <ø> (ø)` | |
   | [common/extension/config\_reader.go](https://codecov.io/gh/apache/dubbo-go/pull/549/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25maWdfcmVhZGVyLmdv) | `0.00% <ø> (ø)` | |
   | [common/extension/configurator.go](https://codecov.io/gh/apache/dubbo-go/pull/549/diff?src=pr&el=tree#diff-Y29tbW9uL2V4dGVuc2lvbi9jb25maWd1cmF0b3IuZ28=) | `0.00% <ø> (ø)` | |
   | ... and [112 more](https://codecov.io/gh/apache/dubbo-go/pull/549/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/549?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/549?src=pr&el=footer). Last update [7d4f1ed...4a0f73e](https://codecov.io/gh/apache/dubbo-go/pull/549?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org

[GitHub] [dubbo-go] hxmhlt removed a comment on pull request #549: Improve code coverage of zookeeper config center

Posted by GitBox <gi...@apache.org>.
hxmhlt removed a comment on pull request #549:
URL: https://github.com/apache/dubbo-go/pull/549#issuecomment-637226796


   > The total coverage is lower. May be some case in previous code not be coveraged.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org

[GitHub] [dubbo-go] Patrick0308 commented on a change in pull request #549: Improve code coverage of zookeeper config center

Posted by GitBox <gi...@apache.org>.
Patrick0308 commented on a change in pull request #549:
URL: https://github.com/apache/dubbo-go/pull/549#discussion_r433843925



##########
File path: config_center/zookeeper/impl.go
##########
@@ -81,32 +79,6 @@ func newZookeeperDynamicConfiguration(url *common.URL) (*zookeeperDynamicConfigu
 
 }
 
-func newMockZookeeperDynamicConfiguration(url *common.URL, opts ...zookeeper.Option) (*zk.TestCluster, *zookeeperDynamicConfiguration, error) {
-	c := &zookeeperDynamicConfiguration{
-		url:      url,
-		rootPath: "/" + url.GetParam(constant.CONFIG_NAMESPACE_KEY, config_center.DEFAULT_GROUP) + "/config",
-	}
-	var (
-		tc  *zk.TestCluster
-		err error
-	)
-	tc, c.client, _, err = zookeeper.NewMockZookeeperClient("test", 15*time.Second, opts...)
-	if err != nil {
-		logger.Errorf("mock zookeeper client start error ,error message is %v", err)
-		return tc, c, err
-	}
-	c.wg.Add(1)
-	go zookeeper.HandleClientRestart(c)
-

Review comment:
       Yeah, it's be covered. I don't know why this coverage be smaller. In my computer, `cluster/cluster_impl/base_cluster_invoker.go` coverage is bigger than 80%.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org

[GitHub] [dubbo-go] Patrick0308 commented on a change in pull request #549: Improve code coverage of zookeeper config center

Posted by GitBox <gi...@apache.org>.
Patrick0308 commented on a change in pull request #549:
URL: https://github.com/apache/dubbo-go/pull/549#discussion_r433843925



##########
File path: config_center/zookeeper/impl.go
##########
@@ -81,32 +79,6 @@ func newZookeeperDynamicConfiguration(url *common.URL) (*zookeeperDynamicConfigu
 
 }
 
-func newMockZookeeperDynamicConfiguration(url *common.URL, opts ...zookeeper.Option) (*zk.TestCluster, *zookeeperDynamicConfiguration, error) {
-	c := &zookeeperDynamicConfiguration{
-		url:      url,
-		rootPath: "/" + url.GetParam(constant.CONFIG_NAMESPACE_KEY, config_center.DEFAULT_GROUP) + "/config",
-	}
-	var (
-		tc  *zk.TestCluster
-		err error
-	)
-	tc, c.client, _, err = zookeeper.NewMockZookeeperClient("test", 15*time.Second, opts...)
-	if err != nil {
-		logger.Errorf("mock zookeeper client start error ,error message is %v", err)
-		return tc, c, err
-	}
-	c.wg.Add(1)
-	go zookeeper.HandleClientRestart(c)
-

Review comment:
       Yeah, it's be covered. I don't know why this coverage be smaller. In my computer, `cluster/cluster_impl/base_cluster_invoker.go` coverage is bigger than 80%, but here the coverage is 62.31%.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org

[GitHub] [dubbo-go] hxmhlt commented on pull request #549: Improve code coverage of zookeeper config center

Posted by GitBox <gi...@apache.org>.
hxmhlt commented on pull request #549:
URL: https://github.com/apache/dubbo-go/pull/549#issuecomment-637226796


   > The total coverage is lower. May be some case in previous code not be coveraged.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org

[GitHub] [dubbo-go] hxmhlt commented on a change in pull request #549: Improve code coverage of zookeeper config center

Posted by GitBox <gi...@apache.org>.
hxmhlt commented on a change in pull request #549:
URL: https://github.com/apache/dubbo-go/pull/549#discussion_r433583313



##########
File path: config_center/zookeeper/impl.go
##########
@@ -81,32 +79,6 @@ func newZookeeperDynamicConfiguration(url *common.URL) (*zookeeperDynamicConfigu
 
 }
 
-func newMockZookeeperDynamicConfiguration(url *common.URL, opts ...zookeeper.Option) (*zk.TestCluster, *zookeeperDynamicConfiguration, error) {
-	c := &zookeeperDynamicConfiguration{
-		url:      url,
-		rootPath: "/" + url.GetParam(constant.CONFIG_NAMESPACE_KEY, config_center.DEFAULT_GROUP) + "/config",
-	}
-	var (
-		tc  *zk.TestCluster
-		err error
-	)
-	tc, c.client, _, err = zookeeper.NewMockZookeeperClient("test", 15*time.Second, opts...)
-	if err != nil {

Review comment:
       zookeeper.NewMockZookeeperClient may delete?

##########
File path: config_center/zookeeper/impl.go
##########
@@ -81,32 +79,6 @@ func newZookeeperDynamicConfiguration(url *common.URL) (*zookeeperDynamicConfigu
 
 }
 
-func newMockZookeeperDynamicConfiguration(url *common.URL, opts ...zookeeper.Option) (*zk.TestCluster, *zookeeperDynamicConfiguration, error) {
-	c := &zookeeperDynamicConfiguration{
-		url:      url,
-		rootPath: "/" + url.GetParam(constant.CONFIG_NAMESPACE_KEY, config_center.DEFAULT_GROUP) + "/config",
-	}
-	var (
-		tc  *zk.TestCluster
-		err error
-	)
-	tc, c.client, _, err = zookeeper.NewMockZookeeperClient("test", 15*time.Second, opts...)
-	if err != nil {
-		logger.Errorf("mock zookeeper client start error ,error message is %v", err)
-		return tc, c, err
-	}
-	c.wg.Add(1)
-	go zookeeper.HandleClientRestart(c)
-

Review comment:
       Is zookeeper.HandleClientRestart(c)  coveraged ?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org

[GitHub] [dubbo-go] Patrick0308 commented on a change in pull request #549: Improve code coverage of zookeeper config center

Posted by GitBox <gi...@apache.org>.
Patrick0308 commented on a change in pull request #549:
URL: https://github.com/apache/dubbo-go/pull/549#discussion_r433840386



##########
File path: config_center/zookeeper/impl.go
##########
@@ -81,32 +79,6 @@ func newZookeeperDynamicConfiguration(url *common.URL) (*zookeeperDynamicConfigu
 
 }
 
-func newMockZookeeperDynamicConfiguration(url *common.URL, opts ...zookeeper.Option) (*zk.TestCluster, *zookeeperDynamicConfiguration, error) {
-	c := &zookeeperDynamicConfiguration{
-		url:      url,
-		rootPath: "/" + url.GetParam(constant.CONFIG_NAMESPACE_KEY, config_center.DEFAULT_GROUP) + "/config",
-	}
-	var (
-		tc  *zk.TestCluster
-		err error
-	)
-	tc, c.client, _, err = zookeeper.NewMockZookeeperClient("test", 15*time.Second, opts...)
-	if err != nil {

Review comment:
       `zookeeper.NewMockZookeeperClient` be used in other place.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org