test(mcp-cli): convert non-boundary cases to in-process, cutting real spawns 78%#8631
Conversation
…s pattern (JSONbored#8587) Convert every run()/runAsync()/StdioClientTransport test case in the 11 mcp-cli-*.test.ts files that does not need the real process boundary to the existing InMemoryTransport/runCli in-process pattern (template: mcp-cli-contributor-profile-inprocess.test.ts), deleting the subprocess version of each converted case. Cases asserting process exit behavior, startup/module-load env resolution, or cross-invocation process isolation are kept as real subprocess spawns per the issue rule. Real subprocess spawn count across the 11 files drops 254 -> 60 (grep -cE ., -76%); vitest cumulative tests time for the same files drops 77.01s -> 36.67s. All 161 test scenarios preserved, zero skips.
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
loopover-orb
Bot
added
the
gittensor:feature
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-25 15:59:31 UTC
Review summary Nits — 6 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
![loopover-orb[bot]](https://avatars.githubusercontent.com/in/4139483?s=60&v=4){kind=small}
1 participant
Convert the subprocess-spawning test cases in the 11 in-scope
mcp-cli-*.test.tsfiles to the existing in-process harness pattern (template:test/unit/mcp-cli-contributor-profile-inprocess.test.ts), per the closed keep/convert rule in the issue. Each converted case replaces itsrun()/runAsync()/StdioClientTransportsubprocess spawn with the same assertions driven through the bin's exportedrunCli/maintainCli/server(dynamic import of the committed.tssource after env setup, relying on the existingisProcessEntrypointguard — the guard itself is untouched). The subprocess version of every converted case is deleted; kept cases still spawn the real CLI.Closes #8587
What changed (transport swap only — no scenario deleted, no
.skip()added)beforeAll, sets the module-load env (LOOPOVER_API_URL,LOOPOVER_CONFIG_DIR, …) before dynamically importingpackages/loopover-mcp/bin/loopover-mcp.ts, and drives converted cases viacaptureStdout(() => mod.runCli([...]))(orInMemoryTransport.createLinkedPair()+mod.serverfor stdio-tool cases), exactly like the template. Per-test fixture variation uses a shared mutable options object (the harness reads options per request), and per-test tokens use call-timeprocess.envreads.mcp-cli-maintain-tools.test.ts's per-testnew StdioClientTransport({ command: "node", args: [bin, "--stdio"] })connection becameInMemoryTransport+ the exportedserver— no subprocess.runExpectingFailureexit-status/{ok:false}-envelope case (a);expect(() => run(...)).toThrowCLI-argv/unknown-command cases (a); version-banner cases (c); config-provenance /--profile-switching / config-path-override cases where the bin resolves the value at module load (c) — profiles' active-profile is a module-load const (bin lines ~365-368), so profile-switch visibility genuinely needs a fresh process; telemetry persistence-across-invocations cases (d) —telemetry status/doctor/configreport the import-time config snapshot, so fresh-process re-read is the test's subject; doctor's "exits non-zero fromdoctor --exit-codewhen a check fails" (a — realprocess.exitwiring; its exit-code-0 sibling now asserts the coderunClireturns in-process, per the handler'sreturn ... ? 1 : 0).result.isError === trueon thecallToolresult (structured tool error, not a thrown process exception) → converted under rule (f).mcp-discovery.test.ts,mcp-local-telemetry-chokepoint.test.ts,vitest.config.ts,.github/workflows/ci.yml, all pool/concurrency settings,test/unit/support/mcp-cli-harness.ts, andpackages/**(the bin already exportsrunCli/server/maintainCli; no new exports were needed). The dynamic.tsimport uses the same const-indirection as the template sotsc --noEmitstays clean (TS5097).Spawn count before/after (
grep -cE '\brun\(|\brunAsync\(', run onmainand on this branch)run)grep -cE 'new StdioClientTransport')Telemetry stays almost fully subprocess by design: its cases assert opt-in persistence across separate CLI invocations (rule d — the bin snapshots its config at module load, so an in-process version would assert nothing). Profiles keeps its
--profile-switching family for the same module-load reason (rule c). Everything the rule allows was converted; the aggregate reduction is well past the 50% floor.Required verification (run fresh on this machine, isolated 11-file runs, same command list as the issue)
BEFORE (clean
origin/mainworktree,npm ci+npx turbo run build --filter=@loopover/engine --filter=@loopover/mcp):AFTER (this branch, same command):
Cumulative "tests" time for the 11 files: 77.01s → 36.67s (−52%). Wall Duration moves less because it is floored by two deliberately kept cases: profiles' #6792 device-flow 429 test (~10s of real poll-interval waiting, unrelated to spawn cost) and the kept startup-semantics subprocess cases.
Per-file breakdown (sum of the verbose reporter's per-test times in the two runs above):
Coverage / hygiene
npm run typecheck(tsc --noEmit) green;git diff --checkclean; no new files (no-inprocessbolt-ons — converted cases live in their original files next to the kept subprocess cases, matching maintain-tools' existing mixed structure).test/**is outside Codecov's patch gate per the issue's non-goals; the changed files are all undertest/unit/.