Skip to content

Commit

Permalink
Add no-hooks-for-single-case rule (fixes #44)
Browse files Browse the repository at this point in the history
  • Loading branch information
jfmengels committed Aug 8, 2016
1 parent d76f13c commit a44dbd2
Show file tree
Hide file tree
Showing 6 changed files with 344 additions and 2 deletions.
1 change: 1 addition & 0 deletions docs/rules/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@
* [no-sibling-hooks](no-sibling-hooks.md) - disallow duplicate uses of a hook at the same level inside a describe
* [no-mocha-arrows](no-mocha-arrows.md) - disallow arrow functions as arguments to mocha globals
* [no-hooks](no-hooks.md) - disallow hooks
* [no-hooks-for-single-case](no-hooks-for-single-case.md) - disallow hooks for a single test or test suite
* [no-top-level-hooks](no-top-level-hooks.md) - disallow top-level hooks
64 changes: 64 additions & 0 deletions docs/rules/no-hooks-for-single-case.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# Disallow hooks for a single test or test suite (no-hooks-for-single-case)

Mocha proposes hooks that allow code to be run before or after every or all tests. This helps define a common setup or teardown process for every test. These hooks are not useful when there is only one test case, as it would then make more sense to move the hooks' operations in the test directly.

## Rule Details

This rule looks for every call to `before`, `after`, `beforeEach` and `afterEach` and reports them if they are called when there is less than two tests and/or tests suites in the same test suite.

The following patterns are considered warnings:

```js
describe('foo', function () {
before(function () { /* ... */ }); // Not allowed as there is only a test suite next to this hook.

describe('bar', function() {
/* ... */
});
});

describe('foo', function () {
after(function () { /* ... */ }); // Not allowed as there is only a test case next to this hook.

it('should XYZ', function() {
/* ... */
});
});

describe('foo', function () {
beforeEach(function () { /* ... */ }); // Not allowed as there is no test suites or cases next to this hook.
});
```

These patterns would not be considered warnings:

```js
describe('foo', function () {
before(function () { /* ... */ });

it('should XYZ', function() {
/* ... */
});

it('should ABC', function() {
/* ... */
});
});

describe('foo', function () {
before(function () { /* ... */ });

it('should XYZ', function() {
/* ... */
});

describe('bar', function() {
/* ... */
});
});
```

## When Not To Use It

* If you use another library which exposes a similar API as mocha (e.g. `before`, `after`), you should turn this rule off, because it would raise warnings.
* If you turned `no-hooks` on, you should turn this rule off, because it would raise several warnings for the same root cause.
1 change: 1 addition & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ module.exports = {
'valid-suite-description': require('./lib/rules/valid-suite-description'),
'no-mocha-arrows': require('./lib/rules/no-mocha-arrows'),
'no-hooks': require('./lib/rules/no-hooks'),
'no-hooks-for-single-case': require('./lib/rules/no-hooks-for-single-case'),
'no-sibling-hooks': require('./lib/rules/no-sibling-hooks'),
'no-top-level-hooks': require('./lib/rules/no-top-level-hooks')
},
Expand Down
56 changes: 56 additions & 0 deletions lib/rules/no-hooks-for-single-case.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
'use strict';

var astUtil = require('../util/ast');

function newDescribeLayer(describeNode) {
return {
describeNode: describeNode,
hookNodes: [],
testCount: 0
};
}

module.exports = function (context) {
var layers = [];

function popLayer(node) {
var layer = layers[layers.length - 1];
if (layer.describeNode === node) {
if (layer.testCount <= 1) {
layer.hookNodes.forEach(function (hookNode) {
context.report({
node: hookNode,
// message: 'Unexpected use of duplicate Mocha `' + name + '` hook'
message: 'Unexpected use of duplicate Mocha `before` hook'
});
});
}
layers.pop();
}
}

return {
Program: function (node) {
layers.push(newDescribeLayer(node));
},

CallExpression: function (node) {
if (astUtil.isDescribe(node)) {
layers[layers.length - 1].testCount += 1;
layers.push(newDescribeLayer(node));
return;
}

if (astUtil.isTestCase(node)) {
layers[layers.length - 1].testCount += 1;
}

if (astUtil.isHookIdentifier(node.callee)) {
layers[layers.length - 1].hookNodes.push(node.callee);
}
},

'CallExpression:exit': popLayer,
'Program:exit': popLayer
};
};
19 changes: 17 additions & 2 deletions lib/util/ast.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
'use strict';

var describeAliases = [ 'describe', 'xdescribe', 'context', 'xcontext' ],
hooks = [ 'before', 'after', 'beforeEach', 'afterEach' ];
hooks = [ 'before', 'after', 'beforeEach', 'afterEach' ],
testCaseNames = [ 'it', 'it.only', 'test', 'test.only', 'specify', 'specify.only' ];

function isDescribeIdentifier(node) {
return node.type === 'Identifier' && describeAliases.indexOf(node.name) !== -1;
Expand All @@ -24,8 +25,22 @@ function isHookIdentifier(node) {
&& hooks.indexOf(node.name) !== -1;
}

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

function isTestCase(node) {
return node
&& node.type === 'CallExpression'
&& testCaseNames.indexOf(getNodeName(node.callee)) > -1;
}

module.exports = {
isDescribe: isDescribe,
isDescribeIdentifier: isDescribeIdentifier,
isHookIdentifier: isHookIdentifier
isHookIdentifier: isHookIdentifier,
isTestCase: isTestCase
};
205 changes: 205 additions & 0 deletions test/rules/no-hooks-for-single-case.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,205 @@
'use strict';

var RuleTester = require('eslint').RuleTester,
rules = require('../../').rules,
ruleTester = new RuleTester();

ruleTester.run('no-hooks-for-single-case', rules['no-hooks-for-single-case'], {

valid: [
[
'describe(function() {',
' it(function() {});',
'});'
].join('\n'),
[
'describe(function() {',
' before(function() {});',
' it(function() {});',
' it(function() {});',
'});'
].join('\n'),
[
'describe(function() {',
' after(function() {});',
' it(function() {});',
' it(function() {});',
'});'
].join('\n'),
[
'describe(function() {',
' beforeEach(function() {});',
' it(function() {});',
' it(function() {});',
'});'
].join('\n'),
[
'describe(function() {',
' afterEach(function() {});',
' it(function() {});',
' it(function() {});',
'});'
].join('\n'),
[
'before(function() {});',
'it(function() {});',
'it(function() {});'
].join('\n'),
[
'describe(function() {',
' before(function() {});',
' it(function() {});',
' describe(function() {});',
'});'
].join('\n'),
[
'describe(function() {',
' before(function() {});',
' it(function() {});',
' xdescribe(function() {});',
'});'
].join('\n'),
[
'describe(function() {',
' before(function() {});',
' describe(function() {});',
' describe(function() {});',
'});'
].join('\n'),
[
'describe(function() {',
' before(function() {});',
' it.only(function() {});',
' it(function() {});',
'});'
].join('\n'),
[
'describe(function() {',
' before(function() {});',
' it(function() {});',
' describe(function() {',
' before(function() {});',
' it(function() {});',
' it(function() {});',
' });',
'});'
].join('\n')
],

invalid: [
{
code: [
'describe(function() {',
' before(function() {});',
'});'
].join('\n'),
errors: [ { message: 'Unexpected use of duplicate Mocha `before` hook', column: 5, line: 2 } ]
},
{
code: [
'describe(function() {',
' before(function() {});',
' it(function() {});',
'});'
].join('\n'),
errors: [ { message: 'Unexpected use of duplicate Mocha `before` hook', column: 5, line: 2 } ]
},
{
code: [
'describe(function() {',
' it(function() {});',
' before(function() {});',
'});'
].join('\n'),
errors: [ { message: 'Unexpected use of duplicate Mocha `before` hook', column: 5, line: 3 } ]
},
{
code: [
'describe(function() {',
' after(function() {});',
' it(function() {});',
'});'
].join('\n'),
errors: [ { message: 'Unexpected use of duplicate Mocha `before` hook', column: 5, line: 2 } ]
},
{
code: [
'describe(function() {',
' beforeEach(function() {});',
' it(function() {});',
'});'
].join('\n'),
errors: [ { message: 'Unexpected use of duplicate Mocha `before` hook', column: 5, line: 2 } ]
},
{
code: [
'describe(function() {',
' afterEach(function() {});',
' it(function() {});',
'});'
].join('\n'),
errors: [ { message: 'Unexpected use of duplicate Mocha `before` hook', column: 5, line: 2 } ]
},
{
code: [
'before(function() {});',
'it(function() {});'
].join('\n'),
errors: [ { message: 'Unexpected use of duplicate Mocha `before` hook', column: 1, line: 1 } ]
},
{
code: [
'describe(function() {',
' before(function() {});',
' describe(function() {});',
'});'
].join('\n'),
errors: [ { message: 'Unexpected use of duplicate Mocha `before` hook', column: 5, line: 2 } ]
},
{
code: [
'describe(function() {',
' before(function() {});',
' xdescribe(function() {});',
'});'
].join('\n'),
errors: [ { message: 'Unexpected use of duplicate Mocha `before` hook', column: 5, line: 2 } ]
},
{
code: [
'describe(function() {',
' before(function() {});',
' it.only(function() {});',
'});'
].join('\n'),
errors: [ { message: 'Unexpected use of duplicate Mocha `before` hook', column: 5, line: 2 } ]
},
{
code: [
'describe(function() {',
' before(function() {});',
' it(function() {});',
' describe(function() {',
' before(function() {});',
' it(function() {});',
' });',
'});'
].join('\n'),
errors: [ { message: 'Unexpected use of duplicate Mocha `before` hook', column: 9, line: 5 } ]
},
{
code: [
'describe(function() {',
' before(function() {});',
' describe(function() {',
' before(function() {});',
' it(function() {});',
' it(function() {});',
' });',
'});'
].join('\n'),
errors: [ { message: 'Unexpected use of duplicate Mocha `before` hook', column: 5, line: 2 } ]
}
]

});

0 comments on commit a44dbd2

Please sign in to comment.