Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

e2e Accessibility #250

Merged
merged 22 commits into from
Aug 17, 2017
Merged

e2e Accessibility #250

merged 22 commits into from
Aug 17, 2017

Conversation

Blackbaud-SteveBrush
Copy link
Member

@Blackbaud-SteveBrush Blackbaud-SteveBrush commented Aug 2, 2017

  • Add the accessibility analyzer anywhere in your e2e specs:
import { SkyA11y } from '@blackbaud/skyux-builder/runtime/testing/e2e';

SkyA11y.run().then((violations: number) => {
  expect(violations).toEqual(0);
  done();
});
  • Rules can be disabled individually via skyuxconfig.json:
{
  "a11y": {
    "rules": {
      "label": { "enabled": false }
    }
  }
}

Or, all rules can be disabled via:

{
  "a11y": false
}

A full list of rules can be found here: https://github.com/dequelabs/axe-core/blob/master/doc/rule-descriptions.md

Addresses: blackbaud/skyux2#590

@Blackbaud-MattGregg This should be quite similar to what you originally submitted. I had to change a few things because of the way unit tests are handled, currently.

@Blackbaud-SteveBrush Blackbaud-SteveBrush changed the title e2e Accessibility [HOLD] e2e Accessibility Aug 2, 2017
@codecov-io
Copy link

codecov-io commented Aug 4, 2017

Codecov Report

Merging #250 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #250   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          54     56    +2     
  Lines        1265   1305   +40     
  Branches      190    195    +5     
=====================================
+ Hits         1265   1305   +40
Flag Coverage Δ
#builder 100% <100%> (ø) ⬆️
#runtime 100% <ø> (ø) ⬆️
#srcapp 100% <ø> (ø) ⬆️
Impacted Files Coverage Δ
runtime/config.ts 100% <ø> (ø) ⬆️
config/axe/axe.config.js 100% <100%> (ø)
lib/a11y-analyzer.js 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da33cdc...6fed70b. Read the comment docs.

@Blackbaud-SteveBrush Blackbaud-SteveBrush changed the title [HOLD] e2e Accessibility e2e Accessibility Aug 5, 2017
@@ -0,0 +1,2 @@
const analyzer = require('../../../lib/a11y-analyzer');
module.exports = { SkyA11y: analyzer };
Copy link
Member Author

@Blackbaud-SteveBrush Blackbaud-SteveBrush Aug 5, 2017

Choose a reason for hiding this comment

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

This is essentially an adaptor to meet the NodeJS requirements to stub out protractor and axe-webdriverjs in the unit tests, and our consuming SPAs requirement to import a TypeScript class. (See the definition file, above.)

@Blackbaud-SteveBrush
Copy link
Member Author

@Blackbaud-BobbyEarl: To answer the question: "What’s the reasoning for the source being JavaScript and having a matching .d.ts file, versus the source just being in TypeScript?".

The main reason the source is in JavaScript is to make stubbing out the various modules possible in the unit test. Since the source file requires protractor and axe-webdriverjs, attempting to stub both of these JS modules as import * as protractor from 'protractor' proved to be impossible as there was no clean way to reach the JavaScript imports before: 1) importing the A11y module into the unit test, or 2) the TypeScript file was transpiled (I spent 2-3 days researching this alone). Here are some articles that explain the issue in some detail:

I did originally write this feature using pure TypeScript, and it worked in the consuming e2e tests just fine. The issue came when I had to write unit tests for it on the Builder side of things. Stubbing out the JS modules just seemed to be impossible. Also keep in mind that this script is technically NOT a runtime feature, as it's not getting consumed by an Angular application. It's literally a server-side script.

To see what I'm talking about, try writing a unit test for the sibling file, /runtime/testing/e2e/host-browser.ts. This file does not have a unit test, and my guess is the test wasn't written due to the problem described above.

@Blackbaud-BobbyEarl
Copy link

Hey @Blackbaud-SteveBrush. Thanks for clarifying for me. I'm not a huge fan of having to give up TS as the source, but I understand the limitations. While it kind of feels like a weird setup, I'm not sure I have a suggestion on a better approach.

For the sake of time, the only thing I could think to do would be to leave the source in Typescript, and test the functionality in an e2e test (e2e/skyux-e2e.e2e-spec.js).

Unless @Blackbaud-PaulCrowder has any additional opinions, I'm ok with this one.

Copy link

@Blackbaud-BobbyEarl Blackbaud-BobbyEarl left a comment

Choose a reason for hiding this comment

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

Besides the empty beforeEach, I think this is the best course of action to keep the feature moving.


describe('config axe', () => {
beforeEach(() => {

Choose a reason for hiding this comment

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

Empty beforeEach

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants