test: rewrite cli.claude config tests to assert JSON output directly (#2311)
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> Co-authored-by: Felix Weinberger <felixweinberger@users.noreply.github.com>
This commit is contained in:
parent
5388bea53a
commit
883d893097
3 changed files with 158 additions and 83 deletions
|
|
@ -33,7 +33,7 @@ def get_claude_config_path() -> Path | None: # pragma: no cover
|
|||
def get_uv_path() -> str:
|
||||
"""Get the full path to the uv executable."""
|
||||
uv_path = shutil.which("uv")
|
||||
if not uv_path: # pragma: no cover
|
||||
if not uv_path:
|
||||
logger.error(
|
||||
"uv executable not found in PATH, falling back to 'uv'. Please ensure uv is installed and in your PATH"
|
||||
)
|
||||
|
|
@ -65,7 +65,7 @@ def update_claude_config(
|
|||
"""
|
||||
config_dir = get_claude_config_path()
|
||||
uv_path = get_uv_path()
|
||||
if not config_dir: # pragma: no cover
|
||||
if not config_dir:
|
||||
raise RuntimeError(
|
||||
"Claude Desktop config directory not found. Please ensure Claude Desktop"
|
||||
" is installed and has been run at least once to initialize its config."
|
||||
|
|
@ -90,7 +90,7 @@ def update_claude_config(
|
|||
config["mcpServers"] = {}
|
||||
|
||||
# Always preserve existing env vars and merge with new ones
|
||||
if server_name in config["mcpServers"] and "env" in config["mcpServers"][server_name]: # pragma: no cover
|
||||
if server_name in config["mcpServers"] and "env" in config["mcpServers"][server_name]:
|
||||
existing_env = config["mcpServers"][server_name]["env"]
|
||||
if env_vars:
|
||||
# New vars take precedence over existing ones
|
||||
|
|
@ -103,22 +103,26 @@ def update_claude_config(
|
|||
|
||||
# Collect all packages in a set to deduplicate
|
||||
packages = {MCP_PACKAGE}
|
||||
if with_packages: # pragma: no cover
|
||||
if with_packages:
|
||||
packages.update(pkg for pkg in with_packages if pkg)
|
||||
|
||||
# Add all packages with --with
|
||||
for pkg in sorted(packages):
|
||||
args.extend(["--with", pkg])
|
||||
|
||||
if with_editable: # pragma: no cover
|
||||
if with_editable:
|
||||
args.extend(["--with-editable", str(with_editable)])
|
||||
|
||||
# Convert file path to absolute before adding to command
|
||||
# Split off any :object suffix first
|
||||
if ":" in file_spec:
|
||||
# First check if we have a Windows path (e.g., C:\...)
|
||||
has_windows_drive = len(file_spec) > 1 and file_spec[1] == ":"
|
||||
|
||||
# Split on the last colon, but only if it's not part of the Windows drive letter
|
||||
if ":" in (file_spec[2:] if has_windows_drive else file_spec):
|
||||
file_path, server_object = file_spec.rsplit(":", 1)
|
||||
file_spec = f"{Path(file_path).resolve()}:{server_object}"
|
||||
else: # pragma: no cover
|
||||
else:
|
||||
file_spec = str(Path(file_spec).resolve())
|
||||
|
||||
# Add mcp run command
|
||||
|
|
@ -127,7 +131,7 @@ def update_claude_config(
|
|||
server_config: dict[str, Any] = {"command": uv_path, "args": args}
|
||||
|
||||
# Add environment variables if specified
|
||||
if env_vars: # pragma: no cover
|
||||
if env_vars:
|
||||
server_config["env"] = env_vars
|
||||
|
||||
config["mcpServers"][server_name] = server_config
|
||||
|
|
|
|||
146
tests/cli/test_claude.py
Normal file
146
tests/cli/test_claude.py
Normal file
|
|
@ -0,0 +1,146 @@
|
|||
"""Tests for mcp.cli.claude — Claude Desktop config file generation."""
|
||||
|
||||
import json
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
|
||||
from mcp.cli.claude import get_uv_path, update_claude_config
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def config_dir(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path:
|
||||
"""Temp Claude config dir with get_claude_config_path and get_uv_path mocked."""
|
||||
claude_dir = tmp_path / "Claude"
|
||||
claude_dir.mkdir()
|
||||
monkeypatch.setattr("mcp.cli.claude.get_claude_config_path", lambda: claude_dir)
|
||||
monkeypatch.setattr("mcp.cli.claude.get_uv_path", lambda: "/fake/bin/uv")
|
||||
return claude_dir
|
||||
|
||||
|
||||
def _read_server(config_dir: Path, name: str) -> dict[str, Any]:
|
||||
config = json.loads((config_dir / "claude_desktop_config.json").read_text())
|
||||
return config["mcpServers"][name]
|
||||
|
||||
|
||||
def test_generates_uv_run_command(config_dir: Path):
|
||||
"""Should write a uv run command that invokes mcp run on the resolved file spec."""
|
||||
assert update_claude_config(file_spec="server.py:app", server_name="my_server")
|
||||
|
||||
resolved = Path("server.py").resolve()
|
||||
assert _read_server(config_dir, "my_server") == {
|
||||
"command": "/fake/bin/uv",
|
||||
"args": ["run", "--frozen", "--with", "mcp[cli]", "mcp", "run", f"{resolved}:app"],
|
||||
}
|
||||
|
||||
|
||||
def test_file_spec_without_object_suffix(config_dir: Path):
|
||||
"""File specs without :object should still resolve to an absolute path."""
|
||||
assert update_claude_config(file_spec="server.py", server_name="s")
|
||||
|
||||
assert _read_server(config_dir, "s")["args"][-1] == str(Path("server.py").resolve())
|
||||
|
||||
|
||||
def test_with_packages_sorted_and_deduplicated(config_dir: Path):
|
||||
"""Extra packages should appear as --with flags, sorted and deduplicated with mcp[cli]."""
|
||||
assert update_claude_config(file_spec="s.py:app", server_name="s", with_packages=["zebra", "aardvark", "zebra"])
|
||||
|
||||
args = _read_server(config_dir, "s")["args"]
|
||||
assert args[:8] == ["run", "--frozen", "--with", "aardvark", "--with", "mcp[cli]", "--with", "zebra"]
|
||||
|
||||
|
||||
def test_with_editable_adds_flag(config_dir: Path, tmp_path: Path):
|
||||
"""with_editable should add --with-editable after the --with flags."""
|
||||
editable = tmp_path / "project"
|
||||
assert update_claude_config(file_spec="s.py:app", server_name="s", with_editable=editable)
|
||||
|
||||
args = _read_server(config_dir, "s")["args"]
|
||||
assert args[4:6] == ["--with-editable", str(editable)]
|
||||
|
||||
|
||||
def test_env_vars_written(config_dir: Path):
|
||||
"""env_vars should be written under the server's env key."""
|
||||
assert update_claude_config(file_spec="s.py:app", server_name="s", env_vars={"KEY": "val"})
|
||||
|
||||
assert _read_server(config_dir, "s")["env"] == {"KEY": "val"}
|
||||
|
||||
|
||||
def test_existing_env_vars_merged_new_wins(config_dir: Path):
|
||||
"""Re-installing should merge env vars, with new values overriding existing ones."""
|
||||
(config_dir / "claude_desktop_config.json").write_text(
|
||||
json.dumps({"mcpServers": {"s": {"env": {"OLD": "keep", "KEY": "old"}}}})
|
||||
)
|
||||
|
||||
assert update_claude_config(file_spec="s.py:app", server_name="s", env_vars={"KEY": "new"})
|
||||
|
||||
assert _read_server(config_dir, "s")["env"] == {"OLD": "keep", "KEY": "new"}
|
||||
|
||||
|
||||
def test_existing_env_vars_preserved_without_new(config_dir: Path):
|
||||
"""Re-installing without env_vars should keep the existing env block intact."""
|
||||
(config_dir / "claude_desktop_config.json").write_text(json.dumps({"mcpServers": {"s": {"env": {"KEEP": "me"}}}}))
|
||||
|
||||
assert update_claude_config(file_spec="s.py:app", server_name="s")
|
||||
|
||||
assert _read_server(config_dir, "s")["env"] == {"KEEP": "me"}
|
||||
|
||||
|
||||
def test_other_servers_preserved(config_dir: Path):
|
||||
"""Installing a new server should not clobber existing mcpServers entries."""
|
||||
(config_dir / "claude_desktop_config.json").write_text(json.dumps({"mcpServers": {"other": {"command": "x"}}}))
|
||||
|
||||
assert update_claude_config(file_spec="s.py:app", server_name="s")
|
||||
|
||||
config = json.loads((config_dir / "claude_desktop_config.json").read_text())
|
||||
assert set(config["mcpServers"]) == {"other", "s"}
|
||||
assert config["mcpServers"]["other"] == {"command": "x"}
|
||||
|
||||
|
||||
def test_raises_when_config_dir_missing(monkeypatch: pytest.MonkeyPatch):
|
||||
"""Should raise RuntimeError when Claude Desktop config dir can't be found."""
|
||||
monkeypatch.setattr("mcp.cli.claude.get_claude_config_path", lambda: None)
|
||||
monkeypatch.setattr("mcp.cli.claude.get_uv_path", lambda: "/fake/bin/uv")
|
||||
|
||||
with pytest.raises(RuntimeError, match="Claude Desktop config directory not found"):
|
||||
update_claude_config(file_spec="s.py:app", server_name="s")
|
||||
|
||||
|
||||
@pytest.mark.parametrize("which_result, expected", [("/usr/local/bin/uv", "/usr/local/bin/uv"), (None, "uv")])
|
||||
def test_get_uv_path(monkeypatch: pytest.MonkeyPatch, which_result: str | None, expected: str):
|
||||
"""Should return shutil.which's result, or fall back to bare 'uv' when not on PATH."""
|
||||
|
||||
def fake_which(cmd: str) -> str | None:
|
||||
return which_result
|
||||
|
||||
monkeypatch.setattr("shutil.which", fake_which)
|
||||
assert get_uv_path() == expected
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"file_spec, expected_last_arg",
|
||||
[
|
||||
("C:\\Users\\server.py", "C:\\Users\\server.py"),
|
||||
("C:\\Users\\server.py:app", "C:\\Users\\server.py:app"),
|
||||
],
|
||||
)
|
||||
def test_windows_drive_letter_not_split(
|
||||
config_dir: Path, monkeypatch: pytest.MonkeyPatch, file_spec: str, expected_last_arg: str
|
||||
):
|
||||
"""Drive-letter paths like 'C:\\server.py' must not be split on the drive colon.
|
||||
|
||||
Before the fix, a bare 'C:\\path\\server.py' would hit rsplit(":", 1) and yield
|
||||
("C", "\\path\\server.py"), calling resolve() on Path("C") instead of the full path.
|
||||
"""
|
||||
seen: list[str] = []
|
||||
|
||||
def fake_resolve(self: Path) -> Path:
|
||||
seen.append(str(self))
|
||||
return self
|
||||
|
||||
monkeypatch.setattr(Path, "resolve", fake_resolve)
|
||||
|
||||
assert update_claude_config(file_spec=file_spec, server_name="s")
|
||||
|
||||
assert seen == ["C:\\Users\\server.py"]
|
||||
assert _read_server(config_dir, "s")["args"][-1] == expected_last_arg
|
||||
|
|
@ -1,75 +0,0 @@
|
|||
import json
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
from mcp.cli.claude import update_claude_config
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def temp_config_dir(tmp_path: Path):
|
||||
"""Create a temporary Claude config directory."""
|
||||
config_dir = tmp_path / "Claude"
|
||||
config_dir.mkdir()
|
||||
return config_dir
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mock_config_path(temp_config_dir: Path):
|
||||
"""Mock get_claude_config_path to return our temporary directory."""
|
||||
with patch("mcp.cli.claude.get_claude_config_path", return_value=temp_config_dir):
|
||||
yield temp_config_dir
|
||||
|
||||
|
||||
def test_command_execution(mock_config_path: Path):
|
||||
"""Test that the generated command can actually be executed."""
|
||||
# Setup
|
||||
server_name = "test_server"
|
||||
file_spec = "test_server.py:app"
|
||||
|
||||
# Update config
|
||||
success = update_claude_config(file_spec=file_spec, server_name=server_name)
|
||||
assert success
|
||||
|
||||
# Read the generated config
|
||||
config_file = mock_config_path / "claude_desktop_config.json"
|
||||
config = json.loads(config_file.read_text())
|
||||
|
||||
# Get the command and args
|
||||
server_config = config["mcpServers"][server_name]
|
||||
command = server_config["command"]
|
||||
args = server_config["args"]
|
||||
|
||||
test_args = [command] + args + ["--help"]
|
||||
|
||||
result = subprocess.run(test_args, capture_output=True, text=True, timeout=20, check=False)
|
||||
|
||||
assert result.returncode == 0
|
||||
assert "usage" in result.stdout.lower()
|
||||
|
||||
|
||||
def test_absolute_uv_path(mock_config_path: Path):
|
||||
"""Test that the absolute path to uv is used when available."""
|
||||
# Mock the shutil.which function to return a fake path
|
||||
mock_uv_path = "/usr/local/bin/uv"
|
||||
|
||||
with patch("mcp.cli.claude.get_uv_path", return_value=mock_uv_path):
|
||||
# Setup
|
||||
server_name = "test_server"
|
||||
file_spec = "test_server.py:app"
|
||||
|
||||
# Update config
|
||||
success = update_claude_config(file_spec=file_spec, server_name=server_name)
|
||||
assert success
|
||||
|
||||
# Read the generated config
|
||||
config_file = mock_config_path / "claude_desktop_config.json"
|
||||
config = json.loads(config_file.read_text())
|
||||
|
||||
# Verify the command is the absolute path
|
||||
server_config = config["mcpServers"][server_name]
|
||||
command = server_config["command"]
|
||||
|
||||
assert command == mock_uv_path
|
||||
Loading…
Reference in a new issue