Skip to content

Clean up npm test plumbing across workspaces#141

Open
MatiasFernandez wants to merge 1 commit into
mainfrom
mfernandez.tweak-monorepo-test-settings
Open

Clean up npm test plumbing across workspaces#141
MatiasFernandez wants to merge 1 commit into
mainfrom
mfernandez.tweak-monorepo-test-settings

Conversation

@MatiasFernandez

@MatiasFernandez MatiasFernandez commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Goal

Bring the monorepo's npm test plumbing closer to a conventional npm-workspaces setup: make the three workspaces (client, server, mcp-server) run tests consistently, remove a latent divergence between how the root and the workspaces invoked their tests, and drop a Node flag that is now a no-op. Behavioral no-op on the supported toolchain (Node pinned to 24.15.0 via .nvmrc) — verified npm test passes across all three workspaces.

What changed

server/package.json — the workspace had no scripts block at all, which is why the root had to cd server && npx vitest run as a workaround. Added a standard "test": "vitest run", so it can be driven like any other workspace.

package.json — the root test hand-chained three legs that each invoked their workspace differently (cd server && npx vitest run, npm test --workspace client, cd mcp-server && npx vitest run). Now:

  • testnpm run test --workspaces --if-present (the documented mechanism for running a script across all workspaces).
  • test:server / test:mcpnpm test --workspace <name>, consistent with the existing test:client, so all per-workspace legs route through the workspace's own script.

mcp-server/package.json + package.json — the mcp-server test script carried NODE_OPTIONS=--experimental-require-module to let CommonJS require() the ESM-only @modelcontextprotocol/sdk. On Node 24 that flag is a no-op: require(ESM) has been enabled by default since Node v23.0.0 / v22.12.0 / v20.19.0. The inline NODE_OPTIONS=... syntax is also POSIX-shell-only (breaks on Windows cmd.exe). So:

  • Dropped the flag; the script is now just "vitest run" (matching server).
  • Added "engines": { "node": ">=22.12" } to the root to make explicit the Node floor the flag used to paper over.

Removing the flag also eliminates the divergence class at its root: with no special flag, the root and workspace invocations can no longer differ.

Verification

  • npm test passes across all three workspaces.
  • npm run test:mcp → 95/95 pass, exit 0, no ExperimentalWarning, no ERR_REQUIRE_ESM, with the flag removed on Node 24.15.0.

@MatiasFernandez MatiasFernandez requested a review from npapagna July 3, 2026 21:57
@MatiasFernandez MatiasFernandez marked this pull request as ready for review July 3, 2026 21:57
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.

1 participant