Bug 1880118: Cypress a11y enhancements #6698

dtaylor113 · 2020-09-21T17:19:31Z

This PR includes 3 a11y enhancements to our Cypress tests:

1. Adds a11y checks to Namespaces and Monitoring tests
2. Ports Protractors other-routes tests to Cypress and does a11y check on the routes:

 Running:  crud/other-routes.spec.ts                                                       (4 of 6)
  Visiting other routes
No BRIDGE_KUBEADMIN_PASSWORD set, skipping login
1 accessibility violation was detected for / page
┌─────────┬─────────────┬───────────┬───────────────────────────────────────┬───────┐
│ (index) │     id      │  impact   │              description              │ nodes │
├─────────┼─────────────┼───────────┼───────────────────────────────────────┼───────┤
│    0    │ 'link-name' │ 'serious' │ 'Ensures links have discernible text' │   5   │
└─────────┴─────────────┴───────────┴───────────────────────────────────────┴───────┘
    ✓ successfully displays view for route: / (10235ms)
    ✓ successfully displays view for route: /k8s/cluster/clusterroles/view (6958ms)
    ✓ successfully displays view for route: /k8s/cluster/nodes (5735ms)
    ✓ successfully displays view for route: /k8s/all-namespaces/events (5852ms)
    ✓ successfully displays view for route: /k8s/all-namespaces/import (5790ms)
    ✓ successfully displays view for route: /api-explorer (7243ms)
    ✓ successfully displays view for route: /api-resource/ns/default/core~v1~Pod (5697ms)
    ✓ successfully displays view for route: /api-resource/ns/default/core~v1~Pod/schema (5656ms)
    ✓ successfully displays view for route: /api-resource/ns/default/core~v1~Pod/instances (5900ms)
    ✓ successfully displays view for route: /api-resource/ns/default/core~v1~Pod/access (5783ms)
    ✓ successfully displays view for route: /k8s/cluster/user.openshift.io~v1~User (5659ms)
    ✓ successfully displays view for route: /k8s/ns/openshift-machine-api/machine.openshift.io~v1beta1~Machine (5756ms)
    ✓ successfully displays view for route: /k8s/ns/openshift-machine-api/machine.openshift.io~v1beta1~MachineSet (5680ms)
    ✓ successfully displays view for route: /k8s/ns/openshift-machine-api/autoscaling.openshift.io~v1beta1~MachineAutoscaler (5824ms)
    ✓ successfully displays view for route: /k8s/ns/openshift-machine-api/machine.openshift.io~v1beta1~MachineHealthCheck (5730ms)
    ✓ successfully displays view for route: /k8s/cluster/machineconfiguration.openshift.io~v1~MachineConfig (5734ms)
    ✓ successfully displays view for route: /k8s/cluster/machineconfiguration.openshift.io~v1~MachineConfigPool (5704ms)
    ✓ successfully displays view for route: /k8s/all-namespaces/monitoring.coreos.com~v1~Alertmanager (5683ms)
    ✓ successfully displays view for route: /k8s/ns/openshift-monitoring/monitoring.coreos.com~v1~Alertmanager/main (5641ms)
    ✓ successfully displays view for route: /settings/cluster (5679ms)
    ✓ successfully displays view for route: /monitoring/query-browser (5671ms)
No BRIDGE_KUBEADMIN_PASSWORD set, skipping logout
    ✓ successfully displays view for route: /search/all-namespaces?kind=config.openshift.io~v1~Console (6087ms)

3. Outputs a11y summary:

A11y Test Results:
┌──────────────────────┬────────┐
│       (index)        │ Values │
├──────────────────────┼────────┤
│   numberA11yChecks   │  '82'  │
│ numberA11yViolations │  '4'   │
└──────────────────────┴────────┘

JIRA Story: https://issues.redhat.com/browse/CONSOLE-2359

openshift-ci-robot · 2020-09-21T17:19:37Z

@dtaylor113: This pull request references Bugzilla bug 1880118, which is valid. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1880118: Cypress a11y enhancements

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2020-09-21T18:23:58Z

@dtaylor113: This pull request references Bugzilla bug 1880118, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1880118: Cypress a11y enhancements

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

spadgett · 2020-09-21T18:40:08Z

frontend/packages/integration-tests-cypress/plugins/index.js

@@ -33,6 +34,12 @@ module.exports = (on, config) => {
      console.table(data);
      return null;
    },
+    readFileMaybe(filename) {


Suggested change

readFileMaybe(filename) {

readFileIfExists(filename) {

spadgett · 2020-09-21T18:43:46Z

frontend/packages/integration-tests-cypress/support/a11y.ts

+    description,
+    nodes: nodes.length,
+  }));
+  window.Cypress.numberA11yViolations += violations.length;


Should we be modifying the Cypress object? I'd think we'd want to avoid mutating that since it's not our API and store this somewhere else.

Ok, how about window.a11y.numberViolations ?

spadgett · 2020-09-21T18:44:13Z

frontend/public/declarations.d.ts

+    numberA11yViolations: number;
+    numberA11yChecks: number;


If we're only accessing these in the Cypress tests, it would be better to avoid adding the types here since this window definition applies to console itself.

spadgett · 2020-09-21T18:46:47Z

frontend/packages/integration-tests-cypress/support/index.ts

+      window.Cypress.numberA11yViolations = Number(a11yResult.numberA11yViolations);
+      window.Cypress.numberA11yChecks = Number(a11yResult.numberA11yChecks);


If you move this inside the try block, you can use const on allyResult.

spadgett · 2020-09-21T18:48:45Z

frontend/packages/integration-tests-cypress/support/index.ts

+      } catch (e) {
+        cy.task(
+          'logError',
+          `couldn't parse cypress-a11y-results.json.  ${JSON.stringify(e, null, ' ')}`,


I don't think we need the full stack trace here, only the error message?

Suggested change

`couldn't parse cypress-a11y-results.json. ${JSON.stringify(e, null, ' ')}`,

`couldn't parse cypress-a11y-results.json. ${e}`,

spadgett · 2020-09-21T18:52:46Z

frontend/packages/integration-tests-cypress/tests/crud/k8-openshift-cruds.spec.ts

@@ -132,8 +135,10 @@ describe('Kubernetes resource CRUD operations', () => {
        // filter should have 3 chip groups: resource, label, and name
        listPage.filter.numberOfActiveFiltersShouldBe(3);
        listPage.rows.shouldExist(name);
-        cy.testA11y(`Search page for ${kind}: ${name}`);
-
+        // Search page same for all resources, so only a11y test once


This isn't true since the lists are different for each resource. I'd go ahead and test each one unless there is a good reason not to.

ok, thought it was the same for every resource.

spadgett · 2020-09-21T18:53:21Z

frontend/packages/integration-tests-cypress/tests/crud/namespace-crud.spec.ts

@@ -25,11 +25,13 @@ describe('Namespace', () => {
    listPage.rows.shouldNotExist(newName);
    listPage.filter.byName(testName);
    listPage.rows.shouldExist(testName); // created via cy.createProject(testName) above
+    cy.testA11y(`Namespace List page`);


Unnecessary backticks. (I thought we had an eslint rule for this.)

Suggested change

cy.testA11y(`Namespace List page`);

cy.testA11y('Namespace List page');

spadgett · 2020-09-21T18:53:29Z

frontend/packages/integration-tests-cypress/tests/crud/namespace-crud.spec.ts


    cy.log('creates the Namespace');
    listPage.clickCreateYAMLbutton();
    modal.shouldBeOpened();
    cy.byTestID('input-name').type(newName);
+    cy.testA11y(`Create Namespace modal`);


Suggested change

cy.testA11y(`Create Namespace modal`);

cy.testA11y('Create Namespace modal');

spadgett · 2020-09-21T18:53:39Z

frontend/packages/integration-tests-cypress/tests/crud/namespace-crud.spec.ts

@@ -41,6 +43,7 @@ describe('Namespace', () => {
    listPage.rows.clickKebabAction(newName, 'Delete Namespace');
    modal.shouldBeOpened();
    cy.byTestID('project-name-input').type(newName);
+    cy.testA11y(`Delete Namespace modal`);


Suggested change

cy.testA11y(`Delete Namespace modal`);

cy.testA11y('Delete Namespace modal');

spadgett · 2020-09-21T18:54:11Z

frontend/packages/integration-tests-cypress/tests/monitoring/monitoring.spec.ts

@@ -45,6 +45,7 @@ describe('Monitoring: Alerts', () => {
    cy.byLegacyTestID('resource-title').should('have.text', 'Alerting');
    listPage.projectDropdownShouldNotExist();
    listPage.rows.shouldBeLoaded();
+    cy.testA11y(`Monitor Alerting list page`);


Unnecessary backticks throughout this file.

openshift-ci-robot · 2020-09-22T15:31:09Z

@dtaylor113: This pull request references Bugzilla bug 1880118, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1880118: Cypress a11y enhancements

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

dtaylor113 · 2020-09-22T15:32:12Z

Hi @spadgett, I believe I have addressed all of your comments. PTAL -thanks

spadgett · 2020-09-22T15:35:46Z

frontend/packages/integration-tests-cypress/support/index.ts

@@ -1,59 +1,37 @@
+/* eslint-disable import/no-duplicates */


Why is this needed?

It is not :-) removed.

spadgett · 2020-09-22T15:37:31Z

frontend/packages/integration-tests-cypress/tests/crud/k8-openshift-cruds.spec.ts

+        // YAML Editor same for all resources, so only a11y test once
+        if (kind === 'Pod') {
+          cy.testA11y(`YAML Editor for ${kind}: ${name}`);
+        }


I thought the YAML Editor html elements would be the same for every resource, only the inner YAML text changes, so no need to rerun a11y checks for every resource, just one would do, right?

I wouldn't assume that will always be true, and there's no virtually no overhead to checking a11y for each resource. And it makes the code simpler as well. I don't see a reason to restrict it to just pods.

dtaylor113 · 2020-09-22T15:42:07Z

spadgett 2 minutes ago Member
I thought the YAML Editor html elements would be the same for every resource, only the inner YAML text changes, so no need to rerun a11y checks for every resource, just one would do, right?

I wouldn't assume that will always be true, and there's no virtually no overhead to checking a11y for each resource. And it makes the code simpler as well. I don't see a reason to restrict it to just pods.

Fixed. Ok, just didn't want to inflate a11y total number of checks run, that's an extra 27 checks which might be the same page/elements.

openshift-ci-robot · 2020-09-22T15:51:24Z

@dtaylor113: This pull request references Bugzilla bug 1880118, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1880118: Cypress a11y enhancements

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

spadgett

/lgtm

Thanks @dtaylor113 👍

spadgett · 2020-09-22T16:28:29Z

frontend/packages/integration-tests-cypress/tests/crud/other-routes.spec.ts

+  ];
+  otherRoutes.forEach((route) => {
+    it(`successfully displays view for route: ${route}`, () => {
+      cy.visit(`${route}`);


nit:

Suggested change

cy.visit(`${route}`);

cy.visit(route);

Hi @spadgett, fixed, pls re-add 'lgtm' label -thanks

spadgett

/lgtm

dtaylor113 · 2020-09-22T20:59:45Z

Hi @spadgett, had to resolve merge conflict, pls re-add 'lgtm' label -thanks

spadgett

/lgtm

openshift-ci-robot · 2020-09-22T21:03:25Z

[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:

~~frontend/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 · 2020-09-23T00:58:23Z

@dtaylor113: Some pull requests linked via external trackers have merged:

The following pull requests linked via external trackers have not merged:

openshift/console#6475 is open

These pull request must merge or be unlinked from the Bugzilla bug in order for it to move to the next state.

Bugzilla bug 1880118 has not been moved to the MODIFIED state.

In response to this:

Bug 1880118: Cypress a11y enhancements

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

dtaylor113 requested review from spadgett, jcaianirh and bipuladh September 21, 2020 17:19

openshift-ci-robot added the bugzilla/severity-low Referenced Bugzilla bug's severity is low for the branch this PR is targeting. label Sep 21, 2020

openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Sep 21, 2020

openshift-ci-robot requested a review from kyoto September 21, 2020 17:19

openshift-ci-robot added the component/core Related to console core functionality label Sep 21, 2020

spadgett reviewed Sep 21, 2020

View reviewed changes

openshift deleted a comment from openshift-ci-robot Sep 22, 2020

dtaylor113 force-pushed the cypress-axe-enhancements branch from 0c6d0f4 to 7da7af2 Compare September 22, 2020 15:29

spadgett reviewed Sep 22, 2020

View reviewed changes

dtaylor113 force-pushed the cypress-axe-enhancements branch 2 times, most recently from 262b4ed to 4d3cafb Compare September 22, 2020 15:46

spadgett approved these changes Sep 22, 2020

View reviewed changes

openshift-ci-robot assigned spadgett Sep 22, 2020

openshift-ci-robot 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 Sep 22, 2020

spadgett reviewed Sep 22, 2020

View reviewed changes

dtaylor113 force-pushed the cypress-axe-enhancements branch from 4d3cafb to c5d2831 Compare September 22, 2020 17:10

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Sep 22, 2020

openshift-ci-robot mentioned this pull request Sep 22, 2020

Bug 1880118: Cypress better headless logging #6629

Merged

spadgett approved these changes Sep 22, 2020

View reviewed changes

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

cypress a11y: expanded coverage, improved reporting

6f0677e

dtaylor113 force-pushed the cypress-axe-enhancements branch from c5d2831 to 6f0677e Compare September 22, 2020 20:59

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Sep 22, 2020

spadgett approved these changes Sep 22, 2020

View reviewed changes

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

openshift-merge-robot merged commit 85b76c6 into openshift:master Sep 23, 2020

openshift-ci-robot mentioned this pull request Sep 23, 2020

Bug 1880118: Improve Cypress checkErrors output #6475

Merged

spadgett added this to the v4.6 milestone Sep 30, 2020

dtaylor113 deleted the cypress-axe-enhancements branch November 25, 2020 14:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1880118: Cypress a11y enhancements #6698

Bug 1880118: Cypress a11y enhancements #6698

dtaylor113 commented Sep 21, 2020 •

edited

openshift-ci-robot commented Sep 21, 2020

openshift-ci-robot commented Sep 21, 2020

spadgett Sep 21, 2020

spadgett Sep 21, 2020

dtaylor113 Sep 21, 2020

spadgett Sep 21, 2020

spadgett Sep 21, 2020

spadgett Sep 21, 2020

spadgett Sep 21, 2020

dtaylor113 Sep 21, 2020

spadgett Sep 21, 2020

spadgett Sep 21, 2020

spadgett Sep 21, 2020

spadgett Sep 21, 2020

openshift-ci-robot commented Sep 22, 2020

dtaylor113 commented Sep 22, 2020

spadgett Sep 22, 2020

dtaylor113 Sep 22, 2020

spadgett Sep 22, 2020

dtaylor113 commented Sep 22, 2020 •

edited

openshift-ci-robot commented Sep 22, 2020

spadgett left a comment

spadgett Sep 22, 2020

dtaylor113 Sep 22, 2020

spadgett left a comment

dtaylor113 commented Sep 22, 2020

spadgett left a comment

openshift-ci-robot commented Sep 22, 2020

openshift-ci-robot commented Sep 23, 2020

		window.Cypress.numberA11yViolations = Number(a11yResult.numberA11yViolations);
		window.Cypress.numberA11yChecks = Number(a11yResult.numberA11yChecks);

	`couldn't parse cypress-a11y-results.json. ${JSON.stringify(e, null, ' ')}`,
	`couldn't parse cypress-a11y-results.json. ${e}`,

	cy.testA11y(`Namespace List page`);
	cy.testA11y('Namespace List page');

	cy.testA11y(`Create Namespace modal`);
	cy.testA11y('Create Namespace modal');

	cy.testA11y(`Delete Namespace modal`);
	cy.testA11y('Delete Namespace modal');

Bug 1880118: Cypress a11y enhancements #6698

Bug 1880118: Cypress a11y enhancements #6698

Conversation

dtaylor113 commented Sep 21, 2020 • edited

openshift-ci-robot commented Sep 21, 2020

openshift-ci-robot commented Sep 21, 2020

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci-robot commented Sep 22, 2020

dtaylor113 commented Sep 22, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dtaylor113 commented Sep 22, 2020 • edited

openshift-ci-robot commented Sep 22, 2020

spadgett left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spadgett left a comment

Choose a reason for hiding this comment

dtaylor113 commented Sep 22, 2020

spadgett left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Sep 22, 2020

openshift-ci-robot commented Sep 23, 2020

dtaylor113 commented Sep 21, 2020 •

edited

dtaylor113 commented Sep 22, 2020 •

edited