From de549c5affbd98ea4f52ad004520042f460836a0 Mon Sep 17 00:00:00 2001 From: justjoehere Date: Mon, 2 Dec 2024 15:08:20 -0500 Subject: [PATCH] Changes to accomodate windows users. --- Windows_Notes.md | 15 ++++++++++ src/fastmcp/cli/cli.py | 8 +++++- tests/resources/test_file_resources.py | 11 +++++-- tests/test_cli.py | 40 ++++++++++++++++++++++++-- 4 files changed, 68 insertions(+), 6 deletions(-) create mode 100644 Windows_Notes.md diff --git a/Windows_Notes.md b/Windows_Notes.md new file mode 100644 index 000000000..8993614af --- /dev/null +++ b/Windows_Notes.md @@ -0,0 +1,15 @@ +# Getting your development environment set up properly +```bash +uv venv +.venv\Scripts\activate +uv pip install -e ".[dev]" +``` + +# Fixing `AttributeError: module 'collections' has no attribute 'Callable'` +- open `.venv\Lib\site-packages\pyreadline\py3k_compat.py` +- change `return isinstance(x, collections.Callable)` to +``` +from collections.abc import Callable +return isinstance(x, Callable) +``` + diff --git a/src/fastmcp/cli/cli.py b/src/fastmcp/cli/cli.py index cd84a6a5f..594caec3d 100644 --- a/src/fastmcp/cli/cli.py +++ b/src/fastmcp/cli/cli.py @@ -67,11 +67,17 @@ def _parse_file_path(file_spec: str) -> Tuple[Path, Optional[str]]: Returns: Tuple of (file_path, server_object) """ - 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 + # and there's actually another colon in the string after the drive letter + if ":" in (file_spec[2:] if has_windows_drive else file_spec): file_str, server_object = file_spec.rsplit(":", 1) else: file_str, server_object = file_spec, None + # Resolve the file path file_path = Path(file_str).expanduser().resolve() if not file_path.exists(): logger.error(f"File not found: {file_path}") diff --git a/tests/resources/test_file_resources.py b/tests/resources/test_file_resources.py index 5b26abe82..15ddd057b 100644 --- a/tests/resources/test_file_resources.py +++ b/tests/resources/test_file_resources.py @@ -1,3 +1,5 @@ +import os + import pytest from pathlib import Path from tempfile import NamedTemporaryFile @@ -28,12 +30,12 @@ class TestFileResource: def test_file_resource_creation(self, temp_file: Path): """Test creating a FileResource.""" resource = FileResource( - uri=f"file://{temp_file}", + uri=temp_file.as_uri(), name="test", description="test file", path=temp_file, ) - assert str(resource.uri) == f"file://{temp_file}" + assert str(resource.uri) == temp_file.as_uri() assert resource.name == "test" assert resource.description == "test file" assert resource.mime_type == "text/plain" # default @@ -94,12 +96,15 @@ class TestFileResource: with pytest.raises(ValueError, match="Error reading file"): await resource.read() + @pytest.mark.skipif( + os.name == "nt", reason="File permissions behave differently on Windows" + ) async def test_permission_error(self, temp_file: Path): """Test reading a file without permissions.""" temp_file.chmod(0o000) # Remove all permissions try: resource = FileResource( - uri=f"file://{temp_file}", + uri=temp_file.as_uri(), name="test", path=temp_file, ) diff --git a/tests/test_cli.py b/tests/test_cli.py index 3015b78e2..4ed0e1895 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,12 +1,13 @@ """Tests for the FastMCP CLI.""" import json -from unittest.mock import patch +from pathlib import Path +from unittest.mock import patch, call import pytest from typer.testing import CliRunner -from fastmcp.cli.cli import app, _parse_env_var +from fastmcp.cli.cli import app, _parse_env_var, _parse_file_path @pytest.fixture @@ -91,6 +92,41 @@ def test_install_with_env_vars(mock_config, server_file, args, expected_env): assert server["env"] == expected_env +def test_parse_file_path_windows_drive(): + """Test parsing a Windows file path with a drive letter.""" + file_spec = r"C:\path\to\file.txt" + with ( + patch("pathlib.Path.exists", return_value=True), + patch("pathlib.Path.is_file", return_value=True), + ): + file_path, server_object = _parse_file_path(file_spec) + assert file_path == Path(r"C:\path\to\file.txt").resolve() + assert server_object is None + + +def test_parse_file_path_with_object(): + """Test parsing a file path with an object specification.""" + file_spec = "/path/to/file.txt:object" + with patch("sys.exit") as mock_exit: + _parse_file_path(file_spec) + + # Check that sys.exit was called twice with code 1 + assert mock_exit.call_count == 2 + mock_exit.assert_has_calls([call(1), call(1)]) + + +def test_parse_file_path_windows_with_object(): + """Test parsing a Windows file path with an object specification.""" + file_spec = r"C:\path\to\file.txt:object" + with ( + patch("pathlib.Path.exists", return_value=True), + patch("pathlib.Path.is_file", return_value=True), + ): + file_path, server_object = _parse_file_path(file_spec) + assert file_path == Path(r"C:\path\to\file.txt").resolve() + assert server_object == "object" + + def test_install_with_env_file(mock_config, server_file, mock_env_file): """Test installing with environment variables from a file.""" runner = CliRunner()