kentcdodds / react-testing-library-course

Test React Components with Jest and React Testing Library on TestingJavaScript.com
https://testingjavascript.com/playlists/test-react-components-with-jest-and-react-testing-library-72cf
Other
1.09k stars 379 forks source link

jest-axe not catching inaccessible forms #25

Closed jrnxf closed 1 year ago

jrnxf commented 3 years ago

Hey Kent!

Been going through your course and am really loving it. I was writing a test today to check the accessibility of a form I new wasn't accessible to begin with, but was surprised that it passed the test. I checked in this repo as well to find the same thing. If you check the a11y.js file, the following test will pass:

test('accessible forms pass axe', async () => {
  const {container} = render(<InaccessibleForm />)
  expect(await axe(container)).toHaveNoViolations()
})

Any idea why? I looked into the issue for a minute online but can't seem to find anyone else experiencing this issue.

kentcdodds commented 3 years ago

Hi @coloradocolby,

I just tried it myself and things are working as expected:

image

I did just push an update to ensure that an error is thrown when it should be, but I'm definitely getting an error when I use the same code you provided:

image

kentcdodds commented 3 years ago

Whoops! Hold it. I just ran npm install to make sure and now I'm getting the same issue you are.

I'm not sure what changed. Maybe @nickcolley can explain what's going wrong here.

To sum up, here's the whole test:

import * as React from 'react'
import {render} from '@testing-library/react'
import {axe} from 'jest-axe'

function InaccessibleForm() {
  return (
    <form>
      <input placeholder="email" />
    </form>
  )
}

function AccessibleForm() {
  return (
    <form>
      <label htmlFor="username">Username</label>
      <input id="username" placeholder="username" />
    </form>
  )
}

test('inaccessible forms fail axe', async () => {
  const {container} = render(<InaccessibleForm />)
  const axeResult = await axe(container)
  expect(() => expect(axeResult).toHaveNoViolations()).toThrow()
  // NOTE: I can't think of a situation where you'd want to test that some HTML
  // actually _does_ have accessibility issues... This is only here for
  // demonstration purposes.
})

test('accessible forms pass axe', async () => {
  const {container} = render(<AccessibleForm />)
  expect(await axe(container)).toHaveNoViolations()
})

And the test output:

 FAIL  src/__tests__/a11y.js
  ✕ inaccessible forms fail axe (160 ms)
  ✓ accessible forms pass axe (53 ms)

  ● inaccessible forms fail axe

    expect(received).toThrow()

    Received function did not throw

      23 |   const {container} = render(<InaccessibleForm />)
      24 |   const axeResult = await axe(container)
    > 25 |   expect(() => expect(axeResult).toHaveNoViolations()).toThrow()
         |                                                        ^
      26 |   // NOTE: I can't think of a situation where you'd want to test that some HTML
      27 |   // actually _does_ have accessibility issues... This is only here for
      28 |   // demonstration purposes.

      at Object.<anonymous> (src/__tests__/a11y.js:25:56)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 passed, 2 total
Snapshots:   0 total
Time:        3.117 s, estimated 4 s
Ran all test suites matching /a11y/i.

Watch Usage: Press w to show more.

Any ideas @nickcolley?

kentcdodds commented 3 years ago

I did just try downgrading to jest-axe@4.0.0 which was the last working version and it's still causing issues so it's probably some transitive dep that broke. But this is a pretty basic example so I'm sure there's something simple we're missing or there's a bug in jest-axe.

jrnxf commented 3 years ago

@kentcdodds Yeah I thought the same thing! It seems to only be on form validations as well. For example if I render <img src="#" /> it will correctly identify no alt tag and report a violation.

aakashsardana commented 2 years ago

I am also facing the same issue. My test cases are passing even though the form is Inaccessible.

pakaponk commented 2 years ago

I came across the same issue when I was going through the lesson with the code from main branch. If you using jest-axe version 4.1.0 onward (which now being used in the main branch), the following form would no longer be considered inaccessible because of this change in version 4.1.0.

<form>
  <input placeholder="email" />
</form>

Screen Shot 2565-09-06 at 12 46 30

To make it fail the test case, we need to remove the placeholder attribute

Screen Shot 2565-09-09 at 11 57 03

I have already sent an email to help@testingjavascript.com about this issue but I am glad to send a pull request if you like.

kentcdodds commented 2 years ago

Looks like the behavior has changed a bit. A PR would be welcome with a comment explaining things a bit.

gavrielrh commented 2 years ago

Hi, I just ran into this exact issue and was equally confused until reading the comments above.

Looks like in 4.1.1 of the W3C accessibility guidelines, placeholder is now a fall-back for accessible name.

@pakaponk are you planning on submitting a PR? I'd be happy to contribute one otherwise :smile: