Add cypress integration test support for Ceph plugin #7165

bipuladh · 2020-11-09T22:15:38Z

Add test for Subscription flow for OCS Operator

bipuladh · 2020-11-09T22:15:56Z

/assign @dtaylor113 @spadgett

bipuladh · 2020-11-10T14:07:23Z

/retest

dtaylor113

Hi, looks great, just a couple of concerns:

Please update test-cypress.sh script to include ceph
Please add afterEach hook
Please investigate and try to avoid using .click({ force: true })
Thanks

frontend/packages/ceph-storage-plugin/integration-tests-cypress/tests/subscribe-to-ocs.spec.ts

dtaylor113 · 2020-11-10T14:15:12Z

frontend/packages/ceph-storage-plugin/integration-tests-cypress/views/olm.ts

@@ -0,0 +1 @@
+export const installSucess = 'svg[fill="#3e8635"]';


This is a little vauge? Is there an ID we can use, or as a fall back, a css selector?

A css selector is there but we will need that color code to verify that it''s green we could use .co-operator-install-page__pkg-indicator > :nth-child(2) > svg[fill="#3e8635"].

I don't think we should be validating specific colors. We should have data attributes for success/error icons and check for those.

frontend/packages/ceph-storage-plugin/integration-tests-cypress/tests/subscribe-to-ocs.spec.ts

frontend/package.json

dtaylor113

Hi, per Cypress Best Practices, each it(...) test should be independant and not rely on state of previous tests. Please refactor/group dependent tests together.
You might end up having only have a single it(..) test, and you can add cy.log(...) for each of the previous it(..) descriptions.

dtaylor113

/lgtm -thanks!

spadgett · 2020-11-10T20:08:52Z

frontend/packages/ceph-storage-plugin/integration-tests-cypress/views/olm.ts

@@ -0,0 +1 @@
+export const installSucess = 'svg[fill="#3e8635"]';


I don't think we should be validating specific colors. We should have data attributes for success/error icons and check for those.

spadgett · 2020-11-10T20:09:06Z

frontend/packages/integration-tests-cypress/views/form.ts

@@ -1,3 +1,4 @@
 export const submitButton = 'button[type=submit]';
 export const errorMessage = '.pf-c-alert.pf-m-inline.pf-m-danger';
 export const successMessage = '.pf-c-alert.pf-m-inline.pf-m-success';
+export const primaryButton = '.pf-c-button.pf-m-primary';


Better to have a data-test attribute if possible.

spadgett · 2020-11-10T20:10:01Z

frontend/packages/ceph-storage-plugin/integration-tests-cypress/tests/subscribe-to-ocs.spec.ts

+    cy.get(installSucess, { timeout: 180000 }).should('be.visible');
+    cy.exec('oc get project openshift-storage -o yaml')
+      .its('stdout')
+      .should('contain', 'openshift.io/cluster-monitoring: "true"');


Don't do a substring match. Parse the JSON and check for the specific annotation.

spadgett · 2020-11-10T20:10:34Z

frontend/packages/ceph-storage-plugin/integration-tests-cypress/tests/subscribe-to-ocs.spec.ts

+    cy.exec('oc get po -n openshift-storage')
+      .its('stdout')
+      .should('contain', 'noobaa-operator')
+      .should('contain', 'ocs-operator')
+      .should('contain', 'rook-ceph-operator');
+  });


Same comment here. We shouldn't be using substring matches on CLI output. Better to get the JSON and parse it.

dtaylor113

Some concerns.

The contents of .../artifacts/e2e-gcp-console/ seems to be:

 gather-audit-logs/
 gather-extra/
 gather-must-gather/
 ipi-conf-gcp/
 ipi-conf/
 ipi-deprovision-deprovision/
 ipi-install-install/
 ipi-install-rbac/
 test/

Whereas I'm used to seeing in .../artifacts/e2e-gcp-console/

 container-logs/
 gui_test_screenshots/
 installer/
 junit/
 metrics/
 network/
 nodes/
 pods/
...

Is this expected because running from ceph-plugin?

Also, there are some weird characters in the Cypress log output in /artifacts/e2e-gcp-console/test/container-logs/test.log:

�[90m====================================================================================================�[39m

�[0m  (�[4m�[1mRun Starting�[22m�[24m)�[0m

�[90m  ┌�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m┐�[39m
�[90m  │�[39m �[90mCypress:�[39m    5.0.0                                                                              �[90m│�[39m
�[90m  │�[39m �[90mBrowser:�[39m    Chrome 84 �[90m(headless)�[39m                                                               �[90m│�[39m
�[90m  │�[39m �[90mSpecs:�[39m      �[0m8 found (app/auth-multiuser-login.spec.ts, crud/k8-openshift-cruds.spec.ts, crud/n�[0m �[90m│�[39m
�[90m  │�[39m             �[0mamespace-crud.spec.ts, crud/other-routes.spec.ts, i18n/pseudolocalization.spec.ts,�[0m �[90m│�[39m
�[90m  │�[39m             �[0m monitoring/monitoring.spec.ts, storage/clone.spec.ts, storage/snapshot.spec.ts)�[0m   �[90m│�[39m
�[90m  └�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m─�[39m�[90m┘�[39m

whereas in /artifacts/e2e-gcp-console/container-logs/test.log it disaplays as:

================================================================================

  (Run Starting)

  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
  │ Cypress:    5.0.0                                                                              │
  │ Browser:    Chrome 84 (headless)                                                               │
  │ Specs:      8 found (app/auth-multiuser-login.spec.ts, crud/k8-openshift-cruds.spec.ts, crud/n │
  │             amespace-crud.spec.ts, crud/other-routes.spec.ts, i18n/pseudolocalization.spec.ts, │
  │              monitoring/monitoring.spec.ts, storage/clone.spec.ts, storage/snapshot.spec.ts)   │
  └────────────────────────────────────────────────────────────────────────────────────────────────┘

test-log in ceph-plugins is located at
/artifacts/e2e-gcp-console/test/container-logs/test.log
whereas elsewhere it is located at
/artifacts/e2e-gcp-console/container-logs/test.log

Just want to make sure these issues/concerns are raised and known.
I'm ok if directory differences are expected, but we do need to fix the character issues because if this PR is merged it looks like it'll mess up the test.log file!

bipuladh · 2020-11-11T17:43:44Z

@dtaylor113 ceph tests would not cause the artifacts to change. The other thing related to cypress test should be something else as these tests are not triggered at all.
I can see these issues you have mentioned in unrelated PRs as well.
FYI https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift_console/7182/pull-ci-openshift-console-master-e2e-gcp-console/1326555242052980736/artifacts/e2e-gcp-console/test/container-logs/test.log

Add test for Subscription flow for OCS Operator

spadgett

/approve

Adding my approval. I'll let @dtaylor113 give the final lgtm

dtaylor113 · 2020-11-11T19:54:38Z

/retest

dtaylor113

/lgtm
Remaining Cypress issues not related to this PR

openshift-ci-robot · 2020-11-11T19:55:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bipuladh, dtaylor113, spadgett

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [spadgett]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot added the component/ceph Related to ceph-storage-plugin label Nov 9, 2020

openshift-ci-robot requested review from afreen23 and jhadvig November 9, 2020 22:15

openshift-ci-robot added component/core Related to console core functionality component/olm Related to OLM labels Nov 9, 2020

openshift-ci-robot assigned dtaylor113 and spadgett Nov 9, 2020

dtaylor113 requested changes Nov 10, 2020

View reviewed changes

bipuladh force-pushed the cypress-ceph branch from 7d03d01 to 330af0d Compare November 10, 2020 17:32

bipuladh requested a review from dtaylor113 November 10, 2020 17:33

openshift deleted a comment from openshift-ci-robot Nov 10, 2020

dtaylor113 requested changes Nov 10, 2020

View reviewed changes

bipuladh force-pushed the cypress-ceph branch from 330af0d to d7ce3e8 Compare November 10, 2020 19:30

bipuladh requested a review from dtaylor113 November 10, 2020 19:30

dtaylor113 approved these changes Nov 10, 2020

View reviewed changes

spadgett reviewed Nov 10, 2020

View reviewed changes

dtaylor113 requested changes Nov 11, 2020

View reviewed changes

bipuladh force-pushed the cypress-ceph branch from d7ce3e8 to f1ce766 Compare November 11, 2020 18:00

openshift-ci-robot added the component/shared Related to console-shared label Nov 11, 2020

bipuladh requested review from dtaylor113 and spadgett November 11, 2020 18:01

Add cypress integration test support for Ceph plugin

fbc326d

Add test for Subscription flow for OCS Operator

bipuladh force-pushed the cypress-ceph branch from f1ce766 to fbc326d Compare November 11, 2020 18:03

spadgett approved these changes Nov 11, 2020

View reviewed changes

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 11, 2020

dtaylor113 approved these changes Nov 11, 2020

View reviewed changes

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 11, 2020

openshift-merge-robot merged commit 026f43a into openshift:master Nov 11, 2020

spadgett added this to the v4.7 milestone Nov 16, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add cypress integration test support for Ceph plugin #7165

Add cypress integration test support for Ceph plugin #7165

bipuladh commented Nov 9, 2020

bipuladh commented Nov 9, 2020

bipuladh commented Nov 10, 2020

dtaylor113 left a comment

dtaylor113 Nov 10, 2020

bipuladh Nov 10, 2020

dtaylor113 Nov 10, 2020

spadgett Nov 10, 2020

dtaylor113 left a comment •

edited

dtaylor113 left a comment

spadgett Nov 10, 2020

spadgett Nov 10, 2020

spadgett Nov 10, 2020

spadgett Nov 10, 2020

dtaylor113 left a comment •

edited

bipuladh commented Nov 11, 2020

spadgett left a comment

dtaylor113 commented Nov 11, 2020

dtaylor113 left a comment

openshift-ci-robot commented Nov 11, 2020

		@@ -0,0 +1 @@
		export const installSucess = 'svg[fill="#3e8635"]';

Add cypress integration test support for Ceph plugin #7165

Add cypress integration test support for Ceph plugin #7165

Conversation

bipuladh commented Nov 9, 2020

bipuladh commented Nov 9, 2020

bipuladh commented Nov 10, 2020

dtaylor113 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dtaylor113 left a comment • edited

Choose a reason for hiding this comment

dtaylor113 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dtaylor113 left a comment • edited

Choose a reason for hiding this comment

bipuladh commented Nov 11, 2020

spadgett left a comment

Choose a reason for hiding this comment

dtaylor113 commented Nov 11, 2020

dtaylor113 left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Nov 11, 2020

dtaylor113 left a comment •

edited

dtaylor113 left a comment •

edited