diff --git a/tests/_helpers.ts b/tests/_helpers.ts new file mode 100644 index 0000000..626e1a3 --- /dev/null +++ b/tests/_helpers.ts @@ -0,0 +1,140 @@ +// Shared test helpers extracted from cli.test.ts / server.test.ts / e2e.test.ts +// / 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, +): 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 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. +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..35d0c27 100644 --- a/tests/server.test.ts +++ b/tests/server.test.ts @@ -1,11 +1,5 @@ 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,63 +12,14 @@ import { } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; - -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; -} - -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
- -`; +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(); @@ -132,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(); } @@ -203,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", @@ -304,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", @@ -329,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", @@ -345,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/); @@ -362,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, @@ -414,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", @@ -580,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(); @@ -614,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(); @@ -707,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",