display external errors on execution (#124)

* display external errors on execution * refactor
bespoken · Oct 3, 2018 · e841d78 · e841d78
1 parent d2932fb
commit e841d78
Show file tree

Hide file tree

Showing 8 changed files with 100 additions and 37 deletions.
diff --git a/__mocks__/virtual-device-sdk.js b/__mocks__/virtual-device-sdk.js
@@ -33,7 +33,11 @@ function handleMessage(message) {
         }
     };
     if (utterance.toLowerCase().includes("help")) response.transcript = "you can say";
-    else if (utterance.toLowerCase().includes("throw error")) throw new Error("error");
+    else if (utterance.toLowerCase().includes("throw error")) {
+        const error = new Error("Error");        
+        error.error = utterance.includes("complex") ? ["Array", "error"] : "Error from virtual device";
+        throw error;
+    }
     else response.transcript = "Here's your fact";
     return response;
 }

diff --git a/lib/runner/Configuration.js b/lib/runner/Configuration.js
@@ -8,6 +8,32 @@ const Util = require("../util/Util");
 const SkillTestingDirectoryName = ".skillTesting";
 
 module.exports = class Configuration {
+
+    static skillTestingConfigDirectory() {
+        return `${getUserHome()}/${SkillTestingDirectoryName}`;
+    }
+
+    static skillTestingConfigPath() {
+        return `${this.skillTestingConfigDirectory()}/config`;
+    }
+
+    static saveConfig(config = {}) {
+        if (!fs.existsSync(this.skillTestingConfigDirectory())) {
+            fs.mkdirSync(this.skillTestingConfigDirectory());
+        }
+        let configBuffer = new Buffer(JSON.stringify(config, null, 4) + "\n");
+        fs.writeFileSync(this.skillTestingConfigPath(), configBuffer);
+    }
+
+    static readConfig() {
+        if (!fs.existsSync(this.skillTestingConfigPath())) {
+            return undefined;
+        }
+        let data = fs.readFileSync(this.skillTestingConfigPath());
+        let config = JSON.parse(data.toString());
+        return config;
+    }
+
     static async configure(json, pathName, cliOverrides, writeConfig) {
         if (writeConfig) {
             this.saveConfig(cliOverrides);
@@ -84,32 +110,7 @@ module.exports = class Configuration {
         this.overrideConfigurationWithEnvVariables();
         this.overrideConfigurationWithCli(cliOverrides);
     }
-
-    static skillTestingConfigDirectory() {
-        return `${getUserHome()}/${SkillTestingDirectoryName}`;
-    }
-
-    static skillTestingConfigPath() {
-        return `${this.skillTestingConfigDirectory()}/config`;
-    }
-
-    static saveConfig(config = {}) {
-        if (!fs.existsSync(this.skillTestingConfigDirectory())) {
-            fs.mkdirSync(this.skillTestingConfigDirectory());
-        }
-        let configBuffer = new Buffer(JSON.stringify(config, null, 4) + "\n");
-        fs.writeFileSync(this.skillTestingConfigPath(), configBuffer);
-    }
-
-    static readConfig() {
-        if (!fs.existsSync(this.skillTestingConfigPath())) {
-            return undefined;
-        }
-        let data = fs.readFileSync(this.skillTestingConfigPath());
-        let config = JSON.parse(data.toString());
-        return config;
-    }
-
+
     jestPath() {
         const defaultJestPath = path.join(__dirname, "../node_modules/.bin/jest");
         return this.json().jestPath ? this.json().jestPath : defaultJestPath;

diff --git a/lib/runner/TestRunner.js b/lib/runner/TestRunner.js
@@ -205,7 +205,7 @@ module.exports = class TestRunner {
             } else if (!assertion.goto) {
                 // If it did not pass, and was NOT a goto, then it is a failure
                 // We do not consider tests that end in goto statements failures if they do not match
-                const error = assertion.toString(response.json);
+                const error = assertion.toString(response.json, response.errorOnProcess);
                 result = new InteractionResult(interaction, assertion, error, response.errorOnProcess);
                 break;
             } 

diff --git a/lib/runner/VirtualDeviceInvoker.js b/lib/runner/VirtualDeviceInvoker.js
@@ -35,7 +35,7 @@ module.exports = class VirtualDeviceInvoker extends Invoker {
 
     async invokeBatch(interactions) {
         const messages = [];
-        
+
         // Keep an array of the actual interactions sent, as some may be skipped
         const messageInteractions = [];
         for (const interaction of interactions) {
@@ -73,20 +73,22 @@ module.exports = class VirtualDeviceInvoker extends Invoker {
         }
 
         let results = new Array(messages.length);
+        let errorOnProcess = undefined;
         if (messages.length > 0) {
             try {
                 results = await this._virtualDevice.batchMessage(messages);
             } catch (error) {
                 debug("Error: " + JSON.stringify(error));
                 results.fill({});
+                errorOnProcess = this.getError(error);
             }
         }
 
         const responses = [];
         for (let i = 0 ;i < results.length; i++) {
             const virtualDeviceResponse = new VirtualDeviceResponse(messageInteractions[i], results[i]);
-            if (Object.keys(results[i]).length === 0) {
-                virtualDeviceResponse.errorOnProcess = true;
+            if (errorOnProcess) {
+                virtualDeviceResponse.errorOnProcess = errorOnProcess;
             }
             responses.push(virtualDeviceResponse);
         }
@@ -103,6 +105,28 @@ module.exports = class VirtualDeviceInvoker extends Invoker {
         }
         return response;
     }
+
+    getError(error){
+        let objectError = undefined;
+        try {
+            objectError = JSON.parse(error);
+        } catch (_e) {
+            objectError = error;
+        }
+
+        if (typeof(objectError) === "string") {
+            return objectError;
+        } else if (typeof(objectError) === "object") {
+            if (objectError.error) {
+                if (typeof(objectError.error) === "string") {
+                    return objectError.error;
+                } else if (Array.isArray(objectError.error)) {
+                    return objectError.error.join(", ");
+                }
+            }
+            return "Error description missing.";
+        }
+    }
 }
 
 class VirtualDeviceResponse extends InvokerResponse {

diff --git a/lib/test/Assertion.js b/lib/test/Assertion.js
@@ -209,7 +209,10 @@ module.exports = class Assertions {
         return json ? jsonpath.value(json, this.path) : undefined;
     }
 
-    toString(json) {
+    toString(json, errorOnResponse) {
+        if (errorOnResponse) {
+            return errorOnResponse;
+        }
         const testSuite = this.interaction && this.interaction.test && this.interaction.test.testSuite; 
         const jsonValue = this.valueAtPath(json);
         const localizedValue = (testSuite && testSuite.getLocalizedValue(this.value)) || this.value;

diff --git a/test/Assertion.test.js b/test/Assertion.test.js
@@ -152,6 +152,16 @@ describe("assertion", () => {
         assertion = new Assertion(undefined, "ignoreCase", "=~", "/ABC/g");
         expect(assertion.evaluate(obj)).toBe(false);
     });
+
+    describe("toString", () => {
+        test("display error", () => {
+            const json = { val: "Here is a test" };
+            const assertion = new Assertion(undefined, "val", "==", "Here is a test");
+            let errorObj = "This is an error";
+            expect(assertion.toString(json, errorObj)).toBe("This is an error");
+        });
+    });
+
 });
 
 class MockResponse extends InvokerResponse {

diff --git a/test/FactSkill/fact-skill-throw-error.yml b/test/FactSkill/fact-skill-throw-error.yml
@@ -26,6 +26,21 @@ configuration:
   - cardTitle  =~ maintitle # Lowercase expected value
   - cardContent != undefined
 
+---
+- test: "Gets a new fact intent"
+- "Help":
+  - prompt:
+    - /.*you can say.*/i
+    - A phrase
+  - shouldEndSession: false
+- "Throw error complex"
+- "Get New Facts":
+  - prompt == "here's your fact"
+  - transcript == "here's your fact"
+  - cardTitle  =~ maInTiTlE # Uppercase expected value
+  - cardTitle  =~ maintitle # Lowercase expected value
+  - cardContent != undefined
+
 # Example with a one-line request and response
 ---
 - "Help": /.*You can say.*/

diff --git a/test/VirtualDeviceInvoker.test.js b/test/VirtualDeviceInvoker.test.js
@@ -308,7 +308,7 @@ describe("virtual device runner", () => {
             const runner = new TestRunner();
 
             const results = await runner.run("test/FactSkill/fact-skill-throw-error.yml");
-            expect(results.length).toEqual(3);
+            expect(results.length).toEqual(4);
 
             expect(results[0].skipped).toBe(false);
             expect(results[0].interactionResults.length).toBe(2);
@@ -318,7 +318,7 @@ describe("virtual device runner", () => {
             expect(results[1].skipped).toBe(true);
             expect(results[1].interactionResults.length).toBe(3);
             expect(results[1].interactionResults[2].error).toBeDefined();
-            expect(results[1].interactionResults[2].errorOnProcess).toBe(true);
+            expect(results[1].interactionResults[2].errorOnProcess).toBeDefined();
         });
 
         test("fail on external error", async () => {
@@ -331,17 +331,23 @@ describe("virtual device runner", () => {
             const runner = new TestRunner();
 
             const results = await runner.run("test/FactSkill/fact-skill-throw-error.yml");
-            expect(results.length).toEqual(3);
+            expect(results.length).toEqual(4);
 
             expect(results[0].skipped).toBe(false);
             expect(results[0].interactionResults.length).toBe(2);
             expect(results[0].interactionResults[0].error).toBeUndefined();
             expect(results[0].interactionResults[1].error).toBeUndefined();
-
+            
             expect(results[1].skipped).toBe(false);
             expect(results[1].interactionResults.length).toBe(3);
             expect(results[1].interactionResults[2].error).toBeDefined();
-            expect(results[1].interactionResults[2].errorOnProcess).toBe(true);
+            expect(results[1].interactionResults[2].error).toBe("Error from virtual device");
+            expect(results[1].interactionResults[2].errorOnProcess).toBeDefined();
+
+            expect(results[2].skipped).toBe(false);
+            expect(results[2].interactionResults.length).toBe(3);
+            expect(results[2].interactionResults[2].error).toBe("Array, error");
+            expect(results[2].interactionResults[2].errorOnProcess).toBeDefined();            
         });
     });
 });