diff --git a/README.md b/README.md index 2bffe40c9..58397e876 100644 --- a/README.md +++ b/README.md @@ -127,6 +127,7 @@ installations requiring long-term consistency. | [no-test-prefixes][] | Disallow using `f` & `x` prefixes to define focused/skipped tests | ![recommended][] | ![fixable-green][] | | [no-test-return-statement][] | Disallow explicitly returning from tests | | | | [no-truthy-falsy][] | Disallow using `toBeTruthy()` & `toBeFalsy()` | | | +| [no-try-expect][] | Prevent `catch` assertions in tests | | | | [prefer-expect-assertions][] | Suggest using `expect.assertions()` OR `expect.hasAssertions()` | | | | [prefer-spy-on][] | Suggest using `jest.spyOn()` | | ![fixable-green][] | | [prefer-strict-equal][] | Suggest using `toStrictEqual()` | | ![fixable-green][] | @@ -177,6 +178,7 @@ https://github.com/dangreenisrael/eslint-plugin-jest-formatting [no-test-prefixes]: docs/rules/no-test-prefixes.md [no-test-return-statement]: docs/rules/no-test-return-statement.md [no-truthy-falsy]: docs/rules/no-truthy-falsy.md +[no-try-expect]: docs/rules/no-try-expect.md [prefer-called-with]: docs/rules/prefer-called-with.md [prefer-expect-assertions]: docs/rules/prefer-expect-assertions.md [prefer-spy-on]: docs/rules/prefer-spy-on.md diff --git a/docs/rules/no-try-expect.md b/docs/rules/no-try-expect.md new file mode 100644 index 000000000..d257bf0bb --- /dev/null +++ b/docs/rules/no-try-expect.md @@ -0,0 +1,56 @@ +# Prevent catch assertions in tests (no-try-expect) + +This rule prevents the use of `expect` inside `catch` blocks. + +## Rule Details + +Expectations inside a `catch` block can be silently skipped. While Jest provides +an `expect.assertions(number)` helper, it might be cumbersome to add this to +every single test. Using `toThrow` concisely guarantees that an exception was +thrown, and that its contents match expectations. + +The following patterns are warnings: + +```js +it('foo', () => { + try { + foo(); // `foo` may be refactored to not throw exceptions, yet still appears to be tested here. + } catch (err) { + expect(err).toMatch(/foo error/); + } +}); + +it('bar', async () => { + try { + await foo(); + } catch (err) { + expect(err).toMatch(/foo error/); + } +}); + +it('baz', async () => { + try { + await foo(); + } catch (err) { + expect(err).toMatchObject({ code: 'MODULE_NOT_FOUND' }); + } +}); +``` + +The following patterns are not warnings: + +```js +it('foo', () => { + expect(() => foo()).toThrow(/foo error/); +}); + +it('bar', async () => { + await expect(fooPromise).rejects.toThrow(/foo error/); +}); + +it('baz', async () => { + await expect(() => foo()).rejects.toThrow( + expect.objectContaining({ code: 'MODULE_NOT_FOUND' }), + ); +}); +``` diff --git a/src/__tests__/rules.test.js b/src/__tests__/rules.test.js index b259c2c38..aa2882b6f 100644 --- a/src/__tests__/rules.test.js +++ b/src/__tests__/rules.test.js @@ -3,7 +3,7 @@ import { resolve } from 'path'; import { rules } from '../'; const ruleNames = Object.keys(rules); -const numberOfRules = 35; +const numberOfRules = 36; describe('rules', () => { it('should have a corresponding doc for each rule', () => { diff --git a/src/rules/__tests__/no-try-expect.test.js b/src/rules/__tests__/no-try-expect.test.js new file mode 100644 index 000000000..f8409ed23 --- /dev/null +++ b/src/rules/__tests__/no-try-expect.test.js @@ -0,0 +1,62 @@ +import { RuleTester } from 'eslint'; +import rule from '../no-try-expect'; + +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 2019, + }, +}); + +ruleTester.run('no-try-catch', rule, { + valid: [ + `it('foo', () => { + expect('foo').toEqual('foo'); + })`, + `it('foo', () => { + expect('bar').toEqual('bar'); + }); + try { + + } catch { + expect('foo').toEqual('foo'); + }`, + `it.skip('foo'); + try { + + } catch { + expect('foo').toEqual('foo'); + }`, + ], + invalid: [ + { + code: `it('foo', () => { + try { + + } catch (err) { + expect(err).toMatch('Error'); + } + })`, + errors: [ + { + messageId: 'noTryExpect', + }, + ], + }, + { + code: `it('foo', async () => { + await wrapper('production', async () => { + try { + + } catch (err) { + expect(err).toMatch('Error'); + } + }) + })`, + errors: [ + { + messageId: 'noTryExpect', + }, + ], + }, + ], +}); diff --git a/src/rules/no-try-expect.js b/src/rules/no-try-expect.js new file mode 100644 index 000000000..aa3a424fa --- /dev/null +++ b/src/rules/no-try-expect.js @@ -0,0 +1,53 @@ +import { getDocsUrl, isTestCase } from './util'; + +export default { + meta: { + docs: { + description: 'Prefer using toThrow for exception tests', + uri: getDocsUrl(__filename), + }, + messages: { + noTryExpect: [ + 'Tests should use Jest‘s exception helpers.', + 'Use "expect(() => yourFunction()).toThrow()" for synchronous tests,', + 'or "await expect(yourFunction()).rejects.toThrow()" for async tests', + ].join(' '), + }, + }, + create(context) { + let isTest = false; + let catchDepth = 0; + + function isThrowExpectCall(node) { + return catchDepth > 0 && node.callee.name === 'expect'; + } + + return { + CallExpression(node) { + if (isTestCase(node)) { + isTest = true; + } else if (isTest && isThrowExpectCall(node)) { + context.report({ + messageId: 'noTryExpect', + node, + }); + } + }, + CatchClause() { + if (isTest) { + ++catchDepth; + } + }, + 'CatchClause:exit'() { + if (isTest) { + --catchDepth; + } + }, + 'CallExpression:exit'(node) { + if (isTestCase(node)) { + isTest = false; + } + }, + }; + }, +};