test(radio): Change tests to react-testing-library and improve coverage #381
Conversation
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
mannycarrera4
changed the title
cypress/helpers/dom.ts
Outdated
| } | ||
| const id = $label.attr('for'); | ||
| try { | ||
| const $input = Cypress.$(`[id="${id}"]`); |
There was a problem hiding this comment.
Are we worried that this doesn't account for wrapping labels?
i.e.
<label>
<input />
</label>| }); | ||
|
|
||
| it('should pass accessibility checks', () => { | ||
| cy.checkA11y(); |
There was a problem hiding this comment.
Random point of discussion: it really bugs me how much of a black box this is. For example, Radio group labels need to be legends - does this check that? Does this ensure aria-describedby is set when there's an error? I have no clue. In general, it makes the a11y specs really unclear. Not sure what the solution is without manually checking it all.
There was a problem hiding this comment.
I asked Accessibility this question a few months ago. They estimate axe-core catches around 13% of accessibility issues. They think we should run it because it is so cheap to do so, but not treat it as a replacement for accessibility specifications.
What you describe should be specifications in our Cypress files.
describe('given the Error story is rendered', () => {
it('should have a legend element', () => {
// get radio input
// .parents('legend').should('exist')
})
it('should have an aria-describedby linking to the input element', () => {
// find radio input and get id to check against the value of the `aria-describedby`
})
})I think the test block description is misleading. It should say something like:
it('should pass axe-core checks', () => {To clarify:
cy.checkA11y() is not a replacement for accessibility specifications. We simply treat it as a black box and not care if we duplicate accessibility checks.
…overage (#390) * test(text-input): Switch tests to RTL * test(text-input): Add static state for testing * test(text-input): Update tests * test(text-input): Update story name * test(text-input): Fix test * test(text-input): Fix test * test(text-input): Fix test * test(text-input): Fix test * test(text-input): Update placeholder test * test(text-input): Update input ref test * test(text-input): Fix lint * test(text-input): Update component state table * fix(text-input): Fix border for disabled text inputs in error * test(text-input): Add cypress test for typing into field * chore: Remove unnecessary depth from imports * test(text-input): Clean up and add RTL tests Co-authored-by: Alex Nicholls <anicholls3@gmail.com>
Co-authored-by: Alex Nicholls <anicholls3@gmail.com>
…testing-radio # Conflicts: # yarn.lock
NicholasBoll
dismissed
anicholls’s stale review
It looks like all requested changes were implemented
Fixes adding test for radio: #286
Checklist
yarn testpassespackage.jsoncanvas-kit-reactand/orcanvas-kit-cssuniversal modules, ifapplicable
Additional References