Skip to content

Commit

Permalink
feat: new rule no-get-by-for-checking-element-not-present (#65)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: rule prefer-expect-query-by has disappeared in favor of new rule no-get-by-for-checking-element-not-present
  • Loading branch information
Thomas Lombart committed Jan 28, 2020
1 parent 6420867 commit 1aa9238
Show file tree
Hide file tree
Showing 8 changed files with 257 additions and 254 deletions.
22 changes: 11 additions & 11 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,17 +134,17 @@ To enable this configuration use the `extends` property in your

## Supported Rules

| Rule | Description | Configurations | Fixable |
| -------------------------------------------------------------- | ------------------------------------------------------------------- | ------------------------------------------------------------------------- | ------------------ |
| [await-async-query](docs/rules/await-async-query.md) | Enforce async queries to have proper `await` | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
| [await-async-utils](docs/rules/await-async-utils.md) | Enforce async utils to be awaited properly | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
| [await-fire-event](docs/rules/await-fire-event.md) | Enforce async fire event methods to be awaited | ![vue-badge][] | |
| [no-await-sync-query](docs/rules/no-await-sync-query.md) | Disallow unnecessary `await` for sync queries | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
| [no-debug](docs/rules/no-debug.md) | Disallow the use of `debug` | ![angular-badge][] ![react-badge][] ![vue-badge][] | |
| [no-dom-import](docs/rules/no-dom-import.md) | Disallow importing from DOM Testing Library | ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] |
| [prefer-expect-query-by](docs/rules/prefer-expect-query-by.md) | Disallow the use of `expect(getBy*)` | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
| [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than just `getBy*` queries | | |
| [consistent-data-testid](docs/rules/consistent-data-testid.md) | Ensure `data-testid` values match a provided regex. | | |
| Rule | Description | Configurations | Fixable |
| --------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------- | ------------------------------------------------------------------------- | ------------------ |
| [await-async-query](docs/rules/await-async-query.md) | Enforce async queries to have proper `await` | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
| [await-async-utils](docs/rules/await-async-utils.md) | Enforce async utils to be awaited properly | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
| [await-fire-event](docs/rules/await-fire-event.md) | Enforce async fire event methods to be awaited | ![vue-badge][] | |
| [consistent-data-testid](docs/rules/consistent-data-testid.md) | Ensure `data-testid` values match a provided regex. | | |
| [no-await-sync-query](docs/rules/no-await-sync-query.md) | Disallow unnecessary `await` for sync queries | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
| [no-debug](docs/rules/no-debug.md) | Disallow the use of `debug` | ![angular-badge][] ![react-badge][] ![vue-badge][] | |
| [no-dom-import](docs/rules/no-dom-import.md) | Disallow importing from DOM Testing Library | ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] |
| [no-get-by-for-checking-element-not-present](docs/rules/no-get-by-for-checking-element-not-present) | Disallow the use of `getBy*` queries when checking elements are not present | | |
| [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than just `getBy*` queries | | |

[build-badge]: https://img.shields.io/travis/testing-library/eslint-plugin-testing-library?style=flat-square
[build-url]: https://travis-ci.org/testing-library/eslint-plugin-testing-library
Expand Down
61 changes: 61 additions & 0 deletions docs/rules/no-get-by-for-checking-element-not-present.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# Disallow the use of `getBy*` queries when checking elements are not present (no-get-by-for-checking-element-not-present)

The (DOM) Testing Library allows to query DOM elements using different types of queries such as `getBy*` and `queryBy*`. Using `getBy*` throws an error in case the element is not found. This is useful when:

- using method like `waitForElement`, which are `async` functions that will wait for the element to be found until a certain timeout, after that the test will fail.
- using `getBy` queries as an assert itself, so if the element is not found the error thrown will work as the check itself within the test.

However, when asserting if an element is not present or waiting for disappearance, using `getBy*` will make the test fail immediately. Instead it is recommended to use `queryBy*`, which does not throw and therefore we can:

- assert element does not exist: `expect(queryByText("Foo")).not.toBeInTheDocument()`
- wait for disappearance: `await waitForElementToBeRemoved(() => queryByText('the mummy'))`

## Rule details

This rule fires whenever:

- `expect` is used to assert element does not exist with `.not.toBeInTheDocument()` or `.toBeNull()` matchers
- `waitForElementToBeRemoved` async util is used to wait for element to be removed from DOM

Examples of **incorrect** code for this rule:

```js
test('some test', () => {
const { getByText } = render(<App />);
expect(getByText('Foo')).not.toBeInTheDocument();
expect(getByText('Foo')).toBeFalsy();
expect(getByText('Foo')).toBeNull();
});
```

```js
test('some test', async () => {
const utils = render(<App />);
await waitForElementToBeRemoved(() => utils.getByText('Foo'));
});
```

Examples of **correct** code for this rule:

```js
test('some test', () => {
const { getByText } = render(<App />);
expect(getByText('Foo')).toBeInTheDocument();
expect(queryByText('Foo')).not.toBeInTheDocument();
expect(queryByText('Foo')).toBeFalsy();
});
```

```js
test('some test', async () => {
const utils = render(<App />);
await waitForElementToBeRemoved(() => utils.queryByText('Foo'));
});
```

## Further Reading

- [Asserting elements are not present](https://testing-library.com/docs/guide-disappearance#asserting-elements-are-not-present)
- [Waiting for disappearance](https://testing-library.com/docs/guide-disappearance#waiting-for-disappearance)
- [jest-dom note about using `getBy` within assertions](https://testing-library.com/docs/ecosystem-jest-dom)
- [Testing Library queries cheatsheet](https://testing-library.com/docs/dom-testing-library/cheatsheet#queries)
81 changes: 0 additions & 81 deletions docs/rules/prefer-expect-query-by.md

This file was deleted.

2 changes: 1 addition & 1 deletion lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const rules = {
'no-await-sync-query': require('./rules/no-await-sync-query'),
'no-debug': require('./rules/no-debug'),
'no-dom-import': require('./rules/no-dom-import'),
'prefer-expect-query-by': require('./rules/prefer-expect-query-by'),
'no-get-by-for-checking-element-not-present': require('./rules/no-get-by-for-checking-element-not-present'),
'prefer-explicit-assert': require('./rules/prefer-explicit-assert'),
};

Expand Down
78 changes: 78 additions & 0 deletions lib/rules/no-get-by-for-checking-element-not-present.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
'use strict';

const { getDocsUrl } = require('../utils');

const falsyMatchers = ['toBeNull', 'toBeFalsy'];

module.exports = {
meta: {
docs: {
category: 'Best Practices',
description:
'Disallow the use of `getBy*` queries when checking elements are not present',
recommended: 'error',
url: getDocsUrl('no-get-by-for-checking-element-not-present'),
},
messages: {
expectQueryBy:
'Use `getBy*` only when checking elements are present, otherwise use `queryBy*`',
},
schema: [],
type: 'suggestion',
fixable: null,
},

create: context => ({
[`Identifier[name=/getBy|getAllBy/]`](node) {
const expectCallNode = findClosestCallNode(node, 'expect');

// expect(getByText("foo"))...
if (expectCallNode) {
const expectStatement = expectCallNode.parent;
const matcher = expectStatement.property.name;

if (matcher === 'not') {
const negatedMatcher = expectStatement.parent.property.name;

if (!falsyMatchers.includes(negatedMatcher)) {
return context.report({
node,
messageId: 'expectQueryBy',
});
}
}

if (falsyMatchers.includes(matcher)) {
return context.report({
node,
messageId: 'expectQueryBy',
});
}
}

const waitCallNode = findClosestCallNode(
node,
'waitForElementToBeRemoved'
);

if (waitCallNode) {
return context.report({
node,
messageId: 'expectQueryBy',
});
}
},
}),
};

function findClosestCallNode(node, name) {
if (!node.parent) {
return false;
}

if (node.type === 'CallExpression' && node.callee.name === name) {
return node;
} else {
return findClosestCallNode(node.parent, name);
}
}
102 changes: 0 additions & 102 deletions lib/rules/prefer-expect-query-by.js

This file was deleted.

0 comments on commit 1aa9238

Please sign in to comment.