Skip to content

Commit

Permalink
feat: create prefer-expect-resolves rule
Browse files Browse the repository at this point in the history
  • Loading branch information
G-Rath committed Apr 11, 2021
1 parent 9c31a8d commit b34f133
Show file tree
Hide file tree
Showing 6 changed files with 188 additions and 43 deletions.
85 changes: 43 additions & 42 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,48 +130,49 @@ installations requiring long-term consistency.

<!-- begin base rules list -->

| Rule | Description | Configurations | Fixable |
| ---------------------------------------------------------------------------- | --------------------------------------------------------------- | ---------------- | ------------ |
| [consistent-test-it](docs/rules/consistent-test-it.md) | Have control over `test` and `it` usages | | ![fixable][] |
| [expect-expect](docs/rules/expect-expect.md) | Enforce assertion to be made in a test body | ![recommended][] | |
| [lowercase-name](docs/rules/lowercase-name.md) | Enforce lowercase test names | | ![fixable][] |
| [no-alias-methods](docs/rules/no-alias-methods.md) | Disallow alias methods | ![style][] | ![fixable][] |
| [no-commented-out-tests](docs/rules/no-commented-out-tests.md) | Disallow commented out tests | ![recommended][] | |
| [no-conditional-expect](docs/rules/no-conditional-expect.md) | Prevent calling `expect` conditionally | ![recommended][] | |
| [no-deprecated-functions](docs/rules/no-deprecated-functions.md) | Disallow use of deprecated functions | ![recommended][] | ![fixable][] |
| [no-disabled-tests](docs/rules/no-disabled-tests.md) | Disallow disabled tests | ![recommended][] | |
| [no-done-callback](docs/rules/no-done-callback.md) | Avoid using a callback in asynchronous tests and hooks | ![recommended][] | ![suggest][] |
| [no-duplicate-hooks](docs/rules/no-duplicate-hooks.md) | Disallow duplicate setup and teardown hooks | | |
| [no-export](docs/rules/no-export.md) | Disallow using `exports` in files containing tests | ![recommended][] | |
| [no-focused-tests](docs/rules/no-focused-tests.md) | Disallow focused tests | ![recommended][] | ![suggest][] |
| [no-hooks](docs/rules/no-hooks.md) | Disallow setup and teardown hooks | | |
| [no-identical-title](docs/rules/no-identical-title.md) | Disallow identical titles | ![recommended][] | |
| [no-if](docs/rules/no-if.md) | Disallow conditional logic | | |
| [no-interpolation-in-snapshots](docs/rules/no-interpolation-in-snapshots.md) | Disallow string interpolation inside snapshots | ![recommended][] | |
| [no-jasmine-globals](docs/rules/no-jasmine-globals.md) | Disallow Jasmine globals | ![recommended][] | ![fixable][] |
| [no-jest-import](docs/rules/no-jest-import.md) | Disallow importing Jest | ![recommended][] | |
| [no-large-snapshots](docs/rules/no-large-snapshots.md) | disallow large snapshots | | |
| [no-mocks-import](docs/rules/no-mocks-import.md) | Disallow manually importing from `__mocks__` | ![recommended][] | |
| [no-restricted-matchers](docs/rules/no-restricted-matchers.md) | Disallow specific matchers & modifiers | | |
| [no-standalone-expect](docs/rules/no-standalone-expect.md) | Disallow using `expect` outside of `it` or `test` blocks | ![recommended][] | |
| [no-test-prefixes](docs/rules/no-test-prefixes.md) | Use `.only` and `.skip` over `f` and `x` | ![recommended][] | ![fixable][] |
| [no-test-return-statement](docs/rules/no-test-return-statement.md) | Disallow explicitly returning from tests | | |
| [prefer-called-with](docs/rules/prefer-called-with.md) | Suggest using `toBeCalledWith()` or `toHaveBeenCalledWith()` | | |
| [prefer-expect-assertions](docs/rules/prefer-expect-assertions.md) | Suggest using `expect.assertions()` OR `expect.hasAssertions()` | | ![suggest][] |
| [prefer-hooks-on-top](docs/rules/prefer-hooks-on-top.md) | Suggest having hooks before any test cases | | |
| [prefer-spy-on](docs/rules/prefer-spy-on.md) | Suggest using `jest.spyOn()` | | ![fixable][] |
| [prefer-strict-equal](docs/rules/prefer-strict-equal.md) | Suggest using `toStrictEqual()` | | ![suggest][] |
| [prefer-to-be-null](docs/rules/prefer-to-be-null.md) | Suggest using `toBeNull()` | ![style][] | ![fixable][] |
| [prefer-to-be-undefined](docs/rules/prefer-to-be-undefined.md) | Suggest using `toBeUndefined()` | ![style][] | ![fixable][] |
| [prefer-to-contain](docs/rules/prefer-to-contain.md) | Suggest using `toContain()` | ![style][] | ![fixable][] |
| [prefer-to-have-length](docs/rules/prefer-to-have-length.md) | Suggest using `toHaveLength()` | ![style][] | ![fixable][] |
| [prefer-todo](docs/rules/prefer-todo.md) | Suggest using `test.todo` | | ![fixable][] |
| [require-to-throw-message](docs/rules/require-to-throw-message.md) | Require a message for `toThrow()` | | |
| [require-top-level-describe](docs/rules/require-top-level-describe.md) | Require test cases and hooks to be inside a `describe` block | | |
| [valid-describe](docs/rules/valid-describe.md) | Enforce valid `describe()` callback | ![recommended][] | |
| [valid-expect](docs/rules/valid-expect.md) | Enforce valid `expect()` usage | ![recommended][] | |
| [valid-expect-in-promise](docs/rules/valid-expect-in-promise.md) | Enforce having return statement when testing with promises | ![recommended][] | |
| [valid-title](docs/rules/valid-title.md) | Enforce valid titles | ![recommended][] | ![fixable][] |
| Rule | Description | Configurations | Fixable |
| ---------------------------------------------------------------------------- | ------------------------------------------------------------------- | ---------------- | ------------ |
| [consistent-test-it](docs/rules/consistent-test-it.md) | Have control over `test` and `it` usages | | ![fixable][] |
| [expect-expect](docs/rules/expect-expect.md) | Enforce assertion to be made in a test body | ![recommended][] | |
| [lowercase-name](docs/rules/lowercase-name.md) | Enforce lowercase test names | | ![fixable][] |
| [no-alias-methods](docs/rules/no-alias-methods.md) | Disallow alias methods | ![style][] | ![fixable][] |
| [no-commented-out-tests](docs/rules/no-commented-out-tests.md) | Disallow commented out tests | ![recommended][] | |
| [no-conditional-expect](docs/rules/no-conditional-expect.md) | Prevent calling `expect` conditionally | ![recommended][] | |
| [no-deprecated-functions](docs/rules/no-deprecated-functions.md) | Disallow use of deprecated functions | ![recommended][] | ![fixable][] |
| [no-disabled-tests](docs/rules/no-disabled-tests.md) | Disallow disabled tests | ![recommended][] | |
| [no-done-callback](docs/rules/no-done-callback.md) | Avoid using a callback in asynchronous tests and hooks | ![recommended][] | ![suggest][] |
| [no-duplicate-hooks](docs/rules/no-duplicate-hooks.md) | Disallow duplicate setup and teardown hooks | | |
| [no-export](docs/rules/no-export.md) | Disallow using `exports` in files containing tests | ![recommended][] | |
| [no-focused-tests](docs/rules/no-focused-tests.md) | Disallow focused tests | ![recommended][] | ![suggest][] |
| [no-hooks](docs/rules/no-hooks.md) | Disallow setup and teardown hooks | | |
| [no-identical-title](docs/rules/no-identical-title.md) | Disallow identical titles | ![recommended][] | |
| [no-if](docs/rules/no-if.md) | Disallow conditional logic | | |
| [no-interpolation-in-snapshots](docs/rules/no-interpolation-in-snapshots.md) | Disallow string interpolation inside snapshots | ![recommended][] | |
| [no-jasmine-globals](docs/rules/no-jasmine-globals.md) | Disallow Jasmine globals | ![recommended][] | ![fixable][] |
| [no-jest-import](docs/rules/no-jest-import.md) | Disallow importing Jest | ![recommended][] | |
| [no-large-snapshots](docs/rules/no-large-snapshots.md) | disallow large snapshots | | |
| [no-mocks-import](docs/rules/no-mocks-import.md) | Disallow manually importing from `__mocks__` | ![recommended][] | |
| [no-restricted-matchers](docs/rules/no-restricted-matchers.md) | Disallow specific matchers & modifiers | | |
| [no-standalone-expect](docs/rules/no-standalone-expect.md) | Disallow using `expect` outside of `it` or `test` blocks | ![recommended][] | |
| [no-test-prefixes](docs/rules/no-test-prefixes.md) | Use `.only` and `.skip` over `f` and `x` | ![recommended][] | ![fixable][] |
| [no-test-return-statement](docs/rules/no-test-return-statement.md) | Disallow explicitly returning from tests | | |
| [prefer-called-with](docs/rules/prefer-called-with.md) | Suggest using `toBeCalledWith()` or `toHaveBeenCalledWith()` | | |
| [prefer-expect-assertions](docs/rules/prefer-expect-assertions.md) | Suggest using `expect.assertions()` OR `expect.hasAssertions()` | | ![suggest][] |
| [prefer-expect-resolves](docs/rules/prefer-expect-resolves.md) | Prefer `await expect(...).resolves` over `expect(await ...)` syntax | | |
| [prefer-hooks-on-top](docs/rules/prefer-hooks-on-top.md) | Suggest having hooks before any test cases | | |
| [prefer-spy-on](docs/rules/prefer-spy-on.md) | Suggest using `jest.spyOn()` | | ![fixable][] |
| [prefer-strict-equal](docs/rules/prefer-strict-equal.md) | Suggest using `toStrictEqual()` | | ![suggest][] |
| [prefer-to-be-null](docs/rules/prefer-to-be-null.md) | Suggest using `toBeNull()` | ![style][] | ![fixable][] |
| [prefer-to-be-undefined](docs/rules/prefer-to-be-undefined.md) | Suggest using `toBeUndefined()` | ![style][] | ![fixable][] |
| [prefer-to-contain](docs/rules/prefer-to-contain.md) | Suggest using `toContain()` | ![style][] | ![fixable][] |
| [prefer-to-have-length](docs/rules/prefer-to-have-length.md) | Suggest using `toHaveLength()` | ![style][] | ![fixable][] |
| [prefer-todo](docs/rules/prefer-todo.md) | Suggest using `test.todo` | | ![fixable][] |
| [require-to-throw-message](docs/rules/require-to-throw-message.md) | Require a message for `toThrow()` | | |
| [require-top-level-describe](docs/rules/require-top-level-describe.md) | Require test cases and hooks to be inside a `describe` block | | |
| [valid-describe](docs/rules/valid-describe.md) | Enforce valid `describe()` callback | ![recommended][] | |
| [valid-expect](docs/rules/valid-expect.md) | Enforce valid `expect()` usage | ![recommended][] | |
| [valid-expect-in-promise](docs/rules/valid-expect-in-promise.md) | Enforce having return statement when testing with promises | ![recommended][] | |
| [valid-title](docs/rules/valid-title.md) | Enforce valid titles | ![recommended][] | ![fixable][] |

<!-- end base rules list -->

Expand Down
53 changes: 53 additions & 0 deletions docs/rules/prefer-expect-resolves.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# Prefer `await expect(...).resolves` over `expect(await ...)` syntax (`prefer-expect-resolves`)

When working with promises, there are two primary ways you can test the resolved
value:

1. use the `resolve` modifier on `expect`
(`await expect(...).resolves.<matcher>` style)
2. `await` the promise and assert against its result
(`expect(await ...).<matcher>` style)

While the second style is arguably less dependent on `jest`, if the promise
rejects it will be treated as a general error, resulting in less predictable
behaviour and output from `jest`.

Additionally, favoring the first style ensures consistency with its `rejects`
counterpart, as there is no way of "awaiting" a rejection.

## Rule details

This rule triggers a warning if an `await` is done within an `expect`, and
recommends using `resolves` instead.

Examples of **incorrect** code for this rule

```js
it('passes', async () => {
expect(await someValue()).toBe(true);
});

it('is true', async () => {
const myPromise = Promise.resolve(true);

expect(await myPromise).toBe(true);
});
```

Examples of **correct** code for this rule

```js
it('passes', async () => {
await expect(someValue()).resolves.toBe(true);
});

it('is true', async () => {
const myPromise = Promise.resolve(true);

await expect(myPromise).resolves.toBe(true);
});

it('errors', async () => {
await expect(Promise.rejects('oh noes!')).rejects.toThrow('oh noes!');
});
```
1 change: 1 addition & 0 deletions src/__tests__/__snapshots__/rules.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ Object {
"jest/no-test-return-statement": "error",
"jest/prefer-called-with": "error",
"jest/prefer-expect-assertions": "error",
"jest/prefer-expect-resolves": "error",
"jest/prefer-hooks-on-top": "error",
"jest/prefer-spy-on": "error",
"jest/prefer-strict-equal": "error",
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/rules.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { existsSync } from 'fs';
import { resolve } from 'path';
import plugin from '../';

const numberOfRules = 45;
const numberOfRules = 46;
const ruleNames = Object.keys(plugin.rules);
const deprecatedRules = Object.entries(plugin.rules)
.filter(([, rule]) => rule.meta.deprecated)
Expand Down
53 changes: 53 additions & 0 deletions src/rules/__tests__/prefer-expect-resolves.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { TSESLint } from '@typescript-eslint/experimental-utils';
import dedent from 'dedent';
import resolveFrom from 'resolve-from';
import rule from '../prefer-expect-resolves';

const ruleTester = new TSESLint.RuleTester({
parser: resolveFrom(require.resolve('eslint'), 'espree'),
parserOptions: {
ecmaVersion: 2017,
},
});

ruleTester.run('prefer-expect-resolves', rule, {
valid: [
dedent`
it('passes', async () => {
await expect(someValue()).resolves.toBe(true);
});
`,
dedent`
it('is true', async () => {
const myPromise = Promise.resolve(true);
await expect(myPromise).resolves.toBe(true);
});
`,
dedent`
it('errors', async () => {
await expect(Promise.rejects('oh noes!')).rejects.toThrow('oh noes!');
});
`,
],
invalid: [
{
code: dedent`
it('passes', async () => {
expect(await someValue()).toBe(true);
});
`,
errors: [{ endColumn: 27, column: 10, messageId: 'expectResolves' }],
},
{
code: dedent`
it('is true', async () => {
const myPromise = Promise.resolve(true);
expect(await myPromise).toBe(true);
});
`,
errors: [{ endColumn: 25, column: 10, messageId: 'expectResolves' }],
},
],
});
37 changes: 37 additions & 0 deletions src/rules/prefer-expect-resolves.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import {
AST_NODE_TYPES,
TSESTree,
} from '@typescript-eslint/experimental-utils';
import { createRule, isExpectCall } from './utils';

export default createRule({
name: __filename,
meta: {
docs: {
category: 'Best Practices',
description:
'Prefer `await expect(...).resolves` over `expect(await ...)` syntax',
recommended: false,
},
messages: {
expectResolves: 'Use `await expect(...).resolves instead.',
},
schema: [],
type: 'suggestion',
},
defaultOptions: [],
create: context => ({
CallExpression(node: TSESTree.CallExpression) {
if (
isExpectCall(node) &&
node.arguments.length &&
node.arguments[0].type === AST_NODE_TYPES.AwaitExpression
) {
context.report({
node: node.arguments[0],
messageId: 'expectResolves',
});
}
},
}),
});

0 comments on commit b34f133

Please sign in to comment.