Skip to content

Commit

Permalink
feat(rules): prefer-hooks-on-top (#425)
Browse files Browse the repository at this point in the history 
* feat(rules): prefer-hooks-on-top

* chore: review changes

* fix: failing test
  • Loading branch information
@Mark1626 @G-Rath
Mark1626 authored and G-Rath committed Oct 12, 2019
1 parent 7017fc7 commit ad3fc06
Show file tree
Hide file tree
Showing 5 changed files with 325 additions and 1 deletion.
2 changes: 2 additions & 0 deletions README.md
View file
Expand Up @@ -136,6 +136,7 @@ installations requiring long-term consistency.
| [no-try-expect][] | Prevent `catch` assertions in tests | | |
| [prefer-called-with][] | Suggest using `toBeCalledWith()` OR `toHaveBeenCalledWith()` | | |
| [prefer-expect-assertions][] | Suggest using `expect.assertions()` OR `expect.hasAssertions()` | | |
| [prefer-hooks-on-top][] | Suggest to have all hooks at top-level before tests | | |
| [prefer-inline-snapshots][] | Suggest using `toMatchInlineSnapshot()` | | ![fixable-green][] |
| [prefer-spy-on][] | Suggest using `jest.spyOn()` | | ![fixable-green][] |
| [prefer-strict-equal][] | Suggest using `toStrictEqual()` | | ![fixable-green][] |
Expand Down Expand Up @@ -191,6 +192,7 @@ https://github.com/dangreenisrael/eslint-plugin-jest-formatting
[prefer-called-with]: docs/rules/prefer-called-with.md
[prefer-expect-assertions]: docs/rules/prefer-expect-assertions.md
[prefer-inline-snapshots]: docs/rules/prefer-inline-snapshots.md
[prefer-hooks-on-top]: docs/rules/prefer-hooks-on-top.md
[prefer-spy-on]: docs/rules/prefer-spy-on.md
[prefer-strict-equal]: docs/rules/prefer-strict-equal.md
[prefer-to-be-null]: docs/rules/prefer-to-be-null.md
Expand Down
96 changes: 96 additions & 0 deletions docs/rules/prefer-hooks-on-top.md
View file
@@ -0,0 +1,96 @@
# Suggest to have all hooks at top-level before tests (prefer-hooks-on-top)

All hooks should be defined before the start of the tests

## Rule Details

Examples of **incorrect** code for this rule

```js
/* eslint jest/prefer-hooks-on-top: "error" */

describe("foo" () => {
beforeEach(() => {
//some hook code
});
test("bar" () => {
some_fn();
});
beforeAll(() => {
//some hook code
});
test("bar" () => {
some_fn();
});
});

// Nested describe scenario
describe("foo" () => {
beforeAll(() => {
//some hook code
});
test("bar" () => {
some_fn();
});
describe("inner_foo" () => {
beforeEach(() => {
//some hook code
});
test("inner bar" () => {
some_fn();
});
test("inner bar" () => {
some_fn();
});
beforeAll(() => {
//some hook code
});
afterAll(() => {
//some hook code
});
test("inner bar" () => {
some_fn();
});
});
});
```

Examples of **correct** code for this rule

```js
/* eslint jest/prefer-hooks-on-top: "error" */

describe("foo" () => {
beforeEach(() => {
//some hook code
});

// Not affected by rule
someSetup();

afterEach(() => {
//some hook code
});
test("bar" () => {
some_fn();
});
});

// Nested describe scenario
describe("foo" () => {
beforeEach(() => {
//some hook code
});
test("bar" () => {
some_fn();
});
describe("inner_foo" () => {
beforeEach(() => {
//some hook code
});
test("inner bar" () => {
some_fn();
});
});
});
```
2 changes: 1 addition & 1 deletion src/__tests__/rules.test.ts
View file
Expand Up @@ -3,7 +3,7 @@ import { resolve } from 'path';
import plugin from '../';

const ruleNames = Object.keys(plugin.rules);
const numberOfRules = 39;
const numberOfRules = 40;

describe('rules', () => {
it('should have a corresponding doc for each rule', () => {
Expand Down
188 changes: 188 additions & 0 deletions src/rules/__tests__/prefer-hooks-on-top.test.ts
View file
@@ -0,0 +1,188 @@
import { TSESLint } from '@typescript-eslint/experimental-utils';
import rule from '../prefer-hooks-on-top';

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

ruleTester.run('basic describe block', rule, {
valid: [
`describe("foo", () => {
beforeEach(() => {
});
someSetupFn();
afterEach(() => {
});
test("bar", () => {
some_fn();
});
});`,
],
invalid: [
{
code: `describe("foo", () => {
beforeEach(() => {
});
test("bar", () => {
some_fn();
});
beforeAll(() => {
});
test("bar", () => {
some_fn();
});
});`,
errors: [
{
messageId: 'noHookOnTop',
column: 11,
line: 7,
},
],
},
],
});

ruleTester.run('multiple describe blocks', rule, {
valid: [
`describe.skip("foo", () => {
beforeEach(() => {
});
beforeAll(() => {
});
test("bar", () => {
some_fn();
});
});
describe("foo", () => {
beforeEach(() => {
});
test("bar", () => {
some_fn();
});
});`,
],

invalid: [
{
code: `describe.skip("foo", () => {
beforeEach(() => {
});
test("bar", () => {
some_fn();
});
beforeAll(() => {
});
test("bar", () => {
some_fn();
});
});
describe("foo", () => {
beforeEach(() => {
});
beforeEach(() => {
});
beforeAll(() => {
});
test("bar", () => {
some_fn();
});
});
describe("foo", () => {
test("bar", () => {
some_fn();
});
beforeEach(() => {
});
beforeEach(() => {
});
beforeAll(() => {
});
});`,
errors: [
{
messageId: 'noHookOnTop',
column: 11,
line: 7,
},
{
messageId: 'noHookOnTop',
column: 11,
line: 28,
},
{
messageId: 'noHookOnTop',
column: 11,
line: 30,
},
{
messageId: 'noHookOnTop',
column: 11,
line: 32,
},
],
},
],
});

ruleTester.run('nested describe blocks', rule, {
valid: [
`describe("foo", () => {
beforeEach(() => {
});
test("bar", () => {
some_fn();
});
describe("inner_foo" , () => {
beforeEach(() => {
});
test("inner bar", () => {
some_fn();
});
});
});`,
],

invalid: [
{
code: `describe("foo", () => {
beforeAll(() => {
});
test("bar", () => {
some_fn();
});
describe("inner_foo" , () => {
beforeEach(() => {
});
test("inner bar", () => {
some_fn();
});
test("inner bar", () => {
some_fn();
});
beforeAll(() => {
});
afterAll(() => {
});
test("inner bar", () => {
some_fn();
});
});
});`,
errors: [
{
messageId: 'noHookOnTop',
column: 13,
line: 16,
},
{
messageId: 'noHookOnTop',
column: 13,
line: 18,
},
],
},
],
});
38 changes: 38 additions & 0 deletions src/rules/prefer-hooks-on-top.ts
View file
@@ -0,0 +1,38 @@
import { createRule, isHook, isTestCase } from './utils';

export default createRule({
name: __filename,
meta: {
docs: {
category: 'Best Practices',
description: 'Suggest to have all hooks at top level',
recommended: false,
},
messages: {
noHookOnTop: 'Move all hooks before test cases',
},
schema: [],
type: 'suggestion',
},
defaultOptions: [],
create(context) {
const hooksContext = [false];
return {
CallExpression(node) {
if (!isHook(node) && isTestCase(node)) {
hooksContext[hooksContext.length - 1] = true;
}
if (hooksContext[hooksContext.length - 1] && isHook(node)) {
context.report({
messageId: 'noHookOnTop',
node,
});
}
hooksContext.push(false);
},
'CallExpression:exit'() {
hooksContext.pop();
},
};
},
});

0 comments on commit ad3fc06

Please sign in to comment.