Skip to content

Commit 2556020

Browse files
G-RathG-Rath
authored
feat: create prefer-expect-resolves rule (#822)
1 parent 3a64aea commit 2556020

File tree

6 files changed

+214
-45
lines changed

6 files changed

+214
-45
lines changed

‎README.md‎

Lines changed: 45 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -150,50 +150,51 @@ installations requiring long-term consistency.
150150

151151
<!-- begin base rules list -->
152152

153-
| Rule | Description | Configurations | Fixable |
154-
| ---------------------------------------------------------------------------- | --------------------------------------------------------------- | ---------------- | ------------ |
155-
| [consistent-test-it](docs/rules/consistent-test-it.md) | Have control over `test` and `it` usages | | ![fixable][] |
156-
| [expect-expect](docs/rules/expect-expect.md) | Enforce assertion to be made in a test body | ![recommended][] | |
157-
| [lowercase-name](docs/rules/lowercase-name.md) | Enforce lowercase test names | | ![fixable][] |
158-
| [max-nested-describe](docs/rules/max-nested-describe.md) | Enforces a maximum depth to nested describe calls | | |
159-
| [no-alias-methods](docs/rules/no-alias-methods.md) | Disallow alias methods | ![style][] | ![fixable][] |
160-
| [no-commented-out-tests](docs/rules/no-commented-out-tests.md) | Disallow commented out tests | ![recommended][] | |
161-
| [no-conditional-expect](docs/rules/no-conditional-expect.md) | Prevent calling `expect` conditionally | ![recommended][] | |
162-
| [no-deprecated-functions](docs/rules/no-deprecated-functions.md) | Disallow use of deprecated functions | ![recommended][] | ![fixable][] |
163-
| [no-disabled-tests](docs/rules/no-disabled-tests.md) | Disallow disabled tests | ![recommended][] | |
164-
| [no-done-callback](docs/rules/no-done-callback.md) | Avoid using a callback in asynchronous tests and hooks | ![recommended][] | ![suggest][] |
165-
| [no-duplicate-hooks](docs/rules/no-duplicate-hooks.md) | Disallow duplicate setup and teardown hooks | | |
166-
| [no-export](docs/rules/no-export.md) | Disallow using `exports` in files containing tests | ![recommended][] | |
167-
| [no-focused-tests](docs/rules/no-focused-tests.md) | Disallow focused tests | ![recommended][] | ![suggest][] |
168-
| [no-hooks](docs/rules/no-hooks.md) | Disallow setup and teardown hooks | | |
169-
| [no-identical-title](docs/rules/no-identical-title.md) | Disallow identical titles | ![recommended][] | |
170-
| [no-if](docs/rules/no-if.md) | Disallow conditional logic | | |
171-
| [no-interpolation-in-snapshots](docs/rules/no-interpolation-in-snapshots.md) | Disallow string interpolation inside snapshots | ![recommended][] | |
172-
| [no-jasmine-globals](docs/rules/no-jasmine-globals.md) | Disallow Jasmine globals | ![recommended][] | ![fixable][] |
173-
| [no-jest-import](docs/rules/no-jest-import.md) | Disallow importing Jest | ![recommended][] | |
174-
| [no-large-snapshots](docs/rules/no-large-snapshots.md) | disallow large snapshots | | |
175-
| [no-mocks-import](docs/rules/no-mocks-import.md) | Disallow manually importing from `__mocks__` | ![recommended][] | |
176-
| [no-restricted-matchers](docs/rules/no-restricted-matchers.md) | Disallow specific matchers & modifiers | | |
177-
| [no-standalone-expect](docs/rules/no-standalone-expect.md) | Disallow using `expect` outside of `it` or `test` blocks | ![recommended][] | |
178-
| [no-test-prefixes](docs/rules/no-test-prefixes.md) | Use `.only` and `.skip` over `f` and `x` | ![recommended][] | ![fixable][] |
179-
| [no-test-return-statement](docs/rules/no-test-return-statement.md) | Disallow explicitly returning from tests | | |
180-
| [prefer-called-with](docs/rules/prefer-called-with.md) | Suggest using `toBeCalledWith()` or `toHaveBeenCalledWith()` | | |
181-
| [prefer-expect-assertions](docs/rules/prefer-expect-assertions.md) | Suggest using `expect.assertions()` OR `expect.hasAssertions()` | | ![suggest][] |
182-
| [prefer-hooks-on-top](docs/rules/prefer-hooks-on-top.md) | Suggest having hooks before any test cases | | |
183-
| [prefer-spy-on](docs/rules/prefer-spy-on.md) | Suggest using `jest.spyOn()` | | ![fixable][] |
184-
| [prefer-strict-equal](docs/rules/prefer-strict-equal.md) | Suggest using `toStrictEqual()` | | ![suggest][] |
185-
| [prefer-to-be](docs/rules/prefer-to-be.md) | Suggest using `toBe()` for primitive literals | | ![fixable][] |
186-
| [prefer-to-be-null](docs/rules/prefer-to-be-null.md) | Suggest using `toBeNull()` | ![style][] | ![fixable][] |
187-
| [prefer-to-be-undefined](docs/rules/prefer-to-be-undefined.md) | Suggest using `toBeUndefined()` | ![style][] | ![fixable][] |
188-
| [prefer-to-contain](docs/rules/prefer-to-contain.md) | Suggest using `toContain()` | ![style][] | ![fixable][] |
189-
| [prefer-to-have-length](docs/rules/prefer-to-have-length.md) | Suggest using `toHaveLength()` | ![style][] | ![fixable][] |
190-
| [prefer-todo](docs/rules/prefer-todo.md) | Suggest using `test.todo` | | ![fixable][] |
191-
| [require-to-throw-message](docs/rules/require-to-throw-message.md) | Require a message for `toThrow()` | | |
192-
| [require-top-level-describe](docs/rules/require-top-level-describe.md) | Require test cases and hooks to be inside a `describe` block | | |
193-
| [valid-describe](docs/rules/valid-describe.md) | Enforce valid `describe()` callback | ![recommended][] | |
194-
| [valid-expect](docs/rules/valid-expect.md) | Enforce valid `expect()` usage | ![recommended][] | |
195-
| [valid-expect-in-promise](docs/rules/valid-expect-in-promise.md) | Enforce having return statement when testing with promises | ![recommended][] | |
196-
| [valid-title](docs/rules/valid-title.md) | Enforce valid titles | ![recommended][] | ![fixable][] |
153+
| Rule | Description | Configurations | Fixable |
154+
| ---------------------------------------------------------------------------- | ------------------------------------------------------------------- | ---------------- | ------------ |
155+
| [consistent-test-it](docs/rules/consistent-test-it.md) | Have control over `test` and `it` usages | | ![fixable][] |
156+
| [expect-expect](docs/rules/expect-expect.md) | Enforce assertion to be made in a test body | ![recommended][] | |
157+
| [lowercase-name](docs/rules/lowercase-name.md) | Enforce lowercase test names | | ![fixable][] |
158+
| [max-nested-describe](docs/rules/max-nested-describe.md) | Enforces a maximum depth to nested describe calls | | |
159+
| [no-alias-methods](docs/rules/no-alias-methods.md) | Disallow alias methods | ![style][] | ![fixable][] |
160+
| [no-commented-out-tests](docs/rules/no-commented-out-tests.md) | Disallow commented out tests | ![recommended][] | |
161+
| [no-conditional-expect](docs/rules/no-conditional-expect.md) | Prevent calling `expect` conditionally | ![recommended][] | |
162+
| [no-deprecated-functions](docs/rules/no-deprecated-functions.md) | Disallow use of deprecated functions | ![recommended][] | ![fixable][] |
163+
| [no-disabled-tests](docs/rules/no-disabled-tests.md) | Disallow disabled tests | ![recommended][] | |
164+
| [no-done-callback](docs/rules/no-done-callback.md) | Avoid using a callback in asynchronous tests and hooks | ![recommended][] | ![suggest][] |
165+
| [no-duplicate-hooks](docs/rules/no-duplicate-hooks.md) | Disallow duplicate setup and teardown hooks | | |
166+
| [no-export](docs/rules/no-export.md) | Disallow using `exports` in files containing tests | ![recommended][] | |
167+
| [no-focused-tests](docs/rules/no-focused-tests.md) | Disallow focused tests | ![recommended][] | ![suggest][] |
168+
| [no-hooks](docs/rules/no-hooks.md) | Disallow setup and teardown hooks | | |
169+
| [no-identical-title](docs/rules/no-identical-title.md) | Disallow identical titles | ![recommended][] | |
170+
| [no-if](docs/rules/no-if.md) | Disallow conditional logic | | |
171+
| [no-interpolation-in-snapshots](docs/rules/no-interpolation-in-snapshots.md) | Disallow string interpolation inside snapshots | ![recommended][] | |
172+
| [no-jasmine-globals](docs/rules/no-jasmine-globals.md) | Disallow Jasmine globals | ![recommended][] | ![fixable][] |
173+
| [no-jest-import](docs/rules/no-jest-import.md) | Disallow importing Jest | ![recommended][] | |
174+
| [no-large-snapshots](docs/rules/no-large-snapshots.md) | disallow large snapshots | | |
175+
| [no-mocks-import](docs/rules/no-mocks-import.md) | Disallow manually importing from `__mocks__` | ![recommended][] | |
176+
| [no-restricted-matchers](docs/rules/no-restricted-matchers.md) | Disallow specific matchers & modifiers | | |
177+
| [no-standalone-expect](docs/rules/no-standalone-expect.md) | Disallow using `expect` outside of `it` or `test` blocks | ![recommended][] | |
178+
| [no-test-prefixes](docs/rules/no-test-prefixes.md) | Use `.only` and `.skip` over `f` and `x` | ![recommended][] | ![fixable][] |
179+
| [no-test-return-statement](docs/rules/no-test-return-statement.md) | Disallow explicitly returning from tests | | |
180+
| [prefer-called-with](docs/rules/prefer-called-with.md) | Suggest using `toBeCalledWith()` or `toHaveBeenCalledWith()` | | |
181+
| [prefer-expect-assertions](docs/rules/prefer-expect-assertions.md) | Suggest using `expect.assertions()` OR `expect.hasAssertions()` | | ![suggest][] |
182+
| [prefer-expect-resolves](docs/rules/prefer-expect-resolves.md) | Prefer `await expect(...).resolves` over `expect(await ...)` syntax | | ![fixable][] |
183+
| [prefer-hooks-on-top](docs/rules/prefer-hooks-on-top.md) | Suggest having hooks before any test cases | | |
184+
| [prefer-spy-on](docs/rules/prefer-spy-on.md) | Suggest using `jest.spyOn()` | | ![fixable][] |
185+
| [prefer-strict-equal](docs/rules/prefer-strict-equal.md) | Suggest using `toStrictEqual()` | | ![suggest][] |
186+
| [prefer-to-be](docs/rules/prefer-to-be.md) | Suggest using `toBe()` for primitive literals | | ![fixable][] |
187+
| [prefer-to-be-null](docs/rules/prefer-to-be-null.md) | Suggest using `toBeNull()` | ![style][] | ![fixable][] |
188+
| [prefer-to-be-undefined](docs/rules/prefer-to-be-undefined.md) | Suggest using `toBeUndefined()` | ![style][] | ![fixable][] |
189+
| [prefer-to-contain](docs/rules/prefer-to-contain.md) | Suggest using `toContain()` | ![style][] | ![fixable][] |
190+
| [prefer-to-have-length](docs/rules/prefer-to-have-length.md) | Suggest using `toHaveLength()` | ![style][] | ![fixable][] |
191+
| [prefer-todo](docs/rules/prefer-todo.md) | Suggest using `test.todo` | | ![fixable][] |
192+
| [require-to-throw-message](docs/rules/require-to-throw-message.md) | Require a message for `toThrow()` | | |
193+
| [require-top-level-describe](docs/rules/require-top-level-describe.md) | Require test cases and hooks to be inside a `describe` block | | |
194+
| [valid-describe](docs/rules/valid-describe.md) | Enforce valid `describe()` callback | ![recommended][] | |
195+
| [valid-expect](docs/rules/valid-expect.md) | Enforce valid `expect()` usage | ![recommended][] | |
196+
| [valid-expect-in-promise](docs/rules/valid-expect-in-promise.md) | Enforce having return statement when testing with promises | ![recommended][] | |
197+
| [valid-title](docs/rules/valid-title.md) | Enforce valid titles | ![recommended][] | ![fixable][] |
197198

198199
<!-- end base rules list -->
199200

‎docs/rules/prefer-expect-resolves.md‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
# Prefer `await expect(...).resolves` over `expect(await ...)` syntax (`prefer-expect-resolves`)
2+
3+
When working with promises, there are two primary ways you can test the resolved
4+
value:
5+
6+
1. use the `resolve` modifier on `expect`
7+
(`await expect(...).resolves.<matcher>` style)
8+
2. `await` the promise and assert against its result
9+
(`expect(await ...).<matcher>` style)
10+
11+
While the second style is arguably less dependent on `jest`, if the promise
12+
rejects it will be treated as a general error, resulting in less predictable
13+
behaviour and output from `jest`.
14+
15+
Additionally, favoring the first style ensures consistency with its `rejects`
16+
counterpart, as there is no way of "awaiting" a rejection.
17+
18+
## Rule details
19+
20+
This rule triggers a warning if an `await` is done within an `expect`, and
21+
recommends using `resolves` instead.
22+
23+
Examples of **incorrect** code for this rule
24+
25+
```js
26+
it('passes', async () => {
27+
expect(await someValue()).toBe(true);
28+
});
29+
30+
it('is true', async () => {
31+
const myPromise = Promise.resolve(true);
32+
33+
expect(await myPromise).toBe(true);
34+
});
35+
```
36+
37+
Examples of **correct** code for this rule
38+
39+
```js
40+
it('passes', async () => {
41+
await expect(someValue()).resolves.toBe(true);
42+
});
43+
44+
it('is true', async () => {
45+
const myPromise = Promise.resolve(true);
46+
47+
await expect(myPromise).resolves.toBe(true);
48+
});
49+
50+
it('errors', async () => {
51+
await expect(Promise.rejects('oh noes!')).rejects.toThrow('oh noes!');
52+
});
53+
```

‎src/__tests__/__snapshots__/rules.test.ts.snap‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ Object {
3737
"jest/no-test-return-statement": "error",
3838
"jest/prefer-called-with": "error",
3939
"jest/prefer-expect-assertions": "error",
40+
"jest/prefer-expect-resolves": "error",
4041
"jest/prefer-hooks-on-top": "error",
4142
"jest/prefer-spy-on": "error",
4243
"jest/prefer-strict-equal": "error",

‎src/__tests__/rules.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { existsSync } from 'fs';
22
import { resolve } from 'path';
33
import plugin from '../';
44

5-
const numberOfRules = 47;
5+
const numberOfRules = 48;
66
const ruleNames = Object.keys(plugin.rules);
77
const deprecatedRules = Object.entries(plugin.rules)
88
.filter(([, rule]) => rule.meta.deprecated)

‎src/rules/__tests__/prefer-expect-resolves.test.ts‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import { TSESLint } from '@typescript-eslint/experimental-utils';
2+
import dedent from 'dedent';
3+
import resolveFrom from 'resolve-from';
4+
import rule from '../prefer-expect-resolves';
5+
6+
const ruleTester = new TSESLint.RuleTester({
7+
parser: resolveFrom(require.resolve('eslint'), 'espree'),
8+
parserOptions: {
9+
ecmaVersion: 2017,
10+
},
11+
});
12+
13+
ruleTester.run('prefer-expect-resolves', rule, {
14+
valid: [
15+
dedent`
16+
it('passes', async () => {
17+
await expect(someValue()).resolves.toBe(true);
18+
});
19+
`,
20+
dedent`
21+
it('is true', async () => {
22+
const myPromise = Promise.resolve(true);
23+
24+
await expect(myPromise).resolves.toBe(true);
25+
});
26+
`,
27+
dedent`
28+
it('errors', async () => {
29+
await expect(Promise.rejects('oh noes!')).rejects.toThrow('oh noes!');
30+
});
31+
`,
32+
],
33+
invalid: [
34+
{
35+
code: dedent`
36+
it('passes', async () => {
37+
expect(await someValue()).toBe(true);
38+
});
39+
`,
40+
output: dedent`
41+
it('passes', async () => {
42+
await expect(someValue()).resolves.toBe(true);
43+
});
44+
`,
45+
errors: [{ endColumn: 27, column: 10, messageId: 'expectResolves' }],
46+
},
47+
{
48+
code: dedent`
49+
it('is true', async () => {
50+
const myPromise = Promise.resolve(true);
51+
52+
expect(await myPromise).toBe(true);
53+
});
54+
`,
55+
output: dedent`
56+
it('is true', async () => {
57+
const myPromise = Promise.resolve(true);
58+
59+
await expect(myPromise).resolves.toBe(true);
60+
});
61+
`,
62+
errors: [{ endColumn: 25, column: 10, messageId: 'expectResolves' }],
63+
},
64+
],
65+
});

0 commit comments

Comments
 (0)