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

fix(no-conditional-expect): check for expects in catchs on promises #819

Merged
merged 1 commit into from Apr 26, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 11 additions & 0 deletions docs/rules/no-conditional-expect.md
Expand Up @@ -3,6 +3,9 @@
This rule prevents the use of `expect` in conditional blocks, such as `if`s &
`catch`s.

This includes using `expect` in callbacks to functions named `catch`, which are
assumed to be promises.

## Rule Details

Jest considered a test to have failed if it throws an error, rather than on if
Expand Down Expand Up @@ -37,6 +40,10 @@ it('baz', async () => {
expect(err).toMatchObject({ code: 'MODULE_NOT_FOUND' });
}
});

it('throws an error', async () => {
await foo().catch(error => expect(error).toBeInstanceOf(error));
});
```

The following patterns are not warnings:
Expand Down Expand Up @@ -67,4 +74,8 @@ it('validates the request', () => {
expect(validRequest).toHaveBeenCalledWith(request);
}
});

it('throws an error', async () => {
await expect(foo).rejects.toThrow(Error);
});
```
106 changes: 106 additions & 0 deletions src/rules/__tests__/no-conditional-expect.test.ts
@@ -1,4 +1,5 @@
import { TSESLint } from '@typescript-eslint/experimental-utils';
import dedent from 'dedent';
import resolveFrom from 'resolve-from';
import rule from '../no-conditional-expect';

Expand Down Expand Up @@ -519,3 +520,108 @@ ruleTester.run('catch conditions', rule, {
},
],
});

ruleTester.run('promises', rule, {
valid: [
`
it('works', async () => {
try {
await Promise.resolve().then(() => {
throw new Error('oh noes!');
});
} catch {
// ignore errors
} finally {
expect(something).toHaveBeenCalled();
}
});
`,
`
it('works', async () => {
await doSomething().catch(error => error);

expect(error).toBeInstanceOf(Error);
});
`,
`
it('works', async () => {
try {
await Promise.resolve().then(() => {
throw new Error('oh noes!');
});
} catch {
// ignore errors
}

expect(something).toHaveBeenCalled();
});
`,
],
invalid: [
{
code: dedent`
it('works', async () => {
await Promise.resolve()
.then(() => { throw new Error('oh noes!'); })
.catch(error => expect(error).toBeInstanceOf(Error));
});
`,
errors: [{ messageId: 'conditionalExpect' }],
},
{
code: dedent`
it('works', async () => {
await Promise.resolve()
.then(() => { throw new Error('oh noes!'); })
.catch(error => expect(error).toBeInstanceOf(Error))
.then(() => { throw new Error('oh noes!'); })
.catch(error => expect(error).toBeInstanceOf(Error))
.then(() => { throw new Error('oh noes!'); })
.catch(error => expect(error).toBeInstanceOf(Error));
});
`,
errors: [{ messageId: 'conditionalExpect' }],
},
{
code: dedent`
it('works', async () => {
await Promise.resolve()
.catch(error => expect(error).toBeInstanceOf(Error))
.catch(error => expect(error).toBeInstanceOf(Error))
.catch(error => expect(error).toBeInstanceOf(Error));
});
`,
errors: [{ messageId: 'conditionalExpect' }],
},
{
code: dedent`
it('works', async () => {
await Promise.resolve()
.catch(error => expect(error).toBeInstanceOf(Error))
.then(() => { throw new Error('oh noes!'); })
.then(() => { throw new Error('oh noes!'); })
.then(() => { throw new Error('oh noes!'); });
});
`,
errors: [{ messageId: 'conditionalExpect' }],
},
{
code: dedent`
it('works', async () => {
await somePromise
.then(() => { throw new Error('oh noes!'); })
.catch(error => expect(error).toBeInstanceOf(Error));
});
`,
errors: [{ messageId: 'conditionalExpect' }],
},
{
code: dedent`
it('works', async () => {
await somePromise.catch(error => expect(error).toBeInstanceOf(Error));
});
`,
errors: [{ messageId: 'conditionalExpect' }],
},
],
});
29 changes: 28 additions & 1 deletion src/rules/no-conditional-expect.ts
@@ -1,11 +1,22 @@
import { TSESTree } from '@typescript-eslint/experimental-utils';
import {
AST_NODE_TYPES,
TSESTree,
} from '@typescript-eslint/experimental-utils';
import {
KnownCallExpression,
createRule,
getTestCallExpressionsFromDeclaredVariables,
isExpectCall,
isSupportedAccessor,
isTestCaseCall,
} from './utils';

const isCatchCall = (
node: TSESTree.CallExpression,
): node is KnownCallExpression<'catch'> =>
node.callee.type === AST_NODE_TYPES.MemberExpression &&
isSupportedAccessor(node.callee.property, 'catch');

export default createRule({
name: __filename,
meta: {
Expand All @@ -24,6 +35,7 @@ export default createRule({
create(context) {
let conditionalDepth = 0;
let inTestCase = false;
let inPromiseCatch = false;

const increaseConditionalDepth = () => inTestCase && conditionalDepth++;
const decreaseConditionalDepth = () => inTestCase && conditionalDepth--;
Expand All @@ -44,17 +56,32 @@ export default createRule({
inTestCase = true;
}

if (isCatchCall(node)) {
inPromiseCatch = true;
}

if (inTestCase && isExpectCall(node) && conditionalDepth > 0) {
context.report({
messageId: 'conditionalExpect',
node,
});
}

if (inPromiseCatch && isExpectCall(node)) {
context.report({
messageId: 'conditionalExpect',
node,
});
}
},
'CallExpression:exit'(node) {
if (isTestCaseCall(node)) {
inTestCase = false;
}

if (isCatchCall(node)) {
inPromiseCatch = false;
}
},
CatchClause: increaseConditionalDepth,
'CatchClause:exit': decreaseConditionalDepth,
Expand Down