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

Fix Installation Tests for Ceph #7639

Merged
merged 1 commit into from
Dec 28, 2020

Conversation

bipuladh
Copy link
Contributor

@bipuladh bipuladh commented Dec 23, 2020

/assign @cloudbehl

  • Enable labeling of the namespace in OLM Page
  • Click Next to adjust for Configure Page
  • Support situations where Operators Panel is already open

@openshift-ci-robot openshift-ci-robot added component/ceph Related to ceph-storage-plugin approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Dec 23, 2020
@cloudbehl
Copy link
Contributor

/test analyze

@afreen23
Copy link
Contributor

We should specify the issue we are fixing in the commit message.

@cloudbehl
Copy link
Contributor

/test analyze

 - Enable labelling of namespace in OLM Page
 - Click Next to adjust for Configure Page
 - Support situations where Operators Panel is already open
@@ -26,6 +26,7 @@ Cypress.Commands.add('install', (mode: 'Internal' | 'Attached' = 'Internal', enc
cy.log('Subscribe to OCS Operator');
cy.byLegacyTestID('operator-install-btn').click({ force: true });
cy.byTestID('Operator recommended namespace:-radio-input').should('be.checked');
cy.byTestID('enable-monitoring').click();
Copy link
Contributor

Choose a reason for hiding this comment

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

is it to support 4.6?

Copy link
Contributor

Choose a reason for hiding this comment

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

can you put a comment, if yes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes. We need to remove it for 4.7

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's required all the time, but we can remove it once 4.7 comes.

Comment on lines -30 to -33
afterEach(() => {
checkErrors();
});

Copy link
Contributor

Choose a reason for hiding this comment

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

put a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be added once we fix i18n issues.

Copy link
Contributor

Choose a reason for hiding this comment

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

can you just comment on the code instead of removing it so we don't miss out?

@cloudbehl
Copy link
Contributor

/lgtm

@cloudbehl
Copy link
Contributor

/test analyze

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 28, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bipuladh, cloudbehl

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:

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

@bipuladh
Copy link
Contributor Author

/retest

@openshift-merge-robot openshift-merge-robot merged commit 0c46330 into openshift:master Dec 28, 2020
@@ -26,6 +26,7 @@ Cypress.Commands.add('install', (mode: 'Internal' | 'Attached' = 'Internal', enc
cy.log('Subscribe to OCS Operator');
cy.byLegacyTestID('operator-install-btn').click({ force: true });
cy.byTestID('Operator recommended namespace:-radio-input').should('be.checked');
cy.byTestID('enable-monitoring').click();
cy.byTestID('install-operator').click();
cy.byTestID('success-icon', { timeout: 180000 }).should('be.visible');
cy.exec('oc get project openshift-storage -o json').then((res) => {
Copy link
Contributor

@GowthamShanmugam GowthamShanmugam Dec 28, 2020

Choose a reason for hiding this comment

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

Up to this part, we can move to some global integration-tests-cypress folder, So anyone can reuse this installation logic by passing the operator name and namespace as param.

What I am suggesting here is, The operator install flow is a pre-requisite for OCS cluster creation flow. Instead of naming it as "install", we can name it something like prepate_storage_cluster or create_ocs_cluster. And prepare_ocs_cluster flow can the call install flow externally. For easier understanding separate these two install and create logic in different functions. (Or) Add "install" as cypress global namespace from global cypress IT and Add "prepare_ocs_cluster" as cypress global namespace in ceph-storage-plugin cypress IT.

Also, we can test our cluster creation logic independently by passing its own parameters.

@spadgett spadgett added this to the v4.7 milestone Jan 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. component/ceph Related to ceph-storage-plugin lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants