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/29 01:28:08 UTC

[GitHub] [apisix-dashboard] LiteSun opened a new pull request #619: feat: added frontend e2e test

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


   Please answer these questions before submitting a pull request
   
   - Why submit this pull request?
   - [ ] Bugfix
   - [x] New feature provided
   - [ ] Improve performance
   
   - Related issues
   #614 
   ___
   ### Bugfix
   - Description
   
   - How to fix?
   
   ___
   ### New feature or improvement
   - Describe the details and related test reports.
   as title


----------------------------------------------------------------
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 #619: feat: added e2e test for Login page

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






----------------------------------------------------------------
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 #619: feat: added e2e test for Login page

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



##########
File path: docs/develop.md
##########
@@ -35,6 +35,8 @@ $ yarn install
 ```sh
 $ yarn start
 ```
+### wirte e2e test case

Review comment:
       md style: need a blank line before and after the title

##########
File path: frontend/src/e2e/README.md
##########
@@ -0,0 +1,41 @@
+<!--
+#
+# 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.
+#
+-->
+
+### Writing test cases
+
+1. Install dependencies then run in development mode
+
+```sh
+$ yarn install && yarn start

Review comment:
       here is an example of APISIX, you can take a look: https://github.com/apache/apisix/blob/master/Makefile#L70




----------------------------------------------------------------
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 #619: feat: added frontend e2e test

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


   Your CI failed because of License issues, fix them first.


----------------------------------------------------------------
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 #619: feat: added frontend e2e test

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



##########
File path: .github/workflows/frontend-e2e-test.yml
##########
@@ -0,0 +1,31 @@
+name: Front-end e2e test
+
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+    branches:
+      - master
+
+jobs:
+  frontend-e2e:
+    name: Front-end e2e test
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+
+      - name: Setup Node.JS environment for Front-end

Review comment:
       `.js`

##########
File path: .github/workflows/frontend-e2e-test.yml
##########
@@ -0,0 +1,31 @@
+name: Front-end e2e test
+
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+    branches:
+      - master
+
+jobs:
+  frontend-e2e:
+    name: Front-end e2e test
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+
+      - name: Setup Node.JS environment for Front-end
+        uses: actions/setup-node@v1
+        with:
+          node-version: 14.x

Review comment:
       10.x 12.x 14.x, currently 14.x is enough

##########
File path: .github/workflows/frontend-e2e-test.yml
##########
@@ -0,0 +1,31 @@
+name: Front-end e2e test
+
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+    branches:
+      - master
+
+jobs:
+  frontend-e2e:
+    name: Front-end e2e test
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+
+      - name: Setup Node.JS environment for Front-end
+        uses: actions/setup-node@v1
+        with:
+          node-version: 14.x
+
+      - name: Install Front-end dependencies
+        run: yarn install
+
+      - name: start Front-end and Test

Review comment:
       1. `Start`
   2. `then 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] liuxiran edited a comment on pull request #619: feat: added e2e test for Login page

Posted by GitBox <gi...@apache.org>.
liuxiran edited a comment on pull request #619:
URL: https://github.com/apache/apisix-dashboard/pull/619#issuecomment-718883982


   just a proposal：define the dom selectors and test datas separately as variables, It may be better to maintain  later.
   
   A test case for a page contains three parts, domselectors, test datas, and test steps.
   * The domselectors maybe changed as the layout of the page changed or UI frame changed;
   * Test datas are also likely to change when data source changed;
   * The test steps are relatively stable
   
   e.g:
   ```javascript
   const domSelectors = {
       inputUserName: '#control-ref_username',
       inputPassWord: '#control-ref_password',
       buttonLogin: '.ant-btn-lg',
       notificationLogin: '.ant-notification-notice-description',
       ...
   };
   const loginFailedDatas = {
       userName: 'admin',
       passWord: '123456'
   };
   const loginSuccessDatas = {
      userName: 'admin',
      passWord: 'admin'
   };
   ```
   
   


----------------------------------------------------------------
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 pull request #619: feat: added e2e test for Login page

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


   It’s really a good example. It’s better if there is a public function for logging in. 
   For other test cases, just calling it is equivalent to logging in to the dashboard.


----------------------------------------------------------------
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] LiteSun commented on a change in pull request #619: feat: added e2e test for Login page

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



##########
File path: frontend/src/e2e/README.md
##########
@@ -0,0 +1,41 @@
+<!--
+#
+# 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.
+#
+-->
+
+### Writing test cases
+
+1. Install dependencies then run in development mode
+
+```sh
+$ yarn install && yarn start

Review comment:
       > How about we use `makefile` to run test cases? In this way, developers only need to know the entry of `make`.
   > 
   > We can implement this separately later.
   
   Good idea, but I'm not familiar with `makefile`  and need time to research it. If that's OK, I'll use it in the future.

##########
File path: frontend/src/e2e/README.md
##########
@@ -0,0 +1,41 @@
+<!--
+#
+# 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.
+#
+-->
+
+### Writing test cases

Review comment:
       > or add a link about how to have a frontend test in `develop.md`
   
   Just added a link in `develop.md`.

##########
File path: frontend/src/e2e/public.js
##########
@@ -0,0 +1,39 @@
+/*
+ * 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.
+ */
+

Review comment:
       > A service file contains some reusable methods and COULD export some public variables, too.
   
   Filename changed.
   




----------------------------------------------------------------
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 #619: feat: added e2e test for Login page

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



##########
File path: src/e2e/Login.e2e.js
##########
@@ -1,33 +1,69 @@
+/*
+ * 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.
+ */
+
 /* eslint-disable import/no-extraneous-dependencies */
 const puppeteer = require('puppeteer');
 
 let browser;
+const BASE_URL = `http://localhost:${process.env.PORT || 8000}`;
+const domSelectors = {
+  inputUserName: '#control-ref_username',
+  inputPassWord: '#control-ref_password',
+  buttonLogin: '.ant-btn-lg',
+  notificationNotice: '.ant-notification-notice',
+  notificationLogin: '.ant-notification-notice-description',
+};
+const loginFailedDatas = {
+  userName: 'admin',
+  passWord: '123456',

Review comment:
       BEM: http://getbem.com/introduction/




----------------------------------------------------------------
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 #619: feat: added e2e test for Login page

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


   just a proposal：define the dom selectors and test datas separately as variables, It may be better to maintain  later.
   
   A test case for a page contains three parts, domselectors, test datas, and test steps.
   * The domselectors maybe changed as the layout of the page changed or UI frame changed;
   * Test datas are also likely to change when data source changed;
   * The test steps are relatively stable
   
   e.g:
   ```javascript
   let domSelectors = {
       inputUserName: '#control-ref_username',
       inputPassWord: '#control-ref_password',
       buttonLogin: '.ant-btn-lg',
       notificationLogin: '.ant-notification-notice-description',
       ...
   };
   let loginFailedDatas = {
       userName: 'admin',
       passWord: '123456'
   };
   let loginSuccessDatas = {
      userName: 'admin',
      passWord: 'admin'
   };
   ```
   
   


----------------------------------------------------------------
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 #619: feat: added e2e test for Login page

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



##########
File path: src/e2e/Login.e2e.js
##########
@@ -0,0 +1,39 @@
+/* eslint-disable import/no-extraneous-dependencies */
+const puppeteer = require('puppeteer');
+
+let browser;
+beforeAll(async () => {
+  browser = await puppeteer.launch({ headless: true });
+});
+
+describe('Login', () => {
+  test('Login fail', async () => {

Review comment:
       it would be better to add  a failed test case: login with empty username and password, because it has different error message and page presentation :)




----------------------------------------------------------------
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 #619: feat: added frontend e2e test

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



##########
File path: .github/workflows/frontend-e2e-test.yml
##########
@@ -0,0 +1,31 @@
+name: Front-end e2e test
+
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+    branches:
+      - master
+
+jobs:
+  frontend-e2e:
+    name: Front-end e2e test
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+
+      - name: Setup Node.JS environment for Front-end
+        uses: actions/setup-node@v1
+        with:
+          node-version: 14.x
+
+      - name: Install Front-end dependencies
+        run: yarn install
+
+      - name: start Front-end and Test
+        run: |
+          yarn start &
+          sleep 10 && 
+          curl http://localhost:8000 &&
+          yarn test

Review comment:
       new line




----------------------------------------------------------------
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 #619: feat: added frontend e2e test

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


   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] juzhiyuan commented on a change in pull request #619: feat: added e2e test for Login page

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



##########
File path: src/e2e/Login.e2e.js
##########
@@ -1,33 +1,69 @@
+/*
+ * 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.
+ */
+
 /* eslint-disable import/no-extraneous-dependencies */
 const puppeteer = require('puppeteer');
 
 let browser;
+const BASE_URL = `http://localhost:${process.env.PORT || 8000}`;
+const domSelectors = {
+  inputUserName: '#control-ref_username',
+  inputPassWord: '#control-ref_password',
+  buttonLogin: '.ant-btn-lg',
+  notificationNotice: '.ant-notification-notice',
+  notificationLogin: '.ant-notification-notice-description',
+};
+const loginFailedDatas = {
+  userName: 'admin',
+  passWord: '123456',

Review comment:
       should be `password` in lowercase
   
   `inputPassWord: '#control-ref_password'`

##########
File path: src/e2e/Login.e2e.js
##########
@@ -1,33 +1,69 @@
+/*
+ * 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.
+ */
+
 /* eslint-disable import/no-extraneous-dependencies */
 const puppeteer = require('puppeteer');
 
 let browser;
+const BASE_URL = `http://localhost:${process.env.PORT || 8000}`;
+const domSelectors = {
+  inputUserName: '#control-ref_username',
+  inputPassWord: '#control-ref_password',
+  buttonLogin: '.ant-btn-lg',
+  notificationNotice: '.ant-notification-notice',
+  notificationLogin: '.ant-notification-notice-description',
+};
+const loginFailedDatas = {
+  userName: 'admin',

Review comment:
       should be `username` because of `inputUserName: '#control-ref_username'`

##########
File path: package.json
##########
@@ -73,6 +74,7 @@
     "react-device-detect": "^1.12.1",
     "react-dom": "^16.8.6",
     "react-helmet-async": "^1.0.4",
+    "start-server-and-test": "^1.11.5",

Review comment:
       What's license?

##########
File path: src/e2e/README.md
##########
@@ -0,0 +1,41 @@
+<!--
+#
+# 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.
+#
+-->
+
+### Writing test cases
+
+1. Installing dependencies and running local development environments
+
+   ```Bash

Review comment:
       sh

##########
File path: package.json
##########
@@ -73,6 +74,7 @@
     "react-device-detect": "^1.12.1",
     "react-dom": "^16.8.6",
     "react-helmet-async": "^1.0.4",
+    "start-server-and-test": "^1.11.5",

Review comment:
       MIT

##########
File path: src/e2e/Login.e2e.js
##########
@@ -1,33 +1,69 @@
+/*
+ * 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.
+ */
+
 /* eslint-disable import/no-extraneous-dependencies */
 const puppeteer = require('puppeteer');
 
 let browser;
+const BASE_URL = `http://localhost:${process.env.PORT || 8000}`;

Review comment:
       why 8000?

##########
File path: src/e2e/Login.e2e.js
##########
@@ -1,33 +1,69 @@
+/*
+ * 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.
+ */
+
 /* eslint-disable import/no-extraneous-dependencies */
 const puppeteer = require('puppeteer');
 
 let browser;
+const BASE_URL = `http://localhost:${process.env.PORT || 8000}`;
+const domSelectors = {
+  inputUserName: '#control-ref_username',
+  inputPassWord: '#control-ref_password',
+  buttonLogin: '.ant-btn-lg',
+  notificationNotice: '.ant-notification-notice',
+  notificationLogin: '.ant-notification-notice-description',
+};
+const loginFailedDatas = {

Review comment:
       omit `s`

##########
File path: src/e2e/README.md
##########
@@ -0,0 +1,41 @@
+<!--
+#
+# 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.
+#
+-->
+

Review comment:
       Should format this file

##########
File path: src/e2e/README.md
##########
@@ -0,0 +1,41 @@
+<!--
+#
+# 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.
+#
+-->
+
+### Writing test cases
+
+1. Installing dependencies and running local development environments
+
+   ```Bash
+   yarn install && yarn start
+   ```
+
+2. Add a new test case file to the `src/e2e` folder
+3. Run test cases
+
+   ```Bash
+   yarn test
+   ```
+
+   If you want to run a particular test file separately, you can execute the following command
+
+   ```Bash
+   yarn test ${yourFileName}.e2e.js
+   ```
+
+   The test results will be displayed on the console.

Review comment:
       Test results will output on the console

##########
File path: src/e2e/README.md
##########
@@ -0,0 +1,41 @@
+<!--
+#
+# 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.
+#
+-->
+
+### Writing test cases
+
+1. Installing dependencies and running local development environments

Review comment:
       `Install dependencies then run in development mode`

##########
File path: src/e2e/README.md
##########
@@ -0,0 +1,41 @@
+<!--
+#
+# 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.
+#
+-->
+
+### Writing test cases
+
+1. Installing dependencies and running local development environments
+
+   ```Bash
+   yarn install && yarn start

Review comment:
       $

##########
File path: src/e2e/README.md
##########
@@ -0,0 +1,41 @@
+<!--
+#
+# 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.
+#
+-->
+
+### Writing test cases
+
+1. Installing dependencies and running local development environments
+
+   ```Bash
+   yarn install && yarn start
+   ```
+
+2. Add a new test case file to the `src/e2e` folder

Review comment:
       under `xxx`

##########
File path: src/e2e/README.zh-CN.md
##########
@@ -0,0 +1,41 @@
+<!--
+#
+# 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.
+#
+-->
+

Review comment:
       same 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] juzhiyuan commented on a change in pull request #619: feat: added e2e test for Login page

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



##########
File path: frontend/src/e2e/Logout.e2e.js
##########
@@ -0,0 +1,56 @@
+/*
+ * 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.
+ */
+
+/* eslint-disable import/no-extraneous-dependencies */
+const puppeteer = require('puppeteer');
+
+const {
+  setupLogin
+} = require('./public')
+
+let browser
+const domSelectors = {
+  userProfile: '.ant-space-horizontal div:nth-child(2)',
+  dropdownMenuItem: '.ant-dropdown-menu-item',
+  buttonLogin:".ant-btn-lg",
+  logoutButton: '.ant-dropdown > ul > li:nth-child(3)',
+};
+
+describe('Logout', () => {
+
+beforeAll(async () => {
+  browser = await puppeteer.launch({
+    headless: true,
+    slowMo: 100
+  });
+});
+
+test('Logout', async () => {
+  const page = await browser.newPage();
+  await setupLogin(page)
+  await page.click(domSelectors.userProfile);
+  await page.waitForSelector(domSelectors.dropdownMenuItem);
+  await page.waitForSelector(domSelectors.logoutButton);
+  await page.click(domSelectors.logoutButton);
+  await page.waitForSelector(domSelectors.buttonLogin);
+  await page.close();
+}, 50000);
+
+afterAll(async () => {
+  await browser.close();
+});

Review comment:
       bad intention 

##########
File path: frontend/src/e2e/public.js
##########
@@ -0,0 +1,39 @@
+/*
+ * 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.
+ */
+

Review comment:
       Not a good filename, how about `service.js`? 

##########
File path: frontend/src/e2e/README.md
##########
@@ -0,0 +1,41 @@
+<!--
+#
+# 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.
+#
+-->
+
+### Writing test cases

Review comment:
       How about moving this part to `docs/develop.md`?

##########
File path: frontend/src/e2e/public.js
##########
@@ -0,0 +1,39 @@
+/*
+ * 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.
+ */
+
+export const BASE_URL = `http://localhost:${process.env.PORT || 8000}`;
+
+const loginSuccessData = {
+    username: 'admin',
+    password: 'admin',
+  };
+
+const domSelectors = {
+    inputUsername: '#control-ref_username',
+    inputPassword: '#control-ref_password',
+    buttonLogin: '.ant-btn-lg',
+    loginSuccessIcon: '.ant-notification-notice-icon-success',
+  };
+
+export const setupLogin = async (page)=>{
+    await page.goto(BASE_URL);

Review comment:
       2 spaces

##########
File path: frontend/src/e2e/Logout.e2e.js
##########
@@ -0,0 +1,56 @@
+/*
+ * 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.
+ */
+
+/* eslint-disable import/no-extraneous-dependencies */
+const puppeteer = require('puppeteer');
+
+const {
+  setupLogin
+} = require('./public')
+
+let browser
+const domSelectors = {
+  userProfile: '.ant-space-horizontal div:nth-child(2)',
+  dropdownMenuItem: '.ant-dropdown-menu-item',
+  buttonLogin:".ant-btn-lg",
+  logoutButton: '.ant-dropdown > ul > li:nth-child(3)',
+};
+
+describe('Logout', () => {
+
+beforeAll(async () => {
+  browser = await puppeteer.launch({
+    headless: true,
+    slowMo: 100
+  });
+});
+
+test('Logout', async () => {
+  const page = await browser.newPage();
+  await setupLogin(page)
+  await page.click(domSelectors.userProfile);
+  await page.waitForSelector(domSelectors.dropdownMenuItem);
+  await page.waitForSelector(domSelectors.logoutButton);
+  await page.click(domSelectors.logoutButton);
+  await page.waitForSelector(domSelectors.buttonLogin);
+  await page.close();
+}, 50000);
+
+afterAll(async () => {
+  await browser.close();
+});

Review comment:
       2 spaces

##########
File path: frontend/src/e2e/README.md
##########
@@ -0,0 +1,41 @@
+<!--
+#
+# 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.
+#
+-->
+
+### Writing test cases

Review comment:
       or add a link about how to have a frontend test in `develop.md` 

##########
File path: frontend/src/e2e/public.js
##########
@@ -0,0 +1,39 @@
+/*
+ * 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.
+ */
+

Review comment:
       A service file contains some reusable methods and COULD export some public variables, too.

##########
File path: docs/develop.zh-CN.md
##########
@@ -38,6 +38,8 @@ $ yarn install
 ```sh
 $ yarn start
 ```
+### 编写 e2e 测试案例
+请参考 [e2e 文档](../frontend/src/e2e/README.zh-CN.md)。

Review comment:
       ![image](https://user-images.githubusercontent.com/2106987/97853990-d3453d80-1d33-11eb-8336-685dfff4395c.png)
   

##########
File path: frontend/src/e2e/README.md
##########
@@ -0,0 +1,41 @@
+<!--
+#
+# 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.
+#
+-->
+
+### Writing test cases

Review comment:
       ![image](https://user-images.githubusercontent.com/2106987/97887925-7f534c80-1d65-11eb-9672-a2dc974a75c6.png)
   
   disharmony IMO

##########
File path: docs/develop.md
##########
@@ -35,6 +35,8 @@ $ yarn install
 ```sh
 $ yarn start
 ```
+### wirte e2e test case

Review comment:
       Typo!




----------------------------------------------------------------
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 #619: feat: added e2e test for Login page

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



##########
File path: frontend/src/e2e/README.zh-CN.md
##########
@@ -0,0 +1,41 @@
+<!--
+#
+# 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.
+#
+-->
+
+### 本地书写测试案例

Review comment:
       书写 -> 编写 :)




----------------------------------------------------------------
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 #619: feat: added frontend e2e test

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



##########
File path: src/e2e/Login.e2e.js
##########
@@ -0,0 +1,39 @@
+/* eslint-disable import/no-extraneous-dependencies */
+const puppeteer = require('puppeteer');
+
+let browser;
+beforeAll(async () => {
+  browser = await puppeteer.launch({ headless: true });
+});
+
+describe('Login', () => {
+  test('Login fail', async () => {

Review comment:
       `failed`




----------------------------------------------------------------
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 #619: feat: added e2e test for Login page

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


   you can refer to https://github.com/apache/apisix/blob/master/Makefile
   
   Thanks,
   Ming Wen
   Twitter: _WenMing
   
   
   litesun <no...@github.com> 于2020年11月2日周一 下午2:43写道：
   
   > *@LiteSun* commented on this pull request.
   > ------------------------------
   >
   > In frontend/src/e2e/README.md
   > <https://github.com/apache/apisix-dashboard/pull/619#discussion_r515766019>
   > :
   >
   > > +#     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.
   > +#
   > +-->
   > +
   > +### Writing test cases
   > +
   > +1. Install dependencies then run in development mode
   > +
   > +```sh
   > +$ yarn install && yarn start
   >
   > How about we use makefile to run test cases? In this way, developers only
   > need to know the entry of make.
   >
   > We can implement this separately later.
   >
   > Good idea, but I'm not familiar with makefile and need time to research
   > it. If that's OK, I'll use it in the future.
   >
   > —
   > You are receiving this because you commented.
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/apisix-dashboard/pull/619#discussion_r515766019>,
   > or unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/AGJZBKY2ANV2XMA5AUN3ZZ3SNZIIJANCNFSM4TDCRUBA>
   > .
   >
   


----------------------------------------------------------------
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 #619: feat: added e2e test for Login page

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


   


----------------------------------------------------------------
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] LiteSun commented on pull request #619: feat: added e2e test for Login page

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


   > just a proposal：define the dom selectors and test datas separately as variables, It may be better to maintain later.
   > 
   > A test case for a page contains three parts, domselectors, test datas, and test steps.
   > 
   > * The domselectors maybe changed as the layout of the page changed or UI frame changed;
   > * Test datas are also likely to change when data source changed;
   > * The test steps are relatively stable
   > 
   > e.g:
   > 
   > ```js
   > const domSelectors = {
   >     inputUserName: '#control-ref_username',
   >     inputPassWord: '#control-ref_password',
   >     buttonLogin: '.ant-btn-lg',
   >     notificationLogin: '.ant-notification-notice-description',
   >     ...
   > };
   > const loginFailedDatas = {
   >     userName: 'admin',
   >     passWord: '123456'
   > };
   > const loginSuccessDatas = {
   >    userName: 'admin',
   >    passWord: 'admin'
   > };
   > ```
   
   I couldn't agree with you more and will update soon.


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