Repository: sjbarag/brs
Pull request #492 (2020-08-05 21:23:47)

fix(exts,mocks): differentiate between scoped and unscoped function mocks
# Change Summary

We currently conflate module-scoped mock functions (`mockComponentPartial`) with universally scoped mock functions (mocks that override any function of the same name, at every scope level above function-scope). This causes issues when the universal mocks aren't copied from environment to environment.

This PR separates scoped vs. unscoped function mocks.


Patch:
diff --git a/src/extensions/mockComponentPartial.ts b/src/extensions/mockComponentPartial.ts
index 3041078..6ab58cb 100644
--- a/src/extensions/mockComponentPartial.ts
+++ b/src/extensions/mockComponentPartial.ts
@@ -31,7 +31,11 @@ export const mockComponentPartial = new Callable("mockComponentPartial", {
                 .filter(([_, value]) => value.kind === ValueKind.Callable)
                 .forEach(([key, value]) => {
                     if (value instanceof Callable) {
-                        maybeComponent?.environment?.setMockFunction(key.toString(), value);
+                        maybeComponent?.environment?.setMockFunction(
+                            key.toString(),
+                            value,
+                            /* isScoped */ true
+                        );
                     }
                 });
         } else {
diff --git a/src/interpreter/Environment.ts b/src/interpreter/Environment.ts
index bf1c558..19f38af 100644
--- a/src/interpreter/Environment.ts
+++ b/src/interpreter/Environment.ts
@@ -49,9 +49,13 @@ export class Environment {
      */
     private mockObjects = new Map<string, RoAssociativeArray>();
     /**
-     * Mocked functions
+     * Unscoped mock functions. These mocks are used across all components and files.
      */
-    private mockFunctions = new Map<string, Callable>();
+    private unscopedMockFunctions = new Map<string, Callable>();
+    /**
+     * Scoped mock functions. These mocks are only used in this component.
+     */
+    private scopedMockFunctions = new Map<string, Callable>();
     /** The BrightScript `m` pointer, analogous to JavaScript's `this` pointer. */
     private mPointer = new RoAssociativeArray([]);
     private rootM: RoAssociativeArray;
@@ -251,18 +255,18 @@ export class Environment {
         newEnvironment.global = new Map(this.global);
         newEnvironment.mPointer = this.mPointer;
         newEnvironment.mockObjects = this.mockObjects;
+        newEnvironment.unscopedMockFunctions = this.unscopedMockFunctions;
+        newEnvironment.focusedNode = this.focusedNode;
+        newEnvironment.nodeDefMap = this.nodeDefMap;
 
         if (includeModuleScope) {
             newEnvironment.module = new Map(this.module);
-            newEnvironment.mockFunctions = this.mockFunctions;
+            newEnvironment.scopedMockFunctions = this.scopedMockFunctions;
         } else {
             newEnvironment.module = new Map<string, BrsType>();
-            newEnvironment.mockFunctions = new Map<string, Callable>();
+            newEnvironment.scopedMockFunctions = new Map<string, Callable>();
         }
 
-        newEnvironment.focusedNode = this.focusedNode;
-        newEnvironment.nodeDefMap = this.nodeDefMap;
-
         return newEnvironment;
     }
 
@@ -287,17 +291,31 @@ export class Environment {
      * places the mockValue function into list of mocks
      * @param functionName the function we are mocking
      * @param mockValue the mock to return
+     * @param isScoped whether this function should be scoped to this module or universally mocked.
      */
-    public setMockFunction(functionName: string, mockValue: Callable): void {
-        this.mockFunctions.set(functionName.toLowerCase(), mockValue);
+    public setMockFunction(
+        functionName: string,
+        mockValue: Callable,
+        isScoped: boolean = false
+    ): void {
+        if (isScoped) {
+            this.scopedMockFunctions.set(functionName.toLowerCase(), mockValue);
+        } else {
+            this.unscopedMockFunctions.set(functionName.toLowerCase(), mockValue);
+        }
     }
 
     /**
-     * retrieves mocked function if it exists
+     * Retrieves mocked function if it exists. If there is a scoped and unscoped function
+     * by the same name, it will return the scoped.
      * @param functionName the function to mock
      */
     public getMockFunction(functionName: string): Callable | BrsInvalid {
-        return this.mockFunctions.get(functionName) || BrsInvalid.Instance;
+        if (this.scopedMockFunctions.has(functionName)) {
+            return this.scopedMockFunctions.get(functionName)!;
+        }
+
+        return this.unscopedMockFunctions.get(functionName) || BrsInvalid.Instance;
     }
 
     /**
@@ -306,7 +324,11 @@ export class Environment {
      * @param possibleMockName the identifier/name for the mocked function
      */
     private isMockedFunction(possibleMockFunction: BrsType, possibleMockName: string): boolean {
-        return possibleMockFunction instanceof Callable && this.mockFunctions.has(possibleMockName);
+        return (
+            possibleMockFunction instanceof Callable &&
+            (this.scopedMockFunctions.has(possibleMockName) ||
+                this.unscopedMockFunctions.has(possibleMockName))
+        );
     }
 
     /**
@@ -328,7 +350,8 @@ export class Environment {
      * resets all of the mocked functions in this environment
      */
     public resetMockFunctions() {
-        this.mockFunctions.clear();
+        this.scopedMockFunctions.clear();
+        this.unscopedMockFunctions.clear();
     }
 
     /**


Test patch:
diff --git a/test/e2e/BrsComponents.test.js b/test/e2e/BrsComponents.test.js
index dff37a3..660468a 100644
--- a/test/e2e/BrsComponents.test.js
+++ b/test/e2e/BrsComponents.test.js
@@ -666,21 +666,4 @@ describe("end to end brightscript functions", () => {
             "Node",
         ]);
     });
-
-    test("components/scripts/MockFunctionsMain.brs", async () => {
-        await execute(
-            [resourceFile("components", "scripts", "MockFunctionsMain.brs")],
-            outputStreams
-        );
-        expect(allArgs(outputStreams.stdout.write).filter((arg) => arg !== "\n")).toEqual([
-            "{fake:'json'}",
-            "GET status: 400",
-            "POST status: 500",
-            "true",
-            "{real: 'json'}",
-            "GET status: 200",
-            "POST status: 200",
-            "false",
-        ]);
-    });
 });
diff --git a/test/e2e/BrsMock.test.js b/test/e2e/BrsMock.test.js
index bbeb025..23609c1 100644
--- a/test/e2e/BrsMock.test.js
+++ b/test/e2e/BrsMock.test.js
@@ -86,10 +86,12 @@ describe("end to end brightscript functions", () => {
             "GET status: 400",
             "POST status: 500",
             "true",
+            "mocked correctly!",
             "{real: 'json'}",
             "GET status: 200",
             "POST status: 200",
             "false",
+            "mocked correctly!",
         ]);
     });
 
diff --git a/test/e2e/resources/components/ComponentMockFunctions.xml b/test/e2e/resources/components/ComponentMockFunctions.xml
index 7bdffcf..1841666 100644
--- a/test/e2e/resources/components/ComponentMockFunctions.xml
+++ b/test/e2e/resources/components/ComponentMockFunctions.xml
@@ -3,6 +3,7 @@
     <script type="text/brightscript" uri="pkg:/components/scripts/ComponentMockFunctions.brs" />
     <script type="text/brightscript" uri="pkg:/components/scripts/MockFunctionsUtils-A.brs" />
     <script type="text/brightscript" uri="pkg:/components/scripts/MockFunctionsUtils-B.brs" />
+    <script type="text/brightscript" uri="pkg:/components/scripts/MockFunctionsUtils-C.brs" />
     <children>
     </children>
 </component>
diff --git a/test/e2e/resources/components/ComponentWithoutMockFunctions.xml b/test/e2e/resources/components/ComponentWithoutMockFunctions.xml
index 3fc79ea..8675afa 100644
--- a/test/e2e/resources/components/ComponentWithoutMockFunctions.xml
+++ b/test/e2e/resources/components/ComponentWithoutMockFunctions.xml
@@ -3,6 +3,7 @@
     <script type="text/brightscript" uri="pkg:/components/scripts/ComponentWithoutMockFunctions.brs" />
     <script type="text/brightscript" uri="pkg:/components/scripts/MockFunctionsUtils-A.brs" />
     <script type="text/brightscript" uri="pkg:/components/scripts/MockFunctionsUtils-B.brs" />
+    <script type="text/brightscript" uri="pkg:/components/scripts/MockFunctionsUtils-C.brs" />
     <children>
     </children>
-</component>
\ No newline at end of file
+</component>
diff --git a/test/e2e/resources/components/scripts/ComponentMockFunctions.brs b/test/e2e/resources/components/scripts/ComponentMockFunctions.brs
index c52d49c..91e99f7 100644
--- a/test/e2e/resources/components/scripts/ComponentMockFunctions.brs
+++ b/test/e2e/resources/components/scripts/ComponentMockFunctions.brs
@@ -4,4 +4,5 @@ sub init()
     print http_get() ' => GET status: 400
     print http_post() ' => POST status: 500
     print isValid() ' => true
+    print shouldBeMocked() ' => "mocked correctly!"
 end sub
diff --git a/test/e2e/resources/components/scripts/ComponentWithoutMockFunctions.brs b/test/e2e/resources/components/scripts/ComponentWithoutMockFunctions.brs
index 68d97f1..04f4a83 100644
--- a/test/e2e/resources/components/scripts/ComponentWithoutMockFunctions.brs
+++ b/test/e2e/resources/components/scripts/ComponentWithoutMockFunctions.brs
@@ -3,4 +3,5 @@ sub init()
     print http_get() ' => GET status: 200
     print http_post() ' => POST status: 200
     print isValid() ' => false
+    print shouldBeMocked() ' => "mocked correctly!"
 end sub
diff --git a/test/e2e/resources/components/scripts/MockFunctionsMain.brs b/test/e2e/resources/components/scripts/MockFunctionsMain.brs
index 6f1e986..b8c6047 100644
--- a/test/e2e/resources/components/scripts/MockFunctionsMain.brs
+++ b/test/e2e/resources/components/scripts/MockFunctionsMain.brs
@@ -14,6 +14,10 @@ sub main()
         end function
     })
 
+    _brs_.mockFunction("shouldBeMocked", function() as dynamic
+        return "mocked correctly!"
+    end function)
+
     m1 = createObject("roSGNode", "ComponentMockFunctions")
     real = createObject("roSGNode", "ComponentWithoutMockFunctions")
 end sub
diff --git a/test/e2e/resources/components/scripts/MockFunctionsUtils-C.brs b/test/e2e/resources/components/scripts/MockFunctionsUtils-C.brs
new file mode 100644
index 0000000..48f84f2
--- /dev/null
+++ b/test/e2e/resources/components/scripts/MockFunctionsUtils-C.brs
@@ -0,0 +1,3 @@
+function shouldBeMocked()
+    return "noooo should be mocked"
+end function