From d5bbe72e62e949cf262a58ab756f1db978ef665c Mon Sep 17 00:00:00 2001 From: Serhii Vasylenko Date: Wed, 13 May 2026 08:39:50 +0200 Subject: [PATCH 1/2] Extract shared test helpers to remove cross-file duplication MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sonar flagged 3.3% duplication on new code in PR #2 (limit 3%). The duplication is real and pre-existing: `startMock`, `textOf`, and HAPPY_FIXTURE were copy-pasted across cli.test.ts, server.test.ts, and e2e.test.ts. PR #2's renames happened to touch lines inside the duplicated blocks, which surfaced the finding. Extract the truly identical pieces to `tests/_helpers.ts`: - `startMock` — byte-for-byte identical in all three files (20 lines × 3 copies). - `textOf` — identical in server.test.ts and e2e.test.ts. - `HAPPY_FIXTURE` — server's 3-section version. cli.test.ts's assertions are a subset of what this fixture exposes, so they continue to pass. Keep per-file: - `spawnClient` / `spawnBuilt` / `runCli` — these differ meaningfully in transport and command. A polymorphic factory would muddy test-local clarity for ~10-line functions. - e2e.test.ts's own HAPPY_FIXTURE — uses distinctive sentinel strings ("E2E Fixture Heading") to prove the e2e tests are exercising their own pipeline, not accidentally hitting the server-test setup. The new file uses `_helpers.ts` (underscore prefix). The runner pattern `tsx --test tests/*.test.ts` (package.json) matches only `*.test.ts` and skips it by name. Net diff in test files: +3 import lines, -125 duplicated lines. The new helpers file is ~63 lines. No behavior change; all 50 tests pass. --- tests/_helpers.ts | 59 ++++++++++++++++++++++++++++++++++++++++++++ tests/cli.test.ts | 44 +-------------------------------- tests/e2e.test.ts | 32 +----------------------- tests/server.test.ts | 52 +------------------------------------- 4 files changed, 62 insertions(+), 125 deletions(-) create mode 100644 tests/_helpers.ts diff --git a/tests/_helpers.ts b/tests/_helpers.ts new file mode 100644 index 0000000..0e7d661 --- /dev/null +++ b/tests/_helpers.ts @@ -0,0 +1,59 @@ +// Shared test helpers extracted from cli.test.ts / server.test.ts / e2e.test.ts +// to remove copy-paste duplication. Not a test file itself — the runner pattern +// `tsx --test tests/*.test.ts` (see package.json) excludes this file by name. + +import { + createServer, + type IncomingMessage, + type ServerResponse, +} from "node:http"; + +export async function startMock( + handler: (req: IncomingMessage, res: ServerResponse) => void, +): Promise<{ url: string; close: () => Promise }> { + const httpServer = createServer(handler); + await new Promise((resolve) => + httpServer.listen(0, "127.0.0.1", () => resolve()), + ); + const address = httpServer.address(); + if (!address || typeof address !== "object") { + throw new Error("mock server address unavailable"); + } + return { + url: `http://127.0.0.1:${address.port}`, + // closeAllConnections() drops keep-alive sockets so close() actually + // resolves; without it the server lingers past the test boundary. + close: () => + new Promise((resolve, reject) => { + httpServer.closeAllConnections(); + httpServer.close((err) => (err ? reject(err) : resolve())); + }), + }; +} + +export function textOf(result: { content: unknown }): string { + const content = result.content as Array<{ type: string; text?: string }>; + return content[0]?.text ?? ""; +} + +// Deterministic Readability-friendly fixture with three

sections so +// server-side tests that assert on multiple sub-headings have material; +// CLI tests assert on a subset and still pass. +export const HAPPY_FIXTURE = ` + +Test Article + +
+
+
+

Test Article Title

+

First substantive paragraph with enough content to pass Readability's heuristics. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. The article contains real prose for the extractor to score positively.

+

Section heading

+

Second paragraph with continuing content. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. More words to give Readability adequate signal.

+

Another section

+

Third paragraph: Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.

+
+
+
copyright
+ +`; diff --git a/tests/cli.test.ts b/tests/cli.test.ts index 5087374..603bdae 100644 --- a/tests/cli.test.ts +++ b/tests/cli.test.ts @@ -6,14 +6,10 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import { execFile } from "node:child_process"; import { promisify } from "node:util"; -import { - createServer, - type IncomingMessage, - type ServerResponse, -} from "node:http"; import { mkdtemp, readFile, rm, stat } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, resolve as resolvePath } from "node:path"; +import { startMock, HAPPY_FIXTURE } from "./_helpers.js"; const execFileAsync = promisify(execFile); @@ -24,44 +20,6 @@ const execFileAsync = promisify(execFile); const TSX_CLI = resolvePath("./node_modules/.bin/tsx"); const ENTRY = resolvePath("src/index.ts"); -const HAPPY_FIXTURE = ` - -Test Article - -
-
-
-

Test Article Title

-

First substantive paragraph with enough content to pass Readability's heuristics. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. The article contains real prose for the extractor to score positively.

-

Section heading

-

Second paragraph with continuing content. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. More words to give Readability adequate signal.

-
-
-
copyright
- -`; - -async function startMock( - handler: (req: IncomingMessage, res: ServerResponse) => void, -): Promise<{ url: string; close: () => Promise }> { - const httpServer = createServer(handler); - await new Promise((resolve) => - httpServer.listen(0, "127.0.0.1", () => resolve()), - ); - const address = httpServer.address(); - if (!address || typeof address !== "object") { - throw new Error("mock server address unavailable"); - } - return { - url: `http://127.0.0.1:${address.port}`, - close: () => - new Promise((resolve, reject) => { - httpServer.closeAllConnections(); - httpServer.close((err) => (err ? reject(err) : resolve())); - }), - }; -} - type RunResult = { code: number; stdout: string; stderr: string }; // Runs `tsx src/index.ts ` and resolves with the full process result. diff --git a/tests/e2e.test.ts b/tests/e2e.test.ts index e637447..22553eb 100644 --- a/tests/e2e.test.ts +++ b/tests/e2e.test.ts @@ -7,16 +7,12 @@ import { test, before } from "node:test"; import assert from "node:assert/strict"; import { execFile, execSync } from "node:child_process"; import { promisify } from "node:util"; -import { - createServer, - type IncomingMessage, - type ServerResponse, -} from "node:http"; import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; import { mkdtemp, readFile, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, resolve as resolvePath } from "node:path"; +import { startMock, textOf } from "./_helpers.js"; const execFileAsync = promisify(execFile); @@ -40,32 +36,6 @@ async function spawnBuilt(env: Record = {}) { return client; } -function textOf(result: { content: unknown }): string { - const content = result.content as Array<{ type: string; text?: string }>; - return content[0]?.text ?? ""; -} - -async function startMock( - handler: (req: IncomingMessage, res: ServerResponse) => void, -): Promise<{ url: string; close: () => Promise }> { - const httpServer = createServer(handler); - await new Promise((resolve) => - httpServer.listen(0, "127.0.0.1", () => resolve()), - ); - const address = httpServer.address(); - if (!address || typeof address !== "object") { - throw new Error("mock server address unavailable"); - } - return { - url: `http://127.0.0.1:${address.port}`, - close: () => - new Promise((resolve, reject) => { - httpServer.closeAllConnections(); - httpServer.close((err) => (err ? reject(err) : resolve())); - }), - }; -} - const HAPPY_FIXTURE = ` E2E Fixture diff --git a/tests/server.test.ts b/tests/server.test.ts index 0d174ef..a8f299d 100644 --- a/tests/server.test.ts +++ b/tests/server.test.ts @@ -1,11 +1,6 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import { spawn } from "node:child_process"; -import { - createServer, - type IncomingMessage, - type ServerResponse, -} from "node:http"; import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; import { @@ -18,6 +13,7 @@ import { } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { startMock, textOf, HAPPY_FIXTURE } from "./_helpers.js"; async function spawnClient(env: Record = {}) { const transport = new StdioClientTransport({ @@ -30,52 +26,6 @@ async function spawnClient(env: Record = {}) { return client; } -function textOf(result: { content: unknown }): string { - const content = result.content as Array<{ type: string; text?: string }>; - return content[0]?.text ?? ""; -} - -async function startMock( - handler: (req: IncomingMessage, res: ServerResponse) => void, -): Promise<{ url: string; close: () => Promise }> { - const httpServer = createServer(handler); - await new Promise((resolve) => - httpServer.listen(0, "127.0.0.1", () => resolve()), - ); - const address = httpServer.address(); - if (!address || typeof address !== "object") { - throw new Error("mock server address unavailable"); - } - return { - url: `http://127.0.0.1:${address.port}`, - close: () => - new Promise((resolve, reject) => { - httpServer.closeAllConnections(); - httpServer.close((err) => (err ? reject(err) : resolve())); - }), - }; -} - -// Deterministic fixture: a small but Readability-friendly article. -const HAPPY_FIXTURE = ` - -Test Article - -
-
-
-

Test Article Title

-

First substantive paragraph with enough content to pass Readability's heuristics. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. The article contains real prose for the extractor to score positively.

-

Section heading

-

Second paragraph with continuing content. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. More words to give Readability adequate signal.

-

Another section

-

Third paragraph: Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.

-
-
-
copyright
- -`; - test("server boots over stdio and completes the initialize handshake", async () => { const client = await spawnClient(); try { From 6d2695c17c72fa1f8e9ab5a5efd12bc82147e725 Mon Sep 17 00:00:00 2001 From: Serhii Vasylenko Date: Wed, 13 May 2026 12:06:36 +0200 Subject: [PATCH 2/2] Extract more shared test helpers, rewire snapshots onto them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the first commit on this branch. Three additional extractions that the initial pass didn't cover; together they push the PR's net LOC delta from -63 to -101 vs main. New helpers in tests/_helpers.ts: - `spawnClient({ name?, env? })` — moved from server.test.ts and parameterized so snapshots.test.ts can share it. Existing callers using env pass `{ env: {...} }` now. - `assertSchemaRejection(client, args, msg)` — captures the F27 pattern (try/catch around callTool to handle SDK-version-variant rejection shapes, then assert no `[code]` prefix leaked). Used at 3 sites (zod-syntax test, savePath relative, savePath tilde). - `spawnAndCaptureExit(args, env)` — captures the env-var-validation pattern (spawn → collect stderr → await exit). Used at 3 sites. - `ERROR_CODE_PREFIX_RE` — single source for the seven `[code]` prefixes the regex was previously inlined at 3 sites. snapshots.test.ts: the inline `createServer` plumbing and `StdioClientTransport` setup are replaced by `startMock` (with a closure over `currentHtml`) and `spawnClient({ name: "snapshot-test" })`. The `Mock` type and `setHtml` wrapper are gone — tests mutate `currentHtml` directly, which the mock handler reads on each request. Avoided one Sonar finding while at it: `opts.env ?? {}` was flagged by S7744 ("empty object is useless"). Object spread already handles `undefined`, so the `?? {}` fallback was strictly redundant — replaced with `...opts?.env`. No behavior change; all 50 tests pass. --- tests/_helpers.ts | 85 +++++++++++++++++++++- tests/server.test.ts | 154 ++++++++++------------------------------ tests/snapshots.test.ts | 59 +++------------ 3 files changed, 130 insertions(+), 168 deletions(-) diff --git a/tests/_helpers.ts b/tests/_helpers.ts index 0e7d661..626e1a3 100644 --- a/tests/_helpers.ts +++ b/tests/_helpers.ts @@ -1,12 +1,17 @@ // Shared test helpers extracted from cli.test.ts / server.test.ts / e2e.test.ts -// to remove copy-paste duplication. Not a test file itself — the runner pattern -// `tsx --test tests/*.test.ts` (see package.json) excludes this file by name. +// / snapshots.test.ts to remove copy-paste duplication. Not a test file itself +// — the runner pattern `tsx --test tests/*.test.ts` (see package.json) excludes +// this file by name. +import assert from "node:assert/strict"; +import { spawn } from "node:child_process"; import { createServer, type IncomingMessage, type ServerResponse, } from "node:http"; +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; export async function startMock( handler: (req: IncomingMessage, res: ServerResponse) => void, @@ -31,11 +36,87 @@ export async function startMock( }; } +export async function spawnClient(opts?: { + name?: string; + env?: Record; +}): Promise { + const transport = new StdioClientTransport({ + command: "tsx", + args: ["src/index.ts"], + env: { ...process.env, ...opts?.env } as Record, + }); + const client = new Client({ + name: opts?.name ?? "markfetch-test", + version: "0.0.0", + }); + await client.connect(transport); + return client; +} + export function textOf(result: { content: unknown }): string { const content = result.content as Array<{ type: string; text?: string }>; return content[0]?.text ?? ""; } +// Matches any of the seven [code] error prefixes the tool emits. Used by +// schema-rejection assertions to prove the handler did NOT run — a [code] +// prefix would mean the call escaped Zod and reached core. +export const ERROR_CODE_PREFIX_RE = + /^\[(network_error|http_error|timeout|unsupported_content_type|extraction_failed|too_large|save_failed)\]/; + +// Asserts that a tool call is rejected at the Zod schema boundary, not by the +// handler. The SDK either throws (some versions) or returns isError:true with +// schema-error text — both are valid rejections. What's NOT valid is a +// [code]-prefixed reply, which would prove the handler ran. +export async function assertSchemaRejection( + client: Client, + args: Record, + failureMessage: string, +): Promise { + let caught = false; + let result: { isError?: boolean; content?: unknown } | undefined; + try { + result = (await client.callTool({ + name: "fetch_markdown", + arguments: args, + })) as { isError?: boolean; content?: unknown }; + } catch { + caught = true; + } + if (!caught) { + assert.equal( + result?.isError, + true, + "schema rejection must surface as isError", + ); + const text = textOf(result as { content: unknown }); + assert.ok( + !ERROR_CODE_PREFIX_RE.test(text), + `${failureMessage}: ${text}`, + ); + } +} + +// One-shot subprocess spawn that returns exit code + stderr. Used by +// startup-failure tests that expect a misconfigured env var to fail fast. +export async function spawnAndCaptureExit( + args: string[], + env: Record, +): Promise<{ exitCode: number; stderr: string }> { + const child = spawn("./node_modules/.bin/tsx", args, { + env: { ...process.env, ...env } as Record, + stdio: ["pipe", "pipe", "pipe"], + }); + let stderr = ""; + child.stderr.on("data", (d: Buffer) => { + stderr += d.toString(); + }); + const exitCode = await new Promise((resolve) => + child.on("exit", (code) => resolve(code ?? -1)), + ); + return { exitCode, stderr }; +} + // Deterministic Readability-friendly fixture with three

sections so // server-side tests that assert on multiple sub-headings have material; // CLI tests assert on a subset and still pass. diff --git a/tests/server.test.ts b/tests/server.test.ts index a8f299d..35d0c27 100644 --- a/tests/server.test.ts +++ b/tests/server.test.ts @@ -1,6 +1,5 @@ import { test } from "node:test"; import assert from "node:assert/strict"; -import { spawn } from "node:child_process"; import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; import { @@ -13,18 +12,14 @@ import { } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { startMock, textOf, HAPPY_FIXTURE } from "./_helpers.js"; - -async function spawnClient(env: Record = {}) { - const transport = new StdioClientTransport({ - command: "tsx", - args: ["src/index.ts"], - env: { ...process.env, ...env } as Record, - }); - const client = new Client({ name: "markfetch-test", version: "0.0.0" }); - await client.connect(transport); - return client; -} +import { + startMock, + textOf, + HAPPY_FIXTURE, + spawnClient, + assertSchemaRejection, + spawnAndCaptureExit, +} from "./_helpers.js"; test("server boots over stdio and completes the initialize handshake", async () => { const client = await spawnClient(); @@ -82,36 +77,16 @@ test("happy path: deterministic fixture → pure markdown body in content[0]", a } }); +// F27 regression-guard: locks against Zod being silently removed and +// "not-a-url" reaching fetch(). test("zod syntax: malformed URL is rejected at the schema boundary, not by the handler", async () => { const client = await spawnClient(); try { - let caught = false; - let result: - | { isError?: boolean; content?: unknown } - | undefined; - try { - result = (await client.callTool({ - name: "fetch_markdown", - arguments: { url: "not-a-url" }, - })) as { isError?: boolean; content?: unknown }; - } catch { - caught = true; - } - // F27 regression-guard: locks against the case where Zod is silently - // removed and "not-a-url" reaches fetch(). The SDK either throws (some - // versions) OR returns isError:true with schema-error text. Either is - // fine — but the text must NOT carry one of our [code] prefixes, which - // would prove the handler ran and returned a tool error. - if (!caught) { - assert.equal(result?.isError, true, "schema rejection must surface as isError"); - const text = textOf(result as { content: unknown }); - assert.ok( - !/^\[(network_error|http_error|timeout|unsupported_content_type|extraction_failed|too_large|save_failed)\]/.test( - text, - ), - `expected schema error, got tool [code] error (handler ran when it shouldn't have): ${text}`, - ); - } + await assertSchemaRejection( + client, + { url: "not-a-url" }, + "expected schema error, got tool [code] error (handler ran when it shouldn't have)", + ); } finally { await client.close(); } @@ -153,7 +128,7 @@ test("error: http_error on 404 response", async () => { test("error: timeout when server hangs and MARKFETCH_TIMEOUT_MS is small", async () => { const mock = await startMock(() => {}); - const client = await spawnClient({ MARKFETCH_TIMEOUT_MS: "200" }); + const client = await spawnClient({ env: { MARKFETCH_TIMEOUT_MS: "200" } }); try { const result = await client.callTool({ name: "fetch_markdown", @@ -254,7 +229,7 @@ test("error: too_large via Content-Length pre-check", async () => { }); res.end(big); }); - const client = await spawnClient({ MARKFETCH_MAX_BYTES: "100" }); + const client = await spawnClient({ env: { MARKFETCH_MAX_BYTES: "100" } }); try { const result = await client.callTool({ name: "fetch_markdown", @@ -279,7 +254,7 @@ test("error: too_large via post-decode body-bytes check (chunked, no Content-Len res.write(big); res.end(); }); - const client = await spawnClient({ MARKFETCH_MAX_BYTES: "100" }); + const client = await spawnClient({ env: { MARKFETCH_MAX_BYTES: "100" } }); try { const result = await client.callTool({ name: "fetch_markdown", @@ -295,16 +270,8 @@ test("error: too_large via post-decode body-bytes check (chunked, no Content-Len }); test("env-var validation: invalid MARKFETCH_TIMEOUT_MS fails fast at startup", async () => { - const child = spawn("./node_modules/.bin/tsx", ["src/index.ts"], { - env: { ...process.env, MARKFETCH_TIMEOUT_MS: "abc" }, - stdio: ["pipe", "pipe", "pipe"], - }); - let stderr = ""; - child.stderr.on("data", (d: Buffer) => { - stderr += d.toString(); - }); - const exitCode = await new Promise((resolve) => { - child.on("exit", (code) => resolve(code ?? -1)); + const { exitCode, stderr } = await spawnAndCaptureExit(["src/index.ts"], { + MARKFETCH_TIMEOUT_MS: "abc", }); assert.notEqual(exitCode, 0, "subprocess must exit non-zero on bad env var"); assert.match(stderr, /MARKFETCH_TIMEOUT_MS/); @@ -312,35 +279,16 @@ test("env-var validation: invalid MARKFETCH_TIMEOUT_MS fails fast at startup", a }); test("env-var validation: negative MARKFETCH_MAX_BYTES is rejected", async () => { - const child = spawn("./node_modules/.bin/tsx", ["src/index.ts"], { - env: { ...process.env, MARKFETCH_MAX_BYTES: "-1" }, - stdio: ["pipe", "pipe", "pipe"], - }); - let stderr = ""; - child.stderr.on("data", (d: Buffer) => { - stderr += d.toString(); - }); - const exitCode = await new Promise((resolve) => { - child.on("exit", (code) => resolve(code ?? -1)); + const { exitCode, stderr } = await spawnAndCaptureExit(["src/index.ts"], { + MARKFETCH_MAX_BYTES: "-1", }); assert.notEqual(exitCode, 0); assert.match(stderr, /MARKFETCH_MAX_BYTES/); }); test("env-var validation: non-Chrome MARKFETCH_USER_AGENT fails fast at startup", async () => { - const child = spawn("./node_modules/.bin/tsx", ["src/index.ts"], { - env: { - ...process.env, - MARKFETCH_USER_AGENT: "Mozilla/5.0 (X11; FreeBSD) Firefox/120.0", - }, - stdio: ["pipe", "pipe", "pipe"], - }); - let stderr = ""; - child.stderr.on("data", (d: Buffer) => { - stderr += d.toString(); - }); - const exitCode = await new Promise((resolve) => { - child.on("exit", (code) => resolve(code ?? -1)); + const { exitCode, stderr } = await spawnAndCaptureExit(["src/index.ts"], { + MARKFETCH_USER_AGENT: "Mozilla/5.0 (X11; FreeBSD) Firefox/120.0", }); assert.notEqual( exitCode, @@ -364,7 +312,7 @@ test("Sec-CH-UA-* client hints are derived from MARKFETCH_USER_AGENT", async () }); const overrideUa = "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/125.0.0.0 Safari/537.36"; - const client = await spawnClient({ MARKFETCH_USER_AGENT: overrideUa }); + const client = await spawnClient({ env: { MARKFETCH_USER_AGENT: overrideUa } }); try { const result = await client.callTool({ name: "fetch_markdown", @@ -530,26 +478,11 @@ test("savePath: relative path is rejected at the schema boundary, not by the han }); const client = await spawnClient(); try { - let caught = false; - let result: { isError?: boolean; content?: unknown } | undefined; - try { - result = (await client.callTool({ - name: "fetch_markdown", - arguments: { url: mock.url, savePath: "relative/path.md" }, - })) as { isError?: boolean; content?: unknown }; - } catch { - caught = true; - } - if (!caught) { - assert.equal(result?.isError, true, "schema rejection must surface as isError"); - const text = textOf(result as { content: unknown }); - assert.ok( - !/^\[(network_error|http_error|timeout|unsupported_content_type|extraction_failed|too_large|save_failed)\]/.test( - text, - ), - `expected schema error, got tool [code] error (handler ran when it shouldn't have): ${text}`, - ); - } + await assertSchemaRejection( + client, + { url: mock.url, savePath: "relative/path.md" }, + "expected schema error, got tool [code] error (handler ran when it shouldn't have)", + ); } finally { await client.close(); await mock.close(); @@ -564,26 +497,11 @@ test("savePath: tilde-path '~/x.md' is rejected at the schema boundary (no auto- }); const client = await spawnClient(); try { - let caught = false; - let result: { isError?: boolean; content?: unknown } | undefined; - try { - result = (await client.callTool({ - name: "fetch_markdown", - arguments: { url: mock.url, savePath: "~/x.md" }, - })) as { isError?: boolean; content?: unknown }; - } catch { - caught = true; - } - if (!caught) { - assert.equal(result?.isError, true); - const text = textOf(result as { content: unknown }); - assert.ok( - !/^\[(network_error|http_error|timeout|unsupported_content_type|extraction_failed|too_large|save_failed)\]/.test( - text, - ), - `tilde path must be rejected at schema, not by handler: ${text}`, - ); - } + await assertSchemaRejection( + client, + { url: mock.url, savePath: "~/x.md" }, + "tilde path must be rejected at schema, not by handler", + ); } finally { await client.close(); await mock.close(); @@ -657,7 +575,7 @@ test("savePath INVARIANT: too_large + savePath → file is NOT written (cap runs }); await withTmpDir(async (dir) => { const savePath = join(dir, "should-not-exist.md"); - const client = await spawnClient({ MARKFETCH_MAX_BYTES: "100" }); + const client = await spawnClient({ env: { MARKFETCH_MAX_BYTES: "100" } }); try { const result = await client.callTool({ name: "fetch_markdown", diff --git a/tests/snapshots.test.ts b/tests/snapshots.test.ts index a6460f5..178c889 100644 --- a/tests/snapshots.test.ts +++ b/tests/snapshots.test.ts @@ -1,15 +1,9 @@ import { test, before, after } from "node:test"; import assert from "node:assert/strict"; -import { - createServer, - type IncomingMessage, - type ServerResponse, -} from "node:http"; -import { Client } from "@modelcontextprotocol/sdk/client/index.js"; -import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; import { readFile, readdir, writeFile } from "node:fs/promises"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; +import { spawnClient, startMock } from "./_helpers.js"; const FIXTURES_DIR = join( dirname(fileURLToPath(import.meta.url)), @@ -22,49 +16,18 @@ const FIXTURES_DIR = join( // to detect drift on subsequent edits. const UPDATE_MODE = process.env.UPDATE_SNAPSHOTS === "1"; -type Mock = { - url: string; - setHtml: (html: string) => void; - close: () => Promise; -}; - -let mock: Mock; -let client: Client; +// Mutated per test to control what the shared mock server responds with; +// captured by the startMock handler's closure below. +let currentHtml = ""; +let mock: Awaited>; +let client: Awaited>; before(async () => { - let currentHtml = ""; - const httpServer = createServer( - (_req: IncomingMessage, res: ServerResponse) => { - res.writeHead(200, { "Content-Type": "text/html; charset=utf-8" }); - res.end(currentHtml); - }, - ); - await new Promise((resolve) => - httpServer.listen(0, "127.0.0.1", () => resolve()), - ); - const address = httpServer.address(); - if (!address || typeof address !== "object") { - throw new Error("mock server address unavailable"); - } - mock = { - url: `http://127.0.0.1:${address.port}`, - setHtml: (html) => { - currentHtml = html; - }, - close: () => - new Promise((resolve, reject) => { - httpServer.closeAllConnections(); - httpServer.close((err) => (err ? reject(err) : resolve())); - }), - }; - - const transport = new StdioClientTransport({ - command: "tsx", - args: ["src/index.ts"], - env: process.env as Record, + mock = await startMock((_req, res) => { + res.writeHead(200, { "Content-Type": "text/html; charset=utf-8" }); + res.end(currentHtml); }); - client = new Client({ name: "snapshot-test", version: "0.0.0" }); - await client.connect(transport); + client = await spawnClient({ name: "snapshot-test" }); }); after(async () => { @@ -80,7 +43,7 @@ const fixtureNames = (await readdir(FIXTURES_DIR)) for (const name of fixtureNames) { test(`snapshot: ${name}`, async () => { const html = await readFile(join(FIXTURES_DIR, `${name}.html`), "utf8"); - mock.setHtml(html); + currentHtml = html; const result = await client.callTool({ name: "fetch_markdown",