Skip to content

Commit 005f23e

Browse files
G-RathG-Rath
committed
feat: create require-hook rule
1 parent 7a49c58 commit 005f23e

File tree

6 files changed

+361
-1
lines changed

6 files changed

+361
-1
lines changed

‎README.md

+1
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ installations requiring long-term consistency.
189189
| [prefer-to-contain](docs/rules/prefer-to-contain.md) | Suggest using `toContain()` | ![style][] | ![fixable][] |
190190
| [prefer-to-have-length](docs/rules/prefer-to-have-length.md) | Suggest using `toHaveLength()` | ![style][] | ![fixable][] |
191191
| [prefer-todo](docs/rules/prefer-todo.md) | Suggest using `test.todo` | | ![fixable][] |
192+
| [require-hook](docs/rules/require-hook.md) | Require setup and teardown code to be within a hook | | |
192193
| [require-to-throw-message](docs/rules/require-to-throw-message.md) | Require a message for `toThrow()` | | |
193194
| [require-top-level-describe](docs/rules/require-top-level-describe.md) | Require test cases and hooks to be inside a `describe` block | | |
194195
| [valid-describe](docs/rules/valid-describe.md) | Enforce valid `describe()` callback | ![recommended][] | |

‎docs/rules/require-hook.md

+149
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
# Require setup and teardown code to be within a hook (`require-hook`)
2+
3+
Often while writing tests you have some setup work that needs to happen before
4+
tests run, and you have some finishing work that needs to happen after tests
5+
run. Jest provides helper functions to handle this.
6+
7+
It's common when writing tests to need to perform setup work that needs to
8+
happen before tests run, and finishing work after tests run.
9+
10+
Because Jest executes all `describe` handlers in a test file _before_ it
11+
executes any of the actual tests, it's important to ensure setup and teardown
12+
work is done inside `before*` and `after*` handlers respectively, rather than
13+
inside the `describe` blocks.
14+
15+
## Rule details
16+
17+
This rule flags any expression that is either at the toplevel of a test file or
18+
directly within the body of a `describe`, _except_ for the following:
19+
20+
- `import` statements
21+
- `const` variables
22+
- `let` _declarations_
23+
- Types
24+
- Calls to the standard Jest globals
25+
26+
This rule flags any function calls within test files that are directly within
27+
the body of a `describe`, and suggests wrapping them in one of the four
28+
lifecycle hooks.
29+
30+
Here is a slightly contrived test file showcasing some common cases that would
31+
be flagged:
32+
33+
```js
34+
import { database, isCity } from '../database';
35+
import { Logger } from '../../../src/Logger';
36+
import { loadCities } from '../api';
37+
38+
jest.mock('../api');
39+
40+
const initializeCityDatabase = () => {
41+
database.addCity('Vienna');
42+
database.addCity('San Juan');
43+
database.addCity('Wellington');
44+
};
45+
46+
const clearCityDatabase = () => {
47+
database.clear();
48+
};
49+
50+
initializeCityDatabase();
51+
52+
test('that persists cities', () => {
53+
expect(database.cities.length).toHaveLength(3);
54+
});
55+
56+
test('city database has Vienna', () => {
57+
expect(isCity('Vienna')).toBeTruthy();
58+
});
59+
60+
test('city database has San Juan', () => {
61+
expect(isCity('San Juan')).toBeTruthy();
62+
});
63+
64+
describe('when loading cities from the api', () => {
65+
let consoleWarnSpy = jest.spyOn(console, 'warn');
66+
67+
loadCities.mockResolvedValue(['Wellington', 'London']);
68+
69+
it('does not duplicate cities', async () => {
70+
await database.loadCities();
71+
72+
expect(database.cities).toHaveLength(4);
73+
});
74+
75+
it('logs any duplicates', async () => {
76+
await database.loadCities();
77+
78+
expect(consoleWarnSpy).toHaveBeenCalledWith(
79+
'Ignored duplicate cities: Wellington',
80+
);
81+
});
82+
});
83+
84+
clearCityDatabase();
85+
```
86+
87+
Here is the same slightly contrived test file showcasing the same common cases
88+
but in ways that would be **not** flagged:
89+
90+
```js
91+
import { database, isCity } from '../database';
92+
import { Logger } from '../../../src/Logger';
93+
import { loadCities } from '../api';
94+
95+
jest.mock('../api');
96+
97+
const initializeCityDatabase = () => {
98+
database.addCity('Vienna');
99+
database.addCity('San Juan');
100+
database.addCity('Wellington');
101+
};
102+
103+
const clearCityDatabase = () => {
104+
database.clear();
105+
};
106+
107+
beforeEach(() => {
108+
initializeCityDatabase();
109+
});
110+
111+
test('that persists cities', () => {
112+
expect(database.cities.length).toHaveLength(3);
113+
});
114+
115+
test('city database has Vienna', () => {
116+
expect(isCity('Vienna')).toBeTruthy();
117+
});
118+
119+
test('city database has San Juan', () => {
120+
expect(isCity('San Juan')).toBeTruthy();
121+
});
122+
123+
describe('when loading cities from the api', () => {
124+
let consoleWarnSpy;
125+
126+
beforeEach(() => {
127+
consoleWarnSpy = jest.spyOn(console, 'warn');
128+
loadCities.mockResolvedValue(['Wellington', 'London']);
129+
});
130+
131+
it('does not duplicate cities', async () => {
132+
await database.loadCities();
133+
134+
expect(database.cities).toHaveLength(4);
135+
});
136+
137+
it('logs any duplicates', async () => {
138+
await database.loadCities();
139+
140+
expect(consoleWarnSpy).toHaveBeenCalledWith(
141+
'Ignored duplicate cities: Wellington',
142+
);
143+
});
144+
});
145+
146+
afterEach(() => {
147+
clearCityDatabase();
148+
});
149+
```

‎src/__tests__/__snapshots__/rules.test.ts.snap

+1
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ Object {
4747
"jest/prefer-to-contain": "error",
4848
"jest/prefer-to-have-length": "error",
4949
"jest/prefer-todo": "error",
50+
"jest/require-hook": "error",
5051
"jest/require-to-throw-message": "error",
5152
"jest/require-top-level-describe": "error",
5253
"jest/unbound-method": "error",

‎src/__tests__/rules.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { existsSync } from 'fs';
22
import { resolve } from 'path';
33
import plugin from '../';
44

5-
const numberOfRules = 48;
5+
const numberOfRules = 49;
66
const ruleNames = Object.keys(plugin.rules);
77
const deprecatedRules = Object.entries(plugin.rules)
88
.filter(([, rule]) => rule.meta.deprecated)

‎src/rules/__tests__/require-hook.test.ts

+146
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
import { TSESLint } from '@typescript-eslint/experimental-utils';
2+
import dedent from 'dedent';
3+
import resolveFrom from 'resolve-from';
4+
import rule from '../require-hook';
5+
6+
const ruleTester = new TSESLint.RuleTester({
7+
parser: resolveFrom(require.resolve('eslint'), 'espree'),
8+
parserOptions: {
9+
ecmaVersion: 2017,
10+
},
11+
});
12+
13+
ruleTester.run('require-hook', rule, {
14+
valid: [
15+
dedent`
16+
test('it', () => {
17+
//
18+
});
19+
`,
20+
dedent`
21+
describe('some tests', () => {
22+
it('is true', () => {
23+
expect(true).toBe(true);
24+
});
25+
});
26+
`,
27+
dedent`
28+
describe('some tests', () => {
29+
it('is true', () => {
30+
expect(true).toBe(true);
31+
});
32+
33+
describe('more tests', () => {
34+
it('is false', () => {
35+
expect(true).toBe(false);
36+
});
37+
});
38+
});
39+
`,
40+
dedent`
41+
describe('some tests', () => {
42+
let consoleLogSpy;
43+
44+
beforeEach(() => {
45+
consoleLogSpy = jest.spyOn(console, 'log');
46+
});
47+
48+
it('prints a message', () => {
49+
printMessage('hello world');
50+
51+
expect(consoleLogSpy).toHaveBeenCalledWith('hello world');
52+
});
53+
});
54+
`,
55+
dedent`
56+
describe('some tests', () => {
57+
beforeEach(() => {
58+
setup();
59+
});
60+
});
61+
`,
62+
dedent`
63+
beforeEach(() => {
64+
initializeCityDatabase();
65+
});
66+
67+
afterEach(() => {
68+
clearCityDatabase();
69+
});
70+
71+
test('city database has Vienna', () => {
72+
expect(isCity('Vienna')).toBeTruthy();
73+
});
74+
75+
test('city database has San Juan', () => {
76+
expect(isCity('San Juan')).toBeTruthy();
77+
});
78+
`,
79+
dedent`
80+
describe('cities', () => {
81+
beforeEach(() => {
82+
initializeCityDatabase();
83+
});
84+
85+
test('city database has Vienna', () => {
86+
expect(isCity('Vienna')).toBeTruthy();
87+
});
88+
89+
test('city database has San Juan', () => {
90+
expect(isCity('San Juan')).toBeTruthy();
91+
});
92+
93+
afterEach(() => {
94+
clearCityDatabase();
95+
});
96+
});
97+
`,
98+
],
99+
invalid: [
100+
{
101+
code: dedent`
102+
describe('some tests', () => {
103+
setup();
104+
});
105+
`,
106+
errors: [
107+
{
108+
messageId: 'useHook',
109+
line: 2,
110+
column: 3,
111+
},
112+
],
113+
},
114+
{
115+
code: dedent`
116+
describe('some tests', () => {
117+
setup();
118+
119+
it('is true', () => {
120+
expect(true).toBe(true);
121+
});
122+
123+
describe('more tests', () => {
124+
setup();
125+
126+
it('is false', () => {
127+
expect(true).toBe(false);
128+
});
129+
});
130+
});
131+
`,
132+
errors: [
133+
{
134+
messageId: 'useHook',
135+
line: 2,
136+
column: 3,
137+
},
138+
{
139+
messageId: 'useHook',
140+
line: 9,
141+
column: 5,
142+
},
143+
],
144+
},
145+
],
146+
});

‎src/rules/require-hook.ts

+63
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import { AST_NODE_TYPES } from '@typescript-eslint/experimental-utils';
2+
import {
3+
createRule,
4+
isDescribeCall,
5+
isFunction,
6+
isHook,
7+
isTestCaseCall,
8+
} from './utils';
9+
10+
export default createRule({
11+
name: __filename,
12+
meta: {
13+
docs: {
14+
category: 'Best Practices',
15+
description: 'Require setup and teardown code to be within a hook',
16+
recommended: false,
17+
},
18+
messages: {
19+
useHook: 'This should be done within a hook',
20+
},
21+
type: 'suggestion',
22+
schema: [],
23+
},
24+
defaultOptions: [],
25+
create(context) {
26+
return {
27+
CallExpression(node) {
28+
if (!isDescribeCall(node) || node.arguments.length < 2) {
29+
return;
30+
}
31+
32+
const [, testFn] = node.arguments;
33+
34+
if (
35+
!isFunction(testFn) ||
36+
testFn.body.type !== AST_NODE_TYPES.BlockStatement
37+
) {
38+
return;
39+
}
40+
41+
for (const nod of testFn.body.body) {
42+
if (
43+
nod.type === AST_NODE_TYPES.ExpressionStatement &&
44+
nod.expression.type === AST_NODE_TYPES.CallExpression
45+
) {
46+
if (
47+
isDescribeCall(nod.expression) ||
48+
isTestCaseCall(nod.expression) ||
49+
isHook(nod.expression)
50+
) {
51+
return;
52+
}
53+
54+
context.report({
55+
node: nod.expression,
56+
messageId: 'useHook',
57+
});
58+
}
59+
}
60+
},
61+
};
62+
},
63+
});

0 commit comments

Comments
 (0)