From 49a99fda56076f51a85e7ac0e49a59e76dfa13f7 Mon Sep 17 00:00:00 2001 From: olaservo Date: Mon, 19 Jan 2026 09:42:36 -0700 Subject: [PATCH] chore(cli): improve test infrastructure - Move express from dependencies to devDependencies (only used in tests) - Add defensive check for child.pid in cli-runner.ts - Extract test URIs to constants (TEST_URI_ENV, TEST_URI_RESOURCE) Co-Authored-By: Claude Opus 4.5 --- cli/__tests__/cli.test.ts | 13 +++++++------ cli/__tests__/helpers/cli-runner.ts | 4 ++-- cli/__tests__/helpers/fixtures.ts | 6 ++++++ cli/__tests__/metadata.test.ts | 10 +++++----- cli/package.json | 2 +- package-lock.json | 2 +- 6 files changed, 22 insertions(+), 15 deletions(-) diff --git a/cli/__tests__/cli.test.ts b/cli/__tests__/cli.test.ts index b263f618c..f199cf21c 100644 --- a/cli/__tests__/cli.test.ts +++ b/cli/__tests__/cli.test.ts @@ -7,6 +7,7 @@ import { } from "./helpers/assertions.js"; import { NO_SERVER_SENTINEL, + TEST_URI_ENV, createSampleTestConfig, createTestConfig, createInvalidConfig, @@ -75,7 +76,7 @@ describe("CLI Tests", () => { "--method", "resources/read", "--uri", - "test://env", + TEST_URI_ENV, ]); expectCliSuccess(result); @@ -114,7 +115,7 @@ describe("CLI Tests", () => { "--method", "resources/read", "--uri", - "test://env", + TEST_URI_ENV, ]); expectCliSuccess(result); @@ -134,7 +135,7 @@ describe("CLI Tests", () => { "--method", "resources/read", "--uri", - "test://env", + TEST_URI_ENV, ]); expectCliSuccess(result); @@ -427,7 +428,7 @@ describe("CLI Tests", () => { "--method", "resources/read", "--uri", - "test://env", + TEST_URI_ENV, ]); expectCliSuccess(result); @@ -522,7 +523,7 @@ describe("CLI Tests", () => { "--method", "resources/read", "--uri", - "test://env", + TEST_URI_ENV, ]); expectCliSuccess(envResult); @@ -630,7 +631,7 @@ describe("CLI Tests", () => { "--method", "resources/read", "--uri", - "test://env", + TEST_URI_ENV, ]); expectCliSuccess(envResult); diff --git a/cli/__tests__/helpers/cli-runner.ts b/cli/__tests__/helpers/cli-runner.ts index 073aa9ae4..a655d01a8 100644 --- a/cli/__tests__/helpers/cli-runner.ts +++ b/cli/__tests__/helpers/cli-runner.ts @@ -50,9 +50,9 @@ export async function runCli( try { if (process.platform === "win32") { child.kill("SIGTERM"); - } else { + } else if (child.pid) { // On Unix, kill the process group - process.kill(-child.pid!, "SIGTERM"); + process.kill(-child.pid, "SIGTERM"); } } catch (e) { // Process might already be dead, try direct kill diff --git a/cli/__tests__/helpers/fixtures.ts b/cli/__tests__/helpers/fixtures.ts index 5914f485c..37f71094f 100644 --- a/cli/__tests__/helpers/fixtures.ts +++ b/cli/__tests__/helpers/fixtures.ts @@ -10,6 +10,12 @@ import { getTestMcpServerCommand } from "./test-server-stdio.js"; */ export const NO_SERVER_SENTINEL = "invalid-command-that-does-not-exist"; +/** + * Test resource URIs used across test files + */ +export const TEST_URI_ENV = "test://env"; +export const TEST_URI_RESOURCE = "test://resource"; + /** * Create a sample test config with test-stdio and test-http servers * Returns a temporary config file path that should be cleaned up with deleteConfigFile() diff --git a/cli/__tests__/metadata.test.ts b/cli/__tests__/metadata.test.ts index 93d5f8ca6..f17fcf07b 100644 --- a/cli/__tests__/metadata.test.ts +++ b/cli/__tests__/metadata.test.ts @@ -11,7 +11,7 @@ import { createAddTool, createTestServerInfo, } from "./helpers/test-fixtures.js"; -import { NO_SERVER_SENTINEL } from "./helpers/fixtures.js"; +import { NO_SERVER_SENTINEL, TEST_URI_RESOURCE } from "./helpers/fixtures.js"; describe("Metadata Tests", () => { describe("General Metadata", () => { @@ -57,7 +57,7 @@ describe("Metadata Tests", () => { serverInfo: createTestServerInfo(), resources: [ { - uri: "test://resource", + uri: TEST_URI_RESOURCE, name: "test-resource", text: "test content", }, @@ -146,7 +146,7 @@ describe("Metadata Tests", () => { serverInfo: createTestServerInfo(), resources: [ { - uri: "test://resource", + uri: TEST_URI_RESOURCE, name: "test-resource", text: "test content", }, @@ -163,7 +163,7 @@ describe("Metadata Tests", () => { "--method", "resources/read", "--uri", - "test://resource", + TEST_URI_RESOURCE, "--metadata", "client=test-client", "--transport", @@ -653,7 +653,7 @@ describe("Metadata Tests", () => { serverInfo: createTestServerInfo(), resources: [ { - uri: "test://resource", + uri: TEST_URI_RESOURCE, name: "test-resource", text: "test content", }, diff --git a/cli/package.json b/cli/package.json index ae24ff79a..1fc44c0a9 100644 --- a/cli/package.json +++ b/cli/package.json @@ -26,13 +26,13 @@ }, "devDependencies": { "@types/express": "^5.0.6", + "express": "^5.2.1", "tsx": "^4.7.0", "vitest": "^4.0.17" }, "dependencies": { "@modelcontextprotocol/sdk": "^1.25.2", "commander": "^13.1.0", - "express": "^5.2.1", "spawn-rx": "^5.1.2" } } diff --git a/package-lock.json b/package-lock.json index e31fc9577..ed4a5a604 100644 --- a/package-lock.json +++ b/package-lock.json @@ -53,7 +53,6 @@ "dependencies": { "@modelcontextprotocol/sdk": "^1.25.2", "commander": "^13.1.0", - "express": "^5.2.1", "spawn-rx": "^5.1.2" }, "bin": { @@ -61,6 +60,7 @@ }, "devDependencies": { "@types/express": "^5.0.6", + "express": "^5.2.1", "tsx": "^4.7.0", "vitest": "^4.0.17" }