Handle the unknown-op status from test commands.

Calva currently fails silently when the test commands fail due to the unknown-op status. Here we handle that error and show a warning message when it occurs. This doesn't address the root cause of the issue, but it will help users to understand the problem. This will help to debug BetterThanTomorrow#1269
marcomorain · Oct 27, 2021 · 6cb9ea7 · 6cb9ea7
1 parent 3377305
commit 6cb9ea7
Show file tree

Hide file tree

Showing 3 changed files with 123 additions and 78 deletions.
diff --git a/src/nrepl/cider.ts b/src/nrepl/cider.ts
@@ -0,0 +1,39 @@
+
+// https://github.com/clojure-emacs/cider-nrepl/blob/a740583c3aa8b582f3097611787a276775131d32/src/cider/nrepl/middleware/test.clj#L45
+export interface TestSummary {
+    ns: number;
+    var: number;
+    test: number;
+    pass: number;
+    fail: number;
+    error: number;
+};
+
+// https://github.com/clojure-emacs/cider-nrepl/blob/a740583c3aa8b582f3097611787a276775131d32/src/cider/nrepl/middleware/test.clj#L97-L112
+export interface TestResult {
+    context: string;
+    index: number;
+    message: string;
+    ns: string;
+    type: string;
+    var: string;
+    expected?: string;
+    'gen-input'?: string;
+    actual?: string;
+    diffs?: unknown;
+    error?: unknown;
+    line?: number
+    file?: string;
+}
+
+// https://github.com/clojure-emacs/cider-nrepl/blob/a740583c3aa8b582f3097611787a276775131d32/src/cider/nrepl/middleware/test.clj#L45-L46
+export interface TestResults {
+    summary: TestSummary;
+    results: {
+        [key: string]: {
+            [key: string]: TestResult[]
+        }
+    }
+    'testing-ns'?: string
+    'gen-input': unknown
+}
diff --git a/src/nrepl/index.ts b/src/nrepl/index.ts
@@ -1,5 +1,6 @@
 import * as net from "net";
 import { BEncoderStream, BDecoderStream } from "./bencode";
+import * as cider from './cider'
 import * as state from './../state';
 import * as util from '../utilities';
 import { PrettyPrintingOptions, disabledPrettyPrinter, getServerSidePrinter } from "../printer";
@@ -13,6 +14,39 @@ import { getStateValue, prettyPrint } from '../../out/cljs-lib/cljs-lib';
 import { getConfig } from '../config';
 import { log, Direction, loggingEnabled } from './logging';
 
+// The result object that is returned from an nREPL command.
+interface Result {
+    id: string
+    op: string
+    session: string
+    status: string[]
+    err?: string
+    error?: string
+    reloading?: string[]
+}
+
+// https://nrepl.org/nrepl/design/handlers.html
+// If the handler being used by an nREPL server does not recognize or cannot
+// perform the operation indicated by a request message’s :op, then it should
+// respond with a message containing a :status of "unknown-op".
+function unknownOperation(res: Result): boolean {
+    return res.status.indexOf("unknown-op") > -1;
+}
+
+// When a test command fails becuase of an unknonw-op (usually caused by missing
+// middleware), we can mark the operation as failed, so that we can show a message
+// in the UI to the user.
+function testResultHandler(resolve: any, reject: any) {
+    return (res: Result): boolean => {
+        if (unknownOperation(res)) {
+            reject("The server does not recognize or cannot perform the '" + res.op + "' operation");
+        } else {
+            resolve(res);
+        }
+        return true;
+    }
+}
+
 /** An nREPL client */
 export class NReplClient {
     private _nextId = 0;
@@ -176,7 +210,7 @@ export class NReplSession {
         }
     }
 
-    messageHandlers: { [id: string]: (msg: any) => boolean } = {};
+    messageHandlers: { [id: string]: (msg: Result) => boolean } = {};
     replType: ReplSessionType = null;
 
     close() {
@@ -395,7 +429,7 @@ export class NReplSession {
             this.client.write({ op: "info", ns, symbol, id, session: this.sessionId })
         })
     }
-    
+
     classpath() {
         return new Promise<any>((resolve, reject) => {
             let id = this.client.nextId;
@@ -407,13 +441,10 @@ export class NReplSession {
         })
     }
 
-    test(ns: string, test: string) {
+    test(ns: string, test: string): Promise<cider.TestResults> {
         return new Promise<any>((resolve, reject) => {
             const id = this.client.nextId;
-            this.messageHandlers[id] = (msg) => {
-                resolve(msg);
-                return true;
-            };
+            this.messageHandlers[id] = testResultHandler(resolve, reject);
             this.client.write({
                 op: "test-var-query",
                 ns,
@@ -431,12 +462,9 @@ export class NReplSession {
     }
 
     testNs(ns: string) {
-        return new Promise<any>((resolve, reject) => {
+        return new Promise<cider.TestResults>((resolve, reject) => {
             let id = this.client.nextId;
-            this.messageHandlers[id] = (msg) => {
-                resolve(msg);
-                return true;
-            }
+            this.messageHandlers[id] = testResultHandler(resolve, reject);
             this.client.write({
                 op: "test-var-query", ns, id, session: this.sessionId, "var-query": {
                     "ns-query": {
@@ -448,34 +476,25 @@ export class NReplSession {
     }
 
     testAll() {
-        return new Promise<any>((resolve, reject) => {
+        return new Promise<cider.TestResults>((resolve, reject) => {
             let id = this.client.nextId;
-            this.messageHandlers[id] = (msg) => {
-                resolve(msg);
-                return true;
-            }
+            this.messageHandlers[id] = testResultHandler(resolve, reject);
             this.client.write({ op: "test-all", id, session: this.sessionId, "load?": true });
         })
     }
 
     retest() {
-        return new Promise<any>((resolve, reject) => {
+        return new Promise<cider.TestResults>((resolve, reject) => {
             let id = this.client.nextId;
-            this.messageHandlers[id] = (msg) => {
-                resolve(msg);
-                return true;
-            }
+            this.messageHandlers[id] = testResultHandler(resolve, reject);
             this.client.write({ op: "retest", id, session: this.sessionId });
         })
     }
 
     loadAll() {
         return new Promise<any>((resolve, reject) => {
             let id = this.client.nextId;
-            this.messageHandlers[id] = (msg) => {
-                resolve(msg);
-                return true;
-            }
+            this.messageHandlers[id] = testResultHandler(resolve, reject);
             this.client.write({ op: "ns-load-all", id, session: this.sessionId });
         })
     }

diff --git a/src/testRunner.ts b/src/testRunner.ts
@@ -5,52 +5,14 @@ import * as util from './utilities';
 import { disabledPrettyPrinter } from './printer';
 import * as outputWindow from './results-output/results-doc';
 import { NReplSession } from './nrepl';
+import * as cider from './nrepl/cider'
 import * as namespace from './namespace';
 import { getSession, updateReplSessionType } from './nrepl/repl-session';
 import * as getText from './util/get-text';
 
 let diagnosticCollection = vscode.languages.createDiagnosticCollection('calva');
 
-// https://github.com/clojure-emacs/cider-nrepl/blob/a740583c3aa8b582f3097611787a276775131d32/src/cider/nrepl/middleware/test.clj#L45
-interface CiderTestSummary {
-    ns: number;
-    var: number;
-    test: number;
-    pass: number;
-    fail: number;
-    error: number;
-};
-
-// https://github.com/clojure-emacs/cider-nrepl/blob/a740583c3aa8b582f3097611787a276775131d32/src/cider/nrepl/middleware/test.clj#L97-L112
-interface CiderTestResult {
-    context: string;
-    index: number;
-    message: string;
-    ns: string;
-    type: string;
-    var: string;
-    expected?: string;
-    'gen-input'?: string;
-    actual?: string;
-    diffs?: unknown;
-    error?: unknown;
-    line?: number
-    file?: string;
-}
-
-// https://github.com/clojure-emacs/cider-nrepl/blob/a740583c3aa8b582f3097611787a276775131d32/src/cider/nrepl/middleware/test.clj#L45-L46
-interface CiderTestResults {
-    summary: CiderTestSummary;
-    results: {
-        [key: string]: {
-            [key: string]: CiderTestResult[]
-        }
-    }
-    'testing-ns'?: string
-    'gen-input': unknown
-}
-
-function resultMessage(resultItem: Readonly<CiderTestResult>): string {
+function resultMessage(resultItem: Readonly<cider.TestResult>): string {
     let msg = [];
     if (!_.isEmpty(resultItem.context) && resultItem.context !== "false")
         msg.push(resultItem.context);
@@ -59,9 +21,9 @@ function resultMessage(resultItem: Readonly<CiderTestResult>): string {
     return `${msg.length > 0 ? msg.join(": ").replace(/\r?\n$/, "") : ''}`;
 }
 
-function reportTests(results: CiderTestResults[], errorStr: string) {
+function reportTests(results: cider.TestResults[]) {
     let diagnostics: { [key: string]: vscode.Diagnostic[] } = {};
-    let total_summary: CiderTestSummary = { test: 0, error: 0, ns: 0, var: 0, fail: 0, pass: 0 };
+    let total_summary: cider.TestSummary = { test: 0, error: 0, ns: 0, var: 0, fail: 0, pass: 0 };
     diagnosticCollection.clear();
 
     for (let result of results) {
@@ -135,7 +97,11 @@ function reportTests(results: CiderTestResults[], errorStr: string) {
 async function runAllTests(document = {}) {
     const session = getSession(util.getFileType(document));
     outputWindow.append("; Running all project tests…");
-    reportTests([await session.testAll()], "Running all tests");
+    try {
+        reportTests([await session.testAll()]);
+    } catch (e) {
+        outputWindow.append('; ' + e)
+    }
     updateReplSessionType();
     outputWindow.appendPrompt();
 }
@@ -145,7 +111,9 @@ function runAllTestsCommand() {
         vscode.window.showInformationMessage('You must connect to a REPL server to run this command.')
         return;
     }
-    runAllTests().catch(() => { });
+    runAllTests().catch((msg) => {
+        vscode.window.showWarningMessage(msg)
+    });
 }
 
 async function considerTestNS(ns: string, session: NReplSession, nss: string[]): Promise<string[]> {
@@ -183,8 +151,12 @@ async function runNamespaceTests(document = {}) {
     if (nss.length > 1) {
         resultPromises.push(session.testNs(nss[1]));
     }
-    const results = await Promise.all(resultPromises);
-    reportTests(results, "Running tests");
+    try {
+        reportTests(await Promise.all(resultPromises));
+    } catch (e) {
+        outputWindow.append('; ' + e)
+    }
+
     outputWindow.setSession(session, ns);
     updateReplSessionType();
     outputWindow.appendPrompt();
@@ -206,8 +178,11 @@ async function runTestUnderCursor() {
     if (test) {
         await evaluate.loadFile(doc, disabledPrettyPrinter);
         outputWindow.append(`; Running test: ${test}…`);
-        const results = [await session.test(ns, test)];
-        reportTests(results, `Running test: ${test}`);
+        try {
+            reportTests([await session.test(ns, test)]);
+        } catch (e) {
+            outputWindow.append('; ' + e)
+        }
     } else {
         outputWindow.append('; No test found at cursor');
     }
@@ -219,22 +194,32 @@ function runTestUnderCursorCommand() {
         vscode.window.showInformationMessage('You must connect to a REPL server to run this command.')
         return;
     }
-    runTestUnderCursor().catch(() => { });
+    runTestUnderCursor().catch((msg) => {
+        vscode.window.showWarningMessage(msg)
+    });
 }
 
 function runNamespaceTestsCommand() {
     if (!util.getConnectedState()) {
         vscode.window.showInformationMessage('You must connect to a REPL server to run this command.')
         return;
     }
-    runNamespaceTests();
+    runNamespaceTests().catch((msg) => {
+        vscode.window.showWarningMessage(msg)
+    });
 }
 
 async function rerunTests(document = {}) {
     let session = getSession(util.getFileType(document))
     await evaluate.loadFile({}, disabledPrettyPrinter);
     outputWindow.append("; Running previously failed tests…");
-    reportTests([await session.retest()], "Retesting");
+
+    try {
+        reportTests([await session.retest()]);
+    } catch (e) {
+        outputWindow.append('; ' + e)
+    }
+
     outputWindow.appendPrompt();
 }
 
@@ -243,7 +228,9 @@ function rerunTestsCommand() {
         vscode.window.showInformationMessage('You must connect to a REPL server to run this command.')
         return;
     }
-    rerunTests();
+    rerunTests().catch((msg) => {
+        vscode.window.showWarningMessage(msg)
+    });
 }
 
 export default {