fix: expectExpect efficiency & speed

\### Rationale Dropped time to process down significantly from test runs of 15ms to 5ms.
testcafe-community · Oct 23, 2021 · 2926638 · 2926638
1 parent d46600a
commit 2926638
Show file tree

Hide file tree

Showing 3 changed files with 109 additions and 70 deletions.
diff --git a/lib/rules/expect-expect.ts b/lib/rules/expect-expect.ts
@@ -3,8 +3,11 @@
  * @author Ben Monro
  * @author codejedi365
  */
-import type { CallExpression } from "@typescript-eslint/types/dist/ast-spec";
-import { determineCodeLocation } from "../utils/locator";
+import type {
+    CallExpression,
+    BaseNode
+} from "@typescript-eslint/types/dist/ast-spec";
+import { determineCodeLocation, isAncestorOf } from "../utils/locator";
 import { createRule } from "../create-rule";
 
 const testFnAttributes = [
@@ -40,63 +43,95 @@ export default createRule({
     create(context) {
         let hasExpect = false;
         let isInsideTest = false;
-        let ignoreExpects = false;
-        return {
-            "CallExpression": (node: CallExpression) => {
-                if (isInsideTest && hasExpect) return; // Short circuit, already found
+        let currentTestNode: CallExpression | null = null;
+        let testFnImplementationNode: BaseNode | null = null;
 
-                let fnName;
-                let objectName;
-                try {
-                    [fnName, objectName] = determineCodeLocation(node);
-                } catch (e) {
-                    // ABORT: Failed to evaluate rule effectively
-                    // since I cannot derive values to determine location in the code
-                    return;
-                }
+        const resetFlags = () => {
+            hasExpect = false;
+            isInsideTest = false;
+            currentTestNode = null;
+            testFnImplementationNode = null;
+        };
 
-                if (isInsideTest) {
-                    if (ignoreExpects) return;
-                    if (fnName === "expect") {
-                        hasExpect = true;
-                        return;
-                    }
-                    if (objectName === "test") {
-                        // only happens in chained methods with internal callbacks
-                        // like test.before(() => {})("my test", async () => {})
-                        // prevents any registering of an expect in the before() callback
-                        ignoreExpects = true;
-                    }
-                    return;
-                }
-                // Determine if inside/chained to a test() function
-                if (objectName !== "test") return;
-                if (fnName === "test" || testFnAttributes.includes(fnName)) {
-                    isInsideTest = true;
-                }
-            },
+        const activateExpectTracking = (node: CallExpression) => {
+            isInsideTest = true;
+            currentTestNode = node;
+            [, testFnImplementationNode] = currentTestNode.arguments;
+        };
 
-            "CallExpression:exit": (node: CallExpression) => {
-                if (!isInsideTest) return; // Short circuit
+        const isInsideTestFnImplementation = (node: CallExpression) => {
+            return testFnImplementationNode
+                ? isAncestorOf(testFnImplementationNode, node)
+                : false;
+        };
 
-                let fnName;
-                let objectName;
-                try {
-                    [fnName, objectName] = determineCodeLocation(node);
-                } catch (e) {
-                    // ABORT: Failed to evaluate rule effectively
-                    // since I cannot derive values to determine location in the code
-                    return;
-                }
-                if (objectName !== "test") return;
-                if (fnName === "test" || testFnAttributes.includes(fnName)) {
-                    if (!hasExpect) {
-                        context.report({ node, messageId: "missingExpect" });
-                    }
-                    hasExpect = false;
-                    isInsideTest = false;
+        const validateExpectWasFound = (node: CallExpression) => {
+            if (!hasExpect) {
+                context.report({ node, messageId: "missingExpect" });
+            }
+            resetFlags();
+        };
+
+        const unknownFnCallENTER = (node: CallExpression) => {
+            if (isInsideTest && hasExpect) return; // Short circuit, already found
+
+            let fnName;
+            let objectName;
+            try {
+                [fnName, objectName] = determineCodeLocation(node);
+            } catch (e) {
+                // ABORT: Failed to evaluate rule effectively
+                // since I cannot derive values to determine location in the code
+                return;
+            }
+
+            // Determine if inside/chained to a test() function
+            if (objectName !== "test") return;
+            if (fnName === "test" || testFnAttributes.includes(fnName)) {
+                activateExpectTracking(node);
+            }
+        };
+
+        const unknownFnCallEXIT = (node: CallExpression) => {
+            if (isInsideTest && node === currentTestNode) {
+                validateExpectWasFound(node);
+            }
+        };
+
+        const testFnCallExpression = "CallExpression[arguments.length=2]";
+        const expectFnCallExpression =
+            "CallExpression[arguments.length=1][callee.property.name=expect]";
+
+        return {
+            // UNCOMMENT to Debug step-by-step
+            // ---------------------------------
+            // "CallExpression": (node: CallExpression) => {
+            //     debugger;
+            // },
+            // "CallExpression:exit": (node: CallExpression) => {
+            //     debugger;
+            // },
+            // ---------------------------------
+            // test("name", () => {}), ENTER
+            [`${testFnCallExpression}[callee.name=test]`]:
+                activateExpectTracking,
+            // test("name", () => {}), EXIT
+            [`${testFnCallExpression}[callee.name=test]:exit`]:
+                unknownFnCallEXIT,
+            [`${testFnCallExpression}[callee.type=CallExpression]`]:
+                unknownFnCallENTER,
+            [`${testFnCallExpression}[callee.type=CallExpression]:exit`]:
+                unknownFnCallEXIT,
+            [`${testFnCallExpression}[callee.type=MemberExpression]`]:
+                unknownFnCallENTER,
+            [`${testFnCallExpression}[callee.type=MemberExpression]:exit`]:
+                unknownFnCallEXIT,
+            // expect(actual), ENTER
+            [expectFnCallExpression]: (node: CallExpression) => {
+                if (!isInsideTest || hasExpect) return; // Short circuit
+                if (isInsideTestFnImplementation(node)) {
+                    hasExpect = true;
                 }
-                ignoreExpects = false;
             }
         };
     }

diff --git a/lib/utils/locator.ts b/lib/utils/locator.ts
@@ -9,6 +9,7 @@ import type {
     Identifier,
     BaseNode
 } from "@typescript-eslint/types/dist/ast-spec";
+import { AST_NODE_TYPES } from "@typescript-eslint/experimental-utils";
 import {
     isCallExpression,
     isIdentifier,
@@ -86,3 +87,19 @@ export function determineCodeLocation(
 ): [FunctionName, ObjectName] {
     return [deriveFunctionName(node), deriveObjectName(node)];
 }
+
+export function isAncestorOf(
+    ancestorNode: BaseNode,
+    childNode: BaseNode
+): boolean {
+    function checkParent(child: BaseNode, parent: BaseNode): boolean {
+        if (child === parent) {
+            return true;
+        }
+        if (child.parent && child.parent.type !== AST_NODE_TYPES.Program) {
+            return checkParent(child.parent, parent);
+        }
+        return false;
+    }
+    return checkParent(childNode, ancestorNode);
+}
diff --git a/tests/lib/rules/expect-expect.test.ts b/tests/lib/rules/expect-expect.test.ts
@@ -199,22 +199,9 @@ describe("Error Handling", () => {
             throw new Error("FATAL: Unable to determine code location");
         });
         expect(() => {
-            ruleFns.CallExpression(fakeNode);
-        }).not.toThrow();
-    });
-    it("Ignores an undeterminable code location internal error of CallExpression:exit", () => {
-        determineCodeLocationMock
-            .mockImplementationOnce(() => {
-                return ["test", "test"];
-            })
-            .mockImplementationOnce(() => {
-                throw new Error("FATAL: Unable to determine code location");
-            });
-        // simulate enter of test();
-        ruleFns.CallExpression(fakeNode);
-        // Test proper exit of test() that fails;
-        expect(() => {
-            ruleFns["CallExpression:exit"](fakeNode);
+            ruleFns[
+                "CallExpression[arguments.length=2][callee.type=CallExpression]"
+            ](fakeNode);
         }).not.toThrow();
     });
 });