From a34ccf980782abf563a9cf7c2fb13608e3753d58 Mon Sep 17 00:00:00 2001 From: David Straub Date: Thu, 5 Sep 2019 17:30:07 -0400 Subject: [PATCH 1/4] Fix no-setup-in-describe to allow Mocha suite config This rule will now allow `this.timeout();`, `this.slow();`, and `this.retries();` in describe blocks, since they're configuration for the suite and not setup. Fixes #208. --- lib/rules/no-setup-in-describe.js | 4 +++- lib/util/ast.js | 12 ++++++++++++ test/rules/no-setup-in-describe.js | 6 ++++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/lib/rules/no-setup-in-describe.js b/lib/rules/no-setup-in-describe.js index fcc1873..a0886c0 100644 --- a/lib/rules/no-setup-in-describe.js +++ b/lib/rules/no-setup-in-describe.js @@ -13,7 +13,9 @@ module.exports = function noSetupInDescribe(context) { const settings = context.settings; function isPureNode(node) { - return astUtils.isHookCall(node) || astUtils.isTestCase(node); + return astUtils.isHookCall(node) || + astUtils.isTestCase(node) || + astUtils.isSuiteConfigCall(node); } function reportCallExpression(callExpression) { diff --git a/lib/util/ast.js b/lib/util/ast.js index aa99877..18a892a 100644 --- a/lib/util/ast.js +++ b/lib/util/ast.js @@ -11,6 +11,7 @@ const describeAliases = [ 'suite', 'xsuite', 'suite.only', 'suite.skip' ]; const hooks = [ 'before', 'after', 'beforeEach', 'afterEach', 'beforeAll', 'afterAll' ]; +const suiteConfig = [ 'timeout', 'slow', 'retries' ]; const testCaseNames = [ 'it', 'it.only', 'it.skip', 'xit', 'test', 'test.only', 'test.skip', @@ -43,6 +44,16 @@ function isHookCall(node) { return isCallExpression(node) && isHookIdentifier(node.callee); } +function isSuiteConfigExpression(node) { + return node.type === 'MemberExpression' && + node.object.type === 'ThisExpression' && + suiteConfig.indexOf(node.property.name) !== -1; +} + +function isSuiteConfigCall(node) { + return isCallExpression(node) && isSuiteConfigExpression(node.callee); +} + function isTestCase(node) { return isCallExpression(node) && testCaseNames.indexOf(getNodeName(node.callee)) > -1; } @@ -102,6 +113,7 @@ module.exports = { getNodeName, isMochaFunctionCall, isHookCall, + isSuiteConfigCall, isStringLiteral, hasParentMochaFunctionCall, findReturnStatement, diff --git a/test/rules/no-setup-in-describe.js b/test/rules/no-setup-in-describe.js index 3109fb1..fe77e48 100644 --- a/test/rules/no-setup-in-describe.js +++ b/test/rules/no-setup-in-describe.js @@ -19,10 +19,16 @@ ruleTester.run('no-setup-in-describe', rule, { 'it("", function () { a[b]; })', 'it("", function () { a["b"]; })', 'describe("", function () { it(); })', + 'describe("", function () { this.slow(1); it(); })', + 'describe("", function () { this.timeout(1); it(); })', + 'describe("", function () { this.retries(1); it(); })', 'describe("", function () { it("", function () { b(); }); })', 'describe("", function () { it("", function () { a.b; }); })', 'describe("", function () { it("", function () { a[b]; }); })', 'describe("", function () { it("", function () { a["b"]; }); })', + 'describe("", function () { it("", function () { this.slow(1); }); })', + 'describe("", function () { it("", function () { this.timeout(1); }); })', + 'describe("", function () { it("", function () { this.retries(1); }); })', 'describe("", function () { function a() { b(); }; it(); })', 'describe("", function () { function a() { b.c; }; it(); })', 'describe("", function () { afterEach(function() { b(); }); it(); })', From 0569acd81aa3035b17c0221cf4089c34114b4899 Mon Sep 17 00:00:00 2001 From: David Straub Date: Thu, 5 Sep 2019 17:40:32 -0400 Subject: [PATCH 2/4] Document no-setup-in-describe suite config exception --- docs/rules/no-setup-in-describe.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/rules/no-setup-in-describe.md b/docs/rules/no-setup-in-describe.md index 6a26ac1..ebb72af 100644 --- a/docs/rules/no-setup-in-describe.md +++ b/docs/rules/no-setup-in-describe.md @@ -18,7 +18,7 @@ Any setup directly in a `describe` is run before all tests execute. This is unde 1. When doing TDD in a large codebase, all setup is run for tests that don't have `only` set. This can add a substantial amount of time per iteration. 2. If global state is altered by the setup of another describe block, your test may be affected. -This rule reports all function calls and use of the dot operator (due to getters and setters) directly in describe blocks. +This rule reports all function calls and use of the dot operator (due to getters and setters) directly in describe blocks. An exception is made for Mocha's suite configuration methods, like `this.timeout();`, which do not represent setup logic. If you're using [dynamically generated tests](https://mochajs.org/#dynamically-generating-tests), you should disable this rule. @@ -68,4 +68,8 @@ describe('something', function () { const { a, b } = setup(); }); }); +describe('something', function () { + this.timeout(5000); + it('should take awhile', function () {}); +}); ``` From 0d838d54cbf8f9176de8fc57962b33c0f6396181 Mon Sep 17 00:00:00 2001 From: David Straub Date: Mon, 9 Sep 2019 10:59:06 -0400 Subject: [PATCH 3/4] Test no-setup-in-describe on disallowed and computed member expressions --- test/rules/no-setup-in-describe.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/rules/no-setup-in-describe.js b/test/rules/no-setup-in-describe.js index fe77e48..0997340 100644 --- a/test/rules/no-setup-in-describe.js +++ b/test/rules/no-setup-in-describe.js @@ -131,6 +131,28 @@ ruleTester.run('no-setup-in-describe', rule, { line: 1, column: 28 } ] + }, { + code: 'describe("", function () { this.a(); });', + errors: [ { + message: 'Unexpected function call in describe block.', + line: 1, + column: 28 + }, { + message: memberExpressionError, + line: 1, + column: 28 + } ] + }, { + code: 'describe("", function () { this["retries"](); });', + errors: [ { + message: 'Unexpected function call in describe block.', + line: 1, + column: 28 + }, { + message: memberExpressionError, + line: 1, + column: 28 + } ] }, { code: 'foo("", function () { a.b; });', settings: { From 64d1db4a9c850d090f78d8f99ef750f34ecd236b Mon Sep 17 00:00:00 2001 From: David Straub Date: Tue, 10 Sep 2019 12:10:10 -0400 Subject: [PATCH 4/4] Fix no-setup-in-describe to allow suite config call via computed member expression --- lib/util/ast.js | 2 +- test/rules/no-setup-in-describe.js | 13 ++----------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/lib/util/ast.js b/lib/util/ast.js index 18a892a..bae0dde 100644 --- a/lib/util/ast.js +++ b/lib/util/ast.js @@ -47,7 +47,7 @@ function isHookCall(node) { function isSuiteConfigExpression(node) { return node.type === 'MemberExpression' && node.object.type === 'ThisExpression' && - suiteConfig.indexOf(node.property.name) !== -1; + suiteConfig.indexOf(getPropertyName(node.property)) !== -1; } function isSuiteConfigCall(node) { diff --git a/test/rules/no-setup-in-describe.js b/test/rules/no-setup-in-describe.js index 0997340..21e98ad 100644 --- a/test/rules/no-setup-in-describe.js +++ b/test/rules/no-setup-in-describe.js @@ -22,6 +22,7 @@ ruleTester.run('no-setup-in-describe', rule, { 'describe("", function () { this.slow(1); it(); })', 'describe("", function () { this.timeout(1); it(); })', 'describe("", function () { this.retries(1); it(); })', + 'describe("", function () { this["retries"](1); it(); })', 'describe("", function () { it("", function () { b(); }); })', 'describe("", function () { it("", function () { a.b; }); })', 'describe("", function () { it("", function () { a[b]; }); })', @@ -29,6 +30,7 @@ ruleTester.run('no-setup-in-describe', rule, { 'describe("", function () { it("", function () { this.slow(1); }); })', 'describe("", function () { it("", function () { this.timeout(1); }); })', 'describe("", function () { it("", function () { this.retries(1); }); })', + 'describe("", function () { it("", function () { this["retries"](1); }); })', 'describe("", function () { function a() { b(); }; it(); })', 'describe("", function () { function a() { b.c; }; it(); })', 'describe("", function () { afterEach(function() { b(); }); it(); })', @@ -142,17 +144,6 @@ ruleTester.run('no-setup-in-describe', rule, { line: 1, column: 28 } ] - }, { - code: 'describe("", function () { this["retries"](); });', - errors: [ { - message: 'Unexpected function call in describe block.', - line: 1, - column: 28 - }, { - message: memberExpressionError, - line: 1, - column: 28 - } ] }, { code: 'foo("", function () { a.b; });', settings: {