Skip to content

Commit 5e01e6b

Browse files
macklinuSimenB
macklinu
authored and
SimenB
committed
feat(rule): add valid-describe rule (#57)
Fixes #18
1 parent 33bdfbf commit 5e01e6b

8 files changed

+342
-18
lines changed

‎README.md

+1
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ for more information about extending configuration files.
8888
| [prefer-to-be-null](docs/rules/prefer-to-be-null.md) | Suggest using `toBeNull()` | | ![fixable](https://img.shields.io/badge/-fixable-green.svg) |
8989
| [prefer-to-be-undefined](docs/rules/prefer-to-be-undefined.md) | Suggest using `toBeUndefined()` | | ![fixable](https://img.shields.io/badge/-fixable-green.svg) |
9090
| [prefer-expect-assertions](docs/rules/prefer-expect-assertions.md) | Suggest using `expect.assertions()` OR `expect.hasAssertions()` | | |
91+
| [valid-describe](docs/rules/valid-describe.md) | Enforce valid `describe()` callback | | |
9192
| [valid-expect](docs/rules/valid-expect.md) | Enforce valid `expect()` usage | ![recommended](https://img.shields.io/badge/-recommended-lightgrey.svg) | |
9293
| [valid-expect-in-promise](docs/rules/valid-expect-in-promise.md) | Enforce having return statement when testing with promises | | |
9394

‎docs/rules/valid-describe.md

+53
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
# Enforce valid `describe()` callback (valid-describe)
2+
3+
Using an improper `describe()` callback function can lead to unexpected test errors.
4+
5+
## Rule Details
6+
7+
This rule validates that the second parameter of a `describe()` function is a callback function. This callback function:
8+
9+
* should not be [async](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/async_function)
10+
* should not contain any parameters
11+
* should not contain any `return` statements
12+
13+
The following `describe` function aliases are also validated:
14+
15+
* `describe`
16+
* `describe.only`
17+
* `describe.skip`
18+
* `fdescribe`
19+
* `xdescribe`
20+
21+
The following patterns are considered warnings:
22+
23+
```js
24+
// Async callback functions are not allowed
25+
describe('myFunction()', async () => {
26+
// ...
27+
});
28+
29+
// Callback function parameters are not allowed
30+
describe('myFunction()', done => {
31+
// ...
32+
});
33+
34+
//
35+
describe('myFunction', () => {
36+
// No return statements are allowed in block of a callback function
37+
return Promise.resolve().then(() => {
38+
it('breaks', () => {
39+
throw new Error('Fail');
40+
});
41+
});
42+
});
43+
```
44+
45+
The following patterns are not considered warnings:
46+
47+
```js
48+
describe('myFunction()', () => {
49+
it('returns a truthy value', () => {
50+
expect(myFunction()).toBeTruthy();
51+
});
52+
});
53+
```

‎index.js

+2
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const noLargeSnapshots = require('./rules/no-large-snapshots');
77
const preferToBeNull = require('./rules/prefer-to-be-null');
88
const preferToBeUndefined = require('./rules/prefer-to-be-undefined');
99
const preferToHaveLength = require('./rules/prefer-to-have-length');
10+
const validDescribe = require('./rules/valid-describe');
1011
const validExpect = require('./rules/valid-expect');
1112
const preferExpectAssertions = require('./rules/prefer-expect-assertions');
1213
const validExpectInPromise = require('./rules/valid-expect-in-promise');
@@ -63,6 +64,7 @@ module.exports = {
6364
'prefer-to-be-null': preferToBeNull,
6465
'prefer-to-be-undefined': preferToBeUndefined,
6566
'prefer-to-have-length': preferToHaveLength,
67+
'valid-describe': validDescribe,
6668
'valid-expect': validExpect,
6769
'prefer-expect-assertions': preferExpectAssertions,
6870
'valid-expect-in-promise': validExpectInPromise,

‎rules/__tests__/valid-describe.test.js

+195
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
'use strict';
2+
3+
const RuleTester = require('eslint').RuleTester;
4+
const rules = require('../..').rules;
5+
6+
const ruleTester = new RuleTester({
7+
parserOptions: {
8+
ecmaVersion: 8,
9+
},
10+
});
11+
12+
ruleTester.run('valid-describe', rules['valid-describe'], {
13+
valid: [
14+
'describe("foo")',
15+
'describe("foo", function() {})',
16+
'describe("foo", () => {})',
17+
'xdescribe("foo", () => {})',
18+
'fdescribe("foo", () => {})',
19+
'describe.only("foo", () => {})',
20+
'describe.skip("foo", () => {})',
21+
`
22+
describe('foo', () => {
23+
it('bar', () => {
24+
return Promise.resolve(42).then(value => {
25+
expect(value).toBe(42)
26+
})
27+
})
28+
})
29+
`,
30+
`
31+
describe('foo', () => {
32+
it('bar', async () => {
33+
expect(await Promise.resolve(42)).toBe(42)
34+
})
35+
})
36+
`,
37+
],
38+
invalid: [
39+
{
40+
code: 'describe("foo", async () => {})',
41+
errors: [{ message: 'No async describe callback', line: 1, column: 17 }],
42+
},
43+
{
44+
code: 'describe("foo", async function () {})',
45+
errors: [{ message: 'No async describe callback', line: 1, column: 17 }],
46+
},
47+
{
48+
code: 'xdescribe("foo", async function () {})',
49+
errors: [{ message: 'No async describe callback', line: 1, column: 18 }],
50+
},
51+
{
52+
code: 'fdescribe("foo", async function () {})',
53+
errors: [{ message: 'No async describe callback', line: 1, column: 18 }],
54+
},
55+
{
56+
code: 'describe.only("foo", async function () {})',
57+
errors: [{ message: 'No async describe callback', line: 1, column: 22 }],
58+
},
59+
{
60+
code: 'describe.skip("foo", async function () {})',
61+
errors: [{ message: 'No async describe callback', line: 1, column: 22 }],
62+
},
63+
{
64+
code: `
65+
describe('sample case', () => {
66+
it('works', () => {
67+
expect(true).toEqual(true);
68+
});
69+
describe('async', async () => {
70+
await new Promise(setImmediate);
71+
it('breaks', () => {
72+
throw new Error('Fail');
73+
});
74+
});
75+
});`,
76+
errors: [{ message: 'No async describe callback', line: 6, column: 27 }],
77+
},
78+
{
79+
code: `
80+
describe('foo', function () {
81+
return Promise.resolve().then(() => {
82+
it('breaks', () => {
83+
throw new Error('Fail')
84+
})
85+
})
86+
})
87+
`,
88+
errors: [
89+
{
90+
message: 'Unexpected return statement in describe callback',
91+
line: 3,
92+
column: 9,
93+
},
94+
],
95+
},
96+
{
97+
code: `
98+
describe('foo', () => {
99+
return Promise.resolve().then(() => {
100+
it('breaks', () => {
101+
throw new Error('Fail')
102+
})
103+
})
104+
describe('nested', () => {
105+
return Promise.resolve().then(() => {
106+
it('breaks', () => {
107+
throw new Error('Fail')
108+
})
109+
})
110+
})
111+
})
112+
`,
113+
errors: [
114+
{
115+
message: 'Unexpected return statement in describe callback',
116+
line: 3,
117+
column: 9,
118+
},
119+
{
120+
message: 'Unexpected return statement in describe callback',
121+
line: 9,
122+
column: 11,
123+
},
124+
],
125+
},
126+
{
127+
code: `
128+
describe('foo', async () => {
129+
await something()
130+
it('does something')
131+
describe('nested', () => {
132+
return Promise.resolve().then(() => {
133+
it('breaks', () => {
134+
throw new Error('Fail')
135+
})
136+
})
137+
})
138+
})
139+
`,
140+
errors: [
141+
{
142+
message: 'No async describe callback',
143+
line: 2,
144+
column: 23,
145+
},
146+
{
147+
message: 'Unexpected return statement in describe callback',
148+
line: 6,
149+
column: 11,
150+
},
151+
],
152+
},
153+
{
154+
code: 'describe("foo", done => {})',
155+
errors: [
156+
{
157+
message: 'Unexpected argument(s) in describe callback',
158+
line: 1,
159+
column: 17,
160+
},
161+
],
162+
},
163+
{
164+
code: 'describe("foo", function (done) {})',
165+
errors: [
166+
{
167+
message: 'Unexpected argument(s) in describe callback',
168+
line: 1,
169+
column: 27,
170+
},
171+
],
172+
},
173+
{
174+
code: 'describe("foo", function (one, two, three) {})',
175+
errors: [
176+
{
177+
message: 'Unexpected argument(s) in describe callback',
178+
line: 1,
179+
column: 27,
180+
},
181+
],
182+
},
183+
{
184+
code: 'describe("foo", async function (done) {})',
185+
errors: [
186+
{ message: 'No async describe callback', line: 1, column: 17 },
187+
{
188+
message: 'Unexpected argument(s) in describe callback',
189+
line: 1,
190+
column: 33,
191+
},
192+
],
193+
},
194+
],
195+
});

‎rules/no-identical-title.js

+1-12
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,6 @@
11
'use strict';
22

3-
const describeAliases = Object.assign(Object.create(null), {
4-
describe: true,
5-
'describe.only': true,
6-
'describe.skip': true,
7-
fdescribe: true,
8-
xdescribe: true,
9-
});
3+
const isDescribe = require('./util').isDescribe;
104

115
const testCaseNames = Object.assign(Object.create(null), {
126
fit: true,
@@ -27,11 +21,6 @@ const getNodeName = node => {
2721
return node.name;
2822
};
2923

30-
const isDescribe = node =>
31-
node &&
32-
node.type === 'CallExpression' &&
33-
describeAliases[getNodeName(node.callee)];
34-
3524
const isTestCase = node =>
3625
node &&
3726
node.type === 'CallExpression' &&

‎rules/util.js

+23
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,27 @@ const argument = node => node.parent.parent.arguments[0];
8080

8181
const argument2 = node => node.parent.parent.parent.arguments[0];
8282

83+
const describeAliases = Object.assign(Object.create(null), {
84+
describe: true,
85+
'describe.only': true,
86+
'describe.skip': true,
87+
fdescribe: true,
88+
xdescribe: true,
89+
});
90+
91+
const getNodeName = node => {
92+
if (node.type === 'MemberExpression') {
93+
return node.object.name + '.' + node.property.name;
94+
}
95+
return node.name;
96+
};
97+
98+
const isDescribe = node =>
99+
node.type === 'CallExpression' && describeAliases[getNodeName(node.callee)];
100+
101+
const isFunction = node =>
102+
node.type === 'FunctionExpression' || node.type === 'ArrowFunctionExpression';
103+
83104
module.exports = {
84105
method: method,
85106
method2: method2,
@@ -95,4 +116,6 @@ module.exports = {
95116
expectNotToEqualCase: expectNotToEqualCase,
96117
expectToBeUndefinedCase: expectToBeUndefinedCase,
97118
expectNotToBeUndefinedCase: expectNotToBeUndefinedCase,
119+
isDescribe: isDescribe,
120+
isFunction: isFunction,
98121
};

‎rules/valid-describe.js

+63
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
'use strict';
2+
3+
const isDescribe = require('./util').isDescribe;
4+
const isFunction = require('./util').isFunction;
5+
6+
const isAsync = node => node.async;
7+
8+
const hasParams = node => node.params.length > 0;
9+
10+
const paramsLocation = params => {
11+
const first = params[0];
12+
const last = params[params.length - 1];
13+
return {
14+
start: {
15+
line: first.loc.start.line,
16+
column: first.loc.start.column,
17+
},
18+
end: {
19+
line: last.loc.end.line,
20+
column: last.loc.end.column,
21+
},
22+
};
23+
};
24+
25+
module.exports = {
26+
meta: {
27+
docs: {
28+
url:
29+
'https://github.com/jest-community/eslint-plugin-jest/blob/master/docs/rules/valid-describe.md',
30+
},
31+
},
32+
create(context) {
33+
return {
34+
CallExpression(node) {
35+
if (isDescribe(node)) {
36+
const callbackFunction = node.arguments[1];
37+
if (callbackFunction && isFunction(callbackFunction)) {
38+
if (isAsync(callbackFunction)) {
39+
context.report({
40+
message: 'No async describe callback',
41+
node: callbackFunction,
42+
});
43+
}
44+
if (hasParams(callbackFunction)) {
45+
context.report({
46+
message: 'Unexpected argument(s) in describe callback',
47+
loc: paramsLocation(callbackFunction.params),
48+
});
49+
}
50+
callbackFunction.body.body.forEach(node => {
51+
if (node.type === 'ReturnStatement') {
52+
context.report({
53+
message: 'Unexpected return statement in describe callback',
54+
node: node,
55+
});
56+
}
57+
});
58+
}
59+
}
60+
},
61+
};
62+
},
63+
};

0 commit comments

Comments
 (0)