Merge pull request #209 from straub/fix/208-no-setup-in-describe-timeout

Fix no-setup-in-describe to allow Mocha suite config
lo1tuma · Sep 11, 2019 · 1e32ad7 · 1e32ad7
2 parents 6c7deb0 + 64d1db4
commit 1e32ad7
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 2 deletions.
diff --git 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 () {});
+});
 ```
diff --git 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
@@ -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(getPropertyName(node.property)) !== -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
@@ -19,10 +19,18 @@ 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 () { 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 () { it("", function () { this["retries"](1); }); })',
         'describe("", function () { function a() { b(); }; it(); })',
         'describe("", function () { function a() { b.c; }; it(); })',
         'describe("", function () { afterEach(function() { b(); }); it(); })',
@@ -125,6 +133,17 @@ 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: 'foo("", function () { a.b; });',
             settings: {