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..9c3731b 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 @@ -105,9 +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: 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. 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..0d93d4f 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 @@ -44,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]); @@ -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; - baseUrl = `http://localhost:${port}`; + const port = await listenOnLoopback(server); + baseUrl = `http://127.0.0.1:${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..6b9b743 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 @@ -135,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; @@ -187,18 +188,19 @@ 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`; + 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'); @@ -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: {