From fe02712ed621d8fc4599b6b2a06c712f2b2731e7 Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Thu, 3 Sep 2026 23:21:59 +0300 Subject: [PATCH 1/3] Separate test suites and fail listener setup fast --- .github/workflows/ci.yml | 2 + README.md | 3 +- docs/specifications/e2e-testing.md | 7 ++-- package.json | 7 ++-- src/routes/mcp/server.test.ts | 7 ++-- tests/benchmark/workspace-capacity.test.ts | 2 +- tests/e2e/harness.ts | 24 +++++------ tests/e2e/instance-isolation.test.ts | 2 +- tests/e2e/listener.test.ts | 26 ++++++++++++ tests/e2e/listener.ts | 48 ++++++++++++++++++++++ tests/e2e/tier-a.test.ts | 2 +- tests/e2e/tier-b.spec.ts | 2 +- vite.config.ts | 14 ++++++- 13 files changed, 118 insertions(+), 28 deletions(-) create mode 100644 tests/e2e/listener.test.ts create mode 100644 tests/e2e/listener.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3c6655a..690b7b5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -61,6 +61,8 @@ jobs: path: coverage/ retention-days: 14 + - run: npm run benchmark:workspace + # tests/e2e/harness.ts serves real HTTP responses through # build/handler.js — without this, every route 404s and tier-b # silently fails to find any DOM element. diff --git a/README.md b/README.md index 79734dc..b9d7f38 100644 --- a/README.md +++ b/README.md @@ -183,7 +183,8 @@ for the shared record primitive. ```sh npm run test # unit and component tests -npm run test:e2e # real MCP↔Yjs and browser-level flows +npm run test:integration # real HTTP/WebSocket protocol tests (Tier A + transport isolation) +npm run test:e2e # protocol integration plus browser-level flows npm run benchmark:workspace # bounded CRDT capacity profile npm run benchmark:workspace:large # manual sharding/persistence profile npm run check # Svelte and TypeScript checks diff --git a/docs/specifications/e2e-testing.md b/docs/specifications/e2e-testing.md index 309b9ca..0881840 100644 --- a/docs/specifications/e2e-testing.md +++ b/docs/specifications/e2e-testing.md @@ -82,7 +82,7 @@ This harness is the only thing that should know how to boot a full server instan ## 4. Tooling and CI placement -- **Tier A** uses Vitest (already the project's test runner — no new dependency for the runner itself; the MCP SDK client and a Yjs client are both already project dependencies via the server-side code). Fast enough (no browser) to run in the normal `npm run test` suite and in CI on every PR. +- **Tier A and adjacent protocol/integration coverage** use Vitest (already the project's test runner — no new dependency for the runner itself; the MCP SDK client and a Yjs client are both already project dependencies via the server-side code). `npm run test:integration` owns real-listener tests: Tier A, instance isolation, and MCP route transport. It is intentionally separate from `npm run test` and `npm run test:coverage`, so environments that cannot bind a local socket can still run unit and component checks. CI runs it once through `npm run test:e2e`. - **Tier B** uses Playwright (new dev dependency). Slower and more flake-prone than Tier A by nature of driving a real browser — run it in CI on every PR too, but keep the tier small (per §2) precisely so this cost stays bounded rather than growing into a full UI-test suite; the PRD's UI is already covered qualitatively by manual dogfooding per the Phase 0/1 success-metrics framing (`prd.md`, "Success Metrics"). ## 5. Relationship to existing and future unit tests @@ -106,8 +106,9 @@ npm run benchmark:workspace:large # `large`: manual pre/post-change comparison The benchmark lives in `tests/benchmark/workspace-capacity.test.ts` and runs in its own Vitest project. It is intentionally excluded from `npm run test` -and coverage: performance work must stay discoverable and repeatable without -making ordinary correctness checks slow or environment-sensitive. Every run +and coverage, and CI invokes the bounded `daily` command once: performance +work must stay discoverable and repeatable without making ordinary correctness +checks slow or environment-sensitive. Every run creates a temporary SQLite database and random local port; it must never point at a developer's running workspace database. diff --git a/package.json b/package.json index 5aa6582..811d34b 100644 --- a/package.json +++ b/package.json @@ -10,14 +10,15 @@ "prepare": "svelte-kit sync || echo ''", "check": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json", "check:watch": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json --watch", - "test:unit": "vitest", - "test:e2e:tier-a": "vitest --run tests/e2e/tier-a.test.ts", + "test:unit": "vitest --project server --project client --project component", + "test:integration": "vitest run --project integration", + "test:e2e:tier-a": "npm run test:integration", "test:e2e:tier-b": "playwright test", "test:e2e": "npm run test:e2e:tier-a && npm run test:e2e:tier-b", "benchmark:workspace": "vitest run --project benchmark", "benchmark:workspace:large": "COMPENDIUM_BENCHMARK_PROFILE=large vitest run --project benchmark", "test": "npm run test:unit -- --run", - "test:coverage": "npm run test:unit -- --run --coverage", + "test:coverage": "vitest run --project server --project client --project component --coverage", "lint": "prettier --check . && eslint .", "format": "prettier --write .", "start": "tsx server.ts", diff --git a/src/routes/mcp/server.test.ts b/src/routes/mcp/server.test.ts index 251168d..852e5c3 100644 --- a/src/routes/mcp/server.test.ts +++ b/src/routes/mcp/server.test.ts @@ -7,6 +7,7 @@ import { GET, POST, DELETE } from './+server'; import { createDocument } from '$lib/data/records'; import { createToken } from '$lib/mcp/tokens'; import { resolveWorkspaceContext } from '$lib/server/workspace-store'; +import { closeTestServer, listenOnLoopback } from '../../../tests/e2e/listener'; // Mirrors tests/e2e/harness.ts's node-request/web-request bridge, but points // at this route's own exported handlers rather than reimplementing MCP @@ -67,14 +68,12 @@ describe('routes/mcp: HTTP transport wiring and bearer-token extraction', () => res.end(); })(); }); - await new Promise((resolve) => server.listen(0, resolve)); - const address = server.address(); - const port = address && typeof address === 'object' ? address.port : 0; + const port = await listenOnLoopback(server); baseUrl = `http://localhost:${port}`; }); afterEach(async () => { - await new Promise((resolve) => server.close(() => resolve())); + if (server) await closeTestServer(server); }); it('serves an authenticated tool call end-to-end over the real MCP HTTP transport', async () => { diff --git a/tests/benchmark/workspace-capacity.test.ts b/tests/benchmark/workspace-capacity.test.ts index b8b47d1..5ea60df 100644 --- a/tests/benchmark/workspace-capacity.test.ts +++ b/tests/benchmark/workspace-capacity.test.ts @@ -91,7 +91,7 @@ describe('CRDT workspace capacity, real shard-aware transport (issue #123)', () }); afterEach(async () => { - await harness.cleanup(); + await harness?.cleanup(); }); it('measures per-shard state, sync, fan-out (with cross-shard isolation), catalog size, and restart cost', async () => { diff --git a/tests/e2e/harness.ts b/tests/e2e/harness.ts index c640c0d..fdb6d87 100644 --- a/tests/e2e/harness.ts +++ b/tests/e2e/harness.ts @@ -13,6 +13,7 @@ import { attachYjsWebSocket } from '$lib/server/attach-ws'; import { createToken, type AccessToken } from '$lib/mcp/tokens'; import { closeDb } from '$lib/server/store'; import { resetWorkspaceStoreForTests } from '$lib/server/workspace-store'; +import { closeTestServer, listenOnLoopback } from './listener'; // adapter-node's built handler (build/handler.js) uses ORIGIN to construct // each request's trusted `url.origin` for its CSRF check — not the raw @@ -187,15 +188,16 @@ export async function createTestHarness(): Promise { const wss = attachYjsWebSocket(server, '/ws'); - await new Promise((resolve) => { - server.listen(0, () => { - const addr = server.address(); - if (addr && typeof addr === 'object') { - port = addr.port; - } - resolve(); - }); - }); + try { + port = await listenOnLoopback(server); + } catch (error) { + await new Promise((resolve) => wss.close(() => resolve())); + await closeTestServer(server); + closeDb(); + resetWorkspaceStoreForTests(); + rmSync(tempDir, { recursive: true, force: true }); + throw error; + } const httpUrl = `http://localhost:${port}`; const wsUrl = `ws://localhost:${port}/ws`; @@ -321,9 +323,7 @@ export async function createTestHarness(): Promise { await new Promise((resolve) => { wss.close(() => resolve()); }); - await new Promise((resolve) => { - server.close(() => resolve()); - }); + await closeTestServer(server); closeDb(); resetWorkspaceStoreForTests(); diff --git a/tests/e2e/instance-isolation.test.ts b/tests/e2e/instance-isolation.test.ts index c72c5f5..aa35b18 100644 --- a/tests/e2e/instance-isolation.test.ts +++ b/tests/e2e/instance-isolation.test.ts @@ -35,7 +35,7 @@ describe('Instance isolation: two configured instances never cross-observe (#111 afterEach(async () => { delete process.env.COMPENDIUM_INSTANCE_ID; - await harness.cleanup(); + await harness?.cleanup(); }); it("list_documents scoped to one instance never returns the other instance's Documents", async () => { diff --git a/tests/e2e/listener.test.ts b/tests/e2e/listener.test.ts new file mode 100644 index 0000000..d65b654 --- /dev/null +++ b/tests/e2e/listener.test.ts @@ -0,0 +1,26 @@ +import { createServer } from 'node:http'; +import { describe, expect, it, vi } from 'vitest'; +import { closeTestServer, listenOnLoopback } from './listener'; + +describe('test listener lifecycle', () => { + it('rejects immediately with the original listener error and removes its temporary listeners', async () => { + const server = createServer(); + const initialListeningListeners = server.listenerCount('listening'); + const bindError = Object.assign(new Error('listen EPERM: operation not permitted 127.0.0.1'), { + code: 'EPERM' + }); + vi.spyOn(server, 'listen').mockImplementation(() => { + queueMicrotask(() => server.emit('error', bindError)); + return server; + }); + + await expect(listenOnLoopback(server)).rejects.toBe(bindError); + expect(server.listenerCount('error')).toBe(0); + expect(server.listenerCount('listening')).toBe(initialListeningListeners); + }); + + it('allows cleanup after partial initialization when the server never listened', async () => { + const server = createServer(); + await expect(closeTestServer(server)).resolves.toBeUndefined(); + }); +}); diff --git a/tests/e2e/listener.ts b/tests/e2e/listener.ts new file mode 100644 index 0000000..6d960e4 --- /dev/null +++ b/tests/e2e/listener.ts @@ -0,0 +1,48 @@ +import type { Server } from 'node:http'; + +/** + * Start a test HTTP server on loopback and propagate bind failures immediately. + * + * Test suites deliberately use an ephemeral port, but they must not silently + * hang when the environment denies listening sockets. The temporary listeners + * are removed whichever event settles first so later server errors retain their + * normal Node handling. + */ +export function listenOnLoopback(server: Server): Promise { + return new Promise((resolve, reject) => { + const onListening = () => { + cleanup(); + const address = server.address(); + if (address && typeof address === 'object') { + resolve(address.port); + return; + } + reject(new Error('Test server started without a TCP address')); + }; + const onError = (error: Error) => { + cleanup(); + reject(error); + }; + const cleanup = () => { + server.removeListener('listening', onListening); + server.removeListener('error', onError); + }; + + server.once('listening', onListening); + server.once('error', onError); + server.listen({ port: 0, host: '127.0.0.1' }); + }); +} + +/** Close a test server without treating a failed startup as a second failure. */ +export function closeTestServer(server: Server): Promise { + return new Promise((resolve, reject) => { + server.close((error) => { + if (error && (error as NodeJS.ErrnoException).code !== 'ERR_SERVER_NOT_RUNNING') { + reject(error); + return; + } + resolve(); + }); + }); +} diff --git a/tests/e2e/tier-a.test.ts b/tests/e2e/tier-a.test.ts index 635b7d4..75954e2 100644 --- a/tests/e2e/tier-a.test.ts +++ b/tests/e2e/tier-a.test.ts @@ -55,7 +55,7 @@ describe('Tier A: Protocol-Level MCP & Yjs E2E Parity', () => { }); afterEach(async () => { - await harness.cleanup(); + await harness?.cleanup(); }); it('1. MCP write_record -> Yjs websocket client observes new content within latency bound', async () => { diff --git a/tests/e2e/tier-b.spec.ts b/tests/e2e/tier-b.spec.ts index 3fdeca1..cb64edd 100644 --- a/tests/e2e/tier-b.spec.ts +++ b/tests/e2e/tier-b.spec.ts @@ -21,7 +21,7 @@ test.describe('Tier B: DOM-visible MCP/Browser parity', () => { }); test.afterEach(async () => { - await harness.cleanup(); + await harness?.cleanup(); }); test('Held-block placeholder appears on MCP hold and resolves atomically on MCP write', async ({ diff --git a/vite.config.ts b/vite.config.ts index 7694279..12eb2a0 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -67,16 +67,28 @@ export default defineConfig({ test: { name: 'server', environment: 'node', - include: ['src/**/*.{test,spec}.{js,ts}', 'tests/**/*.test.{js,ts}'], + include: ['src/**/*.{test,spec}.{js,ts}'], exclude: [ 'src/**/*.svelte.{test,spec}.{js,ts}', + 'src/routes/mcp/server.test.ts', 'tests/**/*.spec.{js,ts}', + 'tests/e2e/**', 'tests/benchmark/**', 'src/lib/client/**/*.{test,spec}.{js,ts}' ], setupFiles: ['./tests/setup/isolate-persistence.ts'] } }, + { + extends: './vite.config.ts', + test: { + name: 'integration', + environment: 'node', + include: ['tests/e2e/**/*.test.{js,ts}', 'src/routes/mcp/server.test.ts'], + setupFiles: ['./tests/setup/isolate-persistence.ts'], + fileParallelism: false + } + }, { extends: './vite.config.ts', test: { From d1d1773c101aef1d1ec57483bb9f225d94874d62 Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Thu, 3 Sep 2026 23:23:35 +0300 Subject: [PATCH 2/3] Keep capacity benchmark out of routine CI --- .github/workflows/ci.yml | 2 -- docs/specifications/e2e-testing.md | 8 ++++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 690b7b5..3c6655a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -61,8 +61,6 @@ jobs: path: coverage/ retention-days: 14 - - run: npm run benchmark:workspace - # tests/e2e/harness.ts serves real HTTP responses through # build/handler.js — without this, every route 404s and tier-b # silently fails to find any DOM element. diff --git a/docs/specifications/e2e-testing.md b/docs/specifications/e2e-testing.md index 0881840..9c3731b 100644 --- a/docs/specifications/e2e-testing.md +++ b/docs/specifications/e2e-testing.md @@ -105,10 +105,10 @@ npm run benchmark:workspace:large # `large`: manual pre/post-change comparison ``` The benchmark lives in `tests/benchmark/workspace-capacity.test.ts` and runs -in its own Vitest project. It is intentionally excluded from `npm run test` -and coverage, and CI invokes the bounded `daily` command once: performance -work must stay discoverable and repeatable without making ordinary correctness -checks slow or environment-sensitive. Every run +in its own Vitest project. It is intentionally excluded from `npm run test`, +coverage, and routine CI: performance work must stay discoverable and +repeatable without making ordinary correctness checks slow or +environment-sensitive. Every run creates a temporary SQLite database and random local port; it must never point at a developer's running workspace database. From 348e84bcda992a544225bfd4942d539b513f27a2 Mon Sep 17 00:00:00 2001 From: Brylie Christopher Oxley Date: Thu, 3 Sep 2026 23:35:11 +0300 Subject: [PATCH 3/3] Use IPv4 loopback URLs in protocol tests --- src/routes/mcp/server.test.ts | 4 ++-- tests/e2e/harness.ts | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/routes/mcp/server.test.ts b/src/routes/mcp/server.test.ts index 852e5c3..0d93d4f 100644 --- a/src/routes/mcp/server.test.ts +++ b/src/routes/mcp/server.test.ts @@ -45,7 +45,7 @@ describe('routes/mcp: HTTP transport wiring and bearer-token extraction', () => beforeEach(async () => { server = createServer((req: IncomingMessage, res: ServerResponse) => { void (async () => { - const request = await nodeRequestToWebRequest(req, 'http://localhost'); + const request = await nodeRequestToWebRequest(req, 'http://127.0.0.1'); let response: Response; if (req.method === 'POST') { response = await POST({ request } as Parameters[0]); @@ -69,7 +69,7 @@ describe('routes/mcp: HTTP transport wiring and bearer-token extraction', () => })(); }); const port = await listenOnLoopback(server); - baseUrl = `http://localhost:${port}`; + baseUrl = `http://127.0.0.1:${port}`; }); afterEach(async () => { diff --git a/tests/e2e/harness.ts b/tests/e2e/harness.ts index fdb6d87..6b9b743 100644 --- a/tests/e2e/harness.ts +++ b/tests/e2e/harness.ts @@ -136,7 +136,7 @@ export async function createTestHarness(): Promise { const server: Server = createServer(async (req, res) => { try { if (req.url?.startsWith('/mcp')) { - const webReq = await nodeRequestToWebRequest(req, `http://localhost:${port}`); + const webReq = await nodeRequestToWebRequest(req, `http://127.0.0.1:${port}`); const auth = webReq.headers.get('authorization'); const token = auth?.startsWith('Bearer ') ? auth.slice('Bearer '.length).trim() : undefined; @@ -199,8 +199,8 @@ export async function createTestHarness(): Promise { throw error; } - const httpUrl = `http://localhost:${port}`; - const wsUrl = `ws://localhost:${port}/ws`; + const httpUrl = `http://127.0.0.1:${port}`; + const wsUrl = `ws://127.0.0.1:${port}/ws`; try { const buildPath = join(process.cwd(), 'build/handler.js');