From 34dfde6812574d01136bab99da5478cfd268e959 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Thu, 23 Jan 2020 18:25:45 +0100 Subject: [PATCH 1/6] docs(await-async-utils): rule details --- docs/rules/await-async-utils.md | 66 +++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 docs/rules/await-async-utils.md diff --git a/docs/rules/await-async-utils.md b/docs/rules/await-async-utils.md new file mode 100644 index 00000000..90d44535 --- /dev/null +++ b/docs/rules/await-async-utils.md @@ -0,0 +1,66 @@ +# Enforce async utils to be awaited properly (await-async-utils) + +Ensure that promises returned by async utils are handled properly. + +## Rule Details + +Testing library provides several utilities for dealing with asynchronous code. These are useful to wait for an element until certain criteria or situation happens. The available async utils are: + +- `wait` +- `waitForElement` +- `waitForDomChange` +- `waitForElementToBeRemoved` + +This rule aims to prevent users from forgetting to handle the returned promise from those async utils, which could lead to unexpected errors in the tests execution. The promises can be handled by using either `await` operator or `then` method. + +Examples of **incorrect** code for this rule: + +```js +test('something incorrectly', async () => { + // ... + wait(() => getByLabelText('email')); + + const [usernameElement, passwordElement] = waitForElement( + () => [ + getByLabelText(container, 'username'), + getByLabelText(container, 'password'), + ], + { container } + ); + + waitForDomChange(() => { return { container } }); + + waitForElementToBeRemoved(() => document.querySelector('div.getOuttaHere')); + // ... +}); +``` + +Examples of **correct** code for this rule: + +```js +test('something correctly', async () => { + // ... + await wait(() => getByLabelText('email')); + + const [usernameElement, passwordElement] = await waitForElement( + () => [ + getByLabelText(container, 'username'), + getByLabelText(container, 'password'), + ], + { container } + ); + + waitForDomChange(() => { return { container } }) + .then(() => console.log('DOM changed!')) + .catch(err => console.log(`Error you need to deal with: ${err}`)); + + waitForElementToBeRemoved( + () => document.querySelector('div.getOuttaHere') + ).then(() => console.log('Element no longer in DOM')); + // ... +}); +``` + +## Further Reading + +- [Async Utilities](https://testing-library.com/docs/dom-testing-library/api-async) From 5caa05c8e063286d51ae9c87fe2bbcba5fd96bdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Fri, 24 Jan 2020 16:43:22 +0100 Subject: [PATCH 2/6] feat: new rule await-async-utils --- README.md | 3 +- docs/rules/await-async-utils.md | 28 ++++-- lib/index.js | 2 + lib/rules/await-async-utils.js | 103 +++++++++++++++++++ lib/utils.js | 8 ++ tests/__snapshots__/index.test.js.snap | 4 + tests/lib/rules/await-async-utils.js | 133 +++++++++++++++++++++++++ 7 files changed, 269 insertions(+), 12 deletions(-) create mode 100644 lib/rules/await-async-utils.js create mode 100644 tests/lib/rules/await-async-utils.js diff --git a/README.md b/README.md index a1aa0bdb..fff1c2ff 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,7 @@ -[![All Contributors](https://img.shields.io/badge/all_contributors-4-orange.svg?style=flat-square)](#contributors) +[![All Contributors](https://img.shields.io/badge/all_contributors-4-orange.svg?style=flat-square)](#contributors-) @@ -137,6 +137,7 @@ To enable this configuration use the `extends` property in your | 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][] | | diff --git a/docs/rules/await-async-utils.md b/docs/rules/await-async-utils.md index 90d44535..b83a1d38 100644 --- a/docs/rules/await-async-utils.md +++ b/docs/rules/await-async-utils.md @@ -19,7 +19,7 @@ Examples of **incorrect** code for this rule: test('something incorrectly', async () => { // ... wait(() => getByLabelText('email')); - + const [usernameElement, passwordElement] = waitForElement( () => [ getByLabelText(container, 'username'), @@ -27,9 +27,11 @@ test('something incorrectly', async () => { ], { container } ); - - waitForDomChange(() => { return { container } }); - + + waitForDomChange(() => { + return { container }; + }); + waitForElementToBeRemoved(() => document.querySelector('div.getOuttaHere')); // ... }); @@ -40,8 +42,9 @@ Examples of **correct** code for this rule: ```js test('something correctly', async () => { // ... + // `await` operator is correct await wait(() => getByLabelText('email')); - + const [usernameElement, passwordElement] = await waitForElement( () => [ getByLabelText(container, 'username'), @@ -49,14 +52,17 @@ test('something correctly', async () => { ], { container } ); - - waitForDomChange(() => { return { container } }) + + // `then` chained method is correct + waitForDomChange(() => { + return { container }; + }) .then(() => console.log('DOM changed!')) .catch(err => console.log(`Error you need to deal with: ${err}`)); - - waitForElementToBeRemoved( - () => document.querySelector('div.getOuttaHere') - ).then(() => console.log('Element no longer in DOM')); + + // return the promise within a function is correct too! + const makeCustomWait = () => + waitForElementToBeRemoved(() => document.querySelector('div.getOuttaHere')); // ... }); ``` diff --git a/lib/index.js b/lib/index.js index 6c25b8b2..c8f17612 100644 --- a/lib/index.js +++ b/lib/index.js @@ -2,6 +2,7 @@ const rules = { 'await-async-query': require('./rules/await-async-query'), + 'await-async-utils': require('./rules/await-async-utils'), 'await-fire-event': require('./rules/await-fire-event'), 'consistent-data-testid': require('./rules/consistent-data-testid'), 'no-await-sync-query': require('./rules/no-await-sync-query'), @@ -13,6 +14,7 @@ const rules = { const recommendedRules = { 'testing-library/await-async-query': 'error', + 'testing-library/await-async-utils': 'error', 'testing-library/no-await-sync-query': 'error', }; diff --git a/lib/rules/await-async-utils.js b/lib/rules/await-async-utils.js new file mode 100644 index 00000000..b2bd1a67 --- /dev/null +++ b/lib/rules/await-async-utils.js @@ -0,0 +1,103 @@ +'use strict'; + +const { getDocsUrl, ASYNC_UTILS } = require('../utils'); + +const VALID_PARENTS = [ + 'AwaitExpression', + 'ArrowFunctionExpression', + 'ReturnStatement', +]; + +const ASYNC_UTILS_REGEXP = new RegExp(`^(${ASYNC_UTILS.join('|')})$`); + +module.exports = { + meta: { + type: 'problem', + docs: { + description: 'Enforce async utils to be awaited properly', + category: 'Best Practices', + recommended: true, + url: getDocsUrl('await-async-utils'), + }, + messages: { + awaitAsyncUtil: 'Promise returned from `{{ name }}` must be handled', + }, + fixable: null, + schema: [], + }, + + create: function(context) { + const testingLibraryUtilUsage = []; + return { + [`CallExpression > Identifier[name=${ASYNC_UTILS_REGEXP}]`](node) { + if (!isAwaited(node.parent.parent) && !isPromiseResolved(node)) { + testingLibraryUtilUsage.push(node); + } + }, + 'Program:exit'() { + testingLibraryUtilUsage.forEach(node => { + const variableDeclaratorParent = node.parent.parent; + + const references = + (variableDeclaratorParent.type === 'VariableDeclarator' && + context + .getDeclaredVariables(variableDeclaratorParent)[0] + .references.slice(1)) || + []; + + if ( + references && + references.length === 0 && + !isAwaited(node.parent.parent) && + !isPromiseResolved(node) + ) { + context.report({ + node, + messageId: 'awaitAsyncUtil', + data: { + name: node.name, + }, + }); + } else { + for (const reference of references) { + const referenceNode = reference.identifier; + if ( + !isAwaited(referenceNode.parent) && + !isPromiseResolved(referenceNode) + ) { + context.report({ + node, + messageId: 'awaitAsyncUtil', + data: { + name: node.name, + }, + }); + + break; + } + } + } + }); + }, + }; + }, +}; + +function isAwaited(node) { + return VALID_PARENTS.includes(node.type); +} + +function isPromiseResolved(node) { + const parent = node.parent; + + const hasAThenProperty = node => + node.type === 'MemberExpression' && node.property.name === 'then'; + + // findByText("foo").then(...) + if (parent.type === 'CallExpression') { + return hasAThenProperty(parent.parent); + } + + // promise.then(...) + return hasAThenProperty(parent); +} diff --git a/lib/utils.js b/lib/utils.js index 55ec08d7..8f477be4 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -48,6 +48,13 @@ const ALL_QUERIES_COMBINATIONS = [ ASYNC_QUERIES_COMBINATIONS, ]; +const ASYNC_UTILS = [ + 'wait', + 'waitForElement', + 'waitForDomChange', + 'waitForElementToBeRemoved', +]; + module.exports = { getDocsUrl, SYNC_QUERIES_VARIANTS, @@ -57,4 +64,5 @@ module.exports = { SYNC_QUERIES_COMBINATIONS, ASYNC_QUERIES_COMBINATIONS, ALL_QUERIES_COMBINATIONS, + ASYNC_UTILS, }; diff --git a/tests/__snapshots__/index.test.js.snap b/tests/__snapshots__/index.test.js.snap index b83bb366..86bf0861 100644 --- a/tests/__snapshots__/index.test.js.snap +++ b/tests/__snapshots__/index.test.js.snap @@ -7,6 +7,7 @@ Object { ], "rules": Object { "testing-library/await-async-query": "error", + "testing-library/await-async-utils": "error", "testing-library/no-await-sync-query": "error", "testing-library/no-debug": "warn", "testing-library/no-dom-import": Array [ @@ -24,6 +25,7 @@ Object { ], "rules": Object { "testing-library/await-async-query": "error", + "testing-library/await-async-utils": "error", "testing-library/no-await-sync-query": "error", "testing-library/no-debug": "warn", "testing-library/no-dom-import": Array [ @@ -41,6 +43,7 @@ Object { ], "rules": Object { "testing-library/await-async-query": "error", + "testing-library/await-async-utils": "error", "testing-library/no-await-sync-query": "error", }, } @@ -53,6 +56,7 @@ Object { ], "rules": Object { "testing-library/await-async-query": "error", + "testing-library/await-async-utils": "error", "testing-library/await-fire-event": "error", "testing-library/no-await-sync-query": "error", "testing-library/no-debug": "warn", diff --git a/tests/lib/rules/await-async-utils.js b/tests/lib/rules/await-async-utils.js new file mode 100644 index 00000000..f2ee36e0 --- /dev/null +++ b/tests/lib/rules/await-async-utils.js @@ -0,0 +1,133 @@ +'use strict'; + +const rule = require('../../../lib/rules/await-async-utils'); +const { ASYNC_UTILS } = require('../../../lib/utils'); +const RuleTester = require('eslint').RuleTester; + +const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2018 } }); + +ruleTester.run('await-async-utils', rule, { + valid: [ + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + test('${asyncUtil} util directly waited with await operator is valid', async () => { + doSomethingElse(); + await ${asyncUtil}(() => getByLabelText('email')); + }); + `, + })), + + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + test('${asyncUtil} util promise saved in var and waited with await operator is valid', async () => { + doSomethingElse(); + const aPromise = ${asyncUtil}(() => getByLabelText('email')); + await aPromise; + }); + `, + })), + + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + test('${asyncUtil} util directly chained with then is valid', () => { + doSomethingElse(); + ${asyncUtil}(() => getByLabelText('email')).then(() => { console.log('done') }); + }); + `, + })), + + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + test('${asyncUtil} util promise saved in var and chained with then is valid', () => { + doSomethingElse(); + const aPromise = ${asyncUtil}(() => getByLabelText('email')); + aPromise.then(() => { console.log('done') }); + }); + `, + })), + + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + test('${asyncUtil} util directly returned in arrow function is valid', async () => { + const makeCustomWait = () => + ${asyncUtil}(() => + document.querySelector('div.getOuttaHere') + ); + }); + `, + })), + + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + test('${asyncUtil} util explicitly returned in arrow function is valid', async () => { + const makeCustomWait = () => { + return ${asyncUtil}(() => + document.querySelector('div.getOuttaHere') + ); + }; + }); + `, + })), + + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + test('${asyncUtil} util returned in regular function is valid', async () => { + function makeCustomWait() { + return ${asyncUtil}(() => + document.querySelector('div.getOuttaHere') + ); + } + }); + `, + })), + + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + test('${asyncUtil} util promise saved in var and returned in function is valid', async () => { + const makeCustomWait = () => { + const aPromise = ${asyncUtil}(() => + document.querySelector('div.getOuttaHere') + ); + + doSomethingElse(); + + return aPromise; + }; + }); + `, + })), + ], + invalid: [ + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + test('${asyncUtil} util not waited', () => { + doSomethingElse(); + ${asyncUtil}(() => getByLabelText('email')); + }); + `, + errors: [{ line: 4, messageId: 'awaitAsyncUtil' }], + })), + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + test('${asyncUtil} util promise saved not waited', () => { + doSomethingElse(); + const aPromise = ${asyncUtil}(() => getByLabelText('email')); + }); + `, + errors: [{ line: 4, column: 28, messageId: 'awaitAsyncUtil' }], + })), + ...ASYNC_UTILS.map(asyncUtil => ({ + code: ` + test('several ${asyncUtil} utils not waited', () => { + ${asyncUtil}(() => getByLabelText('username')); + doSomethingElse(); + ${asyncUtil}(() => getByLabelText('email')); + }); + `, + errors: [ + { line: 3, messageId: 'awaitAsyncUtil' }, + { line: 5, messageId: 'awaitAsyncUtil' }, + ], + })), + ], +}); From 0b2c917d5e031d5e92f29ef084b46413e390fb75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Fri, 24 Jan 2020 16:44:45 +0100 Subject: [PATCH 3/6] docs: update util comment --- lib/rules/await-async-utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/await-async-utils.js b/lib/rules/await-async-utils.js index b2bd1a67..881783e6 100644 --- a/lib/rules/await-async-utils.js +++ b/lib/rules/await-async-utils.js @@ -93,7 +93,7 @@ function isPromiseResolved(node) { const hasAThenProperty = node => node.type === 'MemberExpression' && node.property.name === 'then'; - // findByText("foo").then(...) + // wait(...).then(...) if (parent.type === 'CallExpression') { return hasAThenProperty(parent.parent); } From 3784fa6319b39dafeef55ee924e5168228b95a72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Fri, 24 Jan 2020 16:53:36 +0100 Subject: [PATCH 4/6] test(await-async-utils): add extra test --- tests/lib/rules/await-async-utils.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/lib/rules/await-async-utils.js b/tests/lib/rules/await-async-utils.js index f2ee36e0..b76ed175 100644 --- a/tests/lib/rules/await-async-utils.js +++ b/tests/lib/rules/await-async-utils.js @@ -96,6 +96,14 @@ ruleTester.run('await-async-utils', rule, { }); `, })), + { + code: ` + test('util not related to testing library is valid', async () => { + doSomethingElse(); + waitNotRelatedToTestingLibrary(); + }); + `, + }, ], invalid: [ ...ASYNC_UTILS.map(asyncUtil => ({ From 61779754c8a56ba4a94e436472e5ac48b2d70ddb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Fri, 24 Jan 2020 17:21:59 +0100 Subject: [PATCH 5/6] test(await-async-utils): increase coverage --- tests/lib/rules/await-async-utils.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/lib/rules/await-async-utils.js b/tests/lib/rules/await-async-utils.js index b76ed175..986ce047 100644 --- a/tests/lib/rules/await-async-utils.js +++ b/tests/lib/rules/await-async-utils.js @@ -127,14 +127,14 @@ ruleTester.run('await-async-utils', rule, { ...ASYNC_UTILS.map(asyncUtil => ({ code: ` test('several ${asyncUtil} utils not waited', () => { - ${asyncUtil}(() => getByLabelText('username')); - doSomethingElse(); + const aPromise = ${asyncUtil}(() => getByLabelText('username')); + doSomethingElse(aPromise); ${asyncUtil}(() => getByLabelText('email')); }); `, errors: [ - { line: 3, messageId: 'awaitAsyncUtil' }, - { line: 5, messageId: 'awaitAsyncUtil' }, + { line: 3, column: 28, messageId: 'awaitAsyncUtil' }, + { line: 5, column: 11, messageId: 'awaitAsyncUtil' }, ], })), ], From 9f1005ad624871fda730cc1239f34c8f1df81703 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltra=CC=81n=20Alarco=CC=81n?= Date: Mon, 27 Jan 2020 10:15:03 +0100 Subject: [PATCH 6/6] refactor(await-async-utils): move function definition outside --- lib/rules/await-async-utils.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/rules/await-async-utils.js b/lib/rules/await-async-utils.js index 881783e6..1ddc9936 100644 --- a/lib/rules/await-async-utils.js +++ b/lib/rules/await-async-utils.js @@ -87,12 +87,12 @@ function isAwaited(node) { return VALID_PARENTS.includes(node.type); } +const hasAThenProperty = node => + node.type === 'MemberExpression' && node.property.name === 'then'; + function isPromiseResolved(node) { const parent = node.parent; - const hasAThenProperty = node => - node.type === 'MemberExpression' && node.property.name === 'then'; - // wait(...).then(...) if (parent.type === 'CallExpression') { return hasAThenProperty(parent.parent);