Repository: apache/rocketmq-client-go
Pull request #156 (2019-08-15 05:07:45)

update defaultConsumer.client init AND add push_consumer testing
## What is the purpose of the change

update defaultConsumer.client init AND add push_consumer testing

## Brief changelog

update defaultConsumer.client init AND add push_consumer testing

## Verifying this change

XXXX

Follow this checklist to help us incorporate your contribution quickly and easily. Notice, `it would be helpful if you could finish the following 5 checklist(the last one is not necessary)before request the community to review your PR`.

- [x] Make sure there is a [Github issue](https://github.com/apache/rocketmq/issues) filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue. 
- [x] Format the pull request title like `[ISSUE #123] Fix UnknownException when host config not exist`. Each commit in the pull request should have a meaningful subject line and body.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] Write necessary unit-test(over 80% coverage) to verify your logic correction, more mock a little better when a cross-module dependency exists.
- [ ] If this contribution is large, please file an [Apache Individual Contributor License Agreement](http://www.apache.org/licenses/#clas).


codecov-io: # [Codecov](https://codecov.io/gh/apache/rocketmq-client-go/pull/156?src=pr&el=h1) Report
> Merging [#156](https://codecov.io/gh/apache/rocketmq-client-go/pull/156?src=pr&el=desc) into [native](https://codecov.io/gh/apache/rocketmq-client-go/commit/ae7a2cf084c9fc4aabfca50fcfc24a52083b52f9?src=pr&el=desc) will **increase** coverage by `2.58%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/apache/rocketmq-client-go/pull/156/graphs/tree.svg?width=650&token=CiV9ofPkG3&height=150&src=pr)](https://codecov.io/gh/apache/rocketmq-client-go/pull/156?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           native     #156      +/-   ##
==========================================
+ Coverage   17.06%   19.65%   +2.58%     
==========================================
  Files          42       42              
  Lines        4402     4402              
==========================================
+ Hits          751      865     +114     
+ Misses       3555     3416     -139     
- Partials       96      121      +25
```


| [Impacted Files](https://codecov.io/gh/apache/rocketmq-client-go/pull/156?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [consumer/consumer.go](https://codecov.io/gh/apache/rocketmq-client-go/pull/156/diff?src=pr&el=tree#diff-Y29uc3VtZXIvY29uc3VtZXIuZ28=) | `3.86% <ø> (+3.86%)` | :arrow_up: |
| [consumer/push\_consumer.go](https://codecov.io/gh/apache/rocketmq-client-go/pull/156/diff?src=pr&el=tree#diff-Y29uc3VtZXIvcHVzaF9jb25zdW1lci5nbw==) | `14.31% <100%> (+14.31%)` | :arrow_up: |
| [consumer/statistics.go](https://codecov.io/gh/apache/rocketmq-client-go/pull/156/diff?src=pr&el=tree#diff-Y29uc3VtZXIvc3RhdGlzdGljcy5nbw==) | `60% <0%> (-4.35%)` | :arrow_down: |
| [consumer/offset\_store.go](https://codecov.io/gh/apache/rocketmq-client-go/pull/156/diff?src=pr&el=tree#diff-Y29uc3VtZXIvb2Zmc2V0X3N0b3JlLmdv) | `8.42% <0%> (+6.84%)` | :arrow_up: |
| [consumer/option.go](https://codecov.io/gh/apache/rocketmq-client-go/pull/156/diff?src=pr&el=tree#diff-Y29uc3VtZXIvb3B0aW9uLmdv) | `35.41% <0%> (+35.41%)` | :arrow_up: |
| [consumer/lock.go](https://codecov.io/gh/apache/rocketmq-client-go/pull/156/diff?src=pr&el=tree#diff-Y29uc3VtZXIvbG9jay5nbw==) | `40% <0%> (+40%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq-client-go/pull/156?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/rocketmq-client-go/pull/156?src=pr&el=footer). Last update [ae7a2cf...09ce9c1](https://codecov.io/gh/apache/rocketmq-client-go/pull/156?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

wenfengwang: LGTM

Patch:
diff --git a/consumer/consumer.go b/consumer/consumer.go
index 56836ae..b5bf22f 100644
--- a/consumer/consumer.go
+++ b/consumer/consumer.go
@@ -275,7 +275,7 @@ func (dc *defaultConsumer) start() error {
 		dc.subscriptionDataTable.Store(retryTopic, sub)
 	}
 
-	dc.client = internal.GetOrNewRocketMQClient(dc.option.ClientOptions, nil)
+	//dc.client = internal.GetOrNewRocketMQClient(dc.option.ClientOptions, nil)
 	if dc.model == Clustering {
 		dc.option.ChangeInstanceNameToPID()
 		dc.storage = NewRemoteOffsetStore(dc.consumerGroup, dc.client)
diff --git a/consumer/push_consumer.go b/consumer/push_consumer.go
index 88ab362..3b9747c 100644
--- a/consumer/push_consumer.go
+++ b/consumer/push_consumer.go
@@ -67,6 +67,7 @@ func NewPushConsumer(opts ...Option) (*pushConsumer, error) {
 	internal.RegisterNamsrv(srvs)
 
 	dc := &defaultConsumer{
+		client:         internal.GetOrNewRocketMQClient(defaultOpts.ClientOptions, nil),
 		consumerGroup:  defaultOpts.GroupName,
 		cType:          _PushConsume,
 		state:          internal.StateCreateJust,


Test patch:
diff --git a/consumer/push_consumer_test.go b/consumer/push_consumer_test.go
index f7bc454..2f4dbeb 100644
--- a/consumer/push_consumer_test.go
+++ b/consumer/push_consumer_test.go
@@ -16,3 +16,58 @@ limitations under the License.
 */
 
 package consumer
+
+import (
+	"context"
+	"fmt"
+	"github.com/apache/rocketmq-client-go/internal"
+	"github.com/apache/rocketmq-client-go/primitive"
+	"github.com/golang/mock/gomock"
+	. "github.com/smartystreets/goconvey/convey"
+	"testing"
+)
+
+func mockB4Start(c *pushConsumer) {
+	c.topicSubscribeInfoTable.Store("TopicTest", []*primitive.MessageQueue{})
+}
+
+func TestStart(t *testing.T) {
+	Convey("test Start method", t, func() {
+		c, _ := NewPushConsumer(
+			WithGroupName("testGroup"),
+			WithNameServer([]string{"127.0.0.1:9876"}),
+		)
+
+		ctrl := gomock.NewController(t)
+		defer ctrl.Finish()
+
+		client := internal.NewMockRMQClient(ctrl)
+		c.client = client
+
+		err := c.Subscribe("TopicTest", MessageSelector{}, func(ctx context.Context,
+			msgs ...*primitive.MessageExt) (ConsumeResult, error) {
+			fmt.Printf("subscribe callback: %v \n", msgs)
+			return ConsumeSuccess, nil
+		})
+
+		client.EXPECT().ClientID().Return("127.0.0.1@DEFAULT")
+		client.EXPECT().Start().Return()
+		client.EXPECT().RegisterConsumer(gomock.Any(), gomock.Any()).Return(nil)
+		client.EXPECT().UpdateTopicRouteInfo().AnyTimes().Return()
+
+		Convey("test topic route info not found", func() {
+			client.EXPECT().Shutdown().Return()
+			err = c.Start()
+			So(err.Error(), ShouldContainSubstring, "route info not found")
+		})
+
+		Convey("test topic route info found", func() {
+			client.EXPECT().RebalanceImmediately().Return()
+			client.EXPECT().CheckClientInBroker().Return()
+			client.EXPECT().SendHeartbeatToAllBrokerWithLock().Return()
+			mockB4Start(c)
+			err = c.Start()
+			So(err, ShouldBeNil)
+		})
+	})
+}