diff --git a/README.md b/README.md index a2df8e352..14d603b5c 100644 --- a/README.md +++ b/README.md @@ -209,6 +209,7 @@ installations requiring long-term consistency. | [prefer-equality-matcher](docs/rules/prefer-equality-matcher.md) | Suggest using the built-in equality matchers | | ![suggest][] | | [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 | | ![fixable][] | +| [prefer-hooks-in-order](docs/rules/prefer-hooks-in-order.md) | Prefer having hooks in a consistent order | | | | [prefer-hooks-on-top](docs/rules/prefer-hooks-on-top.md) | Suggest having hooks before any test cases | | | | [prefer-lowercase-title](docs/rules/prefer-lowercase-title.md) | Enforce lowercase test names | | ![fixable][] | | [prefer-snapshot-hint](docs/rules/prefer-snapshot-hint.md) | Prefer including a hint with external snapshots | | | diff --git a/docs/rules/prefer-hooks-in-order.md b/docs/rules/prefer-hooks-in-order.md new file mode 100644 index 000000000..a2d580b07 --- /dev/null +++ b/docs/rules/prefer-hooks-in-order.md @@ -0,0 +1,133 @@ +# Prefer having hooks in a consistent order (`prefer-hooks-in-order`) + +While hooks can be setup in any order, they're always called by `jest` in this +specific order: + +1. `beforeAll` +1. `beforeEach` +1. `afterEach` +1. `afterAll` + +This rule aims to make that more obvious by enforcing grouped hooks be setup in +that order within tests. + +## Rule Details + +Examples of **incorrect** code for this rule + +```js +/* eslint jest/prefer-hooks-in-order: "error" */ + +describe('foo', () => { + beforeEach(() => { + seedMyDatabase(); + }); + + beforeAll(() => { + createMyDatabase(); + }); + + it('accepts this input', () => { + // ... + }); + + it('returns that value', () => { + // ... + }); + + describe('when the database has specific values', () => { + const specificValue = '...'; + + beforeEach(() => { + seedMyDatabase(specificValue); + }); + + it('accepts that input', () => { + // ... + }); + + it('throws an error', () => { + // ... + }); + + afterEach(() => { + clearLogger(); + }); + beforeEach(() => { + mockLogger(); + }); + + it('logs a message', () => { + // ... + }); + }); + + afterAll(() => { + removeMyDatabase(); + }); +}); +``` + +Examples of **correct** code for this rule + +```js +/* eslint jest/prefer-hooks-in-order: "error" */ + +describe('foo', () => { + beforeAll(() => { + createMyDatabase(); + }); + + beforeEach(() => { + seedMyDatabase(); + }); + + it('accepts this input', () => { + // ... + }); + + it('returns that value', () => { + // ... + }); + + describe('when the database has specific values', () => { + const specificValue = '...'; + + beforeEach(() => { + seedMyDatabase(specificValue); + }); + + it('accepts that input', () => { + // ... + }); + + it('throws an error', () => { + // ... + }); + + beforeEach(() => { + mockLogger(); + }); + + afterEach(() => { + clearLogger(); + }); + + it('logs a message', () => { + // ... + }); + }); + + afterAll(() => { + removeMyDatabase(); + }); +}); +``` + +## Also See + +- [`prefer-hooks-on-top`](prefer-hooks-on-top.md) + +## Further Reading + +- [Order of execution of describe and test blocks](https://jestjs.io/docs/setup-teardown#order-of-execution-of-describe-and-test-blocks) diff --git a/src/__tests__/__snapshots__/rules.test.ts.snap b/src/__tests__/__snapshots__/rules.test.ts.snap index 830570ce8..80723e13b 100644 --- a/src/__tests__/__snapshots__/rules.test.ts.snap +++ b/src/__tests__/__snapshots__/rules.test.ts.snap @@ -39,6 +39,7 @@ Object { "jest/prefer-equality-matcher": "error", "jest/prefer-expect-assertions": "error", "jest/prefer-expect-resolves": "error", + "jest/prefer-hooks-in-order": "error", "jest/prefer-hooks-on-top": "error", "jest/prefer-lowercase-title": "error", "jest/prefer-snapshot-hint": "error", diff --git a/src/__tests__/rules.test.ts b/src/__tests__/rules.test.ts index 88f10b1a1..484887325 100644 --- a/src/__tests__/rules.test.ts +++ b/src/__tests__/rules.test.ts @@ -2,7 +2,7 @@ import { existsSync } from 'fs'; import { resolve } from 'path'; import plugin from '../'; -const numberOfRules = 47; +const numberOfRules = 48; const ruleNames = Object.keys(plugin.rules); const deprecatedRules = Object.entries(plugin.rules) .filter(([, rule]) => rule.meta.deprecated) diff --git a/src/rules/__tests__/prefer-hooks-in-order.test.ts b/src/rules/__tests__/prefer-hooks-in-order.test.ts new file mode 100644 index 000000000..ca55d2e22 --- /dev/null +++ b/src/rules/__tests__/prefer-hooks-in-order.test.ts @@ -0,0 +1,690 @@ +import { TSESLint } from '@typescript-eslint/utils'; +import dedent from 'dedent'; +import rule from '../prefer-hooks-in-order'; +import { espreeParser } from './test-utils'; + +const ruleTester = new TSESLint.RuleTester({ + parser: espreeParser, + parserOptions: { + ecmaVersion: 2015, + }, +}); + +ruleTester.run('prefer-hooks-in-order', rule, { + valid: [ + 'beforeAll(() => {})', + 'beforeEach(() => {})', + 'afterEach(() => {})', + 'afterAll(() => {})', + 'describe(() => {})', + dedent` + beforeAll(() => {}); + beforeEach(() => {}); + afterEach(() => {}); + afterAll(() => {}); + `, + dedent` + describe('foo', () => { + someSetupFn(); + beforeEach(() => {}); + afterEach(() => {}); + + test('bar', () => { + someFn(); + }); + }); + `, + dedent` + beforeAll(() => {}); + afterAll(() => {}); + `, + dedent` + beforeEach(() => {}); + afterEach(() => {}); + `, + dedent` + beforeAll(() => {}); + afterEach(() => {}); + `, + dedent` + beforeAll(() => {}); + beforeEach(() => {}); + `, + dedent` + afterEach(() => {}); + afterAll(() => {}); + `, + dedent` + beforeAll(() => {}); + beforeAll(() => {}); + `, + dedent` + describe('my test', () => { + afterEach(() => {}); + afterAll(() => {}); + }); + `, + dedent` + describe('my test', () => { + afterEach(() => {}); + afterAll(() => {}); + + doSomething(); + + beforeAll(() => {}); + beforeEach(() => {}); + }); + `, + dedent` + describe('my test', () => { + afterEach(() => {}); + afterAll(() => {}); + + it('is a test', () => {}); + + beforeAll(() => {}); + beforeEach(() => {}); + }); + `, + dedent` + describe('my test', () => { + afterAll(() => {}); + + describe('when something is true', () => { + beforeAll(() => {}); + beforeEach(() => {}); + }); + }); + `, + dedent` + describe('my test', () => { + afterAll(() => {}); + + describe('when something is true', () => { + beforeAll(() => {}); + beforeEach(() => {}); + + it('does something', () => {}); + + beforeAll(() => {}); + beforeEach(() => {}); + }); + + beforeAll(() => {}); + beforeEach(() => {}); + }); + + describe('my test', () => { + beforeAll(() => {}); + beforeEach(() => {}); + afterAll(() => {}); + + describe('when something is true', () => { + it('does something', () => {}); + + beforeAll(() => {}); + beforeEach(() => {}); + }); + + beforeAll(() => {}); + beforeEach(() => {}); + }); + `, + dedent` + const withDatabase = () => { + beforeAll(() => { + createMyDatabase(); + }); + afterAll(() => { + removeMyDatabase(); + }); + }; + + describe('my test', () => { + withDatabase(); + + afterAll(() => {}); + + describe('when something is true', () => { + beforeAll(() => {}); + beforeEach(() => {}); + + it('does something', () => {}); + + beforeAll(() => {}); + beforeEach(() => {}); + }); + + beforeAll(() => {}); + beforeEach(() => {}); + }); + + describe('my test', () => { + beforeAll(() => {}); + beforeEach(() => {}); + afterAll(() => {}); + + withDatabase(); + + describe('when something is true', () => { + it('does something', () => {}); + + beforeAll(() => {}); + beforeEach(() => {}); + }); + + beforeAll(() => {}); + beforeEach(() => {}); + }); + `, + dedent` + describe('foo', () => { + beforeAll(() => { + createMyDatabase(); + }); + + beforeEach(() => { + seedMyDatabase(); + }); + + it('accepts this input', () => { + // ... + }); + + it('returns that value', () => { + // ... + }); + + describe('when the database has specific values', () => { + const specificValue = '...'; + + beforeEach(() => { + seedMyDatabase(specificValue); + }); + + it('accepts that input', () => { + // ... + }); + + it('throws an error', () => { + // ... + }); + + beforeEach(() => { + mockLogger(); + }); + + afterEach(() => { + clearLogger(); + }); + + it('logs a message', () => { + // ... + }); + }); + + afterAll(() => { + removeMyDatabase(); + }); + }); + `, + dedent` + describe('A file with a lot of test', () => { + beforeAll(() => { + setupTheDatabase(); + createMocks(); + }); + + beforeAll(() => { + doEvenMore(); + }); + + beforeEach(() => { + cleanTheDatabase(); + resetSomeThings(); + }); + + afterEach(() => { + cleanTheDatabase(); + resetSomeThings(); + }); + + afterAll(() => { + closeTheDatabase(); + stop(); + }); + + it('does something', () => { + const thing = getThing(); + expect(thing).toBe('something'); + }); + + it('throws', () => { + // Do something that throws + }); + + describe('Also have tests in here', () => { + afterAll(() => {}); + it('tests something', () => {}); + it('tests something else', () => {}); + beforeAll(()=>{}); + }); + }); + `, + ], + invalid: [ + { + code: dedent` + const withDatabase = () => { + afterAll(() => { + removeMyDatabase(); + }); + beforeAll(() => { + createMyDatabase(); + }); + }; + `, + errors: [ + { + messageId: 'reorderHooks', + data: { currentHook: 'beforeAll', previousHook: 'afterAll' }, + column: 3, + line: 5, + }, + ], + }, + { + code: dedent` + afterAll(() => { + removeMyDatabase(); + }); + beforeAll(() => { + createMyDatabase(); + }); + `, + errors: [ + { + messageId: 'reorderHooks', + data: { currentHook: 'beforeAll', previousHook: 'afterAll' }, + column: 1, + line: 4, + }, + ], + }, + { + code: dedent` + afterAll(() => {}); + beforeAll(() => {}); + `, + errors: [ + { + messageId: 'reorderHooks', + data: { currentHook: 'beforeAll', previousHook: 'afterAll' }, + column: 1, + line: 2, + }, + ], + }, + { + code: dedent` + afterEach(() => {}); + beforeEach(() => {}); + `, + errors: [ + { + // 'beforeEach' hooks should be before any 'afterEach' hooks + messageId: 'reorderHooks', + data: { currentHook: 'beforeEach', previousHook: 'afterEach' }, + column: 1, + line: 2, + }, + ], + }, + { + code: dedent` + afterEach(() => {}); + beforeAll(() => {}); + `, + errors: [ + { + // 'beforeAll' hooks should be before any 'afterEach' hooks + messageId: 'reorderHooks', + data: { currentHook: 'beforeAll', previousHook: 'afterEach' }, + column: 1, + line: 2, + }, + ], + }, + { + code: dedent` + beforeEach(() => {}); + beforeAll(() => {}); + `, + errors: [ + { + messageId: 'reorderHooks', + data: { currentHook: 'beforeAll', previousHook: 'beforeEach' }, + column: 1, + line: 2, + }, + ], + }, + { + code: dedent` + afterAll(() => {}); + afterEach(() => {}); + `, + errors: [ + { + messageId: 'reorderHooks', + data: { currentHook: 'afterEach', previousHook: 'afterAll' }, + column: 1, + line: 2, + }, + ], + }, + { + code: dedent` + afterAll(() => {}); + // The afterEach should do this + // This comment does not matter for the order + afterEach(() => {}); + `, + errors: [ + { + messageId: 'reorderHooks', + data: { currentHook: 'afterEach', previousHook: 'afterAll' }, + column: 1, + line: 4, + }, + ], + }, + { + code: dedent` + afterAll(() => {}); + afterAll(() => {}); + afterEach(() => {}); + `, + errors: [ + { + messageId: 'reorderHooks', + data: { currentHook: 'afterEach', previousHook: 'afterAll' }, + column: 1, + line: 3, + }, + ], + }, + { + code: dedent` + describe('my test', () => { + afterAll(() => {}); + afterEach(() => {}); + }); + `, + errors: [ + { + messageId: 'reorderHooks', + data: { currentHook: 'afterEach', previousHook: 'afterAll' }, + column: 3, + line: 3, + }, + ], + }, + { + code: dedent` + describe('my test', () => { + afterAll(() => {}); + afterEach(() => {}); + + doSomething(); + + beforeEach(() => {}); + beforeAll(() => {}); + }); + `, + errors: [ + { + messageId: 'reorderHooks', + data: { currentHook: 'afterEach', previousHook: 'afterAll' }, + column: 3, + line: 3, + }, + { + messageId: 'reorderHooks', + data: { currentHook: 'beforeAll', previousHook: 'beforeEach' }, + column: 3, + line: 8, + }, + ], + }, + { + code: dedent` + describe('my test', () => { + afterAll(() => {}); + afterEach(() => {}); + + it('is a test', () => {}); + + beforeEach(() => {}); + beforeAll(() => {}); + }); + `, + errors: [ + { + messageId: 'reorderHooks', + data: { currentHook: 'afterEach', previousHook: 'afterAll' }, + column: 3, + line: 3, + }, + { + messageId: 'reorderHooks', + data: { currentHook: 'beforeAll', previousHook: 'beforeEach' }, + column: 3, + line: 8, + }, + ], + }, + { + code: dedent` + describe('my test', () => { + afterAll(() => {}); + + describe('when something is true', () => { + beforeEach(() => {}); + beforeAll(() => {}); + }); + }); + `, + errors: [ + { + messageId: 'reorderHooks', + data: { currentHook: 'beforeAll', previousHook: 'beforeEach' }, + column: 5, + line: 6, + }, + ], + }, + { + code: dedent` + describe('my test', () => { + beforeAll(() => {}); + afterAll(() => {}); + beforeAll(() => {}); + + describe('when something is true', () => { + beforeAll(() => {}); + afterEach(() => {}); + beforeEach(() => {}); + afterEach(() => {}); + }); + }); + `, + errors: [ + { + messageId: 'reorderHooks', + data: { currentHook: 'beforeAll', previousHook: 'afterAll' }, + column: 3, + line: 4, + }, + { + messageId: 'reorderHooks', + data: { currentHook: 'beforeEach', previousHook: 'afterEach' }, + column: 5, + line: 9, + }, + ], + }, + { + code: dedent` + describe('my test', () => { + beforeAll(() => {}); + beforeAll(() => {}); + afterAll(() => {}); + + it('foo nested', () => { + // this is a test + }); + + describe('when something is true', () => { + beforeAll(() => {}); + afterEach(() => {}); + + it('foo nested', () => { + // this is a test + }); + + describe('deeply nested', () => { + afterAll(() => {}); + afterAll(() => {}); + // This comment does nothing + afterEach(() => {}); + + it('foo nested', () => { + // this is a test + }); + }) + beforeEach(() => {}); + afterEach(() => {}); + }); + }); + `, + errors: [ + { + messageId: 'reorderHooks', + data: { currentHook: 'afterEach', previousHook: 'afterAll' }, + column: 7, + line: 22, + }, + ], + }, + { + code: dedent` + describe('my test', () => { + const setupDatabase = () => { + beforeEach(() => { + initDatabase(); + fillWithData(); + }); + beforeAll(() => { + setupMocks(); + }); + }; + + it('foo', () => { + // this is a test + }); + + describe('my nested test', () => { + afterAll(() => {}); + afterEach(() => {}); + + it('foo nested', () => { + // this is a test + }); + }); + }); + `, + errors: [ + { + messageId: 'reorderHooks', + data: { currentHook: 'beforeAll', previousHook: 'beforeEach' }, + column: 5, + line: 7, + }, + { + messageId: 'reorderHooks', + data: { currentHook: 'afterEach', previousHook: 'afterAll' }, + column: 5, + line: 18, + }, + ], + }, + { + code: dedent` + describe('foo', () => { + beforeEach(() => { + seedMyDatabase(); + }); + + beforeAll(() => { + createMyDatabase(); + }); + + it('accepts this input', () => { + // ... + }); + + it('returns that value', () => { + // ... + }); + + describe('when the database has specific values', () => { + const specificValue = '...'; + + beforeEach(() => { + seedMyDatabase(specificValue); + }); + + it('accepts that input', () => { + // ... + }); + + it('throws an error', () => { + // ... + }); + + afterEach(() => { + clearLogger(); + }); + + beforeEach(() => { + mockLogger(); + }); + + it('logs a message', () => { + // ... + }); + }); + + afterAll(() => { + removeMyDatabase(); + }); + }); + `, + errors: [ + { + messageId: 'reorderHooks', + data: { currentHook: 'beforeAll', previousHook: 'beforeEach' }, + column: 3, + line: 6, + }, + { + messageId: 'reorderHooks', + data: { currentHook: 'beforeEach', previousHook: 'afterEach' }, + column: 5, + line: 37, + }, + ], + }, + ], +}); diff --git a/src/rules/prefer-hooks-in-order.ts b/src/rules/prefer-hooks-in-order.ts new file mode 100644 index 000000000..06508af1e --- /dev/null +++ b/src/rules/prefer-hooks-in-order.ts @@ -0,0 +1,78 @@ +import { createRule, isHookCall } from './utils'; + +const HooksOrder = [ + 'beforeAll', + 'beforeEach', + 'afterEach', + 'afterAll', +] as const; + +export default createRule({ + name: __filename, + meta: { + docs: { + category: 'Best Practices', + description: 'Prefer having hooks in a consistent order', + recommended: false, + }, + messages: { + reorderHooks: `\`{{ currentHook }}\` hooks should be before any \`{{ previousHook }}\` hooks`, + }, + schema: [], + type: 'suggestion', + }, + defaultOptions: [], + create(context) { + let previousHookIndex = -1; + let inHook = false; + + return { + CallExpression(node) { + if (inHook) { + // Ignore everything that is passed into a hook + return; + } + + if (!isHookCall(node, context.getScope())) { + // Reset the previousHookIndex when encountering something different from a hook + previousHookIndex = -1; + + return; + } + + inHook = true; + const currentHook = node.callee.name; + const currentHookIndex = HooksOrder.indexOf(currentHook); + + if (currentHookIndex < previousHookIndex) { + context.report({ + messageId: 'reorderHooks', + node, + data: { + previousHook: HooksOrder[previousHookIndex], + currentHook, + }, + }); + + return; + } + + previousHookIndex = currentHookIndex; + }, + 'CallExpression:exit'(node) { + if (isHookCall(node, context.getScope())) { + inHook = false; + + return; + } + + if (inHook) { + return; + } + + // Reset the previousHookIndex when encountering something different from a hook + previousHookIndex = -1; + }, + }; + }, +});