Repository: lo1tuma/eslint-plugin-mocha
Pull request #276 (2021-03-05 08:57:52)

Speed up no-exclusive-tests and no-pending-tests
The `isDescribe` and `isTestCase` are doing a lot of work that is unnecessarily redone on each call expression.
This PR moves this work upfront, when the rule is created and re-uses the result for each call expression.

Please note that the choosen implementation is not a strong opinion. I tried to keep the code churn to a minimum and not export more astUtils internals. Feel free to suggest how you prefer this to be solved differently.

We noticed the performance while linting the [DevTools frontend](https://chromium.googlesource.com/devtools/devtools-frontend/). Running with the following command line:

```sh
TIMING=1 third_party/node/node.py --output scripts/test/run_lint_check_js.js
```

Before the change:
```
Rule                                 | Time (ms) | Relative
:------------------------------------|----------:|--------:
mocha/no-exclusive-tests             |  3697.734 |    35.9%
mocha/no-pending-tests               |  2133.972 |    20.7%
@typescript-eslint/naming-convention |   761.571 |     7.4%
no-whitespace-before-property        |   373.331 |     3.6%
keyword-spacing                      |   359.593 |     3.5%
space-infix-ops                      |   266.812 |     2.6%
@typescript-eslint/func-call-spacing |   180.198 |     1.8%
space-in-parens                      |   175.388 |     1.7%
@typescript-eslint/comma-dangle      |   164.089 |     1.6%
no-trailing-spaces                   |   144.546 |     1.4%
```

After the change (the two eslint-plugin-mocha rules we use no longer show up in the TOP 10):
```
Rule                                 | Time (ms) | Relative
:------------------------------------|----------:|--------:
@typescript-eslint/naming-convention |   758.180 |    17.2%
no-whitespace-before-property        |   348.997 |     7.9%
keyword-spacing                      |   336.037 |     7.6%
space-infix-ops                      |   250.037 |     5.7%
space-in-parens                      |   215.183 |     4.9%
@typescript-eslint/func-call-spacing |   147.527 |     3.4%
@typescript-eslint/comma-dangle      |   144.443 |     3.3%
rulesdir/es_modules_import           |   127.713 |     2.9%
no-trailing-spaces                   |   122.335 |     2.8%
comma-style                          |   112.220 |     2.5%
```


Patch:
diff --git a/benchmarks/runtime.bench.js b/benchmarks/runtime.bench.js
index 9906f5a..94f7609 100644
--- a/benchmarks/runtime.bench.js
+++ b/benchmarks/runtime.bench.js
@@ -86,7 +86,7 @@ function lintManyFilesWithAllRecommendedRules({ numberOfFiles }) {
 describe('runtime', () => {
     it('should not take longer as the defined budget to lint many files with the recommended config', () => {
         const nodeVersionMultiplier = getNodeVersionMultiplier();
-        const budget = 80000000 / cpuSpeed * nodeVersionMultiplier;
+        const budget = 70000000 / cpuSpeed * nodeVersionMultiplier;
 
         const { medianDuration } = runBenchmark(() => {
             lintManyFilesWithAllRecommendedRules({ numberOfFiles: 350 });
diff --git a/lib/util/ast.js b/lib/util/ast.js
index d2e2bba..95fe823 100644
--- a/lib/util/ast.js
+++ b/lib/util/ast.js
@@ -63,21 +63,30 @@ function isFunctionCallWithName(node, names) {
     return isCallExpression(node) && names.includes(getNodeName(node.callee));
 }
 
+// eslint-disable-next-line max-statements
 function createAstUtils(settings) {
     const additionalCustomNames = getAddtionalNames(settings);
 
-    function isDescribe(node, options = {}) {
+    function buildIsDescribeAnswerer(options) {
         const { modifiers = [ 'skip', 'only' ], modifiersOnly = false } = options;
         const describeAliases = getSuiteNames({ modifiersOnly, modifiers, additionalCustomNames });
 
-        return isFunctionCallWithName(node, describeAliases);
+        return (node) => isFunctionCallWithName(node, describeAliases);
     }
 
-    function isTestCase(node, options = {}) {
+    function isDescribe(node, options = {}) {
+        return buildIsDescribeAnswerer(options)(node);
+    }
+
+    function buildIsTestCaseAnswerer(options = {}) {
         const { modifiers = [ 'skip', 'only' ], modifiersOnly = false } = options;
         const testCaseNames = getTestCaseNames({ modifiersOnly, modifiers, additionalCustomNames });
 
-        return isFunctionCallWithName(node, testCaseNames);
+        return (node) => isFunctionCallWithName(node, testCaseNames);
+    }
+
+    function isTestCase(node, options = {}) {
+        return buildIsTestCaseAnswerer(options)(node);
     }
 
     function isSuiteConfigExpression(node) {
@@ -132,7 +141,9 @@ function createAstUtils(settings) {
         isSuiteConfigCall,
         hasParentMochaFunctionCall,
         findReturnStatement,
-        isReturnOfUndefined
+        isReturnOfUndefined,
+        buildIsDescribeAnswerer,
+        buildIsTestCaseAnswerer
     };
 }
 


Test patch:
diff --git a/lib/rules/no-exclusive-tests.js b/lib/rules/no-exclusive-tests.js
index 862bb05..8fc25cc 100644
--- a/lib/rules/no-exclusive-tests.js
+++ b/lib/rules/no-exclusive-tests.js
@@ -12,11 +12,13 @@ module.exports = {
     create(context) {
         const astUtils = createAstUtils(context.settings);
 
+        const options = { modifiers: [ 'only' ], modifiersOnly: true };
+        const isDescribe = astUtils.buildIsDescribeAnswerer(options);
+        const isTestCase = astUtils.buildIsTestCaseAnswerer(options);
+
         return {
             CallExpression(node) {
-                const options = { modifiers: [ 'only' ], modifiersOnly: true };
-
-                if (astUtils.isDescribe(node, options) || astUtils.isTestCase(node, options)) {
+                if (isDescribe(node) || isTestCase(node)) {
                     const callee = node.callee;
 
                     context.report({
diff --git a/lib/rules/no-pending-tests.js b/lib/rules/no-pending-tests.js
index e9c182e..db97e4f 100644
--- a/lib/rules/no-pending-tests.js
+++ b/lib/rules/no-pending-tests.js
@@ -11,9 +11,10 @@ module.exports = {
     },
     create(context) {
         const astUtils = createAstUtils(context.settings);
+        const isTestCase = astUtils.buildIsTestCaseAnswerer();
 
         function isPendingMochaTest(node) {
-            return astUtils.isTestCase(node) &&
+            return isTestCase(node) &&
                 node.arguments.length === 1 &&
                 node.arguments[0].type === 'Literal';
         }