Skip to content

Commit

Permalink
Added support for async functions in no-synchronous-tests
Browse files Browse the repository at this point in the history
  • Loading branch information
jawadst committed Jun 14, 2017
1 parent 0e54998 commit 9fc05c4
Show file tree
Hide file tree
Showing 3 changed files with 174 additions and 41 deletions.
49 changes: 46 additions & 3 deletions docs/rules/no-synchronous-tests.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,53 @@
# Disallow Synchronous Tests (no-synchronous-tests)

Mocha automatically determines whether a test is synchronous or asynchronous based on the arity of the function passed into it. When writing tests for a asynchronous function, omitting the `done` callback or forgetting to return a promise can often lead to false-positive test cases. This rule warns against the implicit synchronous feature, and should be combined with `handle-done-callback` for best results.
Mocha automatically determines whether a test is synchronous or asynchronous based on the arity of the function passed into it. When writing tests for an asynchronous function, omitting the `done` callback or forgetting to return a promise can often lead to false-positive test cases. This rule warns against the implicit synchronous feature, and should be combined with `handle-done-callback` for best results.

## Rule Details

This rule looks for either an asynchronous callback or a return statement within the function body of any mocha test statement. If the mocha callback is not used and a promise is not returned, this rule will raise a warning.
By default, this rule looks for the presence of one of:

- An asynchronous callback.
- An async function provided to a mocha test statement.
- A return statement within the function body of any mocha test statement.

If none of these three options is used in a test method, the rule will raise a warning.

The following patterns are considered warnings:

```js
it('should do foo', function () {
return;
});

it('should do foo', function () {
callback();
});
```

These patterns would not be considered warnings:

```js
it('should do foo', function (done) {
done();
});

it('should do foo', async function () {
await something();
});

it('should do foo', function () {
return promise;
}
```
You can change the acceptable asynchronous test methods to only allow a combination of async functions/callbacks/promises:
```js
rules: {
"mocha/no-synchronous-tests": ["warning", {allowed: ['async', 'callback', 'promise']}]
},
```
### Caveats:
Expand All @@ -21,7 +64,7 @@ it('test name', myTestFn);
## When Not To Use It
* If you are primarily writing synchronous tests, and rarely need the `done` callback or promise functionality.
* If you are primarily writing synchronous tests, and rarely need the `done` callback, promise functionality or async functions.
## Further Reading
Expand Down
124 changes: 87 additions & 37 deletions lib/rules/no-synchronous-tests.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
'use strict';

var R = require('ramda');

module.exports = function (context) {
var possibleAsyncFunctionNames = [
var R = require('ramda'),
defaultAllowedAsyncMethods = [ 'async', 'callback', 'promise' ],
possibleAsyncFunctionNames = [
'it',
'it.only',
'test',
Expand All @@ -16,54 +15,105 @@ module.exports = function (context) {
'afterEach'
];

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

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

function hasParentMochaFunctionCall(functionExpression) {
var name;
return callee.name;
}

if (functionExpression.parent && functionExpression.parent.type === 'CallExpression') {
name = getCalleeName(functionExpression.parent.callee);
return possibleAsyncFunctionNames.indexOf(name) > -1;
}
function hasParentMochaFunctionCall(functionExpression) {
var name;

return false;
if (functionExpression.parent && functionExpression.parent.type === 'CallExpression') {
name = getCalleeName(functionExpression.parent.callee);
return possibleAsyncFunctionNames.indexOf(name) > -1;
}

function hasAsyncCallback(functionExpression) {
return functionExpression.params.length === 1;
}
return false;
}

function findPromiseReturnStatement(nodes) {
return R.find(function (node) {
function hasAsyncCallback(functionExpression) {
return functionExpression.params.length === 1;
}

function isAsyncFunction(functionExpression) {
return functionExpression.async === true;
}

function findPromiseReturnStatement(nodes) {
return R.find(function (node) {
return node.type === 'ReturnStatement' && node.argument && node.argument.type !== 'Literal';
}, nodes);
}, nodes);
}

function doesReturnPromise(functionExpression) {
var bodyStatement = functionExpression.body,
returnStatement = null;

if (bodyStatement.type === 'BlockStatement') {
returnStatement = findPromiseReturnStatement(functionExpression.body.body);
} else if (bodyStatement.type === 'CallExpression') {
// allow arrow statements calling a promise with implicit return.
returnStatement = bodyStatement;
}

function checkPromiseReturn(functionExpression) {
var bodyStatement = functionExpression.body,
returnStatement = null;
return returnStatement !== null
&& typeof returnStatement !== 'undefined';
}

if (bodyStatement.type === 'BlockStatement') {
returnStatement = findPromiseReturnStatement(functionExpression.body.body);
} else if (bodyStatement.type === 'CallExpression') {
// allow arrow statements calling a promise with implicit return.
returnStatement = bodyStatement;
}
function getAllowedAsyncMethocsFromOptions(options) {
if (R.isNil(options.allowed)) {
return defaultAllowedAsyncMethods;
}

if (!returnStatement) {
context.report(functionExpression, 'Unexpected synchronous test.');
}
/* istanbul ignore if */
if (!Array.isArray(options.allowed) || options.allowed.length === 0) {
throw new Error('The `allowed` option for no-synchronous-tests must be a non-empty array.');
}

return options.allowed;
}

module.exports = function (context) {
var options = context.options[0] || {},
allowedAsyncMethods = getAllowedAsyncMethocsFromOptions(options);

function check(node) {
if (hasParentMochaFunctionCall(node) && !hasAsyncCallback(node)) {
checkPromiseReturn(node);
var testAsyncMethods,
isAsyncTest;

if (hasParentMochaFunctionCall(node)) {
// For each allowed async test method, check if it is used in the test
testAsyncMethods = allowedAsyncMethods.map(function (method) {
switch (method) {
case 'async':
return isAsyncFunction(node);

case 'callback':
return hasAsyncCallback(node);

case 'promise':
return doesReturnPromise(node);

/* istanbul ignore next */
default:
throw new Error(
'Unknown async test method "' + method + '".' +
'Possible values: ' + defaultAllowedAsyncMethods.join(', ')
);
}
});

// Check that at least one allowed async test method is used in the test
isAsyncTest = testAsyncMethods.some(function (value) {
return value === true;
});

if (!isAsyncTest) {
context.report(node, 'Unexpected synchronous test.');
}
}
}

Expand Down
42 changes: 41 additions & 1 deletion test/rules/no-synchronous-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,42 @@ ruleTester.run('no-synchronous-tests', rules['no-synchronous-tests'], {
{
code: 'it("", () => promise.then() );',
parserOptions: { ecmaVersion: 6 }
},
{
code: 'it("", async function () { });',
parserOptions: { ecmaVersion: 8 }
},
{
code: 'it("", async function () { return true; });',
parserOptions: { ecmaVersion: 8 }
},
{
code: 'it("", async function (val) { return await new Promise((resolve) => { resolve(val); }); });',
parserOptions: { ecmaVersion: 8 }
},
{
code: 'before("", async function () { });',
parserOptions: { ecmaVersion: 8 }
},
{
code: 'beforeEach("", async function () { });',
parserOptions: { ecmaVersion: 8 }
},
{
code: 'after("", async function () { });',
parserOptions: { ecmaVersion: 8 }
},
{
code: 'afterEach("", async function () { });',
parserOptions: { ecmaVersion: 8 }
},
{
code: 'it("", function (done) { done(); });',
options: []
},
{
code: 'it("", function (done) { done(); });',
options: [ { } ]
}
],

Expand Down Expand Up @@ -76,7 +112,11 @@ ruleTester.run('no-synchronous-tests', rules['no-synchronous-tests'], {
{
code: 'specify.only("", function () {});',
errors: [ { message: 'Unexpected synchronous test.', column: 18, line: 1 } ]
},
{
options: [ { allowed: [ 'callback', 'async' ] } ],
code: 'it("", function () { return promise(); });',
errors: [ { message: 'Unexpected synchronous test.', column: 8, line: 1 } ]
}

]
});

0 comments on commit 9fc05c4

Please sign in to comment.