component-driven / cypress-axe

Test accessibility with axe-core in Cypress
MIT License
622 stars 86 forks source link

Add the option to overwrite cy.visit to automatically include axe-core #35

Closed DesselBane closed 4 years ago

DesselBane commented 4 years ago

Hi this is my first PR so I hope this is in the correct format

What is the problem

Adding cy.injectAxe() after each cy.visit() is repetitive. \ Cypress does not allow to execute a command inside the then() statement of another command. This means that it can't be overwritten like so:

Cypress.Commands.overwrite('visit', (originalFn, url, options = {}) => {
    return originalFn(url, options).then(() => {
      cy.injectAxe()
    })
  })

and when overwriting it like so:

Cypress.Commands.overwrite('visit', (originalFn, url, options = {}) => {
    originalFn(url, options)
    cy.injectAxe()
  })

The injectAxe() method is invoked before the original cy.visit() promise is resolved.

Why implement it like this?

I figured overwriting the cy.visit() command should be optional since not everyone will need/want it.

avanslaars commented 4 years ago

Hi @DesselBane,

Thanks for the PR! The format is great however, I don't think overwriting visit belongs in the library.

While calling cy.injectAxe() might feel repetitive, it is only one line of code and writing it where it is needed makes the actions in the spec file explicit. For example, you could break your specs out with multiple describes and only use cy.injectAxe in the beforeEach for a subset of your tests. It keeps the loading of the library limited to those tests and makes it clear to someone reading the tests that Axe is injected in that subset of tests only. If you overwrite visit, then every test gets axe injected whether it is used or not.

I would avoid overwriting visit for this purpose generally, but if you're going to do it, it should probably be something that is defined in your code base and agreed upon by the team that is writing and maintaining those tests, taking the above concerns into consideration.