You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/11/27 10:56:58 UTC

[GitHub] [apisix-dashboard] idbeta opened a new pull request #897: test: add cHash upstream (E2E)

idbeta opened a new pull request #897:
URL: https://github.com/apache/apisix-dashboard/pull/897


   Please answer these questions before submitting a pull request
   
   - Why submit this pull request?
   - [x] E2E test
   - [ ] New feature provided
   - [ ] Improve performance
   
   - Related issues
   https://github.com/apache/apisix-dashboard/issues/607
   


----------------------------------------------------------------
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



[GitHub] [apisix-dashboard] membphis commented on a change in pull request #897: test: add cHash upstream (E2E)

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #897:
URL: https://github.com/apache/apisix-dashboard/pull/897#discussion_r531543175



##########
File path: api/test/e2e/upstream_test.go
##########
@@ -188,7 +188,60 @@ func TestRoute_Node_Host(t *testing.T) {
 	}
 }
 
-//TODO cHash
+func TestUpstream_cHash(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create cHash upstream",
+			Object:   MangerApiExpect(t),
+			Method:   http.MethodPut,
+			Path:     "/apisix/admin/upstreams/1",
+			Body: `{
+                "nodes": [{
+                    "host": "172.16.238.20",
+                    "port": 1980,
+                    "weight": 1
+				},
+				{
+                    "host": "172.16.238.20",
+                    "port": 1981,
+                    "weight": 2
+                }],
+				"type": "chash",
+				"hash_on":"header",
+				"key": "remote_addr"
+			}`,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc: "create route using the upstream just created",
+			Object:   MangerApiExpect(t),
+			Method:   http.MethodPut,
+			Path:     "/apisix/admin/routes/1",
+			Body: `{
+				"uri": "/server_port",
+				"upstream_id": "1"
+			}`,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			Sleep:        sleepTime,
+		},
+		{
+			caseDesc:     "hit the route just created",
+			Object:       APISIXExpect(t),
+			Method:       http.MethodGet,
+			Path:         "/server_port",
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "1981",
+			Sleep:        sleepTime,
+		},

Review comment:
       delete upstream object




----------------------------------------------------------------
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



[GitHub] [apisix-dashboard] membphis merged pull request #897: test: e2e test chash upstream with key (remote_addr)

Posted by GitBox <gi...@apache.org>.
membphis merged pull request #897:
URL: https://github.com/apache/apisix-dashboard/pull/897


   


----------------------------------------------------------------
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



[GitHub] [apisix-dashboard] idbeta commented on a change in pull request #897: test: add cHash upstream (E2E)

Posted by GitBox <gi...@apache.org>.
idbeta commented on a change in pull request #897:
URL: https://github.com/apache/apisix-dashboard/pull/897#discussion_r531677355



##########
File path: api/test/e2e/upstream_test.go
##########
@@ -188,7 +188,60 @@ func TestRoute_Node_Host(t *testing.T) {
 	}
 }
 
-//TODO cHash
+func TestUpstream_cHash(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create cHash upstream",
+			Object:   MangerApiExpect(t),
+			Method:   http.MethodPut,
+			Path:     "/apisix/admin/upstreams/1",
+			Body: `{
+                "nodes": [{
+                    "host": "172.16.238.20",
+                    "port": 1980,
+                    "weight": 1
+				},
+				{
+                    "host": "172.16.238.20",
+                    "port": 1981,
+                    "weight": 2
+                }],
+				"type": "chash",
+				"hash_on":"header",
+				"key": "remote_addr"
+			}`,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc: "create route using the upstream just created",
+			Object:   MangerApiExpect(t),
+			Method:   http.MethodPut,
+			Path:     "/apisix/admin/routes/1",
+			Body: `{
+				"uri": "/server_port",
+				"upstream_id": "1"
+			}`,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			Sleep:        sleepTime,
+		},
+		{
+			caseDesc:     "hit the route just created",
+			Object:       APISIXExpect(t),
+			Method:       http.MethodGet,
+			Path:         "/server_port",
+			ExpectStatus: http.StatusOK,
+			ExpectBody:   "1981",
+			Sleep:        sleepTime,
+		},

Review comment:
       so far, delete the upstream operation is completed in the last test case `TestUpstream_Delete`




----------------------------------------------------------------
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



[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #897: test: add cHash upstream (E2E)

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #897:
URL: https://github.com/apache/apisix-dashboard/pull/897#issuecomment-734777124


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/897?src=pr&el=h1) Report
   > Merging [#897](https://codecov.io/gh/apache/apisix-dashboard/pull/897?src=pr&el=desc) (58064dc) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/277ae594b213c1c0bfb9aa0d387622cc9c65a6cc?el=desc) (277ae59) will **decrease** coverage by `0.07%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/897/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/897?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #897      +/-   ##
   ==========================================
   - Coverage   43.03%   42.95%   -0.08%     
   ==========================================
     Files          18       18              
     Lines        1271     1271              
   ==========================================
   - Hits          547      546       -1     
   - Misses        632      633       +1     
     Partials       92       92              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/897?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/core/store/store.go](https://codecov.io/gh/apache/apisix-dashboard/pull/897/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmUuZ28=) | `78.28% <0.00%> (-0.66%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/897?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/apisix-dashboard/pull/897?src=pr&el=footer). Last update [277ae59...58064dc](https://codecov.io/gh/apache/apisix-dashboard/pull/897?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



[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #897: test: e2e test chash upstream with key (remote_addr)

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #897:
URL: https://github.com/apache/apisix-dashboard/pull/897#issuecomment-734777124


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/897?src=pr&el=h1) Report
   > Merging [#897](https://codecov.io/gh/apache/apisix-dashboard/pull/897?src=pr&el=desc) (644bb3b) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/95f26122fee30d2fcbd18d98433b106c09e9bdca?el=desc) (95f2612) will **decrease** coverage by `0.07%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/897/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/897?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #897      +/-   ##
   ==========================================
   - Coverage   43.10%   43.02%   -0.08%     
   ==========================================
     Files          18       18              
     Lines        1290     1290              
   ==========================================
   - Hits          556      555       -1     
   - Misses        642      643       +1     
     Partials       92       92              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/897?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/core/store/store.go](https://codecov.io/gh/apache/apisix-dashboard/pull/897/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmUuZ28=) | `78.57% <0.00%> (-0.65%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/897?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/apisix-dashboard/pull/897?src=pr&el=footer). Last update [95f2612...644bb3b](https://codecov.io/gh/apache/apisix-dashboard/pull/897?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



[GitHub] [apisix-dashboard] idbeta commented on a change in pull request #897: test: e2e test chash upstream with key (remote_addr)

Posted by GitBox <gi...@apache.org>.
idbeta commented on a change in pull request #897:
URL: https://github.com/apache/apisix-dashboard/pull/897#discussion_r533952686



##########
File path: api/test/e2e/upstream_test.go
##########
@@ -188,10 +191,198 @@ func TestRoute_Node_Host(t *testing.T) {
 	}
 }
 
-//TODO cHash
-//TODO websocket
+func TestUpstream_chash_remote_addr(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create chash upstream with key (remote_addr)",
+			Object:   ManagerApiExpect(t),
+			Method:   http.MethodPut,
+			Path:     "/apisix/admin/upstreams/1",
+			Body: `{
+				"nodes": [{
+					"host": "172.16.238.20",
+					"port": 1980,
+					"weight": 1
+				},
+				{
+					"host": "172.16.238.20",
+					"port": 1981,
+					"weight": 1
+				},
+				{
+					"host": "172.16.238.20",
+					"port": 1982,
+					"weight": 1
+				}],
+				"type": "chash",
+				"hash_on":"header",
+				"key": "remote_addr"
+			}`,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc: "create route using the upstream just created",
+			Object:   ManagerApiExpect(t),
+			Method:   http.MethodPut,
+			Path:     "/apisix/admin/routes/1",
+			Body: `{
+				"uri": "/server_port",
+				"upstream_id": "1"
+			}`,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			Sleep:        sleepTime,
+		},
+	}
+
+	for _, tc := range tests {
+		testCaseCheck(tc)
+	}
+
+	//hit routes
+	basepath := "http://127.0.0.1:9080/"
+	request, err := http.NewRequest("GET", basepath+"/server_port", nil)
+	request.Header.Add("Authorization", token)
+	var resp *http.Response
+	var respBody []byte
+	var count int
+	res := map[string]int{}
+	for i := 0; i <= 17; i++ {
+		resp, err = http.DefaultClient.Do(request)
+		assert.Nil(t, err)
+		respBody, err = ioutil.ReadAll(resp.Body)
+		body := string(respBody)
+		if _, ok := res[body]; !ok {
+			res[body] = 1
+		} else {
+			res[body] += 1
+		}
+		resp.Body.Close()
+	}
+	assert.Equal(t, 18, res["1980"]+res["1981"]+res["1982"])

Review comment:
       fixed.




----------------------------------------------------------------
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



[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #897: test: add cHash upstream (E2E)

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #897:
URL: https://github.com/apache/apisix-dashboard/pull/897#issuecomment-734777124


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/897?src=pr&el=h1) Report
   > Merging [#897](https://codecov.io/gh/apache/apisix-dashboard/pull/897?src=pr&el=desc) (bf91261) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/a7d8e13e0f97b4636ab8a35576a848c0bf004221?el=desc) (a7d8e13) will **decrease** coverage by `0.07%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/897/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/897?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #897      +/-   ##
   ==========================================
   - Coverage   42.82%   42.74%   -0.08%     
   ==========================================
     Files          18       18              
     Lines        1289     1289              
   ==========================================
   - Hits          552      551       -1     
   - Misses        645      646       +1     
     Partials       92       92              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/897?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/core/store/store.go](https://codecov.io/gh/apache/apisix-dashboard/pull/897/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmUuZ28=) | `78.57% <0.00%> (-0.65%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/897?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/apisix-dashboard/pull/897?src=pr&el=footer). Last update [a7d8e13...bf91261](https://codecov.io/gh/apache/apisix-dashboard/pull/897?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



[GitHub] [apisix-dashboard] nic-chen commented on pull request #897: test: e2e test chash upstream with key (remote_addr)

Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #897:
URL: https://github.com/apache/apisix-dashboard/pull/897#issuecomment-737044040


   LGTM Except for some code format issues.
   


----------------------------------------------------------------
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



[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #897: test: e2e test chash upstream with key (remote_addr)

Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #897:
URL: https://github.com/apache/apisix-dashboard/pull/897#discussion_r533944507



##########
File path: api/test/e2e/upstream_test.go
##########
@@ -188,10 +191,198 @@ func TestRoute_Node_Host(t *testing.T) {
 	}
 }
 
-//TODO cHash
-//TODO websocket
+func TestUpstream_chash_remote_addr(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create chash upstream with key (remote_addr)",
+			Object:   ManagerApiExpect(t),
+			Method:   http.MethodPut,
+			Path:     "/apisix/admin/upstreams/1",
+			Body: `{
+				"nodes": [{
+					"host": "172.16.238.20",
+					"port": 1980,
+					"weight": 1
+				},
+				{
+					"host": "172.16.238.20",
+					"port": 1981,
+					"weight": 1
+				},
+				{
+					"host": "172.16.238.20",
+					"port": 1982,
+					"weight": 1
+				}],
+				"type": "chash",
+				"hash_on":"header",
+				"key": "remote_addr"
+			}`,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc: "create route using the upstream just created",
+			Object:   ManagerApiExpect(t),
+			Method:   http.MethodPut,
+			Path:     "/apisix/admin/routes/1",
+			Body: `{
+				"uri": "/server_port",
+				"upstream_id": "1"
+			}`,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			Sleep:        sleepTime,
+		},
+	}
+
+	for _, tc := range tests {
+		testCaseCheck(tc)
+	}
+
+	//hit routes
+	basepath := "http://127.0.0.1:9080/"
+	request, err := http.NewRequest("GET", basepath+"/server_port", nil)
+	request.Header.Add("Authorization", token)
+	var resp *http.Response
+	var respBody []byte
+	var count int
+	res := map[string]int{}
+	for i := 0; i < 18; i++ {
+		resp, err = http.DefaultClient.Do(request)
+		assert.Nil(t, err)
+		respBody, err = ioutil.ReadAll(resp.Body)
+		body := string(respBody)
+		if _, ok := res[body]; !ok {
+			res[body] = 1
+		} else {
+			res[body] += 1
+		}
+		resp.Body.Close()
+	}

Review comment:
       Please add todo to remember to improve the code format, thanks.




----------------------------------------------------------------
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



[GitHub] [apisix-dashboard] membphis commented on a change in pull request #897: test: e2e test chash upstream with key (remote_addr)

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #897:
URL: https://github.com/apache/apisix-dashboard/pull/897#discussion_r533467653



##########
File path: api/test/e2e/upstream_test.go
##########
@@ -188,10 +191,198 @@ func TestRoute_Node_Host(t *testing.T) {
 	}
 }
 
-//TODO cHash
-//TODO websocket
+func TestUpstream_chash_remote_addr(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create chash upstream with key (remote_addr)",
+			Object:   ManagerApiExpect(t),
+			Method:   http.MethodPut,
+			Path:     "/apisix/admin/upstreams/1",
+			Body: `{
+				"nodes": [{
+					"host": "172.16.238.20",
+					"port": 1980,
+					"weight": 1
+				},
+				{
+					"host": "172.16.238.20",
+					"port": 1981,
+					"weight": 1
+				},
+				{
+					"host": "172.16.238.20",
+					"port": 1982,
+					"weight": 1
+				}],
+				"type": "chash",
+				"hash_on":"header",
+				"key": "remote_addr"
+			}`,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc: "create route using the upstream just created",
+			Object:   ManagerApiExpect(t),
+			Method:   http.MethodPut,
+			Path:     "/apisix/admin/routes/1",
+			Body: `{
+				"uri": "/server_port",
+				"upstream_id": "1"
+			}`,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			Sleep:        sleepTime,
+		},
+	}
+
+	for _, tc := range tests {
+		testCaseCheck(tc)
+	}
+
+	//hit routes
+	basepath := "http://127.0.0.1:9080/"
+	request, err := http.NewRequest("GET", basepath+"/server_port", nil)
+	request.Header.Add("Authorization", token)
+	var resp *http.Response
+	var respBody []byte
+	var count int
+	res := map[string]int{}
+	for i := 0; i <= 17; i++ {
+		resp, err = http.DefaultClient.Do(request)
+		assert.Nil(t, err)
+		respBody, err = ioutil.ReadAll(resp.Body)
+		body := string(respBody)
+		if _, ok := res[body]; !ok {
+			res[body] = 1
+		} else {
+			res[body] += 1
+		}
+		resp.Body.Close()
+	}
+	assert.Equal(t, 18, res["1980"]+res["1981"]+res["1982"])

Review comment:
       this way is meanless. 
   
   I think we can write more comments here or we change to a different way.
   
   




----------------------------------------------------------------
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



[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #897: test: e2e test cHash upstream with key (remote_addr)

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #897:
URL: https://github.com/apache/apisix-dashboard/pull/897#issuecomment-734777124


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/897?src=pr&el=h1) Report
   > Merging [#897](https://codecov.io/gh/apache/apisix-dashboard/pull/897?src=pr&el=desc) (6f1b09d) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/7d79386c0c7adcafb5389215c58ac4e8a0f2e8e8?el=desc) (7d79386) will **increase** coverage by `0.07%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/897/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/897?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #897      +/-   ##
   ==========================================
   + Coverage   43.02%   43.10%   +0.07%     
   ==========================================
     Files          18       18              
     Lines        1290     1290              
   ==========================================
   + Hits          555      556       +1     
   + Misses        643      642       -1     
     Partials       92       92              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/897?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/core/store/store.go](https://codecov.io/gh/apache/apisix-dashboard/pull/897/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmUuZ28=) | `79.22% <0.00%> (+0.64%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/897?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/apisix-dashboard/pull/897?src=pr&el=footer). Last update [7d79386...6f1b09d](https://codecov.io/gh/apache/apisix-dashboard/pull/897?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



[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #897: test: e2e test chash upstream with key (remote_addr)

Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #897:
URL: https://github.com/apache/apisix-dashboard/pull/897#discussion_r533944507



##########
File path: api/test/e2e/upstream_test.go
##########
@@ -188,10 +191,198 @@ func TestRoute_Node_Host(t *testing.T) {
 	}
 }
 
-//TODO cHash
-//TODO websocket
+func TestUpstream_chash_remote_addr(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create chash upstream with key (remote_addr)",
+			Object:   ManagerApiExpect(t),
+			Method:   http.MethodPut,
+			Path:     "/apisix/admin/upstreams/1",
+			Body: `{
+				"nodes": [{
+					"host": "172.16.238.20",
+					"port": 1980,
+					"weight": 1
+				},
+				{
+					"host": "172.16.238.20",
+					"port": 1981,
+					"weight": 1
+				},
+				{
+					"host": "172.16.238.20",
+					"port": 1982,
+					"weight": 1
+				}],
+				"type": "chash",
+				"hash_on":"header",
+				"key": "remote_addr"
+			}`,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc: "create route using the upstream just created",
+			Object:   ManagerApiExpect(t),
+			Method:   http.MethodPut,
+			Path:     "/apisix/admin/routes/1",
+			Body: `{
+				"uri": "/server_port",
+				"upstream_id": "1"
+			}`,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			Sleep:        sleepTime,
+		},
+	}
+
+	for _, tc := range tests {
+		testCaseCheck(tc)
+	}
+
+	//hit routes
+	basepath := "http://127.0.0.1:9080/"
+	request, err := http.NewRequest("GET", basepath+"/server_port", nil)
+	request.Header.Add("Authorization", token)
+	var resp *http.Response
+	var respBody []byte
+	var count int
+	res := map[string]int{}
+	for i := 0; i < 18; i++ {
+		resp, err = http.DefaultClient.Do(request)
+		assert.Nil(t, err)
+		respBody, err = ioutil.ReadAll(resp.Body)
+		body := string(respBody)
+		if _, ok := res[body]; !ok {
+			res[body] = 1
+		} else {
+			res[body] += 1
+		}
+		resp.Body.Close()
+	}

Review comment:
       Please add todo to remember to improve the code format, thanks.
   and other places




----------------------------------------------------------------
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



[GitHub] [apisix-dashboard] codecov-io commented on pull request #897: test: add cHash upstream (E2E)

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #897:
URL: https://github.com/apache/apisix-dashboard/pull/897#issuecomment-734777124


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/897?src=pr&el=h1) Report
   > Merging [#897](https://codecov.io/gh/apache/apisix-dashboard/pull/897?src=pr&el=desc) (8704fb1) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/277ae594b213c1c0bfb9aa0d387622cc9c65a6cc?el=desc) (277ae59) will **not change** coverage.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/897/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/897?src=pr&el=tree)
   
   ```diff
   @@           Coverage Diff           @@
   ##           master     #897   +/-   ##
   =======================================
     Coverage   43.03%   43.03%           
   =======================================
     Files          18       18           
     Lines        1271     1271           
   =======================================
     Hits          547      547           
     Misses        632      632           
     Partials       92       92           
   ```
   
   
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/897?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/apisix-dashboard/pull/897?src=pr&el=footer). Last update [277ae59...8704fb1](https://codecov.io/gh/apache/apisix-dashboard/pull/897?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



[GitHub] [apisix-dashboard] idbeta commented on a change in pull request #897: test: e2e test chash upstream with key (remote_addr)

Posted by GitBox <gi...@apache.org>.
idbeta commented on a change in pull request #897:
URL: https://github.com/apache/apisix-dashboard/pull/897#discussion_r533954451



##########
File path: api/test/e2e/upstream_test.go
##########
@@ -188,10 +191,198 @@ func TestRoute_Node_Host(t *testing.T) {
 	}
 }
 
-//TODO cHash
-//TODO websocket
+func TestUpstream_chash_remote_addr(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create chash upstream with key (remote_addr)",
+			Object:   ManagerApiExpect(t),
+			Method:   http.MethodPut,
+			Path:     "/apisix/admin/upstreams/1",
+			Body: `{
+				"nodes": [{
+					"host": "172.16.238.20",
+					"port": 1980,
+					"weight": 1
+				},
+				{
+					"host": "172.16.238.20",
+					"port": 1981,
+					"weight": 1
+				},
+				{
+					"host": "172.16.238.20",
+					"port": 1982,
+					"weight": 1
+				}],
+				"type": "chash",
+				"hash_on":"header",
+				"key": "remote_addr"
+			}`,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+		},
+		{
+			caseDesc: "create route using the upstream just created",
+			Object:   ManagerApiExpect(t),
+			Method:   http.MethodPut,
+			Path:     "/apisix/admin/routes/1",
+			Body: `{
+				"uri": "/server_port",
+				"upstream_id": "1"
+			}`,
+			Headers:      map[string]string{"Authorization": token},
+			ExpectStatus: http.StatusOK,
+			Sleep:        sleepTime,
+		},
+	}
+
+	for _, tc := range tests {
+		testCaseCheck(tc)
+	}
+
+	//hit routes
+	basepath := "http://127.0.0.1:9080/"
+	request, err := http.NewRequest("GET", basepath+"/server_port", nil)
+	request.Header.Add("Authorization", token)
+	var resp *http.Response
+	var respBody []byte
+	var count int
+	res := map[string]int{}
+	for i := 0; i < 18; i++ {
+		resp, err = http.DefaultClient.Do(request)
+		assert.Nil(t, err)
+		respBody, err = ioutil.ReadAll(resp.Body)
+		body := string(respBody)
+		if _, ok := res[body]; !ok {
+			res[body] = 1
+		} else {
+			res[body] += 1
+		}
+		resp.Body.Close()
+	}

Review comment:
       fixed.




----------------------------------------------------------------
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



[GitHub] [apisix-dashboard] membphis commented on pull request #897: test: e2e test chash upstream with key (remote_addr)

Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #897:
URL: https://github.com/apache/apisix-dashboard/pull/897#issuecomment-737011245


   @nic-chen @moonming @gxthrj @ShiningRush please review this PR if you have time


----------------------------------------------------------------
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



[GitHub] [apisix-dashboard] idbeta commented on a change in pull request #897: test: e2e test cHash upstream with key (remote_addr)

Posted by GitBox <gi...@apache.org>.
idbeta commented on a change in pull request #897:
URL: https://github.com/apache/apisix-dashboard/pull/897#discussion_r533257822



##########
File path: api/test/e2e/upstream_test.go
##########
@@ -188,10 +188,63 @@ func TestRoute_Node_Host(t *testing.T) {
 	}
 }
 
-//TODO cHash
+func TestUpstream_cHash(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create cHash upstream",
+			Object:   MangerApiExpect(t),
+			Method:   http.MethodPut,
+			Path:     "/apisix/admin/upstreams/1",
+			Body: `{
+                "nodes": [{
+                    "host": "172.16.238.20",
+                    "port": 1980,
+                    "weight": 1
+				},
+				{
+                    "host": "172.16.238.20",
+                    "port": 1981,
+                    "weight": 2
+                }],
+				"type": "chash",
+				"hash_on":"header",

Review comment:
       ok, you can check #932 #936 




----------------------------------------------------------------
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



[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #897: test: add cHash upstream (E2E)

Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #897:
URL: https://github.com/apache/apisix-dashboard/pull/897#discussion_r532121800



##########
File path: api/test/e2e/upstream_test.go
##########
@@ -188,10 +188,63 @@ func TestRoute_Node_Host(t *testing.T) {
 	}
 }
 
-//TODO cHash
+func TestUpstream_cHash(t *testing.T) {
+	tests := []HttpTestCase{
+		{
+			caseDesc: "create cHash upstream",
+			Object:   MangerApiExpect(t),
+			Method:   http.MethodPut,
+			Path:     "/apisix/admin/upstreams/1",
+			Body: `{
+                "nodes": [{
+                    "host": "172.16.238.20",
+                    "port": 1980,
+                    "weight": 1
+				},
+				{
+                    "host": "172.16.238.20",
+                    "port": 1981,
+                    "weight": 2
+                }],
+				"type": "chash",
+				"hash_on":"header",

Review comment:
       need cases for different hash_on




----------------------------------------------------------------
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