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.

Sign up for GitHub

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

Jump to bottom

consolidate kubevirt cypress tests for d/s #9496

Merged
merged 1 commit into from Jul 14, 2021
Merged

consolidate kubevirt cypress tests for d/s #9496

merged 1 commit into from Jul 14, 2021

Conversation

gouyang
Copy link
Contributor

@gouyang gouyang commented Jul 14, 2021

  1. add wait to deletesource for all objects
  2. change container image from docker.hub to quay
  3. make sure vm is running before checking vm ssh details
  4. Cloning status is not showing when it's ocs, set it just test with hpp.

@openshift-ci openshift-ci bot added the component/kubevirt Related to kubevirt-plugin label Jul 14, 2021
@gouyang
Copy link
Contributor Author

gouyang commented Jul 14, 2021 •
edited

test results on d/s:
before:
ocs: https://main-jenkins-csb-cnvqe.apps.ocp4.prod.psi.redhat.com/job/test-kubevirt-console-t1-cypress-cnv-4.8/14/testReport/

after:
ocs: https://main-jenkins-csb-cnvqe.apps.ocp4.prod.psi.redhat.com/job/test-kubevirt-console-t1-cypress-cnv-4.8/17/testReport/
hpp: https://main-jenkins-csb-cnvqe.apps.ocp4.prod.psi.redhat.com/job/test-kubevirt-console-t1-cypress-cnv-4.8/20/testReport/

@gouyang
Copy link
Contributor Author

gouyang commented Jul 14, 2021

@yaacov @pcbailey PTAL.

@yaacov
Copy link
Member

yaacov commented Jul 14, 2021

/lgtm

@openshift-ci openshift-ci bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 14, 2021
@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

pcbailey
pcbailey reviewed Jul 14, 2021
View reviewed changes
.../kubevirt-plugin/integration-tests-cypress/tests/ssh/ssh-vm-details-page-testing.includes.ts Outdated
@@ -9,6 +11,9 @@ export default ({ vmName }) =>
.then((link) => cy.visit(link.attr('href')));
cy.byLegacyTestID(vmName).click();
cy.byLegacyTestID('horizontal-link-Details').click();

// ensure vm is running before check ssh details
cy.get(detailsTab.vmStatus, { timeout: 600000 }).should('contain', 'Running');
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add 'Cloning' to the VM_STATUS enum [1] and use it for the status here and in source-image.spec.ts? I'd eventually like to replace them anywhere else we may be hard-coding them, as well. I'd really like to avoid hard-coding constants as much as possible.

[1] https://github.com/openshift/console/blob/master/frontend/packages/kubevirt-plugin/integration-tests-cypress/const/index.ts#L44

Copy link
Contributor Author

Choose a reason for hiding this comment

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

NP, I was thinking of using VM_STATUS while writing this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you! =)

...nd/packages/kubevirt-plugin/integration-tests-cypress/tests/vm-template/source-image.spec.ts Outdated
@@ -57,7 +57,9 @@ describe('test vm template source image', () => {
pvcName: testName,
pvcNamespace: 'default',
});
virtualization.templates.testSource(TEMPLATE_NAME, 'Cloning');
if (Cypress.env('STORAGE_CLASS') === 'hostpath-provisioner') {
virtualization.templates.testSource(TEMPLATE_NAME, 'Cloning');
Copy link
Contributor

Choose a reason for hiding this comment

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

See other 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.

Updated.

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 14, 2021
@pcbailey
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 14, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 14, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gouyang, pcbailey, yaacov

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

@openshift-merge-robot openshift-merge-robot merged commit 5100552 into openshift:master Jul 14, 2021
@gouyang gouyang deleted the fix_cypress_ds branch August 23, 2021 02:19
@spadgett spadgett added this to the v4.9 milestone Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pcbailey pcbailey pcbailey left review comments

@mareklibra mareklibra Awaiting requested review from mareklibra

@rhrazdil rhrazdil Awaiting requested review from rhrazdil

Assignees

@yaacov yaacov

@pcbailey pcbailey

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. component/kubevirt Related to kubevirt-plugin lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
v4.9
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@gouyang @yaacov @openshift-bot @pcbailey @spadgett @openshift-merge-robot