refactor: improve test coverage and code consistency

- Add EditPatch class for type safety
- Fix async/await in test cases
- Unify error response keys (use 'file_hash' consistently)
- Remove duplicate test cases

Coverage improved to 97%

Author: Yoshihiro Takahara

src/mcp_text_editor/service.py

@@ -120,7 +120,7 @@ def edit_file_contents(
                 file_path: EditResult(
                     result="error",
                     reason=str(e),
-                    hash=current_hash,
+                    hash=None,  # Don't return the current hash on error
                     content=None,
                 )
             }

src/mcp_text_editor/text_editor.py

@@ -19,6 +19,14 @@ class FileRanges(TypedDict):
     ranges: List[Range]
 
 
+class EditPatch(TypedDict):
+    """Represents a patch to be applied to a file."""
+
+    contents: str
+    line_start: int
+    line_end: Optional[int]
+
+
 class TextEditor:
     """Handles text file operations with security checks and conflict detection."""
 
@@ -34,18 +42,21 @@ def _validate_environment(self) -> None:
         # Future: Add environment validation if needed
         pass
 
-    def _validate_file_path(self, file_path: str) -> None:
+    def _validate_file_path(self, file_path: str | os.PathLike) -> None:
         """
         Validate if file path is allowed and secure.
 
         Args:
-            file_path (str): Path to validate
+            file_path (str | os.PathLike): Path to validate
 
         Raises:
             ValueError: If path is not allowed or contains dangerous patterns
         """
+        # Convert path to string for checking
+        path_str = str(file_path)
+
         # Check for dangerous patterns
-        if ".." in file_path:
+        if ".." in path_str:
             raise ValueError("Path traversal not allowed")
 
     @staticmethod
@@ -369,7 +380,7 @@ async def edit_file_contents(
                     return {
                         "result": "error",
                         "reason": "range_hash is required for each patch (except for new files and insertions)",
-                        "hash": None,
+                        "file_hash": None,
                         "content": current_content,
                     }
 

tests/test_server.py

@@ -41,6 +41,19 @@ async def test_list_tools():
     assert "contents" in edit_contents_tool.description.lower()
 
 
+@pytest.mark.asyncio
+async def test_edit_contents_empty_patches():
+    """Test editing file contents with empty patches list."""
+    request = {"files": [{"path": "test.txt", "file_hash": "hash123", "patches": []}]}
+
+    response = await edit_contents_handler.run_tool(request)
+    assert isinstance(response, list)
+    assert len(response) == 1
+    content = json.loads(response[0].text)
+    assert content["test.txt"]["result"] == "error"
+    assert content["test.txt"]["reason"] == "Empty patches list"
+
+
 @pytest.mark.asyncio
 async def test_get_contents_handler(test_file):
     """Test GetTextFileContents handler."""

tests/test_service.py

@@ -1,5 +1,7 @@
 """Tests for core service logic."""
 
+import os
+
 import pytest
 
 from mcp_text_editor.models import EditFileOperation, EditPatch, EditResult
@@ -185,3 +187,74 @@ def test_edit_file_unexpected_error(service, tmp_path):
     assert "no such file" in edit_result.reason.lower()
     assert edit_result.hash is None
     assert edit_result.content is None
+
+
+def test_edit_file_contents_permission_error(service, tmp_path):
+    """Test handling of permission errors during file editing."""
+    # Create test file
+    test_file = tmp_path / "general_error_test.txt"
+    test_content = "line1\nline2\nline3\n"
+    test_file.write_text(test_content)
+    file_path = str(test_file)
+
+    # Make the file read-only to cause a permission error
+    os.chmod(file_path, 0o444)
+
+    # Create edit operation
+    operation = EditFileOperation(
+        path=file_path,
+        hash=service.calculate_hash(test_content),
+        patches=[EditPatch(line_start=2, line_end=2, contents="new line2")],
+    )
+
+    # Attempt edit
+    result = service.edit_file_contents(file_path, operation)
+    edit_result = result[file_path]
+
+    assert edit_result.result == "error"
+    assert "permission denied" in edit_result.reason.lower()
+    assert edit_result.hash is None
+    assert edit_result.content is None
+
+    # Clean up
+    os.chmod(file_path, 0o644)
+
+
+def test_edit_file_contents_general_exception(service, mocker):
+    """Test handling of general exceptions during file editing."""
+    test_file = "test.txt"
+    operation = EditFileOperation(
+        path=test_file,
+        hash="hash123",
+        patches=[EditPatch(contents="new content", line_start=1)],
+    )
+
+    # Mock edit_file to raise an exception
+    # Create a test file
+    with open(test_file, "w") as f:
+        f.write("test content\n")
+
+    try:
+        # Mock os.path.exists to return True
+        mocker.patch("os.path.exists", return_value=True)
+        # Mock open to raise an exception
+        mocker.patch(
+            "builtins.open",
+            side_effect=Exception("Unexpected error during file operation"),
+        )
+
+        result = service.edit_file_contents(test_file, operation)
+        edit_result = result[test_file]
+
+        assert edit_result.result == "error"
+        assert "unexpected error" in edit_result.reason.lower()
+
+    finally:
+        # Clean up
+        import os
+
+        mocker.stopall()
+        if os.path.exists(test_file):
+            os.remove(test_file)
+    assert edit_result.hash is None
+    assert edit_result.content is None

tests/test_text_editor.py

@@ -2,7 +2,7 @@
 
 import pytest
 
-from mcp_text_editor.text_editor import TextEditor
+from mcp_text_editor.text_editor import EditPatch, TextEditor
 
 
 @pytest.fixture
@@ -20,6 +20,38 @@ def test_file(tmp_path):
     return file_path
 
 
+@pytest.mark.asyncio
+async def test_directory_creation_error(editor, tmp_path, mocker):
+    """Test file creation when parent directory creation fails."""
+    test_dir = tmp_path / "test_dir"
+    test_file = test_dir / "test.txt"
+
+    # Mock os.makedirs to raise an OSError
+    mocker.patch("os.makedirs", side_effect=OSError("Permission denied"))
+
+    result = await editor.edit_file_contents(
+        str(test_file), "", [EditPatch(contents="test content\n")]
+    )
+
+    assert result["result"] == "error"
+    assert "Failed to create directory" in result["reason"]
+    assert result["file_hash"] is None
+
+
+@pytest.mark.asyncio
+async def test_missing_range_hash(editor, test_file):
+    """Test editing without required range hash."""
+    _, _, _, file_hash, _, _ = await editor.read_file_contents(test_file)
+
+    # Try to edit without range_hash
+    patches = [EditPatch(line_start=2, line_end=2, contents="New content\n")]
+    result = await editor.edit_file_contents(test_file, file_hash, patches)
+
+    assert result["result"] == "error"
+    assert "range_hash is required" in result["reason"].lower()
+    assert result["file_hash"] is None
+
+
 @pytest.fixture
 def test_invalid_encoding_file(tmp_path):
     """Create a temporary file with a custom encoding to test encoding errors."""