test(cli,tui): add in-process source coverage gates (#1484) (#1502)

* test(cli,tui): add in-process source coverage gates (#1484)

CLI and TUI source had no coverage gate: CLI tests spawned the built
binary out-of-process (coverage never saw clients/cli/src) and the TUI
had only two smoke tests. Regressions there were invisible to CI.

CLI:
- Rewrite __tests__/helpers/cli-runner.ts to invoke runCli() in-process
  (capture process.std*.write, map thrown errors to an exit code),
  preserving the CliResult shape so all existing tests run unchanged and
  clients/cli/src is now measured under v8 instrumentation.
- cli.ts: program.exitOverride() so a commander usage error throws
  (catchable in-process) instead of tearing down the worker; --help /
  --version still exit cleanly. Drop the dead, never-imported main().
- Add a thin out-of-process e2e.test.ts that spawns the built binary for
  the shebang / process.exit paths the in-process runner can't reach.
- Add @vitest/coverage-v8, a per-file gate (90/85/80/50) excluding only
  src/index.ts (binary bootstrap), and pool: 'forks' for capture
  isolation. cli.ts lands at ~98% lines / 100% functions.

TUI:
- Add @vitest/coverage-v8 and an interim per-file gate over the
  non-React logic (logger.ts, components/tabsConfig.ts, utils/*); the
  Ink components, App.tsx, and hooks/ are excluded with a documented
  follow-up (#1501). Add tests covering the included modules (100%).

Wiring + docs:
- CI runs each client's test:coverage; root test:cli / test:tui (used by
  validate) and root test:coverage now enforce the gates.
- gitignore clients/**/coverage; update AGENTS.md and client READMEs.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(cli): clarify in-process runner footguns from PR #1502 review

Review notes on the in-process CLI test runner (no behavior change):
- Warn in runCli's docblock that --help/--version must NOT go through the
  in-process runner: exitOverride only re-throws non-zero exits, so exitCode
  0 falls through to process.exit(0) and would kill the vitest worker. Those
  belong in the out-of-process e2e.test.ts layer.
- Document that a timeout orphans the uncancellable invokeCli promise (its
  stdio subprocess stays alive; later writes hit the restored stdout) — a
  known, acceptable trade-off on the hang-only failure path.
- Fix a stale reference to the removed main() (errors now route through
  index.ts).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(cli): fix remaining stale main() reference in vitest exclusion comment

PR #1502 re-review caught a second stale `main()` mention (the first was
fixed in 52c23e41). src/index.ts inlines the bootstrap as
runCli(process.argv).then(...).catch(handleError) — there is no main().
Comment-only; no behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* chore: compose root test:coverage from per-client scripts

Replace the brittle inline cd-chain with a composition of the existing
test:cli / test:tui scripts plus a new matching test:web. Behavior is
unchanged (web → cli → tui coverage gates) but there's a single source of
truth for each client's coverage invocation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Cliff Hall 2026-06-20 15:05:58 -04:00 • committed by GitHub
commit 388ea5a7bd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 1304 additions and 197 deletions

View file

@ -3,10 +3,13 @@
## Running Tests
```bash
# Run all tests
# Run all tests (pretest builds test-servers + the CLI binary)
npm test
# Run in watch mode (useful for test file changes; won't work on CLI source changes without rebuild)
# Run all tests under the per-file coverage gate
npm run test:coverage
# Run in watch mode (useful for test file changes)
npm run test:watch
# Run specific test file
@ -16,29 +19,51 @@ npm run test:cli-headers # headers.test.ts
npm run test:cli-metadata # metadata.test.ts
```
## How the CLI is exercised
Most tests run the CLI **in-process**: `helpers/cli-runner.ts` imports `runCli()`
from `../src/cli.ts` and invokes it directly, capturing `process.stdout`/`stderr`
and mapping thrown errors to an exit code. This is what lets vitest's v8 coverage
instrument `clients/cli/src` (#1484). Because `commander` uses `.exitOverride()`,
a parse/usage error throws instead of calling `process.exit` and tearing down the
worker.
`e2e.test.ts` is the deliberately thin **out-of-process** layer: it spawns the
**built** `build/index.js` as a real subprocess to cover the shebang, the
`index.ts` `isMain` bootstrap, and the actual `process.exit` codes — the only
parts the in-process runner can't reach. (`scripts/smoke-cli.mjs` at the repo
root provides a further end-to-end check of the binary.)
## Test Files
- `cli.test.ts` - Basic CLI functionality: CLI mode, environment variables, config files, resources, prompts, logging, transport types
- `tools.test.ts` - Tool-related tests: Tool discovery, JSON argument parsing, error handling, prompts
- `headers.test.ts` - Header parsing and validation
- `metadata.test.ts` - Metadata functionality: General metadata, tool-specific metadata, parsing, merging, validation
- `methods.test.ts` - Method/option-validation paths not covered elsewhere (resource templates, missing/invalid options, the `--` target separator)
- `error-handler.test.ts` - The binary's `handleError` error sink, exercised in-process with `process.exit` stubbed
- `e2e.test.ts` - Out-of-process spawn of the built binary (exit codes + boot)
## Helpers
The `helpers/` directory contains shared utilities:
- `cli-runner.ts` - Spawns CLI as subprocess and captures output
- `test-mcp-server.ts` - Standalone stdio MCP server script for stdio transport testing
- `instrumented-server.ts` - In-process MCP test server for HTTP/SSE transports with request recording
- `cli-runner.ts` - Invokes `runCli()` in-process and captures stdout/stderr + exit code
- `assertions.ts` - Custom assertion helpers for CLI output validation
- `fixtures.ts` - Test config file generators and temporary directory management
- `fixtures.ts` - Test config/catalog file generators and temporary directory management
HTTP/SSE test servers (for transport tests) come from the
`@modelcontextprotocol/inspector-test-server` package (`createTestServerHttp`),
and the bundled stdio server is launched via `getTestMcpServerCommand()`.
## Notes
- Tests run in parallel across files (Vitest default)
- Tests within a file run sequentially (we have isolated config files and ports, so we could get more aggressive if desired)
- Config files use `crypto.randomUUID()` for uniqueness in parallel execution
- Test files run in separate forked processes (`pool: 'forks'`), so the in-process
runner's `process.std*.write` patching never overlaps across files; tests within
a file run sequentially
- Config files use `crypto.randomUUID()` for uniqueness
- HTTP/SSE servers use dynamic port allocation to avoid conflicts
- Coverage is not used because much of the code that we want to measure is run by a spawned process, so it can't be tracked by Vitest
- /sample-config.json is no longer used by tests - not clear if this file serves some other purpose so leaving it for now
- All tests now use built-in MCP test servers, there are no external dependencies on servers from a registry
- Coverage is enforced per-file (lines ≥ 90, statements ≥ 85, functions ≥ 80,
branches ≥ 50). `src/index.ts` (the binary bootstrap) is excluded because it
only runs in the spawned binary, which coverage can't instrument
- All tests use the built-in MCP test servers — no external/registry dependencies

View file

@ -0,0 +1,70 @@
import { describe, it, expect } from "vitest";
import { spawn } from "node:child_process";
import { resolve, dirname } from "node:path";
import { fileURLToPath } from "node:url";
import { getTestMcpServerCommand } from "@modelcontextprotocol/inspector-test-server";
const here = dirname(fileURLToPath(import.meta.url));
const BIN = resolve(here, "../build/index.js");
interface SpawnResult {
exitCode: number | null;
stdout: string;
stderr: string;
}
/**
* Spawn the **built** CLI binary as a real subprocess. This is the deliberately
* thin out-of-process layer that the in-process suite (cli-runner.ts) cannot
* reach: the shebang, `index.ts`'s `isMain` bootstrap, and the actual
* `process.exit` codes. Functional behavior is covered in-process under the
* coverage gate; this only asserts the binary boots and exits correctly. The
* binary is built by the `pretest` / `test:coverage` scripts before tests run.
*/
function spawnCli(args: string[]): Promise<SpawnResult> {
return new Promise((resolvePromise, reject) => {
const child = spawn("node", [BIN, ...args], {
stdio: ["pipe", "pipe", "pipe"],
detached: process.platform !== "win32",
});
let stdout = "";
let stderr = "";
const timer = setTimeout(() => {
if (process.platform !== "win32" && child.pid != null) {
process.kill(-child.pid, "SIGTERM");
} else {
child.kill("SIGTERM");
}
reject(new Error("E2E CLI timed out"));
}, 15000);
child.stdout.on("data", (d) => (stdout += d.toString()));
child.stderr.on("data", (d) => (stderr += d.toString()));
child.on("error", (err) => {
clearTimeout(timer);
reject(err);
});
child.on("close", (exitCode) => {
clearTimeout(timer);
resolvePromise({ exitCode, stdout, stderr });
});
});
}
describe("CLI binary (out-of-process E2E)", () => {
const { command, args } = getTestMcpServerCommand();
it("exits 0 and prints tools JSON on a successful run", async () => {
const result = await spawnCli([command, ...args, "--method", "tools/list"]);
expect(result.exitCode).toBe(0);
const json = JSON.parse(result.stdout);
expect(Array.isArray(json.tools)).toBe(true);
});
it("exits non-zero when required --method is missing", async () => {
const result = await spawnCli([command, ...args]);
expect(result.exitCode).not.toBe(0);
expect(result.stderr).toContain("Method is required");
});
});

View file

@ -0,0 +1,45 @@
import { describe, it, expect, vi, afterEach } from "vitest";
import { handleError } from "../src/error-handler.js";
/**
* `handleError` is the binary's last-resort error sink (wired up in
* `src/index.ts`). It is exercised in-process here — rather than only through
* the spawned binary — so its source is measured under the CLI coverage gate
* (#1484). `process.exit` is stubbed so asserting on it doesn't tear down the
* test worker.
*/
describe("handleError", () => {
afterEach(() => {
vi.restoreAllMocks();
});
it("logs an Error's message and exits with code 1", () => {
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
const exitSpy = vi
.spyOn(process, "exit")
.mockImplementation((() => undefined) as never);
handleError(new Error("boom"));
expect(errorSpy).toHaveBeenCalledWith("boom");
expect(exitSpy).toHaveBeenCalledWith(1);
});
it("logs a string error verbatim", () => {
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
vi.spyOn(process, "exit").mockImplementation((() => undefined) as never);
handleError("plain failure");
expect(errorSpy).toHaveBeenCalledWith("plain failure");
});
it("falls back to 'Unknown error' for non-Error, non-string values", () => {
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
vi.spyOn(process, "exit").mockImplementation((() => undefined) as never);
handleError({ unexpected: true });
expect(errorSpy).toHaveBeenCalledWith("Unknown error");
});
});

View file

@ -1,10 +1,4 @@
import { spawn } from "child_process";
import { resolve } from "path";
import { fileURLToPath } from "url";
import { dirname } from "path";
const __dirname = dirname(fileURLToPath(import.meta.url));
const CLI_PATH = resolve(__dirname, "../../build/index.js");
import { runCli as invokeCli } from "../../src/cli.js";
export interface CliResult {
exitCode: number | null;
@ -15,83 +9,142 @@ export interface CliResult {
export interface CliOptions {
timeout?: number;
/**
* Accepted for source-compatibility with the previous out-of-process runner.
* The in-process runner shares the test worker's working directory, so it is
* ignored — no current test sets it. The out-of-process E2E layer
* (`e2e.test.ts`) still honors a real cwd.
*/
cwd?: string;
env?: Record<string, string>;
signal?: AbortSignal;
}
type WriteArgs = [
chunk: unknown,
encoding?: unknown,
callback?: (() => void) | undefined,
];
/**
* Run the CLI with given arguments and capture output
* Build a `process.std{out,err}.write` replacement that appends every chunk to
* a buffer and still honors the optional write callback. `awaitableLog` (the
* CLI's stdout writer) passes its resolve handler as that callback, so it MUST
* be invoked or `runCli` would hang forever waiting for the flush.
*/
function captureWrite(append: (text: string) => void) {
return (...args: WriteArgs): boolean => {
const [chunk, encoding, callback] = args;
append(typeof chunk === "string" ? chunk : String(chunk));
const cb = typeof encoding === "function" ? encoding : callback;
if (typeof cb === "function") cb();
return true;
};
}
/**
* Run the CLI **in-process** by importing and invoking `runCli` directly, so
* its source (`clients/cli/src/**`) is measured under vitest's coverage
* instrumentation. The previous implementation spawned `build/index.js` as a
* subprocess, which left CLI source invisible to coverage (#1484).
*
* The returned shape matches the old out-of-process runner exactly
* (`exitCode` / `stdout` / `stderr` / `output`) so every existing test and the
* shared assertion helpers keep working unchanged:
* - stdout/stderr are captured by temporarily patching `process.std*.write`
* (and `console.error`/`console.warn`, which commander and error paths use).
* - A thrown error (bad args, failed connect, unsupported method) maps to
* `exitCode: 1` with the message appended to stderr — mirroring how the real
* binary (`index.ts`) routes errors through `handleError` → `process.exit(1)`.
* - `options.env` is applied to `process.env` for the duration of the call and
* fully restored afterward.
*
* The real binary, its shebang, and actual `process.exit` codes are covered by
* the thin out-of-process layer in `e2e.test.ts` and `scripts/smoke-cli.mjs`.
*
* DO NOT drive `--help` / `--version` through this runner. `cli.ts` sets
* `program.exitOverride((err) => { if (err.exitCode !== 0) throw err })`, so for
* those (exitCode 0) commander does NOT throw — it falls through to its normal
* `process.exit(0)`, which run in-process would terminate the vitest worker.
* Cover `--help` / `--version` in the out-of-process `e2e.test.ts` layer instead.
*/
export async function runCli(
args: string[],
options: CliOptions = {},
): Promise<CliResult> {
return new Promise((resolve, reject) => {
const child = spawn("node", [CLI_PATH, ...args], {
stdio: ["pipe", "pipe", "pipe"],
cwd: options.cwd,
env: { ...process.env, ...options.env },
signal: options.signal,
// Own process group on Unix so timeout can kill CLI + stdio grandchildren.
detached: process.platform !== "win32",
});
let stdout = "";
let stderr = "";
let stdout = "";
let stderr = "";
let resolved = false;
const originalStdoutWrite = process.stdout.write;
const originalStderrWrite = process.stderr.write;
const originalConsoleError = console.error;
const originalConsoleWarn = console.warn;
// Default timeout of 10 seconds (less than vitest's 15s)
const timeoutMs = options.timeout ?? 10000;
const timeout = setTimeout(() => {
if (!resolved) {
resolved = true;
// Kill the process and all its children
try {
if (process.platform !== "win32" && child.pid != null) {
process.kill(-child.pid, "SIGTERM");
} else {
child.kill("SIGTERM");
}
} catch {
// Process might already be dead, try direct kill
try {
child.kill("SIGKILL");
} catch {
// Process is definitely dead
}
}
reject(new Error(`CLI command timed out after ${timeoutMs}ms`));
}
}, timeoutMs);
// Snapshot and apply env overrides; `undefined` records keys that did not
// exist so they can be deleted (not set to the string "undefined") on restore.
const envBackup: Record<string, string | undefined> = {};
if (options.env) {
for (const [key, value] of Object.entries(options.env)) {
envBackup[key] = process.env[key];
process.env[key] = value;
}
}
child.stdout.on("data", (data) => {
stdout += data.toString();
});
process.stdout.write = captureWrite((text) => {
stdout += text;
}) as typeof process.stdout.write;
process.stderr.write = captureWrite((text) => {
stderr += text;
}) as typeof process.stderr.write;
console.error = (...parts: unknown[]) => {
stderr += parts.map(String).join(" ") + "\n";
};
console.warn = (...parts: unknown[]) => {
stderr += parts.map(String).join(" ") + "\n";
};
child.stderr.on("data", (data) => {
stderr += data.toString();
});
// `runCli` reads `process.argv.slice(2)`, so prepend two placeholder entries
// ([node, script]) to mirror how the binary is launched.
const argv = ["node", "inspector-cli", ...args];
child.on("close", (code) => {
if (!resolved) {
resolved = true;
clearTimeout(timeout);
resolve({
exitCode: code,
stdout,
stderr,
output: stdout + stderr,
});
}
});
child.on("error", (error) => {
if (!resolved) {
resolved = true;
clearTimeout(timeout);
reject(error);
}
});
// Safety net for a genuinely hung CLI. Note: when the timeout wins the race
// below, `invokeCli` is NOT cancellable in-process — it keeps running (its
// stdio MCP subprocess stays alive) and any later writes land on the
// *restored* real stdout, since `finally` has already unpatched it. The old
// out-of-process runner could SIGTERM the child group; this one structurally
// cannot. Acceptable because it only fires on the failure path (a hang),
// which no passing test should reach.
const timeoutMs = options.timeout ?? 10000;
let timer: ReturnType<typeof setTimeout> | undefined;
const timeout = new Promise<never>((_, reject) => {
timer = setTimeout(
() => reject(new Error(`CLI command timed out after ${timeoutMs}ms`)),
timeoutMs,
);
});
let exitCode = 0;
try {
await Promise.race([invokeCli(argv), timeout]);
} catch (error) {
exitCode = 1;
stderr += (error instanceof Error ? error.message : String(error)) + "\n";
} finally {
if (timer) clearTimeout(timer);
process.stdout.write = originalStdoutWrite;
process.stderr.write = originalStderrWrite;
console.error = originalConsoleError;
console.warn = originalConsoleWarn;
for (const [key, value] of Object.entries(envBackup)) {
if (value === undefined) delete process.env[key];
else process.env[key] = value;
}
}
return {
exitCode,
stdout,
stderr,
output: stdout + stderr,
};
}

View file

@ -0,0 +1,86 @@
import { describe, it, expect } from "vitest";
import { getTestMcpServerCommand } from "@modelcontextprotocol/inspector-test-server";
import { runCli } from "./helpers/cli-runner.js";
import {
expectCliSuccess,
expectCliFailure,
expectJsonStructure,
expectOutputContains,
} from "./helpers/assertions.js";
/**
* Covers the CLI method and option-validation branches that the broader
* integration suites don't reach, keeping `src/cli.ts` above the per-file
* coverage gate (#1484). All connect to the bundled stdio test server, whose
* default config advertises resource templates, prompts, and logging.
*/
describe("CLI method coverage", () => {
const { command, args } = getTestMcpServerCommand();
it("lists resource templates", async () => {
const result = await runCli([
command,
...args,
"--cli",
"--method",
"resources/templates/list",
]);
expectCliSuccess(result);
const json = expectJsonStructure(result, ["resourceTemplates"]);
expect(Array.isArray(json.resourceTemplates)).toBe(true);
expect(json.resourceTemplates.length).toBeGreaterThan(0);
});
it("rejects logging/setLevel without a --log-level", async () => {
const result = await runCli([
command,
...args,
"--cli",
"--method",
"logging/setLevel",
]);
expectCliFailure(result);
expectOutputContains(result, "Log level is required");
});
it("treats args before a `--` separator as the server target", async () => {
const result = await runCli([
command,
...args,
"--",
"--method",
"tools/list",
]);
expectCliSuccess(result);
const json = expectJsonStructure(result, ["tools"]);
expect(Array.isArray(json.tools)).toBe(true);
});
it("rejects an invalid --transport value before connecting", async () => {
const result = await runCli([
command,
...args,
"--cli",
"--method",
"tools/list",
"--transport",
"carrier-pigeon",
]);
expectCliFailure(result);
expectOutputContains(result, "Invalid transport type");
});
it("surfaces a commander usage error (missing option argument) as a failure", async () => {
// `--method` with no value makes commander call its own error-exit path;
// exitOverride re-throws it (exitCode !== 0) so the in-process runner can
// catch it instead of the worker being torn down.
const result = await runCli([command, ...args, "--method"]);
expectCliFailure(result);
expectOutputContains(result, "argument missing");
});
});