Skip to content

Commit

Permalink
[test] Check a11y tree inclusion in CI only (#18433)
Browse files Browse the repository at this point in the history
  • Loading branch information
eps1lon authored Nov 19, 2019
1 parent 9c2f734 commit 3f42da1
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 16 deletions.
16 changes: 14 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,13 @@ how to fix the issues.
##### ci/circleci: test_unit-1

Runs the unit tests in a `jsdom` environment. If this fails then `yarn test:unit`
should fail locally as well. You can narrow the scope of tests run with `yarn test:unit --grep ComponentName`.
should<sup>[1](#accessiblity-tree-exclusion)</sup> fail locally as well. You can narrow the scope of tests run with `yarn test:unit --grep ComponentName`.

##### ci/circleci: test_browser-1

Runs the unit tests in multiple browsers (via Browserstack). The log of the failed
build should list which browsers failed. If Chrome failed then `yarn test:karma`
should fail locally as well. If other browsers failed debugging might be trickier.
should<sup>[1](#accessiblity-tree-exclusion)</sup> fail locally as well. If other browsers failed debugging might be trickier.

##### ci/circleci: test_regression-1

Expand Down Expand Up @@ -158,6 +158,18 @@ it's always appreciated if it can be improved.
There are various other checks done by Netlify to check the integrity of our docs. Click
on _Details_ to find out more about them.

#### Caveats

##### Accessiblity tree exclusion

Our tests also explicitly document which parts of the queried element are included in
the accessibility (a11y) tree and which are excluded. This check is fairly expensive which
is why it is disabled when tests are run locally by default. The rationale being
that in almost all cases including or excluding elements from a query-set depending
on their a11y-tree membership makes no difference. The queries where this does
make a difference explicitly include this check e.g. `getByRole('button', { hidden: false })` (see [byRole documentation](https://testing-library.com/docs/dom-testing-library/api-queries#byrole) for more information).
To see if your test (`test:browser` or `test:unit`) behaves the same between CI and local environment set the environment variable `CI` to `'true'`.

### Testing the documentation site

The documentation site is built with Material-UI and contains examples of all the components.
Expand Down
10 changes: 5 additions & 5 deletions packages/material-ui/src/Breadcrumbs/Breadcrumbs.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ describe('<Breadcrumbs />', () => {
</Breadcrumbs>,
);

expect(getAllByRole('listitem')).to.have.length(2);
expect(getAllByRole('listitem', { hidden: false })).to.have.length(2);
expect(getByRole('list')).to.have.text('first/second');
});

Expand All @@ -56,7 +56,7 @@ describe('<Breadcrumbs />', () => {
</Breadcrumbs>,
);

const listitems = getAllByRole('listitem');
const listitems = getAllByRole('listitem', { hidden: false });
expect(listitems).to.have.length(3);
expect(getByRole('list')).to.have.text('first//ninth');
expect(listitems[1].querySelector('[data-mui-test="MoreHorizIcon"]')).to.be.ok;
Expand All @@ -77,9 +77,9 @@ describe('<Breadcrumbs />', () => {
</Breadcrumbs>,
);

getAllByRole('listitem')[2].click();
getAllByRole('listitem', { hidden: false })[2].click();

expect(getAllByRole('listitem')).to.have.length(3);
expect(getAllByRole('listitem', { hidden: false })).to.have.length(3);
});

describe('warnings', () => {
Expand All @@ -100,7 +100,7 @@ describe('<Breadcrumbs />', () => {
<span>fourth</span>
</Breadcrumbs>,
);
expect(getAllByRole('listitem')).to.have.length(4);
expect(getAllByRole('listitem', { hidden: false })).to.have.length(4);
expect(getByRole('list')).to.have.text('first/second/third/fourth');
expect(consoleErrorMock.callCount()).to.equal(2); // strict mode renders twice
expect(consoleErrorMock.args()[0][0]).to.include(
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/Select/Select.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ describe('<Select />', () => {
});

expect(handleBlur.callCount).to.equal(0);
expect(queryByRole('listbox')).to.be.null;
expect(queryByRole('listbox', { hidden: false })).to.be.null;
});

it('options should have a data-value attribute', () => {
Expand Down
9 changes: 4 additions & 5 deletions packages/material-ui/test/integration/Menu.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ describe('<Menu /> integration', () => {
});

it('closes the menu when Tabbing while the list is active', () => {
const { getByRole, queryByRole } = render(<ButtonMenu />);
const { getByRole } = render(<ButtonMenu />);

getByRole('button').focus();
getByRole('button').click();
Expand All @@ -324,11 +324,11 @@ describe('<Menu /> integration', () => {
// react-transition-group uses one commit per state transition so we need to wait a bit
clock.tick(0);

expect(queryByRole('menu')).to.be.null;
expect(getByRole('menu', { hidden: true })).to.be.inaccessible;
});

it('closes the menu when the backdrop is clicked', () => {
const { getByRole, queryByRole } = render(<ButtonMenu />);
const { getByRole } = render(<ButtonMenu />);
const button = getByRole('button');

button.focus();
Expand All @@ -337,7 +337,6 @@ describe('<Menu /> integration', () => {
document.querySelector('[data-mui-test="Backdrop"]').click();
clock.tick(0);

// TODO use getByRole with hidden and match that it's inaccessible
expect(queryByRole('menu')).to.be.null;
expect(getByRole('menu', { hidden: true })).to.be.inaccessible;
});
});
16 changes: 13 additions & 3 deletions test/utils/init.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,18 @@
import enzyme from 'enzyme/build/index';
import enzyme from 'enzyme';
import Adapter from 'enzyme-adapter-react-16';
import * as testingLibrary from '@testing-library/react/pure';
import consoleError from './consoleError';
import './initMatchers';

consoleError();

enzyme.configure({ adapter: new Adapter() });

// checking if an element is hidden is quite expensive
// this is only done in CI as a fail safe. It can still explicitly be checked
// in the test files which helps documenting what is part of the DOM but hidden
// from assistive technology
const defaultHidden = !process.env.CI;
// adds verbosity for something that might be confusing
console.warn(`${defaultHidden ? 'including' : 'excluding'} inaccessible elements by default`);
testingLibrary.configure({ defaultHidden });

consoleError();

0 comments on commit 3f42da1

Please sign in to comment.