Skip to content

test: reuse serial Vitest worker - #187

Merged
mohanagy merged 2 commits into
developmentfrom
fix/122-lifecycle-root-cause
Jul 22, 2026
Merged

test: reuse serial Vitest worker#187
mohanagy merged 2 commits into
developmentfrom
fix/122-lifecycle-root-cause

Conversation

@mohanagy

@mohanagy mohanagy commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Summary

  • keep serial test files inside one isolated Vitest fork instead of cold-forking once per file
  • preserve real MCP subprocess coverage and every existing startup/shutdown budget
  • add retained fixture-entry, MCP-initialized, sanitized health, and original-cause diagnostics to the exact recurring restart boundary
  • lock the runner behavior with a focused repository contract

Root cause

fileParallelism: false serialized 108 files but Vitest still replaced the fork worker between files. Process-backed tests therefore cold-started their real MCP Node child while the suite was also repeatedly cold-starting its outer worker. Under aggregate load this intermittently delayed an inner fixture past the existing 1s/5s integration budgets; the failure migrated between files and every exact case passed immediately in isolation.

A control run with only poolOptions.forks.singleFork kept one worker PID for the full suite and passed. The committed configuration also keeps isolate: true, so module isolation remains enabled.

TDD evidence

  • reproduced unchanged on supported Node 22.22.3: 1 failure after 1,248 passes in the failed-restart replacement startup case
  • exact failing case passed focused in 297 ms
  • added the runner contract and observed it fail because singleFork was absent
  • implemented the smallest config change and observed the focused contract pass

Security impact

  • test-runner configuration only; no production runtime, public API, package, secret-store, or network behavior changes
  • per-file module isolation remains explicitly enabled
  • real MCP subprocesses, shutdown/startup budgets, process containment, assertions, and coverage thresholds remain unchanged
  • failure diagnostics contain only allowlisted lifecycle state and preserve the original test error as cause; they do not include environment values, tokens, credentials, or raw provider output

Validation

  • npm test: 1,250 passed, 24 platform-skipped
  • npm run test:core: 404 passed, 24 platform-skipped
  • npm run test:coverage: 1,250 passed, 24 platform-skipped; 95.39% statements, 91.85% branches
  • npm run lint
  • npm run typecheck
  • npm run build
  • npm run smoke:cli
  • npm run check:pack: 45 files verified
  • npm run test:package: 20 passed

No timeout, skip, assertion, coverage threshold, production lifecycle behavior, or process containment was changed.

Closes #122

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Vitest now reuses a single isolated fork for serial process-backed tests. The upstream restart teardown test adds startup markers, waits for initialization state, reports structured restart diagnostics, and removes its temporary directory during cleanup.

Changes

Test reliability

Layer / File(s) Summary
Vitest fork isolation contract
vitest.config.ts, tests/release-config.test.ts
Serial tests reuse one isolated fork, and a contract test verifies the required concurrency and isolation settings.
Restart teardown diagnostics
tests/upstream-manager.test.ts
The restart test tracks startup and initialization markers, reports structured error and health data on failure, and cleans up its temporary directory.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Poem

A bunny watched one fork stay near,
While restart markers made things clear.
Health states hopped into a chart,
Temp files vanished—clean and smart.
Tests now thump their paws: “Success!”

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address #122 by reusing one serial Vitest fork and preserving process coverage, budgets, and lifecycle assertions.
Out of Scope Changes check ✅ Passed No clear out-of-scope changes were introduced; the edits stay within the test and configuration contract.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly and concisely describes the main change: reusing the serial Vitest worker.
Description check ✅ Passed The description covers the required Summary, Security impact, and Validation sections with relevant details and results.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/122-lifecycle-root-cause

Comment @coderabbitai help to get the list of available commands.

@mohanagy
mohanagy merged commit 456a365 into development Jul 22, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test: diagnose multi-upstream availability cascade under coverage

1 participant