New API RFC

[sapegin]

Issues with the current API

• Multiple optional positional arguments in the checkA11y() method make it awkward to use and very difficult to extend. We have several open pull requests that are adding a new argument at the end.
• Requires too much setup to have violations summary either in the terminal or JSON that could be supplied to another too.
• Doesn't support Cypress chaining in the checkA11y() method, and uses a custom context argument. This makes the usage less idiomatic and doesn't allow integration with tools like Cypress Testing Library.
• No way to define defaults for the checkA11y() method, we have to pass things like violationCallback in every call or create a custom command.

Related issues: #40, #49, #62, #67, #68

New API proposal

1. Remove the context argument of the checkA11y() method, and rely on the Cypress chaining instead:

cy.checkA11y() // Check the whole document
cy.getByTestId('my-modal').checkA11y() // Check part of the document

2. Replace the rest of the arguments with an optional object of the following shape:

interface CypressAxeOptions {
  axeOptions?: axe.RunOptions, // This used to be `options` argument
  shouldFail?: (violations: RunResults[], results: RunResults[]) => boolean,  // Replaces `includedImpacts` and `skipFailures`
  reporters?: Reporter[]
}

type Reporter = (results: RunResults[]) => void

interface RunResults {
  filename: string, // Test file
  label?: string, // Label passed to checkA11y
  results: axe.Result[]
}

The defaults are going to be:

{
  axeOptions: undefined,
  shouldFail: (violations) => violations.length > 0,  // Fail on any number of violations
  reporters: [
   require('cypress-axe/cli-reporter')
  ]
}

Reporters are replacing the violationCallback: they are more flexible, have access to both passes and violations, and you could have global reporters that would run after all tests, or pass a reporter for a separate checkA11y() method call. The default reporter is printing an overall summary after all tests.

3. Add a new optional label to the checkA11y() method:

cy.checkA11y(options?: CypressAxeOptions, label?: string)

Similar to labels describe() or it() methods, to identify results of a particular call.

4. Add a new method to define defaults:

cy.configureCypressAxe(options: CypressAxeOptions)

It should accept the same object as the checkA11y() method, and set default values that could be overwritten in checkA11y().

---

I hope this covers all the use cases we have. I'm not sure that all the names are clear, so if you have any feedback, leave a comment ;-)

[kristina-hager]

I've worked now with several of the test framework axe extensions. Based on that experience, I have some thoughts about the approach cypress-axe is taking to provide many options for filtering out and reporting.

Many of the extensions are just taking on the job of making it easy to inject axe and scan the html and just returning results to the users. e.g.:
https://www.npmjs.com/package/@testcafe-community/axe#using-full-axe-result-object-and-axeconfigure -- that's the latest testcafe axe. This allows the user to simply get the results object back from axe-core and do what is needed with it.

jest-axe takes a similar approach: https://github.com/nickcolley/jest-axe#testing-react

For cypress-axe, could it work to drop some of the proposed options from checkA11y?

{
  axeOptions: undefined,   // want to keep this one
  maxViolations: 0,  // remove this and let user create their own assert
  minImpact: 'minor', //remove this and let the user inspect the results and create their own assert
  reporters: [ // remove this and give the full results object back to user
   {
      format: 'cli'
    }
  ]
}

checkAlly would then look something like (the following is slightly pseudocode)

let results = cy.checkAlly({ axeOptions: axeOptions });
createHtmlReport( options ); //  from import { createHtmlReport } from 'axe-html-reporter';
expect(results.violations).to.be.empty;

and then, as shown, the user can do what they need with filtering, reporting and then calling an assert to pass or fail the test.
As mentioned, the above is slightly pseudocode. As in, the createHtmlReport would need to be done with a task given how cypress works. I wanted to be brief for clarity.

However, I think the above approach could reduce some maintenance burden from this library as some of the requests like #40 and #62 could then be easily resolved by the coder instead of having to be handled in this package.

I also want to call out that there are a few more open source reporters that have come out recently:
https://github.com/lpelypenko/axe-html-reporter
https://github.com/lpelypenko/axe-result-pretty-print

These can be used in any javascript code, so it doesn't matter which test framework you have.

[sapegin]

@kristina-hager Thanks for the comment!

The problem with this approach is that it requires too much setup from the user to have anything useful. I want to have a zero-config (as much as possible) tool that allows extension. Not a tool where I have to do everything myself, and spend hours or days googling and reading docs, trying to glue several libraries that are supposed to work together but just don't ;-) Honestly, I'm really tired of this.

  maxViolations: 0,  // remove this and let user create their own assert
  minImpact: 'minor', //remove this and let the user inspect the results and create their own assert

The reason for these two options is to allow gradual adoption. I'm not sure what you mean by “their onw assert” and how it will help with that. But I agree there might be a better API.

reporters: [ // remove this and give the full results object back to user

This is exactly what it does but I don't see reasons why not provide useful defaults.

I also want to call out that there are a few more open source reporters that have come out recently:

This is interesting, I'll definitely have a look!

[aldosuwandi]

I second @kristina-hager 's comment to make cy.checkAlly to return axeResults and drop the assertions and reporter parts.

Specifying maxViolations and minImpact might be good enough for some users, but other users might care about different things, so they want to implement their own assertion logic.

Also, by decoupling those parts, I believe it won't make cypress-axe extremely difficult to use, since other similar extensions also making the same approach. For example, this one: https://github.com/dequelabs/axe-core-npm/tree/develop/packages/webdriverjs, where the only responsibility is to inject axe-core and analyze.

[kristina-hager]

The request really is about separation of concerns. I'll make a PR to illustrate what I mean.

[kristina-hager]

Here's the PR (still WIP) to illustrate what I mean. I still need to test this and beef up readme changes.
However, again, the idea here is to decouple the reporting/filtering objectives from injecting axe and analyzing.

[sapegin]

Sorry but such API is extremely difficult to use. If that's what you really need, you could use axe-core directly. I'm happy to make the API more flexible, if it doesn't cover any use cases, but it shouldn't be at the cost of the majority of the users.

[kristina-hager]

Well, I suppose reasonable people may disagree on this matter.

However, all the extra functionality this package wraps may be ignored by the user especially if the following changes can be included:

• return the 'results' object from the function
  (this object can be filtered w/ the additional, optional features like 'minImpact')
• change the argument skipFailures (optional, defaults to false) to failOnViolations (optional, defaults to false). This way users can opt in to having the assert coded up for them. However, users interested in inspecting the results object themselves and doing their own asserts have an easy path.

In addition, this change would bring this library in line with the majority of other axe-core extensions that do not provide such features and do not do the assert for the user while allowing the additional bells/whistles that some find appealing to remain.

[sapegin]

return the 'results' object from the function

👍

change the argument skipFailures (optional, defaults to false) to failOnViolations (optional, defaults to false). This way users can opt in to having the assert coded up for them. However, users interested in inspecting the results object themselves and doing their own asserts have an easy path.

This will make an unfortunate default making the library do nothing by default, I'm sure it will confuse many people trying to use it. Let's keep the defaults helpful, you could change them as you wish via the new cy.configureCypressAxe() method.

However, I think we can make a more generic method for managing failure instead of proposed maxViolations, minImpact and failOnViolations:

shouldFail(violations: RunResults[], results: RunResults[]) => boolean

With default value that would fail on any number of violations. It will be one line function to implement maxViolations or minImpact, and just return false for your case.

[kristina-hager]

This will make an unfortunate default making the library do nothing by default

Well, again, I think this is a matter where we don't agree. Based on experience with other libs, the injecting of axe is the confusing part. With just this covered, the library has value.
If in your experience users find it hard to write javascript to filter results and to make an assert, then so be it. I don't have this experience at all, but the lib can keep this functionality if it's worth your maintenance time.

Ultimately, I'm advocating for the use case we have, which is that we need the full results object back for analysis and users can generally handle writing their asserts (or they can use the embedded assert functionality if they like).

That said:

However, I think we can make a more generic method for managing failure instead of proposed maxViolations, minImpact and failOnViolations

This sounds interesting! I'd look forward to seeing the new draft of the API. There have been enough changes that i've lost track a bit.

[sapegin]

I've updated the initial comment so we have the single source of truth here.

I've also simplified reporters: now it's just a function that gets the full results object and is free to do what it wishes with it.

[kristina-hager]

Thanks! looks basically good to me. i'd be glad to review any PR when you're ready.
We have an internal wrapper of the old cypress-axe that I'm keen to get rid of and move on to this new version.

[sapegin]

I think I want to migrate the code to TypeScript first — it will be easier to work on the new API, and also we'll have our own typings.

[kristina-hager]

Hahah, well, I won't get into debates about typescript vs javascript for relatively small codebases. 😄
If you think maintenance in TS is easier, then it works for you! TS also seems to work fine for importing into JS.

[sapegin]

Hahah, well, I won't get into debates about typescript vs javascript for relatively small codebases. 😄

Very good idea.

[kyrstenkelly]

I am a fan of this new API proposal. I agree that returning all results is useful in case someone wants to do something more custom, but I also personally appreciate having a nice default reporter so I don't have to put in extra effort there.

[kristina-hager]

@sapegin - is this the API you will go with? If so, I may move our internal fork to align to ease the transition back to this version when it is ready. That is, unless you have some plan to release this anytime "soon" (e.g. this year?).

[sgregson]

Came to this issue by searching the tracker for the ability to assert/log on a threshold of "needs review" items, aka the results.incomplete collection – Excited to see what looks like the addition of that ability through both the shouldFail and reporters proposed API @sapegin 🚀

[Cyara-Andrew-Watson]

I agree with sgregson - my issue with not having the full results object, is that the current cypress-axe provides access to only the violations object... by default the form-field-multiple-labels rule is listed as "needs review" (https://github.com/dequelabs/axe-core/blob/develop/doc/rule-descriptions.md) which is in the "incomplete" array, not the violations list (https://www.deque.com/axe/core-documentation/api-documentation/#results-object)... so at the moment it would appear your facade on axe-core is preventing me from accessing / reporting on these types of rule results... Also other rules can do both - for example color-contrast will fail for solid backgrounds but return needs-review for gradient backgrounds (and I'm sure there are others)... so these results end up in two different locations.

I'll try increasing the violation level on that rule (and perhaps others)... but it took me a while to figure out why the "rules were not running" - (experimental rules are not run by default, but all the others are)... it's just being filtered out for me, like warnings don't matter. Please consider changing the interface to allow for either the full result object to be returned, or wrap the other properties on it or some-such-thing... eg have a fuller, better facade on the result object, than just filtering the violations.

[dwightjack]

Hi,

is there a schedule for the implementation of the new API? In the near future, I will start working on a11y testing with Cypress. The new proposed API fits my requirements so I will be happy to contribute to the development.

[kristina-hager]

Is there still any effort planned on these changes? Just checking. My team made an internal fork to deal with some needed updates, but we'd much rather move back to open source.

[tfrijsewijk]

Chaining would be awesome 🎉

[rkrisztian]

I also welcome more flexible custom reporting.

The reporting format I would like to use:
https://sparkbox.com/foundry/cypress_and_axe_tutorial_automated_accessibility_testing_tools

But right now cypress-axe works like this:

        if (violations.length) {
            if (violationCallback) {
                violationCallback(violations);
            }
            violations.forEach(function (v) {
                var selectors = v.nodes
                    .reduce(function (acc, node) { return acc.concat(node.target); }, [])
                    .join(', ');
                Cypress.log({
                    $el: Cypress.$(selectors),
                    name: 'a11y error!',
                    consoleProps: function () { return v; },
                    message: v.id + " on " + v.nodes.length + " Node" + (v.nodes.length === 1 ? '' : 's'),
                });
            });
        }

So I can't configure a custom reporting without having the default reporting in there too (well, unless I use tricks like patch-package, which shouldn't be needed though).

[oussamabadr]

Can you just please make a callback to access all axe-core results object ?

IMHO, making small changes will be helpful instead of big changes, an by receiving feedback, things will be clearer if it worth the effort or not (Just my thought).