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

Cypress integration testing #4831

Merged

Conversation

dtaylor113
Copy link
Contributor

@dtaylor113 dtaylor113 commented Mar 26, 2020

Todo:

  • Balaji Makram @makambalaji on the devconsole ui team is working on a cypress cucumber integration.
  • we do need to think about how static plugins can contribute tests. Add plugin integration tests to protractor #2051 is what we did for protractor. Notes from @spadgett "this might be less critical since we are looking at pulling plugins out of the console into their own repos, so they would have separate ci"
  • add ability to run suites of cypress tests ('release', 'e2e', etc). before we can move these tests out of Protractor. Investigate Cypress Groups Tracked via JIRA
  • investigate integrating patternfly-a11y / axe with cypress axe plugin. Tracked via JIRA
  • turn on video file support?
  • add getError and getLogs checks like this in our Cypress tests to catch runtime errors in the browser.
  • limit use of class selectors in favor of data-test
  • remove specific timeouts
  • change to byTestID(), byTestActionID(), & byLegacyTestID
  • refactor: remove utils, rename cy-command-additions to support
  • read BRIDGE_KUBEADMIN_IDP from an env var set in the test prow script
  • implement multiple cypress reporters
  • integrate with using OpenShift with Authentication
  • update README with instructions for running cypress.
  • move cypress to monorepo/packages directory structure
  • do simple CRUD test
  • split commands.js file into several context helper files
  • enable eslint for cypress tests
  • convert all cypress config files to typescript (ie. 'commands.js')

Todo (CI):

  • Start xvfb in test-prow.sh according to these instructions Tracked via JIRA
  • investigate Cypress Dashboard features/cost. We were approved for OpenShift license for Cypress Dashboard usage. Working locally. Need to figure out how to add to GitHub CI processes.
  • push new console docker image to quay (@spadgett)

Cypress trade-offs

To launch cypress:

console>./clean-frontend.sh
console>./build-frontend.sh
- start console GUI -
cd frontend
frontend>yarn test-cypress

After test has run screen recording below shows how to step back through the run, showing how the UI (DOM snapshots) changed during the test run. You can even step through the Before All and After All setup/teardown methods from the GUI:

Additional Resources

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 26, 2020
@openshift-ci-robot openshift-ci-robot added the component/core Related to console core functionality label Mar 26, 2020
@dtaylor113 dtaylor113 force-pushed the cypress-integration-testing branch 3 times, most recently from cde559d to 1b05e9f Compare April 2, 2020 15:09
@benjaminapetersen
Copy link
Contributor

For the README, when we add instructions, need to figure out if it only works local with no auth.

@benjaminapetersen
Copy link
Contributor

benjaminapetersen commented Apr 3, 2020

I added a few things to the cypress config, picks up the xml output, etc:

{
  "integrationFolder": "cypress/tests",
  "screenshotsFolder": "../gui_test_screenshots/cypress/screenshots",
  "videosFolder": "../gui_test_screenshots/cypress/videos",
  "reporter": "junit",
  "reporterOptions": {
    "mochaFile": "../gui_test_screenshots/cypress-output-[hash].xml",
    "toConsole": true
  }
}

looks like
https://github.com/you54f/cypress-multi-reporters
is the thing to go with if we need multiple outputs.

@benjaminapetersen, Implemented cypress-multi-reporters. 'spec' outputs to console, mochajunit xml outputs to 'gui-test-screenshots`

@dtaylor113 dtaylor113 force-pushed the cypress-integration-testing branch 2 times, most recently from 78eb1c8 to 66033c9 Compare April 3, 2020 14:55
@spadgett
Copy link
Member

@dtaylor113 We might want to wait for the alert rows to appear before we attempt to filter.

@dtaylor113
Copy link
Contributor Author

@dtaylor113 We might want to wait for the alert rows to appear before we attempt to filter.

Hi @spadgett, ok added a test for rows to be loaded before filtering.

@dtaylor113
Copy link
Contributor Author

The tests start typing even before the heading appears

Hi @spadgett, not true, there is a test which passes, checks that the Heading is 'Alerting'.

@spadgett
Copy link
Member

The tests start typing even before the heading appears

Hi @spadgett, not true, there is a test which passes, checks that the Heading is 'Alerting'.

You're right, the page was just scrolled down a bit to where the heading was hidden.

I wonder if it's an issue with virtualized table scrolling behavior. We've seen bugs there. (This table probably doesn't need to be virtualized, but that's not a simple change.)

Copy link
Member

@spadgett spadgett left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jul 28, 2020
@spadgett
Copy link
Member

@dtaylor113 merge conflict in package.json :(

@kyoto
Copy link
Member

kyoto commented Jul 29, 2020

/hold
Needs rebase

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 29, 2020
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 29, 2020
@kyoto
Copy link
Member

kyoto commented Jul 29, 2020

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 29, 2020
@openshift openshift deleted a comment from openshift-bot Jul 29, 2020
@openshift openshift deleted a comment from openshift-bot Jul 29, 2020
@openshift openshift deleted a comment from openshift-bot Jul 29, 2020
@openshift openshift deleted a comment from openshift-bot Jul 29, 2020
@openshift openshift deleted a comment from openshift-bot Jul 29, 2020
@openshift openshift deleted a comment from openshift-ci-robot Jul 29, 2020
@openshift openshift deleted a comment from spadgett Jul 29, 2020
Copy link
Member

@spadgett spadgett left a comment

Choose a reason for hiding this comment

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

/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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:

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 22cc9de into openshift:master Jul 29, 2020
@spadgett spadgett added this to the v4.6 milestone Jul 30, 2020
@dtaylor113 dtaylor113 deleted the cypress-integration-testing branch November 25, 2020 14:22
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/core Related to console core functionality component/monitoring Related to monitoring 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