Skip to content

Commit

Permalink
feat(rule): add valid-describe rule (#57)
Browse files Browse the repository at this point in the history
Fixes #18
  • Loading branch information
macklinu authored and SimenB committed Feb 9, 2018
1 parent 33bdfbf commit 5e01e6b
Show file tree
Hide file tree
Showing 8 changed files with 342 additions and 18 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ for more information about extending configuration files.
| [prefer-to-be-null](docs/rules/prefer-to-be-null.md) | Suggest using `toBeNull()` | | ![fixable](https://img.shields.io/badge/-fixable-green.svg) |
| [prefer-to-be-undefined](docs/rules/prefer-to-be-undefined.md) | Suggest using `toBeUndefined()` | | ![fixable](https://img.shields.io/badge/-fixable-green.svg) |
| [prefer-expect-assertions](docs/rules/prefer-expect-assertions.md) | Suggest using `expect.assertions()` OR `expect.hasAssertions()` | | |
| [valid-describe](docs/rules/valid-describe.md) | Enforce valid `describe()` callback | | |
| [valid-expect](docs/rules/valid-expect.md) | Enforce valid `expect()` usage | ![recommended](https://img.shields.io/badge/-recommended-lightgrey.svg) | |
| [valid-expect-in-promise](docs/rules/valid-expect-in-promise.md) | Enforce having return statement when testing with promises | | |

Expand Down
53 changes: 53 additions & 0 deletions docs/rules/valid-describe.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# Enforce valid `describe()` callback (valid-describe)

Using an improper `describe()` callback function can lead to unexpected test errors.

## Rule Details

This rule validates that the second parameter of a `describe()` function is a callback function. This callback function:

* should not be [async](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/async_function)
* should not contain any parameters
* should not contain any `return` statements

The following `describe` function aliases are also validated:

* `describe`
* `describe.only`
* `describe.skip`
* `fdescribe`
* `xdescribe`

The following patterns are considered warnings:

```js
// Async callback functions are not allowed
describe('myFunction()', async () => {
// ...
});

// Callback function parameters are not allowed
describe('myFunction()', done => {
// ...
});

//
describe('myFunction', () => {
// No return statements are allowed in block of a callback function
return Promise.resolve().then(() => {
it('breaks', () => {
throw new Error('Fail');
});
});
});
```

The following patterns are not considered warnings:

```js
describe('myFunction()', () => {
it('returns a truthy value', () => {
expect(myFunction()).toBeTruthy();
});
});
```
2 changes: 2 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const noLargeSnapshots = require('./rules/no-large-snapshots');
const preferToBeNull = require('./rules/prefer-to-be-null');
const preferToBeUndefined = require('./rules/prefer-to-be-undefined');
const preferToHaveLength = require('./rules/prefer-to-have-length');
const validDescribe = require('./rules/valid-describe');
const validExpect = require('./rules/valid-expect');
const preferExpectAssertions = require('./rules/prefer-expect-assertions');
const validExpectInPromise = require('./rules/valid-expect-in-promise');
Expand Down Expand Up @@ -63,6 +64,7 @@ module.exports = {
'prefer-to-be-null': preferToBeNull,
'prefer-to-be-undefined': preferToBeUndefined,
'prefer-to-have-length': preferToHaveLength,
'valid-describe': validDescribe,
'valid-expect': validExpect,
'prefer-expect-assertions': preferExpectAssertions,
'valid-expect-in-promise': validExpectInPromise,
Expand Down
195 changes: 195 additions & 0 deletions rules/__tests__/valid-describe.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,195 @@
'use strict';

const RuleTester = require('eslint').RuleTester;
const rules = require('../..').rules;

const ruleTester = new RuleTester({
parserOptions: {
ecmaVersion: 8,
},
});

ruleTester.run('valid-describe', rules['valid-describe'], {
valid: [
'describe("foo")',
'describe("foo", function() {})',
'describe("foo", () => {})',
'xdescribe("foo", () => {})',
'fdescribe("foo", () => {})',
'describe.only("foo", () => {})',
'describe.skip("foo", () => {})',
`
describe('foo', () => {
it('bar', () => {
return Promise.resolve(42).then(value => {
expect(value).toBe(42)
})
})
})
`,
`
describe('foo', () => {
it('bar', async () => {
expect(await Promise.resolve(42)).toBe(42)
})
})
`,
],
invalid: [
{
code: 'describe("foo", async () => {})',
errors: [{ message: 'No async describe callback', line: 1, column: 17 }],
},
{
code: 'describe("foo", async function () {})',
errors: [{ message: 'No async describe callback', line: 1, column: 17 }],
},
{
code: 'xdescribe("foo", async function () {})',
errors: [{ message: 'No async describe callback', line: 1, column: 18 }],
},
{
code: 'fdescribe("foo", async function () {})',
errors: [{ message: 'No async describe callback', line: 1, column: 18 }],
},
{
code: 'describe.only("foo", async function () {})',
errors: [{ message: 'No async describe callback', line: 1, column: 22 }],
},
{
code: 'describe.skip("foo", async function () {})',
errors: [{ message: 'No async describe callback', line: 1, column: 22 }],
},
{
code: `
describe('sample case', () => {
it('works', () => {
expect(true).toEqual(true);
});
describe('async', async () => {
await new Promise(setImmediate);
it('breaks', () => {
throw new Error('Fail');
});
});
});`,
errors: [{ message: 'No async describe callback', line: 6, column: 27 }],
},
{
code: `
describe('foo', function () {
return Promise.resolve().then(() => {
it('breaks', () => {
throw new Error('Fail')
})
})
})
`,
errors: [
{
message: 'Unexpected return statement in describe callback',
line: 3,
column: 9,
},
],
},
{
code: `
describe('foo', () => {
return Promise.resolve().then(() => {
it('breaks', () => {
throw new Error('Fail')
})
})
describe('nested', () => {
return Promise.resolve().then(() => {
it('breaks', () => {
throw new Error('Fail')
})
})
})
})
`,
errors: [
{
message: 'Unexpected return statement in describe callback',
line: 3,
column: 9,
},
{
message: 'Unexpected return statement in describe callback',
line: 9,
column: 11,
},
],
},
{
code: `
describe('foo', async () => {
await something()
it('does something')
describe('nested', () => {
return Promise.resolve().then(() => {
it('breaks', () => {
throw new Error('Fail')
})
})
})
})
`,
errors: [
{
message: 'No async describe callback',
line: 2,
column: 23,
},
{
message: 'Unexpected return statement in describe callback',
line: 6,
column: 11,
},
],
},
{
code: 'describe("foo", done => {})',
errors: [
{
message: 'Unexpected argument(s) in describe callback',
line: 1,
column: 17,
},
],
},
{
code: 'describe("foo", function (done) {})',
errors: [
{
message: 'Unexpected argument(s) in describe callback',
line: 1,
column: 27,
},
],
},
{
code: 'describe("foo", function (one, two, three) {})',
errors: [
{
message: 'Unexpected argument(s) in describe callback',
line: 1,
column: 27,
},
],
},
{
code: 'describe("foo", async function (done) {})',
errors: [
{ message: 'No async describe callback', line: 1, column: 17 },
{
message: 'Unexpected argument(s) in describe callback',
line: 1,
column: 33,
},
],
},
],
});
13 changes: 1 addition & 12 deletions rules/no-identical-title.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
'use strict';

const describeAliases = Object.assign(Object.create(null), {
describe: true,
'describe.only': true,
'describe.skip': true,
fdescribe: true,
xdescribe: true,
});
const isDescribe = require('./util').isDescribe;

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

const isDescribe = node =>
node &&
node.type === 'CallExpression' &&
describeAliases[getNodeName(node.callee)];

const isTestCase = node =>
node &&
node.type === 'CallExpression' &&
Expand Down
23 changes: 23 additions & 0 deletions rules/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,27 @@ const argument = node => node.parent.parent.arguments[0];

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

const describeAliases = Object.assign(Object.create(null), {
describe: true,
'describe.only': true,
'describe.skip': true,
fdescribe: true,
xdescribe: true,
});

const getNodeName = node => {
if (node.type === 'MemberExpression') {
return node.object.name + '.' + node.property.name;
}
return node.name;
};

const isDescribe = node =>
node.type === 'CallExpression' && describeAliases[getNodeName(node.callee)];

const isFunction = node =>
node.type === 'FunctionExpression' || node.type === 'ArrowFunctionExpression';

module.exports = {
method: method,
method2: method2,
Expand All @@ -95,4 +116,6 @@ module.exports = {
expectNotToEqualCase: expectNotToEqualCase,
expectToBeUndefinedCase: expectToBeUndefinedCase,
expectNotToBeUndefinedCase: expectNotToBeUndefinedCase,
isDescribe: isDescribe,
isFunction: isFunction,
};
63 changes: 63 additions & 0 deletions rules/valid-describe.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
'use strict';

const isDescribe = require('./util').isDescribe;
const isFunction = require('./util').isFunction;

const isAsync = node => node.async;

const hasParams = node => node.params.length > 0;

const paramsLocation = params => {
const first = params[0];
const last = params[params.length - 1];
return {
start: {
line: first.loc.start.line,
column: first.loc.start.column,
},
end: {
line: last.loc.end.line,
column: last.loc.end.column,
},
};
};

module.exports = {
meta: {
docs: {
url:
'https://github.com/jest-community/eslint-plugin-jest/blob/master/docs/rules/valid-describe.md',
},
},
create(context) {
return {
CallExpression(node) {
if (isDescribe(node)) {
const callbackFunction = node.arguments[1];
if (callbackFunction && isFunction(callbackFunction)) {
if (isAsync(callbackFunction)) {
context.report({
message: 'No async describe callback',
node: callbackFunction,
});
}
if (hasParams(callbackFunction)) {
context.report({
message: 'Unexpected argument(s) in describe callback',
loc: paramsLocation(callbackFunction.params),
});
}
callbackFunction.body.body.forEach(node => {
if (node.type === 'ReturnStatement') {
context.report({
message: 'Unexpected return statement in describe callback',
node: node,
});
}
});
}
}
},
};
},
};
Loading

0 comments on commit 5e01e6b

Please sign in to comment.