From 84abf02f559361be1d6908d39166ea74eb820041 Mon Sep 17 00:00:00 2001 From: Milly Date: Sat, 11 May 2024 06:49:17 +0900 Subject: [PATCH] :+1: Improved test error handling --- README.md | 2 + conf.ts | 13 +++++++ runner.ts | 73 +++++++++++++++++++++++++++++++----- with.ts | 104 +++++++++++++++++++++++++++++++++++++-------------- with_test.ts | 76 ++++++++++++++++++++++++++----------- 5 files changed, 208 insertions(+), 60 deletions(-) diff --git a/README.md b/README.md index 701b0c0..8488417 100644 --- a/README.md +++ b/README.md @@ -24,6 +24,8 @@ used in the unit tests of denops plugins. > - `DENOPS_TEST_NVIM_EXECUTABLE`: Path to the Neovim executable (default: > "nvim") > - `DENOPS_TEST_VERBOSE`: `1` or `true` to print Vim messages (echomsg) +> - `DENOPS_TEST_CONNECT_TIMEOUT`: Timeout [ms] for connecting to Vim/Neovim +> (default: 30000) If you want to test denops plugins with a real Vim and/or Neovim process, use the `test` function to define a test case, as shown below: diff --git a/conf.ts b/conf.ts index 7c37f18..51a0499 100644 --- a/conf.ts +++ b/conf.ts @@ -29,6 +29,12 @@ export interface Config { * It refers to the environment variable 'DENOPS_TEST_VERBOSE'. */ verbose: boolean; + + /** + * Timeout for connecting to Vim/Neovim. + * It refers to the environment variable 'DENOPS_TEST_CONNECT_TIMEOUT'. + */ + connectTimeout?: number; } /** @@ -43,6 +49,7 @@ export interface Config { * - `DENOPS_TEST_VIM_EXECUTABLE`: Path to the Vim executable (default: "vim") * - `DENOPS_TEST_NVIM_EXECUTABLE`: Path to the Neovim executable (default: "nvim") * - `DENOPS_TEST_VERBOSE`: `1` or `true` to print Vim messages (echomsg) + * - `DENOPS_TEST_CONNECT_TIMEOUT`: Timeout [ms] for connecting to Vim/Neovim (default: 30000) * * It throws an error if the environment variable 'DENOPS_TEST_DENOPS_PATH' is * not set. @@ -58,11 +65,17 @@ export function getConfig(): Config { ); } const verbose = Deno.env.get("DENOPS_TEST_VERBOSE"); + const connectTimeout = Number.parseInt( + Deno.env.get("DENOPS_TEST_CONNECT_TIMEOUT") ?? "", + ); conf = { denopsPath: resolve(denopsPath), vimExecutable: Deno.env.get("DENOPS_TEST_VIM_EXECUTABLE") ?? "vim", nvimExecutable: Deno.env.get("DENOPS_TEST_NVIM_EXECUTABLE") ?? "nvim", verbose: verbose === "1" || verbose === "true", + connectTimeout: Number.isNaN(connectTimeout) || connectTimeout <= 0 + ? undefined + : connectTimeout, }; return conf; } diff --git a/runner.ts b/runner.ts index 7a2f4f9..48df795 100644 --- a/runner.ts +++ b/runner.ts @@ -1,3 +1,4 @@ +import { mergeReadableStreams } from "https://deno.land/std@0.210.0/streams/merge_readable_streams.ts"; import { is } from "https://deno.land/x/unknownutil@v3.11.0/mod.ts"; import { unreachable } from "https://deno.land/x/errorutil@v0.1.1/mod.ts"; import { Config, getConfig } from "./conf.ts"; @@ -18,6 +19,25 @@ export interface RunOptions verbose?: boolean; } +/** + * Represents results of the runner. + */ +export interface RunResult extends AsyncDisposable { + /** + * Aborts the process. + */ + close(): void; + /** + * Wait the process closed and returns status. + */ + waitClosed(): Promise; +} + +type WaitClosedResult = { + status: Deno.CommandStatus; + output?: string; +}; + /** * Checks if the provided mode is a valid `RunMode`. */ @@ -34,22 +54,51 @@ export function run( mode: RunMode, cmds: string[], options: RunOptions = {}, -): Deno.ChildProcess { +): RunResult { const conf = getConfig(); - const verbose = options.verbose ?? conf.verbose; + const { verbose = conf.verbose } = options; const [cmd, args] = buildArgs(conf, mode); args.push(...cmds.flatMap((c) => ["-c", c])); - if (verbose) { - args.unshift("--cmd", "redir >> /dev/stdout"); - } + const aborter = new AbortController(); + const { signal } = aborter; const command = new Deno.Command(cmd, { args, env: options.env, stdin: "piped", - stdout: verbose ? "inherit" : "null", - stderr: verbose ? "inherit" : "null", + stdout: "piped", + stderr: "piped", + signal, }); - return command.spawn(); + const proc = command.spawn(); + let outputStream = mergeReadableStreams( + proc.stdout.pipeThrough(new TextDecoderStream(), { signal }), + proc.stderr.pipeThrough(new TextDecoderStream(), { signal }), + ); + if (verbose) { + const [consoleStream] = [, outputStream] = outputStream.tee(); + consoleStream.pipeTo( + new WritableStream({ write: (data) => console.error(data) }), + ).catch(() => {}); + } + return { + close() { + aborter.abort("close"); + }, + async waitClosed() { + const [status, output] = await Promise.all([ + proc.status, + Array.fromAsync(outputStream) + .then((list) => list.join("")) + .catch(() => undefined), + ]); + await proc.stdin.abort(); + return { status, output }; + }, + async [Symbol.asyncDispose]() { + this.close(); + await this.waitClosed(); + }, + }; } function buildArgs(conf: Config, mode: RunMode): [string, string[]] { @@ -67,6 +116,7 @@ function buildArgs(conf: Config, mode: RunMode): [string, string[]] { "-X", // Disable xterm "-e", // Start Vim in Ex mode "-s", // Silent or batch mode ("-e" is required before) + "-V1", // Verbose level 1 (Echo messages to stderr) "-c", "visual", // Go to Normal mode ], @@ -74,7 +124,12 @@ function buildArgs(conf: Config, mode: RunMode): [string, string[]] { case "nvim": return [ conf.nvimExecutable, - ["--clean", "--embed", "--headless", "-n"], + [ + "--clean", + "--headless", + "-n", // Disable swap file + "-V1", // Verbose level 1 (Echo messages to stderr) + ], ]; default: unreachable(mode); diff --git a/with.ts b/with.ts index 4ea7c68..7490472 100644 --- a/with.ts +++ b/with.ts @@ -29,6 +29,8 @@ export interface WithDenopsOptions { prelude?: string[]; /** Vim commands to be executed after the start of Denops */ postlude?: string[]; + /** Timeout for connecting to Vim/Neovim */ + connectTimeout?: number; } /** @@ -66,34 +68,44 @@ export async function withDenops( options: WithDenopsOptions = {}, ) { const conf = getConfig(); - const name = options.pluginName ?? PLUGIN_NAME; + const { + pluginName = PLUGIN_NAME, + verbose = conf.verbose, + prelude = [], + postlude = [], + connectTimeout = conf.connectTimeout ?? CONNECT_TIMEOUT, + } = options; const plugin = new URL("./plugin.ts", import.meta.url); const commands = [ - ...(options.prelude ?? []), + ...prelude, "let g:denops#_test = 1", `set runtimepath^=${conf.denopsPath.replace(/ /g, "\\ ")}`, [ "try", - ` call denops#server#wait_async({ -> denops#plugin#load('${name}', '${plugin}') })`, + ` call denops#server#wait_async({ -> denops#plugin#load('${pluginName}', '${plugin}') })`, "catch /^Vim\\%((\\a\\+)\\)\\=:E117:/", - ` execute 'autocmd User DenopsReady call denops#plugin#register(''${name}'', ''${plugin}'')'`, + ` execute 'autocmd User DenopsReady call denops#plugin#register(''${pluginName}'', ''${plugin}'')'`, "endtry", ].join(" | "), "call denops#server#start()", - ...(options.postlude ?? []), + ...postlude, ]; - const listener = Deno.listen({ + const aborter = new AbortController(); + const { signal } = aborter; + using listener = Deno.listen({ hostname: "127.0.0.1", port: 0, // Automatically select a free port }); - const proc = run(mode, commands, { - verbose: options.verbose, - env: { - "DENOPS_TEST_ADDRESS": JSON.stringify(listener.addr), - }, - }); - const conn = await deadline(listener.accept(), CONNECT_TIMEOUT); - try { + const getConn = async () => { + try { + return await deadline(listener.accept(), connectTimeout, { signal }); + } catch (cause: unknown) { + throw new Error("[denops-test] Connection failed.", { cause }); + } finally { + listener.close(); + } + }; + const createSession = (conn: Deno.Conn) => { const session = new Session(conn.readable, conn.writable, { errorSerializer, }); @@ -105,7 +117,9 @@ export async function withDenops( `[denops-test] Unexpected error occurred for message ${message}: ${err}`, ); }; - session.start(); + return session; + }; + const createDenops = async (session: Session) => { const client = new Client(session, { errorDeserializer, }); @@ -114,7 +128,7 @@ export async function withDenops( "call", ["denops#_internal#meta#get"], ) as Meta; - const denops = new DenopsImpl(name, meta, client); + const denops = new DenopsImpl(pluginName, meta, client); session.dispatcher = { dispatch: (name, args) => { assert(name, is.String); @@ -122,17 +136,49 @@ export async function withDenops( return denops.dispatcher[name](...args); }, }; - // Workaround for an unexpected "leaking async ops" - // https://github.com/denoland/deno/issues/15425#issuecomment-1368245954 - await new Promise((resolve) => setTimeout(resolve, 0)); - await main(denops); - await session.shutdown(); - } finally { - listener.close(); - proc.kill(); - await Promise.all([ - proc.stdin?.close(), - proc.output(), - ]); - } + return denops; + }; + const perform = async () => { + using conn = await getConn(); + const session = createSession(conn); + session.start(); + try { + const denops = await createDenops(session); + + // Workaround for an unexpected "leaking async ops" + // https://github.com/denoland/deno/issues/15425#issuecomment-1368245954 + // Maybe fixed in v1.41.0 + // https://github.com/denoland/deno/pull/22413 + // TODO: Remove this workaround when Deno minimum version changes to v1.41.0 or higher + await new Promise((resolve) => setTimeout(resolve, 0)); + + await main(denops); + } finally { + try { + await session.shutdown(); + } catch { + // Already shutdown, do nothing. + } + } + }; + await using runner = run(mode, commands, { + verbose, + env: { + "DENOPS_TEST_ADDRESS": JSON.stringify(listener.addr), + }, + }); + await Promise.race([ + perform(), + runner.waitClosed().then(({ status, output }) => { + aborter.abort("closed"); + if (!status.success) { + const suffix = output?.length + ? `:\n------- output -------\n${output}\n----- output end -----` + : "."; + throw new Error( + `[denops-test] Process aborted (${mode}, code=${status.code})${suffix}`, + ); + } + }), + ]); } diff --git a/with_test.ts b/with_test.ts index 04bde1d..4fce556 100644 --- a/with_test.ts +++ b/with_test.ts @@ -1,29 +1,61 @@ import { assert, assertFalse, + assertRejects, } from "https://deno.land/std@0.210.0/assert/mod.ts"; +import { + assertSpyCalls, + spy, +} from "https://deno.land/std@0.210.0/testing/mock.ts"; +import type { Denops } from "https://deno.land/x/denops_core@v6.0.2/mod.ts"; import { withDenops } from "./with.ts"; -Deno.test( - "test(mode:vim) start vim to test denops features", - async () => { - let called = false; - await withDenops("vim", async (denops) => { - assertFalse(await denops.call("has", "nvim")); - called = true; - }); - assert(called, "withDenops main is not called"); - }, -); +Deno.test("test(mode:vim) start vim to test denops features", async () => { + const main = spy(async (denops: Denops) => { + assertFalse(await denops.call("has", "nvim")); + }); + await withDenops("vim", main); + assertSpyCalls(main, 1); +}); + +Deno.test("test(mode:nvim) start nvim to test denops features", async () => { + const main = spy(async (denops: Denops) => { + assert(await denops.call("has", "nvim")); + }); + await withDenops("nvim", main); + assertSpyCalls(main, 1); +}); + +for (const mode of ["vim", "nvim"] as const) { + Deno.test(`test(mode:${mode}) rejects if process aborted`, async () => { + const fn = spy(() => {}); + await assertRejects( + async () => { + await withDenops(mode, fn, { + prelude: [ + "echomsg 'foobar'", + "cquit", + ], + }); + }, + Error, + "foobar", + ); + assertSpyCalls(fn, 0); + }); -Deno.test( - "test(mode:nvim) start vim to test denops features", - async () => { - let called = false; - await withDenops("nvim", async (denops) => { - assert(await denops.call("has", "nvim")); - called = true; - }); - assert(called, "withDenops main is not called"); - }, -); + Deno.test(`test(mode:${mode}) rejects if connection failed`, async () => { + const fn = spy(() => {}); + await assertRejects( + async () => { + await withDenops(mode, fn, { + prelude: ["sleep 1"], // Set sleep [s] longer than timeout + connectTimeout: 10, // Set timeout [ms] shorter than sleep + }); + }, + Error, + "Connection failed", + ); + assertSpyCalls(fn, 0); + }); +}