Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(rules): no-expect-resolves #364

Merged
merged 7 commits into from
Aug 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ installations requiring long-term consistency.
| [no-disabled-tests][] | Disallow disabled tests | ![recommended][] | |
| [no-duplicate-hooks][] | Disallow duplicate hooks within a `describe` block | | |
| [no-empty-title][] | Disallow empty titles | | |
| [no-expect-resolves][] | Disallow using `expect().resolves` | | |
| [no-export][] | Disallow export from test files | | |
| [no-focused-tests][] | Disallow focused tests | ![recommended][] | |
| [no-hooks][] | Disallow setup and teardown hooks | | |
Expand Down Expand Up @@ -166,6 +167,7 @@ https://github.com/dangreenisrael/eslint-plugin-jest-formatting
[no-disabled-tests]: docs/rules/no-disabled-tests.md
[no-duplicate-hooks]: docs/rules/no-duplicate-hooks.md
[no-empty-title]: docs/rules/no-empty-title.md
[no-expect-resolves]: docs/rules/no-expect-resolves.md
[no-export]: docs/rules/no-export.md
[no-focused-tests]: docs/rules/no-focused-tests.md
[no-hooks]: docs/rules/no-hooks.md
Expand Down
29 changes: 29 additions & 0 deletions docs/rules/no-expect-resolves.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Avoid using `expect().resolves` (no-expect-resolves)

Jest allows you to test a promise resolve value using `await expect().resolves`.
For consistency and readability this rule bans `expect().resolves` in favor of
`expect(await promise)`.

## Rule details

This rule triggers a warning if `expect().resolves` is used.

This rule is disabled by default.

### Default configuration

The following patterns is considered warning:

```js
test('some test', async () => {
await expect(Promise.resolve(1)).resolves.toBe(1);
});
```

The following pattern is not considered warning:

```js
test('some test', async () => {
expect(await Promise.resolve(1)).toBe(1);
});
```
2 changes: 1 addition & 1 deletion src/__tests__/rules.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { resolve } from 'path';
import { rules } from '../';

const ruleNames = Object.keys(rules);
const numberOfRules = 37;
const numberOfRules = 38;

describe('rules', () => {
it('should have a corresponding doc for each rule', () => {
Expand Down
24 changes: 24 additions & 0 deletions src/rules/__tests__/no-expect-resolves.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { TSESLint } from '@typescript-eslint/experimental-utils';
import rule from '../no-expect-resolves';

const ruleTester = new TSESLint.RuleTester({
parserOptions: {
ecmaVersion: 2017,
},
});

ruleTester.run('no-expect-resolves', rule, {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this rule should probably have more tests, but I ran it against Jest and found no errors (at least no false positives and no crashes)

valid: [
`test('some test', async () => {
expect(await Promise.resolve(1)).toBe(1);
});`,
],
invalid: [
{
code: `test('some test', async () => {
await expect(Promise.resolve(1)).resolves.toBe(1);
});`,
errors: [{ endColumn: 55, column: 47, messageId: 'expectResolves' }],
},
],
});
40 changes: 40 additions & 0 deletions src/rules/no-expect-resolves.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import {
AST_NODE_TYPES,
TSESTree,
} from '@typescript-eslint/experimental-utils';
import { createRule, isExpectCall } from './tsUtils';

function isIdentifierResolves(node: TSESTree.MemberExpression) {
return (
node.property.type === AST_NODE_TYPES.Identifier &&
node.property.name === 'resolves'
);
}

function isExpectResolves(node: TSESTree.MemberExpression) {
return isExpectCall(node.object) && isIdentifierResolves(node);
}

export default createRule({
name: __filename,
meta: {
docs: {
category: 'Best Practices',
description: 'Disallow expect.resolves',
recommended: false,
},
messages: {
expectResolves: 'Use `expect(await promise)` instead.',
},
schema: [],
type: 'suggestion',
},
defaultOptions: [],
create: context => ({
MemberExpression(node) {
if (isExpectResolves(node)) {
context.report({ node: node.property, messageId: 'expectResolves' });
}
},
}),
});