From 4a87d73812fa7963c1f55127887fecb3ea7dedfb Mon Sep 17 00:00:00 2001 From: AHMET BAYHAN BAYRAMOGLU <49499275+ABB65@users.noreply.github.com> Date: Thu, 9 Jul 2026 11:17:32 +0300 Subject: [PATCH] test(mcp): cut write-path suite spawns via init-once template clone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of the slow write-path suite is not git's work but the cost of spawning a subprocess from inside a loaded vitest worker on macOS: a full model_save fans out to 92 git processes whose total execution is ~0.5s, yet the test takes ~17s. Standalone, git --version spawns in ~12ms; inside a vitest worker it is ~170ms (even /usr/bin/true is ~148ms vs ~3ms) — the fork/posix_spawn cost scales with the worker's address space, not with git. Confirmed not fixable by config (identical across forks/threads/vmThreads; unaffected by FD limit, env size, git gc/maintenance/fsync, or the binary), and raising maxForks backfires on kernel VM-lock contention. The only lever is spawning git fewer times. - tests/support/project.ts: makeInitedTemplate (run contentrain_init ONCE in beforeAll) + cloneTemplate (cp -R per test = zero git spawns) + shared createClient/parseResult/initGitRepo (were inlined identically ~9x). - normalize.test.ts: 19 read-only contentrain_scan tests now share ONE beforeAll fixture instead of re-initing per test (~250s -> 13s standalone). - model.test.ts, workflow.test.ts: template-clone per test keeps full isolation while dropping the ~28-spawn init from every beforeEach (model 130s -> 55s; workflow ~270s -> 209s, body-mutation-bound). Same test counts (normalize 19, model 8, workflow 15); combined run with the heaviest unrefactored write-path files is 134/134 green. --- packages/mcp/tests/support/project.ts | 89 +++++++++++++++++++++ packages/mcp/tests/tools/model.test.ts | 74 ++++++------------ packages/mcp/tests/tools/normalize.test.ts | 73 +++-------------- packages/mcp/tests/tools/workflow.test.ts | 91 ++++++++-------------- 4 files changed, 160 insertions(+), 167 deletions(-) create mode 100644 packages/mcp/tests/support/project.ts diff --git a/packages/mcp/tests/support/project.ts b/packages/mcp/tests/support/project.ts new file mode 100644 index 0000000..5eaef51 --- /dev/null +++ b/packages/mcp/tests/support/project.ts @@ -0,0 +1,89 @@ +import { cp, mkdtemp } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { simpleGit } from 'simple-git' +import { Client } from '@modelcontextprotocol/sdk/client/index.js' +import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js' +import { createServer } from '../../src/server.js' + +/** + * Shared test-support for the MCP write-path suites. + * + * Why this exists: on macOS every `git` subprocess spawned from inside a + * loaded vitest worker costs ~150ms (≈14× the standalone cost) because the + * fork/posix_spawn work scales with the worker's address space — not with + * git itself, whose own runtime per command is ~1–20ms. A single + * `contentrain_init` transaction spawns ~28 git processes, so re-running it + * in every `beforeEach` dominates suite wall-clock. + * + * The fix is to spawn git as few times as possible: + * - read-only suites build ONE fixture in `beforeAll` and share it; + * - mutating suites build ONE inited template in `beforeAll` and give each + * test an isolated copy via {@link cloneTemplate} — a recursive file copy + * spawns zero git processes, versus ~28 for a fresh init. + */ + +/** Build an MCP client wired to a fresh in-memory server over the given root. */ +export async function createClient(projectRoot: string): Promise { + const server = createServer(projectRoot) + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair() + const client = new Client({ name: 'test-client', version: '1.0.0' }) + await Promise.all([ + client.connect(clientTransport), + server.connect(serverTransport), + ]) + return client +} + +/** Parse the JSON payload out of a tool result's first text content block. */ +export function parseResult(result: unknown): Record { + const content = (result as { content: Array<{ text: string }> }).content + return JSON.parse(content[0]!.text) as Record +} + +/** git init + identity + one empty commit. The minimal committable repo. */ +export async function initGitRepo(dir: string): Promise { + const git = simpleGit(dir) + await git.init() + await git.addConfig('user.name', 'Test') + await git.addConfig('user.email', 'test@test.com') + await git.commit('initial', { '--allow-empty': null, '--no-verify': null }) +} + +/** + * Create a temp git repo and run `contentrain_init` once, returning the path. + * Call this in `beforeAll`; hand each test a private copy with + * {@link cloneTemplate}. `prepare` runs after the repo is committed but + * before init — use it to lay down source files that must exist at init time. + */ +export async function makeInitedTemplate(opts?: { + locales?: string[] + prepare?: (dir: string) => Promise +}): Promise { + const dir = await mkdtemp(join(tmpdir(), 'cr-template-')) + await initGitRepo(dir) + if (opts?.prepare) { + await opts.prepare(dir) + const git = simpleGit(dir) + await git.add('.') + await git.commit('fixture sources', { '--no-verify': null }) + } + const client = await createClient(dir) + await client.callTool({ + name: 'contentrain_init', + arguments: opts?.locales ? { locales: opts.locales } : {}, + }) + return dir +} + +/** + * Copy an inited template into a fresh temp dir — an isolated, ready-to-use + * project with zero git subprocesses. The `.git` directory copies as plain + * files; the template leaves no worktrees behind, so the copy is a fully + * functional repo at its new path. + */ +export async function cloneTemplate(template: string): Promise { + const dir = await mkdtemp(join(tmpdir(), 'cr-clone-')) + await cp(template, dir, { recursive: true }) + return dir +} diff --git a/packages/mcp/tests/tools/model.test.ts b/packages/mcp/tests/tools/model.test.ts index da21b7d..afad3e2 100644 --- a/packages/mcp/tests/tools/model.test.ts +++ b/packages/mcp/tests/tools/model.test.ts @@ -1,42 +1,19 @@ -import { describe, expect, it, beforeEach, afterEach, vi } from 'vitest' +import { describe, expect, it, beforeAll, beforeEach, afterAll, afterEach, vi } from 'vitest' vi.setConfig({ testTimeout: 120000, hookTimeout: 120000 }) -import { join } from 'node:path' -import { mkdtemp, rm, writeFile } from 'node:fs/promises' +import { mkdtemp, rm } from 'node:fs/promises' import { tmpdir } from 'node:os' +import { join } from 'node:path' import { simpleGit } from 'simple-git' -import { Client } from '@modelcontextprotocol/sdk/client/index.js' -import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js' -import { createServer } from '../../src/server.js' +import type { Client } from '@modelcontextprotocol/sdk/client/index.js' +import { createClient, cloneTemplate, initGitRepo, makeInitedTemplate } from '../support/project.js' import { pathExists, readJson } from '../../src/util/fs.js' import type { ModelDefinition } from '@contentrain/types' +let template: string let testDir: string let client: Client -async function initProject(dir: string): Promise { - const git = simpleGit(dir) - await git.init() - await git.addConfig('user.name', 'Test') - await git.addConfig('user.email', 'test@test.com') - await writeFile(join(dir, '.gitkeep'), '') - await git.add('.') - await git.commit('initial') -} - -async function createTestClient(projectRoot: string): Promise { - const server = createServer(projectRoot) - const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair() - - const c = new Client({ name: 'test-client', version: '1.0.0' }) - await Promise.all([ - c.connect(clientTransport), - server.connect(serverTransport), - ]) - - return c -} - function parseResult(result: unknown): Record { const content = (result as { content: Array<{ text: string }> }).content const text = content[0]!.text @@ -47,14 +24,19 @@ function parseResult(result: unknown): Record { } } -beforeEach(async () => { - testDir = await mkdtemp(join(tmpdir(), 'cr-model-test-')) - await initProject(testDir) - client = await createTestClient(testDir) +// Init the project ONCE; each test gets an isolated copy via a file-copy +// (zero git spawns) instead of re-running the ~28-spawn init transaction. +beforeAll(async () => { + template = await makeInitedTemplate() +}) - // Initialize project first - await client.callTool({ name: 'contentrain_init', arguments: {} }) - client = await createTestClient(testDir) +afterAll(async () => { + await rm(template, { recursive: true, force: true }) +}) + +beforeEach(async () => { + testDir = await cloneTemplate(template) + client = await createClient(testDir) }) afterEach(async () => { @@ -113,7 +95,7 @@ describe('contentrain_model_save', () => { }, }) - client = await createTestClient(testDir) + client = await createClient(testDir) // Update const result = await client.callTool({ @@ -168,15 +150,9 @@ describe('contentrain_model_save', () => { it('returns error when not initialized', async () => { const emptyDir = await mkdtemp(join(tmpdir(), 'cr-empty-')) - const git = simpleGit(emptyDir) - await git.init() - await git.addConfig('user.name', 'Test') - await git.addConfig('user.email', 'test@test.com') - await writeFile(join(emptyDir, '.gitkeep'), '') - await git.add('.') - await git.commit('initial') - - const emptyClient = await createTestClient(emptyDir) + await initGitRepo(emptyDir) + + const emptyClient = await createClient(emptyDir) const result = await emptyClient.callTool({ name: 'contentrain_model_save', arguments: { @@ -210,7 +186,7 @@ describe('contentrain_model_delete', () => { }, }) - client = await createTestClient(testDir) + client = await createClient(testDir) const result = await client.callTool({ name: 'contentrain_model_delete', @@ -241,7 +217,7 @@ describe('contentrain_model_delete', () => { }, }) - client = await createTestClient(testDir) + client = await createClient(testDir) // Create referencing model await client.callTool({ @@ -259,7 +235,7 @@ describe('contentrain_model_delete', () => { }, }) - client = await createTestClient(testDir) + client = await createClient(testDir) // Try to delete authors — should be blocked const result = await client.callTool({ diff --git a/packages/mcp/tests/tools/normalize.test.ts b/packages/mcp/tests/tools/normalize.test.ts index 02e54c2..0a7dd6f 100644 --- a/packages/mcp/tests/tools/normalize.test.ts +++ b/packages/mcp/tests/tools/normalize.test.ts @@ -1,45 +1,15 @@ -import { describe, expect, it, beforeEach, afterEach, vi } from 'vitest' +import { describe, expect, it, beforeAll, afterAll, vi } from 'vitest' vi.setConfig({ testTimeout: 120000, hookTimeout: 120000 }) import { join } from 'node:path' import { mkdtemp, rm, writeFile, mkdir } from 'node:fs/promises' import { tmpdir } from 'node:os' -import { simpleGit } from 'simple-git' -import { Client } from '@modelcontextprotocol/sdk/client/index.js' -import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js' -import { createServer } from '../../src/server.js' +import type { Client } from '@modelcontextprotocol/sdk/client/index.js' +import { createClient, initGitRepo, makeInitedTemplate, parseResult } from '../support/project.js' let testDir: string let client: Client -async function initGitRepo(dir: string): Promise { - const git = simpleGit(dir) - await git.init() - await git.addConfig('user.name', 'Test') - await git.addConfig('user.email', 'test@test.com') - await writeFile(join(dir, '.gitkeep'), '') - await git.add('.') - await git.commit('initial') -} - -async function createTestClient(projectRoot: string): Promise { - const server = createServer(projectRoot) - const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair() - - const c = new Client({ name: 'test-client', version: '1.0.0' }) - await Promise.all([ - c.connect(clientTransport), - server.connect(serverTransport), - ]) - - return c -} - -function parseResult(result: unknown): Record { - const content = (result as { content: Array<{ text: string }> }).content - return JSON.parse(content[0]!.text) as Record -} - async function createSourceFiles(dir: string): Promise { // Create directory structure await mkdir(join(dir, 'src', 'pages'), { recursive: true }) @@ -126,27 +96,14 @@ export function MainLayout({ children }: { children: React.ReactNode }) { `) } -beforeEach(async () => { - testDir = await mkdtemp(join(tmpdir(), 'cr-scan-test-')) - await initGitRepo(testDir) - - // Create source files before init so they exist during project setup - await createSourceFiles(testDir) - - // Commit source files so git is clean - const git = simpleGit(testDir) - await git.add('.') - await git.commit('add source files') - - // Create client and initialize project - client = await createTestClient(testDir) - await client.callTool({ name: 'contentrain_init', arguments: {} }) - - // Re-create client after init - client = await createTestClient(testDir) +// contentrain_scan is read-only (no disk/git writes), so all tests share ONE +// inited fixture built once — instead of paying a ~28-git-spawn init per test. +beforeAll(async () => { + testDir = await makeInitedTemplate({ prepare: createSourceFiles }) + client = await createClient(testDir) }) -afterEach(async () => { +afterAll(async () => { await rm(testDir, { recursive: true, force: true }) }) @@ -396,15 +353,9 @@ describe('contentrain_scan mode:candidates', () => { it('returns error if not initialized', async () => { const emptyDir = await mkdtemp(join(tmpdir(), 'cr-scan-empty-')) - const git = simpleGit(emptyDir) - await git.init() - await git.addConfig('user.name', 'Test') - await git.addConfig('user.email', 'test@test.com') - await writeFile(join(emptyDir, '.gitkeep'), '') - await git.add('.') - await git.commit('initial') - - const emptyClient = await createTestClient(emptyDir) + await initGitRepo(emptyDir) + + const emptyClient = await createClient(emptyDir) const result = await emptyClient.callTool({ name: 'contentrain_scan', arguments: { mode: 'candidates' }, diff --git a/packages/mcp/tests/tools/workflow.test.ts b/packages/mcp/tests/tools/workflow.test.ts index f06de58..15244da 100644 --- a/packages/mcp/tests/tools/workflow.test.ts +++ b/packages/mcp/tests/tools/workflow.test.ts @@ -1,46 +1,18 @@ -import { describe, expect, it, beforeEach, afterEach, vi } from 'vitest' +import { describe, expect, it, beforeAll, beforeEach, afterAll, afterEach, vi } from 'vitest' vi.setConfig({ testTimeout: 120000, hookTimeout: 120000 }) import { join } from 'node:path' import { mkdtemp, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { simpleGit } from 'simple-git' -import { Client } from '@modelcontextprotocol/sdk/client/index.js' -import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js' -import { createServer } from '../../src/server.js' +import type { Client } from '@modelcontextprotocol/sdk/client/index.js' +import { createClient, cloneTemplate, makeInitedTemplate, parseResult } from '../support/project.js' import { readJson, writeJson } from '../../src/util/fs.js' +let template: string let testDir: string let client: Client -async function initProject(dir: string): Promise { - const git = simpleGit(dir) - await git.init() - await git.addConfig('user.name', 'Test') - await git.addConfig('user.email', 'test@test.com') - await writeFile(join(dir, '.gitkeep'), '') - await git.add('.') - await git.commit('initial') -} - -async function createTestClient(projectRoot: string): Promise { - const server = createServer(projectRoot) - const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair() - - const c = new Client({ name: 'test-client', version: '1.0.0' }) - await Promise.all([ - c.connect(clientTransport), - server.connect(serverTransport), - ]) - - return c -} - -function parseResult(result: unknown): Record { - const content = (result as { content: Array<{ text: string }> }).content - return JSON.parse(content[0]!.text) as Record -} - async function createModel( c: Client, id: string, @@ -59,17 +31,22 @@ async function createModel( fields, }, }) - return createTestClient(testDir) + return createClient(testDir) } -beforeEach(async () => { - testDir = await mkdtemp(join(tmpdir(), 'cr-workflow-test-')) - await initProject(testDir) - client = await createTestClient(testDir) +// Init the en+tr project ONCE; each mutating test gets an isolated copy via a +// file-copy (zero git spawns) instead of re-running the ~28-spawn init. +beforeAll(async () => { + template = await makeInitedTemplate({ locales: ['en', 'tr'] }) +}) - // Initialize project with en + tr locales - await client.callTool({ name: 'contentrain_init', arguments: { locales: ['en', 'tr'] } }) - client = await createTestClient(testDir) +afterAll(async () => { + await rm(template, { recursive: true, force: true }) +}) + +beforeEach(async () => { + testDir = await cloneTemplate(template) + client = await createClient(testDir) }) afterEach(async () => { @@ -94,7 +71,7 @@ describe('contentrain_validate', () => { }, }) - client = await createTestClient(testDir) + client = await createClient(testDir) const result = await client.callTool({ name: 'contentrain_validate', @@ -127,7 +104,7 @@ describe('contentrain_validate', () => { }, }) - client = await createTestClient(testDir) + client = await createClient(testDir) const result = await client.callTool({ name: 'contentrain_validate', @@ -166,7 +143,7 @@ describe('contentrain_validate', () => { }, }) - client = await createTestClient(testDir) + client = await createClient(testDir) const result = await client.callTool({ name: 'contentrain_validate', @@ -198,7 +175,7 @@ describe('contentrain_validate', () => { }, }) - client = await createTestClient(testDir) + client = await createClient(testDir) const result = await client.callTool({ name: 'contentrain_validate', @@ -231,7 +208,7 @@ describe('contentrain_validate', () => { }, }) - client = await createTestClient(testDir) + client = await createClient(testDir) const result = await client.callTool({ name: 'contentrain_validate', @@ -264,7 +241,7 @@ describe('contentrain_validate', () => { }, }) - client = await createTestClient(testDir) + client = await createClient(testDir) const result = await client.callTool({ name: 'contentrain_validate', @@ -296,7 +273,7 @@ describe('contentrain_validate', () => { }, }) - client = await createTestClient(testDir) + client = await createClient(testDir) // Manually inject orphan meta entry const metaPath = join(testDir, '.contentrain', 'meta', 'authors', 'en.json') @@ -309,7 +286,7 @@ describe('contentrain_validate', () => { await git.add('.') await git.commit('add orphan meta') - client = await createTestClient(testDir) + client = await createClient(testDir) const result = await client.callTool({ name: 'contentrain_validate', @@ -341,7 +318,7 @@ describe('contentrain_validate', () => { }, }) - client = await createTestClient(testDir) + client = await createClient(testDir) const metaPath = join(testDir, '.contentrain', 'meta', 'authors', 'en.json') const metaData = await readJson>>(metaPath) ?? {} @@ -352,7 +329,7 @@ describe('contentrain_validate', () => { await git.add('.') await git.commit('remove author meta') - client = await createTestClient(testDir) + client = await createClient(testDir) const result = await client.callTool({ name: 'contentrain_validate', @@ -383,7 +360,7 @@ describe('contentrain_validate', () => { }, }) - client = await createTestClient(testDir) + client = await createClient(testDir) const result = await client.callTool({ name: 'contentrain_validate', @@ -417,7 +394,7 @@ describe('contentrain_validate', () => { }, }) - client = await createTestClient(testDir) + client = await createClient(testDir) // Manually mess up the order const contentPath = join(testDir, '.contentrain', 'content', 'blog', 'authors', 'en.json') @@ -437,7 +414,7 @@ describe('contentrain_validate', () => { await git.add('.') await git.commit('mess up order') - client = await createTestClient(testDir) + client = await createClient(testDir) const result = await client.callTool({ name: 'contentrain_validate', @@ -472,7 +449,7 @@ describe('contentrain_validate', () => { }, }) - client = await createTestClient(testDir) + client = await createClient(testDir) const result = await client.callTool({ name: 'contentrain_validate', @@ -503,7 +480,7 @@ describe('contentrain_validate', () => { }, }) - client = await createTestClient(testDir) + client = await createClient(testDir) const result = await client.callTool({ name: 'contentrain_validate', @@ -539,7 +516,7 @@ describe('contentrain_validate', () => { }, }) - client = await createTestClient(testDir) + client = await createClient(testDir) const result = await client.callTool({ name: 'contentrain_validate', @@ -585,7 +562,7 @@ describe('contentrain_submit', () => { await freshGit.add('.') await freshGit.commit('add config') - const freshClient = await createTestClient(freshDir) + const freshClient = await createClient(freshDir) const result = await freshClient.callTool({ name: 'contentrain_submit',