From 6cb9ea782ec40d28170a81c1b30e04aac92d2282 Mon Sep 17 00:00:00 2001 From: Marc O'Morain Date: Wed, 27 Oct 2021 12:34:32 +0100 Subject: [PATCH] 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 #1269 --- src/nrepl/cider.ts | 39 ++++++++++++++++++++ src/nrepl/index.ts | 71 +++++++++++++++++++++++------------- src/testRunner.ts | 91 ++++++++++++++++++++-------------------------- 3 files changed, 123 insertions(+), 78 deletions(-) create mode 100644 src/nrepl/cider.ts diff --git a/src/nrepl/cider.ts b/src/nrepl/cider.ts new file mode 100644 index 000000000..456d3b2f5 --- /dev/null +++ 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 index 049de303f..3df3677a7 100644 --- 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((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 { return new Promise((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((resolve, reject) => { + return new Promise((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,23 +476,17 @@ export class NReplSession { } testAll() { - return new Promise((resolve, reject) => { + return new Promise((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((resolve, reject) => { + return new Promise((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 }); }) } @@ -472,10 +494,7 @@ export class NReplSession { loadAll() { return new Promise((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 index e3fc93316..c7b40b3bf 100644 --- 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): string { +function resultMessage(resultItem: Readonly): string { let msg = []; if (!_.isEmpty(resultItem.context) && resultItem.context !== "false") msg.push(resultItem.context); @@ -59,9 +21,9 @@ function resultMessage(resultItem: Readonly): 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 { @@ -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,7 +194,9 @@ 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() { @@ -227,14 +204,22 @@ function runNamespaceTestsCommand() { 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 {