Repository: ember-cli/eslint-plugin-ember (js)

Issue:
New rule: no-settled-after-test-helper
This rule should lint against using the `settled()` test helper directly after test helpers that already call `settled()` internally. Having to use this pattern is usually a sign of a deeper root cause issue that should be fixed.

**Bad**

```js
await click('.foo');
await settled();
```

```js
await fillIn('.foo');
await settled();
```

**Good**

```js
await waitFor('.foo');
await settled();
```

/cc @rwjblue 

Pull request:
Add new rule `no-settled-after-test-helper`
Resolves #984 

Patch:
diff --git a/README.md b/README.md
index 19e8f05c..9da1d9ca 100644
--- a/README.md
+++ b/README.md
@@ -188,6 +188,7 @@ Rules are grouped by category to help you understand their purpose. Each rule ha
 | :white_check_mark: | [no-pause-test](./docs/rules/no-pause-test.md) | disallow usage of the `pauseTest` helper in tests |
 |  | [no-replace-test-comments](./docs/rules/no-replace-test-comments.md) | disallow 'Replace this with your real tests' comments in test files |
 | :white_check_mark: | [no-restricted-resolver-tests](./docs/rules/no-restricted-resolver-tests.md) | disallow the use of patterns that use the restricted resolver in tests |
+| :wrench: | [no-settled-after-test-helper](./docs/rules/no-settled-after-test-helper.md) | disallow usage of `await settled()` right after test helper that calls it internally |
 | :white_check_mark: | [no-test-and-then](./docs/rules/no-test-and-then.md) | disallow usage of the `andThen` test wait helper |
 | :white_check_mark: | [no-test-import-export](./docs/rules/no-test-import-export.md) | disallow importing of "-test.js" in a test file and exporting from a test file |
 | :white_check_mark: | [no-test-module-for](./docs/rules/no-test-module-for.md) | disallow usage of `moduleFor`, `moduleForComponent`, etc |
diff --git a/lib/index.js b/lib/index.js
index 25af9fff..f3c6c2ac 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -52,6 +52,7 @@ module.exports = {
     'no-replace-test-comments': require('./rules/no-replace-test-comments'),
     'no-restricted-resolver-tests': require('./rules/no-restricted-resolver-tests'),
     'no-restricted-service-injections': require('./rules/no-restricted-service-injections'),
+    'no-settled-after-test-helper': require('./rules/no-settled-after-test-helper'),
     'no-side-effects': require('./rules/no-side-effects'),
     'no-test-and-then': require('./rules/no-test-and-then'),
     'no-test-import-export': require('./rules/no-test-import-export'),


Test patch:
diff --git a/docs/rules/no-settled-after-test-helper.md b/docs/rules/no-settled-after-test-helper.md
new file mode 100644
index 00000000..ab166075
--- /dev/null
+++ b/docs/rules/no-settled-after-test-helper.md
@@ -0,0 +1,64 @@
+# no-settled-after-test-helper
+
+:wrench: The `--fix` option on the [command line](https://eslint.org/docs/user-guide/command-line-interface#fixing-problems) can automatically fix some of the problems reported by this rule.
+
+Most of the test helper functions in
+[`@ember/test-helpers`](https://github.com/emberjs/ember-test-helpers) call
+`settled()` internally, which causes the test to wait until any effects of e.g.
+the `click()` operation have settled. This means calling `await settled()` after
+calling one of these test helpers is redundant and should not be necessary.
+
+In some cases this pattern is mistakenly used to "wait a little more", which
+usually indicated a deeper root cause issue, like a race condition or missing
+test waiter somewhere. This pattern only works sometimes because it causes the
+test to continue on the next tick of the JS runloop, instead of continuing
+directly. It is highly recommended to fix the root cause in these cases instead
+of relying on such brittle workarounds.
+
+## Rule Details
+
+This rule warns about cases where `await settled()` is used right after a call
+to a test helper function that already calls `settled()` internally (or
+`settled()` itself).
+
+## Examples
+
+Examples of **incorrect** code for this rule:
+
+```js
+test('...', async function (assert) {
+  await click('.foo');
+  await settled();
+});
+```
+
+```js
+test('...', async function (assert) {
+  await fillIn('.foo');
+  await settled();
+});
+```
+
+```js
+test('...', async function (assert) {
+  await settled();
+  await settled();
+});
+```
+
+Examples of **correct** code for this rule:
+
+```js
+test('...', async function (assert) {
+  await waitFor('.foo');
+  await settled();
+});
+```
+
+## Migration
+
+* Have a look at the [ember-test-waiters](https://github.com/emberjs/ember-test-waiters) project
+
+## References
+
+* [`settled()`](https://github.com/emberjs/ember-test-helpers/blob/master/API.md#settled)
diff --git a/lib/rules/no-settled-after-test-helper.js b/lib/rules/no-settled-after-test-helper.js
new file mode 100644
index 00000000..4ab035e7
--- /dev/null
+++ b/lib/rules/no-settled-after-test-helper.js
@@ -0,0 +1,125 @@
+'use strict';
+
+const ERROR_MESSAGE =
+  'Do not call `settled()` right after a test helper that already calls it internally.';
+
+const SETTLING_TEST_HELPERS = new Set([
+  'blur',
+  'clearRender',
+  'click',
+  'doubleClick',
+  'fillIn',
+  'focus',
+  'render',
+  'settled',
+  'tap',
+  'triggerEvent',
+  'triggerKeyEvent',
+  'typeIn',
+  'visit',
+]);
+
+module.exports = {
+  meta: {
+    type: 'suggestion',
+    docs: {
+      description:
+        'disallow usage of `await settled()` right after test helper that calls it internally',
+      category: 'Testing',
+      url:
+        'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-settled-after-test-helper.md',
+    },
+    fixable: 'code',
+    schema: [],
+  },
+
+  ERROR_MESSAGE,
+
+  create(context) {
+    const settlingTestHelpers = new Set();
+
+    return {
+      ImportDeclaration(node) {
+        const { source, specifiers } = node;
+        if (source.type !== 'Literal' || source.value !== '@ember/test-helpers') {
+          return;
+        }
+
+        for (const specifier of specifiers) {
+          if (specifier.type !== 'ImportSpecifier') {
+            continue;
+          }
+
+          const { imported, local } = specifier;
+          if (imported.type !== 'Identifier' || local.type !== 'Identifier') {
+            continue;
+          }
+
+          if (SETTLING_TEST_HELPERS.has(imported.name)) {
+            settlingTestHelpers.add(local.name);
+          }
+        }
+      },
+
+      ExpressionStatement(node) {
+        if (!isAwaitSettled(node)) {
+          return;
+        }
+
+        const { parent } = node;
+        /* istanbul ignore next */
+        if (!Array.isArray(parent.body)) {
+          return;
+        }
+
+        const index = parent.body.indexOf(node);
+        if (index < 1) {
+          return;
+        }
+
+        const prevStatement = parent.body[index - 1];
+        if (!isAwaitSettlingTestHelper(prevStatement, settlingTestHelpers)) {
+          return;
+        }
+
+        context.report({
+          node,
+          message: ERROR_MESSAGE,
+          fix(fixer) {
+            return fixer.remove(node);
+          },
+        });
+      },
+    };
+  },
+};
+
+function isAwaitSettled(node) {
+  const { expression } = node;
+  if (expression.type !== 'AwaitExpression') {
+    return false;
+  }
+
+  const { argument } = expression;
+  if (argument.type !== 'CallExpression') {
+    return false;
+  }
+
+  const { callee } = argument;
+  return callee.type === 'Identifier' && callee.name === 'settled';
+}
+
+function isAwaitSettlingTestHelper(node, settlingTestHelpers) {
+  const { expression } = node;
+  if (expression.type !== 'AwaitExpression') {
+    return false;
+  }
+
+  const { argument } = expression;
+  if (argument.type !== 'CallExpression') {
+    return false;
+  }
+
+  const { callee } = argument;
+  return callee.type === 'Identifier' && settlingTestHelpers.has(callee.name);
+}
diff --git a/tests/lib/rules/no-settled-after-test-helper.js b/tests/lib/rules/no-settled-after-test-helper.js
new file mode 100644
index 00000000..3285480c
--- /dev/null
+++ b/tests/lib/rules/no-settled-after-test-helper.js
@@ -0,0 +1,145 @@
+const RuleTester = require('eslint').RuleTester;
+const rule = require('../../../lib/rules/no-settled-after-test-helper');
+
+const { ERROR_MESSAGE } = rule;
+
+const ruleTester = new RuleTester({
+  parser: require.resolve('babel-eslint'),
+  parserOptions: {
+    ecmaVersion: 2015,
+    sourceType: 'module',
+  },
+});
+
+ruleTester.run('no-settled-after-test-helper', rule, {
+  valid: [
+    `
+      import { waitFor, settled } from '@ember/test-helpers';
+
+      async () => {
+        await waitFor('.foo');
+        await settled();
+      }
+    `,
+    `
+      import { waitFor, settled } from '@ember/test-helpers';
+
+      async () => {
+        await waitFor('.foo');
+        await settled;
+      }
+    `,
+    `
+      import { settled } from '@ember/test-helpers';
+
+      async () => {
+        await settled();
+      }
+    `,
+    `
+      import { settled } from '@ember/test-helpers';
+
+      async () => {
+        this.set('foo', 42);
+        await settled();
+      }
+    `,
+    `
+      import { settled } from '@ember/test-helpers';
+
+      async () => {
+        let promise = waitFor('.foo');
+        await promise;
+        await settled();
+      }
+    `,
+    `
+      import { settled } from '@ember/test-helpers';
+
+      function click() {}
+
+      async () => {
+        await click('.foo');
+        await settled();
+      }
+    `,
+  ],
+  invalid: [
+    {
+      code: `
+        import { click, settled } from '@ember/test-helpers';
+
+        async () => {
+          await click('.foo');
+          await settled();
+        }
+      `,
+      output: `
+        import { click, settled } from '@ember/test-helpers';
+
+        async () => {
+          await click('.foo');
+\u0020\u0020\u0020\u0020\u0020\u0020\u0020\u0020\u0020\u0020
+        }
+      `,
+      errors: [{ message: ERROR_MESSAGE, line: 6, column: 11, endLine: 6, endColumn: 27 }],
+    },
+    {
+      code: `
+        import { fillIn, settled } from '@ember/test-helpers';
+
+        async () => {
+          await fillIn('.foo');
+          await settled();
+        }
+      `,
+      output: `
+        import { fillIn, settled } from '@ember/test-helpers';
+
+        async () => {
+          await fillIn('.foo');
+\u0020\u0020\u0020\u0020\u0020\u0020\u0020\u0020\u0020\u0020
+        }
+      `,
+      errors: [{ message: ERROR_MESSAGE, line: 6, column: 11, endLine: 6, endColumn: 27 }],
+    },
+    {
+      code: `
+        import { settled } from '@ember/test-helpers';
+
+        async () => {
+          await settled();
+          await settled();
+        }
+      `,
+      output: `
+        import { settled } from '@ember/test-helpers';
+
+        async () => {
+          await settled();
+\u0020\u0020\u0020\u0020\u0020\u0020\u0020\u0020\u0020\u0020
+        }
+      `,
+      errors: [{ message: ERROR_MESSAGE, line: 6, column: 11, endLine: 6, endColumn: 27 }],
+    },
+    {
+      code: `
+        import { click as cliiiick, settled } from '@ember/test-helpers';
+
+        async () => {
+          await cliiiick('.foo');
+          await settled();
+        }
+      `,
+      output: `
+        import { click as cliiiick, settled } from '@ember/test-helpers';
+
+        async () => {
+          await cliiiick('.foo');
+\u0020\u0020\u0020\u0020\u0020\u0020\u0020\u0020\u0020\u0020
+        }
+      `,
+      errors: [{ message: ERROR_MESSAGE, line: 6, column: 11, endLine: 6, endColumn: 27 }],
+    },
+  ],
+});