From 604e7a74b346eea1c5628a1c035c303bc39e2ecd Mon Sep 17 00:00:00 2001 From: Jeroen Engels Date: Sun, 21 Feb 2016 12:34:14 +0100 Subject: [PATCH] add max-asserts rule --- docs/rules/max-asserts.md | 48 +++++++++++++++++++++++++++ index.js | 6 ++-- readme.md | 2 ++ rules/max-asserts.js | 35 ++++++++++++++++++++ rules/no-identical-title.js | 2 -- test/max-asserts.js | 66 +++++++++++++++++++++++++++++++++++++ util.js | 38 +++++++++++++++++---- 7 files changed, 187 insertions(+), 10 deletions(-) create mode 100644 docs/rules/max-asserts.md create mode 100644 rules/max-asserts.js create mode 100644 test/max-asserts.js diff --git a/docs/rules/max-asserts.md b/docs/rules/max-asserts.md new file mode 100644 index 00000000..9997273b --- /dev/null +++ b/docs/rules/max-asserts.md @@ -0,0 +1,48 @@ +# Limit the number of assertions in a test + +Limit the amount of assertions in a test to enforce splitting up large tests into smaller ones. + +Skipped assertions are counted. + + +## Fail + +```js +/*eslint max-asserts: [2, 5]*/ +const test = require('ava'); + +test('tests with too may asserts', (t) => { + const array = getArrayUpTo(6); + + t.is(array.length, 6); + t.is(array[0], 1); + t.is(array[1], 2); + t.is(array[2], 3); + t.is(array[3], 4); + t.is(array[4], 5); + t.is(array[5], 6); +}); +``` + + +## Pass + +```js +const test = require('ava'); + +test('my test name', (t) => { + +test('tests with too may asserts', (t) => { + const array = getArrayUpTo(6); + + t.same(array, [1, 2, 3, 4, 5, 6]); +}); +``` + +## Options + +The rule takes one option, a number, which is the maximum number of assertions for each test. The default is 5. + +You can set the option in configuration like this: + +"max-asserts": [2, 5] diff --git a/index.js b/index.js index 581b3500..0d735b51 100644 --- a/index.js +++ b/index.js @@ -2,16 +2,18 @@ module.exports = { rules: { + 'max-asserts': require('./rules/max-asserts'), 'no-cb-test': require('./rules/no-cb-test'), - 'no-identical-title': require('./rules/no-identical-title') + 'no-identical-title': require('./rules/no-identical-title'), 'no-only-test': require('./rules/no-only-test'), 'no-skip-test': require('./rules/no-skip-test'), 'test-ended': require('./rules/test-ended'), 'test-title': require('./rules/test-title') }, rulesConfig: { + 'max-asserts': [0, 5], 'no-cb-test': 0, - 'no-identical-title': 0 + 'no-identical-title': 0, 'no-only-test': 0, 'no-skip-test': 0, 'test-ended': 0, diff --git a/readme.md b/readme.md index 54b10deb..157681f2 100644 --- a/readme.md +++ b/readme.md @@ -22,6 +22,7 @@ Configure it in package.json. "ava" ], "rules": { + "ava/max-asserts": [2, 5], "ava/no-cb-test": 2, "ava/no-identical-title": 2, "ava/no-only-test": 2, @@ -38,6 +39,7 @@ Configure it in package.json. The rules will only activate in test files. +- [max-asserts](docs/rules/max-asserts) - Limit the number of assertions in a test. - [no-cb-test](docs/rules/no-cb-test.md) - Ensure no `test.cb()` is used. - [no-identical-title](docs/rules/no-identical-title.md) - Ensure no tests have the same title. - [no-only-test](docs/rules/no-only-test.md) - Ensure no test.only() are present. diff --git a/rules/max-asserts.js b/rules/max-asserts.js new file mode 100644 index 00000000..76b55085 --- /dev/null +++ b/rules/max-asserts.js @@ -0,0 +1,35 @@ +'use strict'; +var util = require('../util'); + +module.exports = function (context) { + var maxAssertions = context.options[0] || 5; + var isTestFile = false; + var assertionCount = 0; + + return { + CallExpression: function (node) { + if (util.isTestFile(node)) { + isTestFile = true; + return; + } + + var callee = node.callee; + var name = callee.type === 'MemberExpression' ? callee.object.name : callee.name; + if (name === 'test') { + assertionCount = 0; + } + }, + + MemberExpression: function (node) { + if (isTestFile) { + // node.parent.type !== 'MemberExpression' --> don't count `t.skip.is(...)` twice + if (node.parent.type !== 'MemberExpression' && util.isAssertion(node)) { + assertionCount++; + if (assertionCount > maxAssertions) { + context.report(node, 'Too many assertions.'); + } + } + } + } + }; +}; diff --git a/rules/no-identical-title.js b/rules/no-identical-title.js index 03264f75..93feee58 100644 --- a/rules/no-identical-title.js +++ b/rules/no-identical-title.js @@ -2,9 +2,7 @@ var util = require('../util'); module.exports = function (context) { - var ifMultiple = context.options[0] === "if-multiple"; var isTestFile = false; - var testCount = 0; var usedTitles = []; return { diff --git a/test/max-asserts.js b/test/max-asserts.js new file mode 100644 index 00000000..4c70573b --- /dev/null +++ b/test/max-asserts.js @@ -0,0 +1,66 @@ +import test from 'ava'; +import {RuleTester} from 'eslint'; +import rule from '../rules/max-asserts'; + +const ruleTester = new RuleTester({ + env: { + es6: true + } +}); + +const errors = [{ruleId: 'max-asserts'}]; +const header = `const test = require('ava');\n`; +function nbAssertions(n) { + return Array.from({length: n}).map(() => 't.is(1, 1);').join('\n'); +} + +test(() => { + ruleTester.run('max-asserts', rule, { + valid: [ + `${header} test(function (t) { ${nbAssertions(3)} });`, + ` ${header} + test(function (t) { ${nbAssertions(3)} }); + test(function (t) { ${nbAssertions(3)} }); + `, + `${header} test(function (t) { t.plan(5); ${nbAssertions(5)} });`, + `${header} test(function (t) { t.skip.is(1, 1); ${nbAssertions(4)} });`, + `${header} test.cb(function (t) { ${nbAssertions(5)} t.end(); });`, + { + code: `${header} test(function (t) { ${nbAssertions(3)} });`, + options: [3] + }, + // shouldn't be triggered since it's not a test file + `test(function (t) { ${nbAssertions(10)} });` + ], + invalid: [ + { + code: `${header} test(function (t) { ${nbAssertions(6)} });`, + errors + }, + { + code: ` ${header} + test(function (t) { ${nbAssertions(3)} }); + test(function (t) { ${nbAssertions(6)} }); + `, + errors + }, + { + code: `${header} test(function (t) { t.plan(5); ${nbAssertions(6)} });`, + errors + }, + { + code: `${header} test(function (t) { t.skip.is(1, 1); ${nbAssertions(5)} });`, + errors + }, + { + code: `${header} test.cb(function (t) { ${nbAssertions(6)} t.end(); });`, + errors + }, + { + code: `${header} test(function (t) { ${nbAssertions(4)} });`, + options: [3], + errors + } + ] + }); +}); diff --git a/util.js b/util.js index 34bfc852..da464fbc 100644 --- a/util.js +++ b/util.js @@ -24,6 +24,13 @@ exports.isTestType = function (callExp, type) { callee.property.name === type; }; +function nameOfRootObject(node) { + if (node.object.type === 'MemberExpression') { + return nameOfRootObject(node.object); + } + return node.object.name; +} + /** * Checks whether the given MemberExpression node has `test` as the root object. * @param {ASTNode} node Node of type MemberExpression @@ -34,9 +41,28 @@ exports.isTestType = function (callExp, type) { * // => true * @return {Boolean} */ -exports.isCallFromTestObject = function isCallFromTestObject(node) { - if (node.object.type === 'MemberExpression') { - return isCallFromTestObject(node.object); - } - return node.object.name === 'test'; -} +exports.isCallFromTestObject = function (node) { + return nameOfRootObject(node) === 'test'; +}; + +/** + * Checks whether the given MemberExpression node is an assertion. + * Expects the assertion object to be `t`. + * Doesn't count `t.plan()` and `t.end()` as assertions. + * @param {ASTNode} node Node of type MemberExpression + * @example + * isAssertion(toASTNode('t.is(1, 1)')) + * // => true + * isAssertion(toASTNode('test.is(1, 1)')) + * // => false + * isAssertion(toASTNode('t.plan(5)')) + * // => false + * isAssertion(toASTNode('t.end()')) + * // => false + * @return {Boolean} + */ +exports.isAssertion = function (node) { + const notAssertionMethods = ['plan', 'end']; + + return nameOfRootObject(node) === 't' && notAssertionMethods.indexOf(node.property.name) === -1; +};