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/10/28 03:19:26 UTC
[GitHub] [apisix-dashboard] nic-chen opened a new pull request #612: test: add backend e2e test
nic-chen opened a new pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612
Please answer these questions before submitting a pull request
- Why submit this pull request?
- [ ] Bugfix
- [ ] New feature provided
- [ ] Improve performance
----------------------------------------------------------------
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] juzhiyuan commented on pull request #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#issuecomment-720449497
Just waiting for this request changes[1].
[1] https://github.com/apache/apisix-dashboard/pull/612#discussion_r515580619
----------------------------------------------------------------
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 #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#issuecomment-719365741
> If running a test case fails, does the error message contain the key information of the test case? For example, the title.
yes.
it provide test function name, file name and which line failed. like:
```
Error Trace: reporter.go:23
chain.go:21
response.go:585
response.go:151
route_test.go:79
Error:
expected status equal to:
"200 OK"
but got:
"400 Bad Request"
Test: TestRoute_Host
```
----------------------------------------------------------------
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 #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#discussion_r514907337
##########
File path: .github/workflows/deploy-api.yml
##########
@@ -0,0 +1,60 @@
+name: Deploy API to Azure
+
+on:
Review comment:
deleted. It's brought from branch master.
----------------------------------------------------------------
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] juzhiyuan commented on a change in pull request #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#discussion_r514930872
##########
File path: .github/workflows/e2e-test-backend.yml
##########
@@ -0,0 +1,52 @@
+name: Backend E2E Test
+
+on:
+ push:
+ branches:
+ - master
+ pull_request:
+ branches:
+ - master
+ - v2.0
+
+jobs:
+ run-test:
+ runs-on: ubuntu-latest
+
+ steps:
+ - uses: actions/checkout@v2
+
+ - name: install runtime
+ run: |
+ sudo apt-get update
+ sudo add-apt-repository ppa:longsleep/golang-backports
+ sudo apt update
+ export GO111MOUDULE=on
+ sudo apt install golang-1.14-go
+
+ - name: build manager api
+ working-directory: ./api
+ run: |
+ docker build -t managerapi:ci .
+
+ - name: run docker compose
+ working-directory: ./api/test/docker
+ run: |
+ docker-compose up -d
+ sleep 5
+ docker logs managerapi
+
+ # - name: run manager api
Review comment:
Remove those commented lines or uncomment them, please.
----------------------------------------------------------------
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] moonming commented on pull request #612: test: add backend e2e test
Posted by GitBox <gi...@apache.org>.
moonming commented on pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#issuecomment-718501120
this PR has a big title, will you add all e2e test cases in this PR?
----------------------------------------------------------------
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 #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#discussion_r515748954
##########
File path: api/test/e2e/route_test.go
##########
@@ -0,0 +1,233 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestRoute_Host_Params(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "invalid host",
+ Object: MangerApiExpect(t),
+ Path: "/apisix/admin/routes/r1",
+ Method: http.MethodPut,
+ Body: `{
+ "uri": "/hello_",
+ "host": "$%$foo.com",
+ "upstream": {
+ "nodes": {
+ "172.16.238.120:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ },
+ {
+ caseDesc: "invalid hosts",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello_",
Review comment:
bad indentation
##########
File path: api/test/e2e/route_test.go
##########
@@ -0,0 +1,233 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestRoute_Host_Params(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "invalid host",
Review comment:
I think we can remove the case `invalid host` for E2E.
we can put them to a different function, eg: `TestRoute_Invalid_Host`.
##########
File path: api/test/e2e/route_test.go
##########
@@ -0,0 +1,233 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestRoute_Host_Params(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "invalid host",
+ Object: MangerApiExpect(t),
+ Path: "/apisix/admin/routes/r1",
+ Method: http.MethodPut,
+ Body: `{
+ "uri": "/hello_",
+ "host": "$%$foo.com",
+ "upstream": {
+ "nodes": {
+ "172.16.238.120:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ },
+ {
+ caseDesc: "invalid hosts",
Review comment:
ditto
##########
File path: api/test/e2e/route_test.go
##########
@@ -0,0 +1,233 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestRoute_Host_Params(t *testing.T) {
Review comment:
how about these steps? it seems clear:
1. hit route `/hello` -> will fail, no route
2. add route, `/hello` -> success
3. hit route `/hello` -> success
4. hit route `/not_found` ->will fail, not match route
5. delete route `/hello` -> success
6. hit route `/hello` -> will fail, no route
##########
File path: api/test/e2e/route_test.go
##########
@@ -0,0 +1,233 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestRoute_Host_Params(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "invalid host",
+ Object: MangerApiExpect(t),
+ Path: "/apisix/admin/routes/r1",
+ Method: http.MethodPut,
+ Body: `{
+ "uri": "/hello_",
+ "host": "$%$foo.com",
+ "upstream": {
+ "nodes": {
+ "172.16.238.120:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ },
+ {
+ caseDesc: "invalid hosts",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello_",
Review comment:
please fix similar points
##########
File path: api/test/e2e/base.go
##########
@@ -0,0 +1,134 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "bytes"
+ "io/ioutil"
+ "net/http"
+ "testing"
+ "time"
+
+ "github.com/gavv/httpexpect/v2"
+ "github.com/tidwall/gjson"
+)
+
+var token string
+
+func init() {
+ //login to get auth token
+ requestBody := []byte(`{
+ "username": "admin",
+ "password": "admin"
+ }`)
+
+ url := "http://127.0.0.1:8080/apisix/admin/user/login"
+ req, err := http.NewRequest(http.MethodPost, url, bytes.NewBuffer(requestBody))
+ if err != nil {
+ panic(err)
+ }
+
+ client := &http.Client{}
+ resp, err := client.Do(req)
+ if err != nil {
+ panic(err)
+ }
+
+ defer resp.Body.Close()
+
+ body, err := ioutil.ReadAll(resp.Body)
+ if err != nil {
+ panic(err)
+ }
+
+ respond := gjson.ParseBytes(body)
+ token = respond.Get("data.token").String()
+}
+
+func MangerApiExpect(t *testing.T) *httpexpect.Expect {
+ return httpexpect.New(t, "http://127.0.0.1:8080")
+}
+
+func APISIXExpect(t *testing.T) *httpexpect.Expect {
+ return httpexpect.New(t, "http://127.0.0.1:9080")
+}
+
+var sleepTime = time.Duration(100) * time.Millisecond
+
+type HttpTestCase struct {
+ caseDesc string
+ Object *httpexpect.Expect
+ Method string
+ Path string
+ Body string
+ Headers map[string]string
+ ExpectStatus int
+ ExpectCode int
+ ExpectMessage string
+ ExpectBody string
+ Sleep time.Duration //ms
+}
+
+func testCaseCheck(tc HttpTestCase) {
+ //init
+ expectObj := tc.Object
+ var req *httpexpect.Request
+ switch tc.Method {
+ case http.MethodGet:
+ req = expectObj.GET(tc.Path)
+ case http.MethodPut:
+ req = expectObj.PUT(tc.Path)
+ case http.MethodPost:
+ req = expectObj.POST(tc.Path)
+ case http.MethodDelete:
+ req = expectObj.DELETE(tc.Path)
+ case http.MethodPatch:
+ req = expectObj.PATCH(tc.Path)
+ default:
+ }
+
+ if req == nil {
+ panic("fail to init request")
+ }
+
+ if tc.Sleep != 0 {
+ time.Sleep(tc.Sleep)
+ }
+
+ //set header
+ for key, val := range tc.Headers {
+ req.WithHeader(key, val)
+ }
+
+ //set body
+ if tc.Body != "" {
+ req.WithText(tc.Body)
+ }
+
+ //respond check
+ resp := req.Expect()
+
+ //match http status
+ if tc.ExpectStatus != 0 {
+ resp.Status(tc.ExpectStatus)
+ }
+
+ //match body
Review comment:
@nic-chen any news about this?
##########
File path: api/test/docker/manager-api-conf.json
##########
@@ -0,0 +1,31 @@
+{
+ "conf": {
+ "syslog": {
+ "host": "127.0.0.1"
+ },
+ "listen": {
+ "host": "0.0.0.0",
+ "port": 8080
+ },
+ "dag-lib-path": "",
+ "etcd": {
+ "endpoints": "172.16.238.10:2379,172.16.238.11:2379,172.16.238.12:2379"
+ }
+ },
+ "authentication": {
+ "session": {
+ "secret": "secret",
+ "expireTime": 3600
+ },
+ "user": [
+ {
+ "username": "admin",
+ "password": "admin"
+ },
+ {
Review comment:
two problems:
1. `user` -> `users`: I will create a new issue to fix this.
2. why do we need two users here?
##########
File path: api/test/docker/manager-api-conf.json
##########
@@ -0,0 +1,31 @@
+{
+ "conf": {
+ "syslog": {
+ "host": "127.0.0.1"
+ },
+ "listen": {
+ "host": "0.0.0.0",
+ "port": 8080
+ },
+ "dag-lib-path": "",
+ "etcd": {
+ "endpoints": "172.16.238.10:2379,172.16.238.11:2379,172.16.238.12:2379"
+ }
+ },
+ "authentication": {
+ "session": {
+ "secret": "secret",
+ "expireTime": 3600
+ },
+ "user": [
+ {
+ "username": "admin",
+ "password": "admin"
+ },
+ {
Review comment:
support multiple user to login, I got it.
----------------------------------------------------------------
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] juzhiyuan commented on a change in pull request #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#discussion_r514812818
##########
File path: .github/workflows/test-api.yml
##########
@@ -42,8 +42,9 @@ jobs:
sudo apt install golang-1.14-go
- name: run test
+
Review comment:
remove this line
##########
File path: api/test/e2e/route_test.go
##########
@@ -0,0 +1,164 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+ "time"
+)
+
+func TestRoute_Host(t *testing.T) {
+
+ //create route use hosts
+ MangerApiExpect(t).PUT("/apisix/admin/routes/r1").WithText(`{
+ "uri": "/hello_",
+ "hosts": ["foo.com", "*.bar.com"],
+ "upstream": {
+ "nodes": {
+ "172.16.238.120:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`).
Review comment:
done?
##########
File path: .github/workflows/deploy-api.yml
##########
@@ -0,0 +1,60 @@
+name: Deploy API to Azure
+
+on:
Review comment:
Do we still need this action? I'm rewriting the Dockerfile so that we could use only 1 container to deploy both backend and frontend.
##########
File path: .github/workflows/e2e-test-backend.yml
##########
@@ -0,0 +1,49 @@
+name: Backend E2E Test
+
+on:
+ push:
+ branches:
+ - master
+ pull_request:
+ branches:
+ - master
Review comment:
We could add v2.0 here to trigger test
----------------------------------------------------------------
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 #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#discussion_r514912142
##########
File path: api/test/e2e/route_test.go
##########
@@ -0,0 +1,164 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+ "time"
+)
+
+func TestRoute_Host(t *testing.T) {
+
+ //create route use hosts
+ MangerApiExpect(t).PUT("/apisix/admin/routes/r1").WithText(`{
+ "uri": "/hello_",
+ "hosts": ["foo.com", "*.bar.com"],
+ "upstream": {
+ "nodes": {
+ "172.16.238.120:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`).
+ WithHeader("Authorization", accessToken).
+ Expect().
+ Status(http.StatusOK)
+
+ //sleep
+ time.Sleep(time.Duration(100) * time.Millisecond)
+
+ //hit route -- not found
+ APISIXExpect(t).GET("/not_found").
+ Expect().
+ Status(http.StatusNotFound)
+
+ //hit route -- not found, wrong host
+ APISIXExpect(t).GET("/hello_").
+ WithHeader("Host", "not_found.com").
+ Expect().
+ Status(http.StatusNotFound)
+
+ //hit route - ok
+ APISIXExpect(t).GET("/hello_").
+ WithHeader("Host", "foo.com").
+ Expect().
+ Status(http.StatusOK)
+
+ //create route -- invalid hosts
+ MangerApiExpect(t).PUT("/apisix/admin/routes/r2").WithText(`{
+ "uri": "/hello_",
+ "hosts": ["$%$foo.com", "*.bar.com"],
+ "upstream": {
+ "nodes": {
+ "172.16.238.120:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`).
Review comment:
sure
----------------------------------------------------------------
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 #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#discussion_r514822176
##########
File path: api/go.sum
##########
@@ -1,43 +1,14 @@
-cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
Review comment:
why we delete so many dependency libraries?
they are only used for testing?
##########
File path: api/test/e2e/route_test.go
##########
@@ -0,0 +1,164 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+ "time"
+)
+
+func TestRoute_Host(t *testing.T) {
+
+ //create route use hosts
Review comment:
bad indentation
##########
File path: .github/workflows/deploy-api.yml
##########
@@ -0,0 +1,60 @@
+name: Deploy API to Azure
+
+on:
Review comment:
one PR for one thing, for E2E test, I think we do not need to update `deploy-api.yml`, all right?
##########
File path: api/test/e2e/route_test.go
##########
@@ -0,0 +1,164 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+ "time"
+)
+
+func TestRoute_Host(t *testing.T) {
+
+ //create route use hosts
+ MangerApiExpect(t).PUT("/apisix/admin/routes/r1").WithText(`{
+ "uri": "/hello_",
+ "hosts": ["foo.com", "*.bar.com"],
+ "upstream": {
+ "nodes": {
+ "172.16.238.120:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`).
+ WithHeader("Authorization", accessToken).
+ Expect().
+ Status(http.StatusOK)
+
+ //sleep
+ time.Sleep(time.Duration(100) * time.Millisecond)
+
+ //hit route -- not found
+ APISIXExpect(t).GET("/not_found").
+ Expect().
+ Status(http.StatusNotFound)
+
+ //hit route -- not found, wrong host
+ APISIXExpect(t).GET("/hello_").
+ WithHeader("Host", "not_found.com").
+ Expect().
+ Status(http.StatusNotFound)
+
+ //hit route - ok
+ APISIXExpect(t).GET("/hello_").
+ WithHeader("Host", "foo.com").
+ Expect().
+ Status(http.StatusOK)
+
+ //create route -- invalid hosts
+ MangerApiExpect(t).PUT("/apisix/admin/routes/r2").WithText(`{
+ "uri": "/hello_",
+ "hosts": ["$%$foo.com", "*.bar.com"],
+ "upstream": {
+ "nodes": {
+ "172.16.238.120:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`).
Review comment:
bad indentation? if yes, please fix other similar points
----------------------------------------------------------------
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 #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#discussion_r514931895
##########
File path: .github/workflows/e2e-test-backend.yml
##########
@@ -0,0 +1,52 @@
+name: Backend E2E Test
+
+on:
+ push:
+ branches:
+ - master
+ pull_request:
+ branches:
+ - master
+ - v2.0
+
+jobs:
+ run-test:
+ runs-on: ubuntu-latest
+
+ steps:
+ - uses: actions/checkout@v2
+
+ - name: install runtime
+ run: |
+ sudo apt-get update
+ sudo add-apt-repository ppa:longsleep/golang-backports
+ sudo apt update
+ export GO111MOUDULE=on
+ sudo apt install golang-1.14-go
+
+ - name: build manager api
+ working-directory: ./api
+ run: |
+ docker build -t managerapi:ci .
+
+ - name: run docker compose
+ working-directory: ./api/test/docker
+ run: |
+ docker-compose up -d
+ sleep 5
+ docker logs managerapi
+
+ # - name: run manager api
Review comment:
will remove when CI pass
----------------------------------------------------------------
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] juzhiyuan merged pull request #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
juzhiyuan merged pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612
----------------------------------------------------------------
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] juzhiyuan commented on a change in pull request #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#discussion_r514829914
##########
File path: api/test/e2e/route_test.go
##########
@@ -0,0 +1,164 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+ "time"
+)
+
+func TestRoute_Host(t *testing.T) {
+
+ //create route use hosts
+ MangerApiExpect(t).PUT("/apisix/admin/routes/r1").WithText(`{
+ "uri": "/hello_",
+ "hosts": ["foo.com", "*.bar.com"],
+ "upstream": {
+ "nodes": {
+ "172.16.238.120:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`).
+ WithHeader("Authorization", accessToken).
+ Expect().
+ Status(http.StatusOK)
+
+ //sleep
+ time.Sleep(time.Duration(100) * time.Millisecond)
+
+ //hit route -- not found
+ APISIXExpect(t).GET("/not_found").
+ Expect().
+ Status(http.StatusNotFound)
+
+ //hit route -- not found, wrong host
+ APISIXExpect(t).GET("/hello_").
+ WithHeader("Host", "not_found.com").
+ Expect().
+ Status(http.StatusNotFound)
+
+ //hit route - ok
+ APISIXExpect(t).GET("/hello_").
+ WithHeader("Host", "foo.com").
+ Expect().
+ Status(http.StatusOK)
+
+ //create route -- invalid hosts
+ MangerApiExpect(t).PUT("/apisix/admin/routes/r2").WithText(`{
+ "uri": "/hello_",
+ "hosts": ["$%$foo.com", "*.bar.com"],
+ "upstream": {
+ "nodes": {
+ "172.16.238.120:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`).
Review comment:
We may use some tools to lint those files automatically, cc @nic-chen
[1] https://golangci-lint.run/
----------------------------------------------------------------
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] liuxiran commented on a change in pull request #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
liuxiran commented on a change in pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#discussion_r514955038
##########
File path: api/test/e2e/base.go
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "bytes"
+ "fmt"
+ "io/ioutil"
+ "net/http"
+ "testing"
+
+ "github.com/gavv/httpexpect/v2"
+ "github.com/tidwall/gjson"
+)
+
+var token string
+
+func init() {
+ //login to get auth token
+ requestBody := []byte(`{
+ "username": "admin",
Review comment:
Please adjust the indentation :)
----------------------------------------------------------------
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 #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#discussion_r514293952
##########
File path: api/test/e2e/route_test.go
##########
@@ -0,0 +1,164 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+ "time"
+)
+
+func TestRoute_Host(t *testing.T) {
+
+ //create route use hosts
+ MangerApiExpect(t).PUT("/apisix/admin/routes/r1").WithText(`{
+ "uri": "/hello_",
+ "hosts": ["foo.com", "*.bar.com"],
+ "upstream": {
+ "nodes": {
+ "172.16.238.120:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`).
+ WithHeader("Authorization", accessToken).
+ Expect().
+ Status(http.StatusOK)
+
+ //sleep
+ time.Sleep(time.Duration(100) * time.Millisecond)
+
+ //hit route -- not found
+ APISIXExpect(t).GET("/not_found").
+ Expect().
+ Status(http.StatusNotFound)
+
+ //hit route -- not found, wrong host
Review comment:
bad indentation
##########
File path: api/test/e2e/route_test.go
##########
@@ -0,0 +1,164 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+ "time"
+)
+
+func TestRoute_Host(t *testing.T) {
+
+ //create route use hosts
+ MangerApiExpect(t).PUT("/apisix/admin/routes/r1").WithText(`{
+ "uri": "/hello_",
+ "hosts": ["foo.com", "*.bar.com"],
+ "upstream": {
+ "nodes": {
+ "172.16.238.120:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`).
+ WithHeader("Authorization", accessToken).
+ Expect().
+ Status(http.StatusOK)
+
+ //sleep
+ time.Sleep(time.Duration(100) * time.Millisecond)
+
+ //hit route -- not found
+ APISIXExpect(t).GET("/not_found").
+ Expect().
+ Status(http.StatusNotFound)
+
+ //hit route -- not found, wrong host
+ APISIXExpect(t).GET("/hello_").
+ WithHeader("Host", "not_found.com").
+ Expect().
+ Status(http.StatusNotFound)
+
+ //hit route - ok
Review comment:
ditto
##########
File path: api/test/e2e/route_test.go
##########
@@ -0,0 +1,164 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+ "time"
+)
+
+func TestRoute_Host(t *testing.T) {
+
+ //create route use hosts
+ MangerApiExpect(t).PUT("/apisix/admin/routes/r1").WithText(`{
+ "uri": "/hello_",
+ "hosts": ["foo.com", "*.bar.com"],
+ "upstream": {
+ "nodes": {
+ "172.16.238.120:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`).
Review comment:
bad indentation
##########
File path: api/test/e2e/route_test.go
##########
@@ -0,0 +1,164 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+ "time"
+)
+
+func TestRoute_Host(t *testing.T) {
+
+ //create route use hosts
+ MangerApiExpect(t).PUT("/apisix/admin/routes/r1").WithText(`{
+ "uri": "/hello_",
+ "hosts": ["foo.com", "*.bar.com"],
+ "upstream": {
+ "nodes": {
+ "172.16.238.120:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`).
+ WithHeader("Authorization", accessToken).
+ Expect().
+ Status(http.StatusOK)
+
+ //sleep
+ time.Sleep(time.Duration(100) * time.Millisecond)
+
+ //hit route -- not found
+ APISIXExpect(t).GET("/not_found").
+ Expect().
+ Status(http.StatusNotFound)
+
+ //hit route -- not found, wrong host
+ APISIXExpect(t).GET("/hello_").
+ WithHeader("Host", "not_found.com").
+ Expect().
+ Status(http.StatusNotFound)
+
+ //hit route - ok
+ APISIXExpect(t).GET("/hello_").
+ WithHeader("Host", "foo.com").
+ Expect().
+ Status(http.StatusOK)
+
+ //create route -- invalid hosts
+ MangerApiExpect(t).PUT("/apisix/admin/routes/r2").WithText(`{
+ "uri": "/hello_",
+ "hosts": ["$%$foo.com", "*.bar.com"],
+ "upstream": {
+ "nodes": {
+ "172.16.238.120:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`).
+ WithHeader("Authorization", accessToken).
+ Expect().
+ Status(http.StatusBadRequest)
+
+ //create route -- invalid type for hosts
+ MangerApiExpect(t).PUT("/apisix/admin/routes/r2").WithText(`{
+ "uri": "/hello_",
Review comment:
bad indentation too
----------------------------------------------------------------
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] moonming commented on a change in pull request #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#discussion_r515802418
##########
File path: api/conf/conf.go
##########
@@ -36,8 +36,8 @@ const (
EnvDEV = "dev"
EnvLOCAL = "local"
- confJsonPath = "/go/manager-api/conf.json"
- schemaPath = "/go/manager-api/schema.json"
+ confJsonPath = "/go/manager-api/conf.json"
Review comment:
why create a root dir for `/go/`?
----------------------------------------------------------------
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] liuxiran commented on pull request #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
liuxiran commented on pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#issuecomment-719435033
Is it necessary to add a test case for `TestRoute_Host`: create a route without both host and hosts ?
----------------------------------------------------------------
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] gxthrj commented on a change in pull request #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#discussion_r515752507
##########
File path: api/test/docker/docker-compose.yaml
##########
@@ -0,0 +1,183 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+version: "3.6"
+
+services:
+ node1:
+ image: quay.io/coreos/etcd:v3.4.0
+ ports:
+ - "2379:2379"
+ expose:
+ - 2379
+ - 2380
+ networks:
+ apisix_dashboard_e2e:
+ ipv4_address: 172.16.238.100
+ environment:
+ - ETCDCTL_API=3
+ command:
+ - /usr/local/bin/etcd
+ - --data-dir=/etcd-data
+ - --name
+ - node1
+ - --initial-advertise-peer-urls
+ - http://172.16.238.100:2380
+ - --listen-peer-urls
+ - http://0.0.0.0:2380
+ - --advertise-client-urls
+ - http://172.16.238.100:2379
+ - --listen-client-urls
+ - http://0.0.0.0:2379
+ - --initial-cluster
+ - node1=http://172.16.238.100:2380,node2=http://172.16.238.101:2380,node3=http://172.16.238.102:2380
+ - --initial-cluster-state
+ - new
+ - --initial-cluster-token
+ - docker-etcd
+
+ node2:
+ image: quay.io/coreos/etcd:v3.4.0
+ networks:
+ apisix_dashboard_e2e:
+ ipv4_address: 172.16.238.101
+ environment:
+ - ETCDCTL_API=3
+ ports:
+ - "3379:2379"
+ expose:
+ - 2379
+ - 2380
+ command:
+ - /usr/local/bin/etcd
+ - --data-dir=/etcd-data
+ - --name
+ - node2
+ - --initial-advertise-peer-urls
+ - http://172.16.238.101:2380
+ - --listen-peer-urls
+ - http://0.0.0.0:2380
+ - --advertise-client-urls
+ - http://172.16.238.101:2379
+ - --listen-client-urls
+ - http://0.0.0.0:2379
+ - --initial-cluster
+ - node1=http://172.16.238.100:2380,node2=http://172.16.238.101:2380,node3=http://172.16.238.102:2380
+ - --initial-cluster-state
+ - new
+ - --initial-cluster-token
+ - docker-etcd
+
+ node3:
+ image: quay.io/coreos/etcd:v3.4.0
+ networks:
+ apisix_dashboard_e2e:
+ ipv4_address: 172.16.238.102
+ environment:
+ - ETCDCTL_API=3
+ ports:
+ - "4379:2379"
Review comment:
`4379` is a strange port,
we can remove Host port mapping if we do not need this
----------------------------------------------------------------
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] moonming commented on pull request #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
moonming commented on pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#issuecomment-718788120
Where can we get the code coverage?
----------------------------------------------------------------
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 #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#issuecomment-719366751
> Where can we get the code coverage?
we could run for it in an other pr.
----------------------------------------------------------------
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 #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#issuecomment-718511547
> The test should not modify any base code and dependencies
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] nic-chen commented on pull request #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#issuecomment-719443334
> Is it necessary to add a test case for `TestRoute_Host`: create a route without both host and hosts ?
done. 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] moonming commented on a change in pull request #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#discussion_r514890091
##########
File path: api/test/e2e/route_test.go
##########
@@ -0,0 +1,164 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+ "time"
+)
+
+func TestRoute_Host(t *testing.T) {
+
+ //create route use hosts
+ MangerApiExpect(t).PUT("/apisix/admin/routes/r1").WithText(`{
+ "uri": "/hello_",
+ "hosts": ["foo.com", "*.bar.com"],
+ "upstream": {
+ "nodes": {
+ "172.16.238.120:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`).
+ WithHeader("Authorization", accessToken).
+ Expect().
+ Status(http.StatusOK)
+
+ //sleep
+ time.Sleep(time.Duration(100) * time.Millisecond)
+
+ //hit route -- not found
+ APISIXExpect(t).GET("/not_found").
+ Expect().
+ Status(http.StatusNotFound)
+
+ //hit route -- not found, wrong host
+ APISIXExpect(t).GET("/hello_").
+ WithHeader("Host", "not_found.com").
+ Expect().
+ Status(http.StatusNotFound)
+
+ //hit route - ok
+ APISIXExpect(t).GET("/hello_").
+ WithHeader("Host", "foo.com").
+ Expect().
+ Status(http.StatusOK)
+
+ //create route -- invalid hosts
+ MangerApiExpect(t).PUT("/apisix/admin/routes/r2").WithText(`{
+ "uri": "/hello_",
+ "hosts": ["$%$foo.com", "*.bar.com"],
+ "upstream": {
+ "nodes": {
+ "172.16.238.120:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`).
Review comment:
> We may use some tools to lint those files automatically, cc @nic-chen
>
> [1] https://golangci-lint.run/
we can add to this linter to CI too.
----------------------------------------------------------------
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 #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#discussion_r514966671
##########
File path: api/test/e2e/base.go
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "bytes"
+ "fmt"
+ "io/ioutil"
+ "net/http"
+ "testing"
+
+ "github.com/gavv/httpexpect/v2"
+ "github.com/tidwall/gjson"
+)
+
+var token string
+
+func init() {
+ //login to get auth token
+ requestBody := []byte(`{
+ "username": "admin",
Review comment:
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] nic-chen commented on a change in pull request #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#discussion_r514904437
##########
File path: api/go.sum
##########
@@ -1,43 +1,14 @@
-cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
Review comment:
yes. I remove some dependency lib and It's auto generated by `Go Module`. and e2e run as an independent project.
----------------------------------------------------------------
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] juzhiyuan commented on a change in pull request #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#discussion_r514888873
##########
File path: api/test/e2e/route_test.go
##########
@@ -41,126 +41,136 @@ func TestRoute_Host(t *testing.T) {
Expect().
Status(http.StatusOK)
- //sleep
- time.Sleep(time.Duration(100) * time.Millisecond)
+ //sleep
+ time.Sleep(time.Duration(100) * time.Millisecond)
//hit route -- not found
APISIXExpect(t).GET("/not_found").
Expect().
Status(http.StatusNotFound)
- //hit route -- not found, wrong host
- APISIXExpect(t).GET("/hello_").
- WithHeader("Host", "not_found.com").
- Expect().
- Status(http.StatusNotFound)
-
- //hit route - ok
- APISIXExpect(t).GET("/hello_").
- WithHeader("Host", "foo.com").
- Expect().
- Status(http.StatusOK)
-
- //create route -- invalid hosts
- MangerApiExpect(t).PUT("/apisix/admin/routes/r2").WithText(`{
- "uri": "/hello_",
- "hosts": ["$%$foo.com", "*.bar.com"],
- "upstream": {
+ //hit route -- not found, wrong host
+ APISIXExpect(t).GET("/hello_").
+ WithHeader("Host", "not_found.com").
+ Expect().
+ Status(http.StatusNotFound)
+
+ //hit route - ok
+ APISIXExpect(t).GET("/hello_").
+ WithHeader("Host", "foo.com").
+ Expect().
+ Status(http.StatusOK)
+
+ //create route -- invalid hosts
+ MangerApiExpect(t).
+ PUT("/apisix/admin/routes/r2").
+ WithText(`{
+ "uri": "/hello_",
+ "hosts": ["$%$foo.com", "*.bar.com"],
+ "upstream": {
+ "nodes": {
+ "172.16.238.120:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }`).
+ WithHeader("Authorization", accessToken).
+ Expect().
+ Status(http.StatusBadRequest)
+
+ //create route -- invalid type for hosts
+ MangerApiExpect(t).
+ PUT("/apisix/admin/routes/r2").
+ WithText(`{
+ "uri": "/hello_",
+ "hosts": [1, "*.bar.com"],
+ "upstream": {
"nodes": {
- "172.16.238.120:1980": 1
+ "172.16.238.120:1980": 1
},
"type": "roundrobin"
- }
- }`).
- WithHeader("Authorization", accessToken).
- Expect().
- Status(http.StatusBadRequest)
-
- //create route -- invalid type for hosts
- MangerApiExpect(t).PUT("/apisix/admin/routes/r2").WithText(`{
- "uri": "/hello_",
- "hosts": [1, "*.bar.com"],
- "upstream": {
- "nodes": {
- "172.16.238.120:1980": 1
- },
- "type": "roundrobin"
- }
- }`).
- WithHeader("Authorization", accessToken).
- Expect().
- //Status(http.StatusBadRequest)
- JSON().Object().ValueNotEqual("code", 0)
+ }
+ }`).
+ WithHeader("Authorization", accessToken).
Review comment:
`accessToken` is not a good name IMO, `token` would be more clearer and without ambiguity.
----------------------------------------------------------------
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] juzhiyuan commented on pull request #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#issuecomment-719478498
Wow! All tests have passed, could we merge this PR?
----------------------------------------------------------------
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] liuxiran commented on a change in pull request #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
liuxiran commented on a change in pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#discussion_r514954657
##########
File path: api/test/docker/manager-api-conf.json
##########
@@ -0,0 +1,32 @@
+{
+ "conf": {
+ "syslog": {
+ "host": "127.0.0.1"
+ },
+ "listen": {
+ "host": "",
+ "port": 8080
+ },
+ "dag-lib-path": "",
+ "etcd": {
+ "endpoints": "172.16.238.100:2379,172.16.238.101:2379,172.16.238.102:2379"
+ }
+ },
+ "authentication": {
+ "session": {
+ "secret": "secret",
+ "expireTime": 3600
+ },
+ "user": [
+ {
+ "username": "admin",
+ "password": "admin"
+ },
+ {
+ "username": "user",
+ "password": "user"
+ }
+ ]
+ }
+ }
+
Review comment:
new line here
----------------------------------------------------------------
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 #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#issuecomment-719375169
> some cases, we need to set different `api/test/docker/apisix_config.yaml`.
>
> how to support this case?
I think `matrix` for github actions is good for this case. or we could do it use shell script. we should do this in a separate PR,
----------------------------------------------------------------
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 #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#discussion_r515746607
##########
File path: api/test/docker/docker-compose.yaml
##########
@@ -0,0 +1,183 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+version: "3.6"
+
+services:
+ node1:
+ image: quay.io/coreos/etcd:v3.4.0
+ ports:
+ - "2379:2379"
+ expose:
+ - 2379
+ - 2380
+ networks:
+ apisix_dashboard_e2e:
+ ipv4_address: 172.16.238.100
+ environment:
+ - ETCDCTL_API=3
+ command:
+ - /usr/local/bin/etcd
+ - --data-dir=/etcd-data
+ - --name
+ - node1
+ - --initial-advertise-peer-urls
+ - http://172.16.238.100:2380
+ - --listen-peer-urls
+ - http://0.0.0.0:2380
+ - --advertise-client-urls
+ - http://172.16.238.100:2379
+ - --listen-client-urls
+ - http://0.0.0.0:2379
+ - --initial-cluster
+ - node1=http://172.16.238.100:2380,node2=http://172.16.238.101:2380,node3=http://172.16.238.102:2380
+ - --initial-cluster-state
+ - new
+ - --initial-cluster-token
+ - docker-etcd
+
+ node2:
+ image: quay.io/coreos/etcd:v3.4.0
+ networks:
+ apisix_dashboard_e2e:
+ ipv4_address: 172.16.238.101
+ environment:
+ - ETCDCTL_API=3
+ ports:
+ - "3379:2379"
+ expose:
+ - 2379
+ - 2380
+ command:
+ - /usr/local/bin/etcd
+ - --data-dir=/etcd-data
+ - --name
+ - node2
+ - --initial-advertise-peer-urls
+ - http://172.16.238.101:2380
+ - --listen-peer-urls
+ - http://0.0.0.0:2380
+ - --advertise-client-urls
+ - http://172.16.238.101:2379
+ - --listen-client-urls
+ - http://0.0.0.0:2379
+ - --initial-cluster
+ - node1=http://172.16.238.100:2380,node2=http://172.16.238.101:2380,node3=http://172.16.238.102:2380
+ - --initial-cluster-state
+ - new
+ - --initial-cluster-token
+ - docker-etcd
+
+ node3:
+ image: quay.io/coreos/etcd:v3.4.0
+ networks:
+ apisix_dashboard_e2e:
+ ipv4_address: 172.16.238.102
+ environment:
+ - ETCDCTL_API=3
+ ports:
+ - "4379:2379"
+ expose:
+ - 2379
+ - 2380
+ command:
+ - /usr/local/bin/etcd
+ - --data-dir=/etcd-data
+ - --name
+ - node3
+ - --initial-advertise-peer-urls
+ - http://172.16.238.102:2380
+ - --listen-peer-urls
+ - http://0.0.0.0:2380
+ - --advertise-client-urls
+ - http://172.16.238.102:2379
+ - --listen-client-urls
+ - http://0.0.0.0:2379
+ - --initial-cluster
+ - node1=http://172.16.238.100:2380,node2=http://172.16.238.101:2380,node3=http://172.16.238.102:2380
+ - --initial-cluster-state
+ - new
+ - --initial-cluster-token
+ - docker-etcd
+
+ upstream:
+ image: johz/upstream:v2.0
+ restart: always
+ ports:
+ - '80:80/tcp'
+ - '1980:1980/tcp'
+ - '1981:1981/tcp'
+ - '1982:1982/tcp'
+ - '1983:1983/tcp'
+ - '1984:1984/tcp'
+ networks:
+ apisix_dashboard_e2e:
+ ipv4_address: 172.16.238.120
Review comment:
will update next move.
##########
File path: api/test/docker/docker-compose.yaml
##########
@@ -0,0 +1,183 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+version: "3.6"
+
+services:
+ node1:
+ image: quay.io/coreos/etcd:v3.4.0
+ ports:
+ - "2379:2379"
+ expose:
+ - 2379
+ - 2380
+ networks:
+ apisix_dashboard_e2e:
+ ipv4_address: 172.16.238.100
+ environment:
+ - ETCDCTL_API=3
+ command:
+ - /usr/local/bin/etcd
+ - --data-dir=/etcd-data
+ - --name
+ - node1
+ - --initial-advertise-peer-urls
+ - http://172.16.238.100:2380
+ - --listen-peer-urls
+ - http://0.0.0.0:2380
+ - --advertise-client-urls
+ - http://172.16.238.100:2379
+ - --listen-client-urls
+ - http://0.0.0.0:2379
+ - --initial-cluster
+ - node1=http://172.16.238.100:2380,node2=http://172.16.238.101:2380,node3=http://172.16.238.102:2380
+ - --initial-cluster-state
+ - new
+ - --initial-cluster-token
+ - docker-etcd
+
+ node2:
+ image: quay.io/coreos/etcd:v3.4.0
+ networks:
+ apisix_dashboard_e2e:
+ ipv4_address: 172.16.238.101
+ environment:
+ - ETCDCTL_API=3
+ ports:
+ - "3379:2379"
+ expose:
+ - 2379
+ - 2380
+ command:
+ - /usr/local/bin/etcd
+ - --data-dir=/etcd-data
+ - --name
+ - node2
+ - --initial-advertise-peer-urls
+ - http://172.16.238.101:2380
+ - --listen-peer-urls
+ - http://0.0.0.0:2380
+ - --advertise-client-urls
+ - http://172.16.238.101:2379
+ - --listen-client-urls
+ - http://0.0.0.0:2379
+ - --initial-cluster
+ - node1=http://172.16.238.100:2380,node2=http://172.16.238.101:2380,node3=http://172.16.238.102:2380
+ - --initial-cluster-state
+ - new
+ - --initial-cluster-token
+ - docker-etcd
+
+ node3:
+ image: quay.io/coreos/etcd:v3.4.0
+ networks:
+ apisix_dashboard_e2e:
+ ipv4_address: 172.16.238.102
+ environment:
+ - ETCDCTL_API=3
+ ports:
+ - "4379:2379"
+ expose:
+ - 2379
+ - 2380
+ command:
+ - /usr/local/bin/etcd
+ - --data-dir=/etcd-data
+ - --name
+ - node3
+ - --initial-advertise-peer-urls
+ - http://172.16.238.102:2380
+ - --listen-peer-urls
+ - http://0.0.0.0:2380
+ - --advertise-client-urls
+ - http://172.16.238.102:2379
+ - --listen-client-urls
+ - http://0.0.0.0:2379
+ - --initial-cluster
+ - node1=http://172.16.238.100:2380,node2=http://172.16.238.101:2380,node3=http://172.16.238.102:2380
+ - --initial-cluster-state
+ - new
+ - --initial-cluster-token
+ - docker-etcd
+
+ upstream:
+ image: johz/upstream:v2.0
+ restart: always
+ ports:
+ - '80:80/tcp'
+ - '1980:1980/tcp'
+ - '1981:1981/tcp'
+ - '1982:1982/tcp'
+ - '1983:1983/tcp'
+ - '1984:1984/tcp'
+ networks:
+ apisix_dashboard_e2e:
+ ipv4_address: 172.16.238.120
Review comment:
updated.
##########
File path: api/internal/handler/route/route.go
##########
@@ -250,6 +250,11 @@ func (h *Handler) Update(c droplet.Context) (interface{}, error) {
input.Route.ID = input.ID
}
+ if input.Route.Host != "" && len(input.Route.Hosts) > 0 {
Review comment:
sure, I will create a issue on APISIX repo to fix it.
##########
File path: api/test/e2e/route_test.go
##########
@@ -0,0 +1,233 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestRoute_Host_Params(t *testing.T) {
Review comment:
I had split them into `TestRoute_Create_With_Hosts`, `TestRoute_Update_Routes_With_Hosts`, `TestRoute_Delete_Routes_With_Hosts`
##########
File path: api/test/docker/docker-compose.yaml
##########
@@ -0,0 +1,183 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+version: "3.6"
+
+services:
+ node1:
+ image: quay.io/coreos/etcd:v3.4.0
+ ports:
+ - "2379:2379"
+ expose:
+ - 2379
+ - 2380
+ networks:
+ apisix_dashboard_e2e:
+ ipv4_address: 172.16.238.100
+ environment:
+ - ETCDCTL_API=3
+ command:
+ - /usr/local/bin/etcd
+ - --data-dir=/etcd-data
+ - --name
+ - node1
+ - --initial-advertise-peer-urls
+ - http://172.16.238.100:2380
+ - --listen-peer-urls
+ - http://0.0.0.0:2380
+ - --advertise-client-urls
+ - http://172.16.238.100:2379
+ - --listen-client-urls
+ - http://0.0.0.0:2379
+ - --initial-cluster
+ - node1=http://172.16.238.100:2380,node2=http://172.16.238.101:2380,node3=http://172.16.238.102:2380
+ - --initial-cluster-state
+ - new
+ - --initial-cluster-token
+ - docker-etcd
+
+ node2:
+ image: quay.io/coreos/etcd:v3.4.0
+ networks:
+ apisix_dashboard_e2e:
+ ipv4_address: 172.16.238.101
+ environment:
+ - ETCDCTL_API=3
+ ports:
+ - "3379:2379"
+ expose:
+ - 2379
+ - 2380
+ command:
+ - /usr/local/bin/etcd
+ - --data-dir=/etcd-data
+ - --name
+ - node2
+ - --initial-advertise-peer-urls
+ - http://172.16.238.101:2380
+ - --listen-peer-urls
+ - http://0.0.0.0:2380
+ - --advertise-client-urls
+ - http://172.16.238.101:2379
+ - --listen-client-urls
+ - http://0.0.0.0:2379
+ - --initial-cluster
+ - node1=http://172.16.238.100:2380,node2=http://172.16.238.101:2380,node3=http://172.16.238.102:2380
+ - --initial-cluster-state
+ - new
+ - --initial-cluster-token
+ - docker-etcd
+
+ node3:
+ image: quay.io/coreos/etcd:v3.4.0
+ networks:
+ apisix_dashboard_e2e:
+ ipv4_address: 172.16.238.102
+ environment:
+ - ETCDCTL_API=3
+ ports:
+ - "4379:2379"
+ expose:
+ - 2379
+ - 2380
+ command:
+ - /usr/local/bin/etcd
+ - --data-dir=/etcd-data
+ - --name
+ - node3
+ - --initial-advertise-peer-urls
+ - http://172.16.238.102:2380
+ - --listen-peer-urls
+ - http://0.0.0.0:2380
+ - --advertise-client-urls
+ - http://172.16.238.102:2379
+ - --listen-client-urls
+ - http://0.0.0.0:2379
+ - --initial-cluster
+ - node1=http://172.16.238.100:2380,node2=http://172.16.238.101:2380,node3=http://172.16.238.102:2380
+ - --initial-cluster-state
+ - new
+ - --initial-cluster-token
+ - docker-etcd
+
+ upstream:
+ image: johz/upstream:v2.0
+ restart: always
+ ports:
+ - '80:80/tcp'
+ - '1980:1980/tcp'
+ - '1981:1981/tcp'
+ - '1982:1982/tcp'
+ - '1983:1983/tcp'
+ - '1984:1984/tcp'
+ networks:
+ apisix_dashboard_e2e:
+ ipv4_address: 172.16.238.120
Review comment:
I try to use `10.1.1.*` as docker subnetwork IP, It's ok in my local, but CI fail. so revert that first, maybe try later.
##########
File path: api/conf/conf.go
##########
@@ -36,8 +36,8 @@ const (
EnvDEV = "dev"
EnvLOCAL = "local"
- confJsonPath = "/go/manager-api/conf.json"
- schemaPath = "/go/manager-api/schema.json"
+ confJsonPath = "/go/manager-api/conf.json"
Review comment:
docker need this path..
----------------------------------------------------------------
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] juzhiyuan commented on a change in pull request #612: test: add e2e test for field hosts in `route` api
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #612:
URL: https://github.com/apache/apisix-dashboard/pull/612#discussion_r515944815
##########
File path: api/test/e2e/base.go
##########
@@ -0,0 +1,134 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "bytes"
+ "io/ioutil"
+ "net/http"
+ "testing"
+ "time"
+
+ "github.com/gavv/httpexpect/v2"
+ "github.com/tidwall/gjson"
+)
+
+var token string
+
+func init() {
+ //login to get auth token
+ requestBody := []byte(`{
+ "username": "admin",
+ "password": "admin"
+ }`)
+
+ url := "http://127.0.0.1:8080/apisix/admin/user/login"
+ req, err := http.NewRequest(http.MethodPost, url, bytes.NewBuffer(requestBody))
+ if err != nil {
+ panic(err)
+ }
+
+ client := &http.Client{}
+ resp, err := client.Do(req)
+ if err != nil {
+ panic(err)
+ }
+
+ defer resp.Body.Close()
+
+ body, err := ioutil.ReadAll(resp.Body)
+ if err != nil {
+ panic(err)
+ }
+
+ respond := gjson.ParseBytes(body)
+ token = respond.Get("data.token").String()
+}
+
+func MangerApiExpect(t *testing.T) *httpexpect.Expect {
+ return httpexpect.New(t, "http://127.0.0.1:8080")
+}
+
+func APISIXExpect(t *testing.T) *httpexpect.Expect {
+ return httpexpect.New(t, "http://127.0.0.1:9080")
+}
+
+var sleepTime = time.Duration(100) * time.Millisecond
+
+type HttpTestCase struct {
+ caseDesc string
+ Object *httpexpect.Expect
+ Method string
+ Path string
+ Body string
+ Headers map[string]string
+ ExpectStatus int
+ ExpectCode int
+ ExpectMessage string
+ ExpectBody string
+ Sleep time.Duration //ms
+}
+
+func testCaseCheck(tc HttpTestCase) {
+ //init
+ expectObj := tc.Object
+ var req *httpexpect.Request
+ switch tc.Method {
+ case http.MethodGet:
+ req = expectObj.GET(tc.Path)
+ case http.MethodPut:
+ req = expectObj.PUT(tc.Path)
+ case http.MethodPost:
+ req = expectObj.POST(tc.Path)
+ case http.MethodDelete:
+ req = expectObj.DELETE(tc.Path)
+ case http.MethodPatch:
+ req = expectObj.PATCH(tc.Path)
+ default:
+ }
+
+ if req == nil {
+ panic("fail to init request")
+ }
+
+ if tc.Sleep != 0 {
+ time.Sleep(tc.Sleep)
+ }
+
+ //set header
+ for key, val := range tc.Headers {
+ req.WithHeader(key, val)
+ }
+
+ //set body
+ if tc.Body != "" {
+ req.WithText(tc.Body)
+ }
+
+ //respond check
+ resp := req.Expect()
+
+ //match http status
+ if tc.ExpectStatus != 0 {
+ resp.Status(tc.ExpectStatus)
+ }
+
+ //match body
Review comment:
This issue will address this question
https://github.com/apache/apisix-dashboard/issues/667
##########
File path: api/test/e2e/route_test.go
##########
@@ -0,0 +1,231 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+ "time"
+)
+
+func TestRoute_Host(t *testing.T) {
Review comment:
If you agree with this comment, kindly mark this PR as approval, or keep requesting changes.
----------------------------------------------------------------
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