Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[test] Check a11y tree inclusion in CI only #18433

Merged
merged 3 commits into from
Nov 19, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 explicityl 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).
eps1lon marked this conversation as resolved.
Show resolved Hide resolved
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();