Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: add the create and delete plugin in drawer #1597

Merged
merged 20 commits into from Mar 22, 2021
Merged

test: add the create and delete plugin in drawer #1597

merged 20 commits into from Mar 22, 2021

Conversation

stu01509
Copy link
Contributor

Please answer these questions before submitting a pull request

New feature or improvement

  • Describe the details and related test reports.

Add the cypress test case to testing the plugin delete in the drawer.
image


Please add the corresponding test cases if necessary.

@codecov-io
Copy link

codecov-io commented Mar 16, 2021

Codecov Report

Merging #1597 (8331f4a) into master (548bbe3) will increase coverage by 0.19%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1597      +/-   ##
==========================================
+ Coverage   71.05%   71.24%   +0.19%     
==========================================
  Files          47       47              
  Lines        3116     3116              
==========================================
+ Hits         2214     2220       +6     
+ Misses        658      653       -5     
+ Partials      244      243       -1     
Flag Coverage Δ
backend-e2e-test 61.97% <ø> (+0.19%) ⬆️
backend-e2e-test-ginkgo 47.75% <ø> (-0.17%) ⬇️
backend-unit-test 51.81% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
api/internal/core/store/store.go 89.15% <0.00%> (+1.20%) ⬆️
api/internal/core/storage/etcd.go 47.27% <0.00%> (+3.63%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 548bbe3...8331f4a. Read the comment docs.

@juzhiyuan
Copy link
Member

also cc @guoqqqi to review

Comment on lines 33 to 37
cy.fixture('plugin-dataset.json').as('cases');
cy.get('@cases').then((cases) => {
cy.configurePlugins(cases);
});
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this test we don't need to create all the plugins, it takes up more time than necessary, we just need to create a few plugin examples to test that the deletion functions properly.

@@ -45,7 +45,6 @@ context('Create and Delete Plugin List', () => {
.then(() => {
cy.get('.ant-drawer-footer').contains('button', 'Delete').click();
cy.contains('button', 'Confirm').click({ force: true });
cy.get(this.domSelector.empty).should('be.visible');
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assertion ensures that the plugin can be removed in its entirety properly, and I think it's best that we don't remove it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any update here?

@guoqqqi
Copy link
Member

guoqqqi commented Mar 16, 2021

Hi, We need to test the functionality not only on the plug-in page but also on the route page and the service page to make sure it works.

@LiteSun
Copy link
Member

LiteSun commented Mar 19, 2021

ping~

@nic-chen
Copy link
Member

ping @stu01509

@stu01509
Copy link
Contributor Author

Hi, We need to test the functionality not only on the plug-in page but also on the route page and the service page to make sure it works.

Hi

Actually, I only found it the plugin page, where could I find in the route page and services page?

@guoqqqi
Copy link
Member

guoqqqi commented Mar 19, 2021

Hi,you can find it in the Create, Configure Plugin step.

@guoqqqi
Copy link
Member

guoqqqi commented Mar 20, 2021

CI error: https://github.com/apache/apisix-dashboard/pull/1597/checks?check_run_id=2148899222
This CI error has been fixed in #1584.

@stu01509
Copy link
Contributor Author

Hi @guoqqqi and @juzhiyuan

Update the commit.

@nic-chen
Copy link
Member

@stu01509 FE e2e test failed

@guoqqqi
Copy link
Member

guoqqqi commented Mar 21, 2021

The CI error is because the plugin has not been removed. Please refer to:
web/cypress/integration/plugin/create-edit-delete-plugin.spec.js
image

@stu01509
Copy link
Contributor Author

stu01509 commented Mar 21, 2021

I updated the commit, but it seems like occurred the error in backend test case.

*/
/* eslint-disable no-undef */

context('Create and Delete Plugin List', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
context('Create and Delete Plugin List', () => {
context('Delete Plugin List with the Drawer', () => {

Comment on lines 34 to 45
cy.contains('basic-auth').parents('.ant-card-bordered').within(() => {
cy.get('button').click({ force: true });
});

cy.get('.ant-drawer-content').should('be.visible').within(() => {
cy.get('#disable').click();
cy.get('.ant-switch-checked').should('exist');
});

cy.contains('button', 'Submit').click();
cy.get('.ant-drawer-content', { timeout }).should('not.exist');
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use this.domSelector... and this.data..., refer to other test cases.

cy.visit('/plugin/list');
cy.get(this.domSelector.refresh).click();
cy.contains('button', 'Edit').click();
cy.get('.ant-drawer-footer').contains('button', 'Delete').click({ force: true });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dom element can be moved to public data for other tests use.

Comment on lines 76 to 94
cy.contains('prometheus').parents('.ant-card-bordered').within(() => {
cy.get('button').click({ force: true });
});

cy.get('.ant-drawer-content').should('be.visible').within(() => {
cy.get('#disable').click();
cy.get('.ant-switch-checked').should('exist');
});

cy.contains('button', 'Submit').click();
cy.get('.ant-drawer-content', { timeout }).should('not.exist');

cy.contains('prometheus').parents('.ant-card-bordered').within(() => {
cy.get('button').click({ force: true });
});
cy.contains('button', 'Cancel').click();

cy.get('.ant-drawer-footer').contains('button', 'Delete').click({ force: true });
cy.contains('button', 'Confirm').click({ force: true });

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also use this.domSelector.. and this.data...
I think another step should be added: check that the plugin was successfully removed after it was deleted.

Comment on lines 41 to 59
cy.contains('prometheus').parents('.ant-card-bordered').within(() => {
cy.get('button').click({ force: true });
});

cy.get('.ant-drawer-content').should('be.visible').within(() => {
cy.get('#disable').click();
cy.get('.ant-switch-checked').should('exist');
});

cy.contains('button', 'Submit').click();
cy.get('.ant-drawer-content', { timeout }).should('not.exist');

cy.contains('prometheus').parents('.ant-card-bordered').within(() => {
cy.get('button').click({ force: true });
});

cy.get('.ant-drawer-footer').contains('button', 'Delete').click({ force: true });
cy.contains('button', 'Confirm').click({ force: true });

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@stu01509
Copy link
Contributor Author

Hi @guoqqqi

Update the commit, please review it again.

@nic-chen
Copy link
Member

@stu01509 please sync codes from branch master

@stu01509
Copy link
Contributor Author

Hello @nic-chen

Update the commit.

@LiteSun
Copy link
Member

LiteSun commented Mar 22, 2021

Hi, @guoqqqi, I remember that you developed this delete feature, please make sure that this test is fully covered

@guoqqqi
Copy link
Member

guoqqqi commented Mar 22, 2021

Hi, @stu01509 Again these pages need to be tested.
image

@guoqqqi
Copy link
Member

guoqqqi commented Mar 22, 2021

As the function is in a public component, it can be tested once and there is no need to add tests for other pages. Thanks.

@LiteSun
Copy link
Member

LiteSun commented Mar 22, 2021

As the function is in a public component, it can be tested once and there is no need to add tests for other pages. Thanks.

The tests corresponding to route and service are unnecessary? @guoqqqi

@stu01509
Copy link
Contributor Author

Update the commit.

…u01509/apisix-dashboard into fix-#1411-add-delete-plugin-in-darwer
@membphis membphis merged commit 85efb37 into apache:master Mar 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test: delete button in plugin drawer
9 participants