From 1b52bd7bcb93a5d4071acddc09109308bb208d8d Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 26 Dec 2021 13:20:25 +1300 Subject: [PATCH 1/9] feat: create `prefer-hooks-in-order` rule --- docs/rules/prefer-hooks-in-order.md | 133 ++++++ .../__tests__/prefer-hooks-in-order.test.ts | 393 ++++++++++++++++++ src/rules/prefer-hooks-in-order.ts | 69 +++ 3 files changed, 595 insertions(+) create mode 100644 docs/rules/prefer-hooks-in-order.md create mode 100644 src/rules/__tests__/prefer-hooks-in-order.test.ts create mode 100644 src/rules/prefer-hooks-in-order.ts diff --git a/docs/rules/prefer-hooks-in-order.md b/docs/rules/prefer-hooks-in-order.md new file mode 100644 index 000000000..d8b68fa70 --- /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/rules/__tests__/prefer-hooks-in-order.test.ts b/src/rules/__tests__/prefer-hooks-in-order.test.ts new file mode 100644 index 000000000..3ec2d8528 --- /dev/null +++ b/src/rules/__tests__/prefer-hooks-in-order.test.ts @@ -0,0 +1,393 @@ +import { TSESLint } from '@typescript-eslint/experimental-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(() => {})', + 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` + 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(() => {}); + }); + `, + ], + invalid: [ + { + code: dedent` + const withDatabase = () => { + afterAll(() => { + removeMyDatabase(); + }); + beforeAll(() => { + createMyDatabase(); + }); + }; + `, + errors: [ + { + messageId: 'reorderHooks', + data: { currentHook: 'beforeAll', previousHook: 'afterAll' }, + column: 1, + line: 2, + }, + ], + }, + { + code: dedent` + afterAll(() => { + removeMyDatabase(); + }); + beforeAll(() => { + createMyDatabase(); + }); + `, + errors: [ + { + messageId: 'reorderHooks', + data: { currentHook: 'beforeAll', previousHook: 'afterAll' }, + column: 1, + line: 2, + }, + ], + }, + { + 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(() => {}); + 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, + }, + ], + }, + ], +}); diff --git a/src/rules/prefer-hooks-in-order.ts b/src/rules/prefer-hooks-in-order.ts new file mode 100644 index 000000000..b8e4b1f6e --- /dev/null +++ b/src/rules/prefer-hooks-in-order.ts @@ -0,0 +1,69 @@ +import { createRule, isDescribeCall, isHook } from './utils'; + +const HooksOrder = [ + 'beforeAll', + 'beforeEach', + 'afterEach', + 'afterAll', +] as const; + +type HookName = typeof HooksOrder[number]; + +export default createRule({ + name: __filename, + meta: { + docs: { + category: 'Best Practices', + description: 'Suggest having hooks before any test cases', + recommended: false, + }, + messages: { + reorderHooks: `\`{{ currentHook }}\` hooks should be before any \`{{ previousHook }}\` hooks`, + }, + schema: [], + type: 'suggestion', + }, + defaultOptions: [], + create(context) { + const hookContexts: Array = [null]; + + return { + CallExpression(node) { + if (!isHook(node)) { + hookContexts.unshift(null); + + return; + } + + const { name: currentHook } = node.callee; + const [previousHook] = hookContexts; + + if (previousHook) { + if (currentHook === previousHook) { + return; + } + + const previousHookIndex = HooksOrder.indexOf(previousHook); + const currentHookIndex = HooksOrder.indexOf(currentHook); + + if (previousHookIndex <= currentHookIndex) { + return; + } + + context.report({ + messageId: 'reorderHooks', + data: { currentHook, previousHook }, + node, + }); + } + + hookContexts[0] = currentHook; + }, + 'CallExpression:exit'(node) { + if (!isHook(node)) { + hookContexts.shift(); + } + }, + }; + }, +}); From 26a77f0de12a213d4b9b150b3ec320a749d321d1 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 26 Dec 2021 15:24:29 +1300 Subject: [PATCH 2/9] wip --- src/rules/prefer-hooks-in-order.ts | 32 +++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/src/rules/prefer-hooks-in-order.ts b/src/rules/prefer-hooks-in-order.ts index b8e4b1f6e..a249859c7 100644 --- a/src/rules/prefer-hooks-in-order.ts +++ b/src/rules/prefer-hooks-in-order.ts @@ -1,4 +1,4 @@ -import { createRule, isDescribeCall, isHook } from './utils'; +import { createRule, isHook } from './utils'; const HooksOrder = [ 'beforeAll', @@ -26,11 +26,31 @@ export default createRule({ defaultOptions: [], create(context) { const hookContexts: Array = [null]; + // const hookContexts = [false]; + // let previousHook: HookName | null = null; return { CallExpression(node) { + // on enter: + // if "is hook: + // if "have seen a previous hook": + // if "previous hook name is different": + // - compare hook orders + // - update previous hook name + // else: + // - mark previous hook name as null + // + // ---------- + // if "hook": + // if: "have seen a previous hook" + // if: previous hook name is different + // - check hook order + // - update previous hook name + // else: + // - mark previous hook name as null if (!isHook(node)) { hookContexts.unshift(null); + // hookContexts.unshift(true); return; } @@ -57,12 +77,14 @@ export default createRule({ }); } - hookContexts[0] = currentHook; + // previousHook = currentHook; + hookContexts.unshift(currentHook); + // hookContexts[0] = currentHook; }, 'CallExpression:exit'(node) { - if (!isHook(node)) { - hookContexts.shift(); - } + // if (!isHook(node)) { + hookContexts.shift(); + // } }, }; }, From fc6aeb86301475021d90a3f03c5b9e4f2343304e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?A=C3=A4ron=20Trippaers?= Date: Thu, 5 May 2022 22:17:44 +0200 Subject: [PATCH 3/9] Implement prefer-hooks-in-order logic --- .../__tests__/prefer-hooks-in-order.test.ts | 6 +- src/rules/prefer-hooks-in-order.ts | 83 ++++++++----------- 2 files changed, 36 insertions(+), 53 deletions(-) diff --git a/src/rules/__tests__/prefer-hooks-in-order.test.ts b/src/rules/__tests__/prefer-hooks-in-order.test.ts index 3ec2d8528..b42db3879 100644 --- a/src/rules/__tests__/prefer-hooks-in-order.test.ts +++ b/src/rules/__tests__/prefer-hooks-in-order.test.ts @@ -189,8 +189,8 @@ ruleTester.run('prefer-hooks-in-order', rule, { { messageId: 'reorderHooks', data: { currentHook: 'beforeAll', previousHook: 'afterAll' }, - column: 1, - line: 2, + column: 3, + line: 5, }, ], }, @@ -208,7 +208,7 @@ ruleTester.run('prefer-hooks-in-order', rule, { messageId: 'reorderHooks', data: { currentHook: 'beforeAll', previousHook: 'afterAll' }, column: 1, - line: 2, + line: 4, }, ], }, diff --git a/src/rules/prefer-hooks-in-order.ts b/src/rules/prefer-hooks-in-order.ts index a249859c7..9f9bc6055 100644 --- a/src/rules/prefer-hooks-in-order.ts +++ b/src/rules/prefer-hooks-in-order.ts @@ -7,8 +7,6 @@ const HooksOrder = [ 'afterAll', ] as const; -type HookName = typeof HooksOrder[number]; - export default createRule({ name: __filename, meta: { @@ -25,66 +23,51 @@ export default createRule({ }, defaultOptions: [], create(context) { - const hookContexts: Array = [null]; - // const hookContexts = [false]; - // let previousHook: HookName | null = null; + let previousHookIndex = -1; + let inHook = false; return { CallExpression(node) { - // on enter: - // if "is hook: - // if "have seen a previous hook": - // if "previous hook name is different": - // - compare hook orders - // - update previous hook name - // else: - // - mark previous hook name as null - // - // ---------- - // if "hook": - // if: "have seen a previous hook" - // if: previous hook name is different - // - check hook order - // - update previous hook name - // else: - // - mark previous hook name as null - if (!isHook(node)) { - hookContexts.unshift(null); - // hookContexts.unshift(true); - + if (inHook) { + // Ignore everything that is passed into a hook return; } - const { name: currentHook } = node.callee; - const [previousHook] = hookContexts; - - if (previousHook) { - if (currentHook === previousHook) { - return; - } + if (!isHook(node)) { + // Reset the previousHookIndex when encountering something different from a hook + previousHookIndex = -1; + } - const previousHookIndex = HooksOrder.indexOf(previousHook); - const currentHookIndex = HooksOrder.indexOf(currentHook); + if (isHook(node)) { + inHook = true; + const currentHook = node.callee.name; + const currentHookIndex = HooksOrder.findIndex( + name => name === currentHook, + ); - if (previousHookIndex <= currentHookIndex) { - return; + if (currentHookIndex < previousHookIndex) { + context.report({ + messageId: 'reorderHooks', + node, + data: { + previousHook: HooksOrder[previousHookIndex], + currentHook, + }, + }); + } else { + previousHookIndex = currentHookIndex; } - - context.report({ - messageId: 'reorderHooks', - data: { currentHook, previousHook }, - node, - }); } - - // previousHook = currentHook; - hookContexts.unshift(currentHook); - // hookContexts[0] = currentHook; }, 'CallExpression:exit'(node) { - // if (!isHook(node)) { - hookContexts.shift(); - // } + if (!isHook(node) && !inHook) { + // Reset the previousHookIndex when encountering something different from a hook + previousHookIndex = -1; + } + + if (isHook(node)) { + inHook = false; + } }, }; }, From 339ffa12de821dfc959da4e9a90631d4c9f79c03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?A=C3=A4ron=20Trippaers?= Date: Thu, 5 May 2022 22:44:08 +0200 Subject: [PATCH 4/9] Fix all rules tests --- src/__tests__/__snapshots__/rules.test.ts.snap | 1 + src/__tests__/rules.test.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/__tests__/__snapshots__/rules.test.ts.snap b/src/__tests__/__snapshots__/rules.test.ts.snap index ce45d2b14..c4dd93476 100644 --- a/src/__tests__/__snapshots__/rules.test.ts.snap +++ b/src/__tests__/__snapshots__/rules.test.ts.snap @@ -37,6 +37,7 @@ Object { "jest/prefer-called-with": "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-spy-on": "error", diff --git a/src/__tests__/rules.test.ts b/src/__tests__/rules.test.ts index dc6ac6b4e..d37e1bd4a 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 = 43; +const numberOfRules = 44; const ruleNames = Object.keys(plugin.rules); const deprecatedRules = Object.entries(plugin.rules) .filter(([, rule]) => rule.meta.deprecated) From ae843fa7e5db6361881fc948c61e3c1add2327e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?A=C3=A4ron=20Trippaers?= Date: Sat, 14 May 2022 22:06:34 +0200 Subject: [PATCH 5/9] ci: run prettier and docs generation, fix test description --- README.md | 1 + docs/rules/prefer-hooks-in-order.md | 2 +- src/rules/prefer-hooks-in-order.ts | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) 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 index d8b68fa70..a2d580b07 100644 --- a/docs/rules/prefer-hooks-in-order.md +++ b/docs/rules/prefer-hooks-in-order.md @@ -22,7 +22,7 @@ describe('foo', () => { beforeEach(() => { seedMyDatabase(); }); - + beforeAll(() => { createMyDatabase(); }); diff --git a/src/rules/prefer-hooks-in-order.ts b/src/rules/prefer-hooks-in-order.ts index 9f9bc6055..5cf1e96ea 100644 --- a/src/rules/prefer-hooks-in-order.ts +++ b/src/rules/prefer-hooks-in-order.ts @@ -12,7 +12,7 @@ export default createRule({ meta: { docs: { category: 'Best Practices', - description: 'Suggest having hooks before any test cases', + description: 'Prefer having hooks in a consistent order', recommended: false, }, messages: { From 9ad5080fbda436d3774ed1318dc8dc463d43c294 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?A=C3=A4ron=20Trippaers?= Date: Mon, 16 May 2022 20:22:05 +0200 Subject: [PATCH 6/9] test: Add examples to the tests --- .../__tests__/prefer-hooks-in-order.test.ts | 118 ++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/src/rules/__tests__/prefer-hooks-in-order.test.ts b/src/rules/__tests__/prefer-hooks-in-order.test.ts index 9290e5fbb..0d0220b19 100644 --- a/src/rules/__tests__/prefer-hooks-in-order.test.ts +++ b/src/rules/__tests__/prefer-hooks-in-order.test.ts @@ -172,6 +172,57 @@ ruleTester.run('prefer-hooks-in-order', rule, { 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(); + }); + }); + `, ], invalid: [ { @@ -389,5 +440,72 @@ ruleTester.run('prefer-hooks-in-order', rule, { }, ], }, + { + 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, + }, + ], + }, ], }); From 01380ab97ea65c5c5c621dfe20ababa97c75039f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?A=C3=A4ron=20Trippaers?= Date: Mon, 16 May 2022 20:59:16 +0200 Subject: [PATCH 7/9] test: Add some more complicated tests --- .../__tests__/prefer-hooks-in-order.test.ts | 179 ++++++++++++++++++ 1 file changed, 179 insertions(+) diff --git a/src/rules/__tests__/prefer-hooks-in-order.test.ts b/src/rules/__tests__/prefer-hooks-in-order.test.ts index 0d0220b19..ca55d2e22 100644 --- a/src/rules/__tests__/prefer-hooks-in-order.test.ts +++ b/src/rules/__tests__/prefer-hooks-in-order.test.ts @@ -16,6 +16,7 @@ ruleTester.run('prefer-hooks-in-order', rule, { 'beforeEach(() => {})', 'afterEach(() => {})', 'afterAll(() => {})', + 'describe(() => {})', dedent` beforeAll(() => {}); beforeEach(() => {}); @@ -53,6 +54,10 @@ ruleTester.run('prefer-hooks-in-order', rule, { afterEach(() => {}); afterAll(() => {}); `, + dedent` + beforeAll(() => {}); + beforeAll(() => {}); + `, dedent` describe('my test', () => { afterEach(() => {}); @@ -223,6 +228,49 @@ ruleTester.run('prefer-hooks-in-order', rule, { }); }); `, + 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: [ { @@ -335,6 +383,22 @@ ruleTester.run('prefer-hooks-in-order', rule, { }, ], }, + { + 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(() => {}); @@ -440,6 +504,121 @@ ruleTester.run('prefer-hooks-in-order', rule, { }, ], }, + { + 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', () => { From a50896a74986e47554f5ffd3a1d383a106b259d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?A=C3=A4ron=20Trippaers?= Date: Tue, 17 May 2022 12:09:34 +0200 Subject: [PATCH 8/9] ci: change isHook to isHookCall --- src/rules/prefer-hooks-in-order.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/rules/prefer-hooks-in-order.ts b/src/rules/prefer-hooks-in-order.ts index 5cf1e96ea..9a94df58f 100644 --- a/src/rules/prefer-hooks-in-order.ts +++ b/src/rules/prefer-hooks-in-order.ts @@ -1,4 +1,4 @@ -import { createRule, isHook } from './utils'; +import { createRule, isHookCall } from './utils'; const HooksOrder = [ 'beforeAll', @@ -33,12 +33,12 @@ export default createRule({ return; } - if (!isHook(node)) { + if (!isHookCall(node, context.getScope())) { // Reset the previousHookIndex when encountering something different from a hook previousHookIndex = -1; } - if (isHook(node)) { + if (isHookCall(node, context.getScope())) { inHook = true; const currentHook = node.callee.name; const currentHookIndex = HooksOrder.findIndex( @@ -60,12 +60,12 @@ export default createRule({ } }, 'CallExpression:exit'(node) { - if (!isHook(node) && !inHook) { + if (!isHookCall(node, context.getScope()) && !inHook) { // Reset the previousHookIndex when encountering something different from a hook previousHookIndex = -1; } - if (isHook(node)) { + if (isHookCall(node, context.getScope())) { inHook = false; } }, From 57b9f1aa81548e9fe63eb875ca38518df79c1c42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?A=C3=A4ron=20Trippaers?= Date: Wed, 18 May 2022 08:13:14 +0200 Subject: [PATCH 9/9] feat: use early returns consistently --- src/rules/prefer-hooks-in-order.ts | 50 ++++++++++++++++-------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/src/rules/prefer-hooks-in-order.ts b/src/rules/prefer-hooks-in-order.ts index 9a94df58f..06508af1e 100644 --- a/src/rules/prefer-hooks-in-order.ts +++ b/src/rules/prefer-hooks-in-order.ts @@ -36,38 +36,42 @@ export default createRule({ if (!isHookCall(node, context.getScope())) { // Reset the previousHookIndex when encountering something different from a hook previousHookIndex = -1; + + return; } - if (isHookCall(node, context.getScope())) { - inHook = true; - const currentHook = node.callee.name; - const currentHookIndex = HooksOrder.findIndex( - name => name === currentHook, - ); + 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, - }, - }); - } else { - previousHookIndex = currentHookIndex; - } + 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) { - // Reset the previousHookIndex when encountering something different from a hook - previousHookIndex = -1; - } - if (isHookCall(node, context.getScope())) { inHook = false; + + return; + } + + if (inHook) { + return; } + + // Reset the previousHookIndex when encountering something different from a hook + previousHookIndex = -1; }, }; },