Skip to content

Commit 058a5b3

Browse files
committed
test: rewrite cli.claude config tests to assert JSON output directly
The previous test_command_execution shelled out to 'uv run --frozen --with mcp[cli] mcp run <path> --help' with a 20s timeout to verify the generated command was executable. Appending --help only proves uv is installed; it doesn't validate the command would actually launch a server. update_claude_config is a config-file writer — its contract is producing correct JSON. Test that directly: - Assert exact command + args structure (replaces the subprocess smoke test) - Mock get_uv_path so tests don't depend on the host's uv install - Cover previously-untested option paths: with_packages (sorted/deduped), with_editable, env_vars, env merge-on-reinstall, file specs without :object, preserving other mcpServers entries - Add get_uv_path tests for shutil.which resolution and fallback Removes 7 now-stale '# pragma: no cover' annotations from claude.py. Moves the file to tests/cli/test_claude.py to mirror src/mcp/cli/claude.py.
1 parent 5388bea commit 058a5b3

File tree

3 files changed

+124
-82
lines changed

3 files changed

+124
-82
lines changed

src/mcp/cli/claude.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ def get_claude_config_path() -> Path | None: # pragma: no cover
3333
def get_uv_path() -> str:
3434
"""Get the full path to the uv executable."""
3535
uv_path = shutil.which("uv")
36-
if not uv_path: # pragma: no cover
36+
if not uv_path:
3737
logger.error(
3838
"uv executable not found in PATH, falling back to 'uv'. Please ensure uv is installed and in your PATH"
3939
)
@@ -65,7 +65,7 @@ def update_claude_config(
6565
"""
6666
config_dir = get_claude_config_path()
6767
uv_path = get_uv_path()
68-
if not config_dir: # pragma: no cover
68+
if not config_dir:
6969
raise RuntimeError(
7070
"Claude Desktop config directory not found. Please ensure Claude Desktop"
7171
" is installed and has been run at least once to initialize its config."
@@ -90,7 +90,7 @@ def update_claude_config(
9090
config["mcpServers"] = {}
9191

9292
# Always preserve existing env vars and merge with new ones
93-
if server_name in config["mcpServers"] and "env" in config["mcpServers"][server_name]: # pragma: no cover
93+
if server_name in config["mcpServers"] and "env" in config["mcpServers"][server_name]:
9494
existing_env = config["mcpServers"][server_name]["env"]
9595
if env_vars:
9696
# New vars take precedence over existing ones
@@ -103,22 +103,22 @@ def update_claude_config(
103103

104104
# Collect all packages in a set to deduplicate
105105
packages = {MCP_PACKAGE}
106-
if with_packages: # pragma: no cover
106+
if with_packages:
107107
packages.update(pkg for pkg in with_packages if pkg)
108108

109109
# Add all packages with --with
110110
for pkg in sorted(packages):
111111
args.extend(["--with", pkg])
112112

113-
if with_editable: # pragma: no cover
113+
if with_editable:
114114
args.extend(["--with-editable", str(with_editable)])
115115

116116
# Convert file path to absolute before adding to command
117117
# Split off any :object suffix first
118118
if ":" in file_spec:
119119
file_path, server_object = file_spec.rsplit(":", 1)
120120
file_spec = f"{Path(file_path).resolve()}:{server_object}"
121-
else: # pragma: no cover
121+
else:
122122
file_spec = str(Path(file_spec).resolve())
123123

124124
# Add mcp run command
@@ -127,7 +127,7 @@ def update_claude_config(
127127
server_config: dict[str, Any] = {"command": uv_path, "args": args}
128128

129129
# Add environment variables if specified
130-
if env_vars: # pragma: no cover
130+
if env_vars:
131131
server_config["env"] = env_vars
132132

133133
config["mcpServers"][server_name] = server_config

tests/cli/test_claude.py

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
"""Tests for mcp.cli.claude — Claude Desktop config file generation."""
2+
3+
import json
4+
from pathlib import Path
5+
from typing import Any
6+
7+
import pytest
8+
9+
from mcp.cli.claude import get_uv_path, update_claude_config
10+
11+
12+
@pytest.fixture
13+
def config_dir(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path:
14+
"""Temp Claude config dir with get_claude_config_path and get_uv_path mocked."""
15+
claude_dir = tmp_path / "Claude"
16+
claude_dir.mkdir()
17+
monkeypatch.setattr("mcp.cli.claude.get_claude_config_path", lambda: claude_dir)
18+
monkeypatch.setattr("mcp.cli.claude.get_uv_path", lambda: "/fake/bin/uv")
19+
return claude_dir
20+
21+
22+
def _read_server(config_dir: Path, name: str) -> dict[str, Any]:
23+
config = json.loads((config_dir / "claude_desktop_config.json").read_text())
24+
return config["mcpServers"][name]
25+
26+
27+
def test_generates_uv_run_command(config_dir: Path):
28+
"""Should write a uv run command that invokes mcp run on the resolved file spec."""
29+
assert update_claude_config(file_spec="server.py:app", server_name="my_server")
30+
31+
resolved = Path("server.py").resolve()
32+
assert _read_server(config_dir, "my_server") == {
33+
"command": "/fake/bin/uv",
34+
"args": ["run", "--frozen", "--with", "mcp[cli]", "mcp", "run", f"{resolved}:app"],
35+
}
36+
37+
38+
def test_file_spec_without_object_suffix(config_dir: Path):
39+
"""File specs without :object should still resolve to an absolute path."""
40+
assert update_claude_config(file_spec="server.py", server_name="s")
41+
42+
assert _read_server(config_dir, "s")["args"][-1] == str(Path("server.py").resolve())
43+
44+
45+
def test_with_packages_sorted_and_deduplicated(config_dir: Path):
46+
"""Extra packages should appear as --with flags, sorted and deduplicated with mcp[cli]."""
47+
assert update_claude_config(file_spec="s.py:app", server_name="s", with_packages=["zebra", "aardvark", "zebra"])
48+
49+
args = _read_server(config_dir, "s")["args"]
50+
assert args[:8] == ["run", "--frozen", "--with", "aardvark", "--with", "mcp[cli]", "--with", "zebra"]
51+
52+
53+
def test_with_editable_adds_flag(config_dir: Path, tmp_path: Path):
54+
"""with_editable should add --with-editable after the --with flags."""
55+
editable = tmp_path / "project"
56+
assert update_claude_config(file_spec="s.py:app", server_name="s", with_editable=editable)
57+
58+
args = _read_server(config_dir, "s")["args"]
59+
assert args[4:6] == ["--with-editable", str(editable)]
60+
61+
62+
def test_env_vars_written(config_dir: Path):
63+
"""env_vars should be written under the server's env key."""
64+
assert update_claude_config(file_spec="s.py:app", server_name="s", env_vars={"KEY": "val"})
65+
66+
assert _read_server(config_dir, "s")["env"] == {"KEY": "val"}
67+
68+
69+
def test_existing_env_vars_merged_new_wins(config_dir: Path):
70+
"""Re-installing should merge env vars, with new values overriding existing ones."""
71+
(config_dir / "claude_desktop_config.json").write_text(
72+
json.dumps({"mcpServers": {"s": {"env": {"OLD": "keep", "KEY": "old"}}}})
73+
)
74+
75+
assert update_claude_config(file_spec="s.py:app", server_name="s", env_vars={"KEY": "new"})
76+
77+
assert _read_server(config_dir, "s")["env"] == {"OLD": "keep", "KEY": "new"}
78+
79+
80+
def test_existing_env_vars_preserved_without_new(config_dir: Path):
81+
"""Re-installing without env_vars should keep the existing env block intact."""
82+
(config_dir / "claude_desktop_config.json").write_text(json.dumps({"mcpServers": {"s": {"env": {"KEEP": "me"}}}}))
83+
84+
assert update_claude_config(file_spec="s.py:app", server_name="s")
85+
86+
assert _read_server(config_dir, "s")["env"] == {"KEEP": "me"}
87+
88+
89+
def test_other_servers_preserved(config_dir: Path):
90+
"""Installing a new server should not clobber existing mcpServers entries."""
91+
(config_dir / "claude_desktop_config.json").write_text(json.dumps({"mcpServers": {"other": {"command": "x"}}}))
92+
93+
assert update_claude_config(file_spec="s.py:app", server_name="s")
94+
95+
config = json.loads((config_dir / "claude_desktop_config.json").read_text())
96+
assert set(config["mcpServers"]) == {"other", "s"}
97+
assert config["mcpServers"]["other"] == {"command": "x"}
98+
99+
100+
def test_raises_when_config_dir_missing(monkeypatch: pytest.MonkeyPatch):
101+
"""Should raise RuntimeError when Claude Desktop config dir can't be found."""
102+
monkeypatch.setattr("mcp.cli.claude.get_claude_config_path", lambda: None)
103+
monkeypatch.setattr("mcp.cli.claude.get_uv_path", lambda: "/fake/bin/uv")
104+
105+
with pytest.raises(RuntimeError, match="Claude Desktop config directory not found"):
106+
update_claude_config(file_spec="s.py:app", server_name="s")
107+
108+
109+
@pytest.mark.parametrize("which_result, expected", [("/usr/local/bin/uv", "/usr/local/bin/uv"), (None, "uv")])
110+
def test_get_uv_path(monkeypatch: pytest.MonkeyPatch, which_result: str | None, expected: str):
111+
"""Should return shutil.which's result, or fall back to bare 'uv' when not on PATH."""
112+
113+
def fake_which(cmd: str) -> str | None:
114+
return which_result
115+
116+
monkeypatch.setattr("shutil.which", fake_which)
117+
assert get_uv_path() == expected

tests/client/test_config.py

Lines changed: 0 additions & 75 deletions
This file was deleted.

0 commit comments

Comments
 (0)