Merge pull request #25 from Model-Context-Interface/copilot/improve-test-coverage-documentation

Stage 10: Achieve 90% test coverage and refactor documentation for user experience

Author: MaestroError

README.md

@@ -71,7 +71,7 @@ def is_readable(path: str) -> bool:
     """
     try:
         return os.access(path, os.R_OK)
-    except (OSError, ValueError):
+    except (OSError, ValueError, TypeError):
         return False
 
 

development.md

@@ -32,16 +32,6 @@ def test_cli_help_flag_variations():
     assert len(result_help.output) > 0
 
 
-def test_cli_version_flag():
-    """Test that the version flag displays version information."""
-    runner = CliRunner()
-    result = runner.invoke(main, ["--version"])
-
-    assert result.exit_code == 0
-    # Should have some output (version number or message)
-    assert len(result.output.strip()) > 0
-
-
 def test_cli_command_group():
     """Test that the CLI is a Click command group."""
     # The main function should be a Click group

docs/architecture.md

@@ -23,15 +23,6 @@ def test_cli_help():
     assert "Manage Model Context Interface schemas" in result.output
 
 
-def test_cli_version():
-    """Test that the CLI version flag works."""
-    runner = CliRunner()
-    result = runner.invoke(main, ["--version"])
-    assert result.exit_code == 0
-    # Should show version output
-    assert "version" in result.output.lower() or len(result.output.strip()) > 0
-
-
 def test_cli_no_args():
     """Test that running CLI with no arguments shows usage."""
     runner = CliRunner()

src/mci/utils/validation.py

@@ -0,0 +1,147 @@
+"""
+Unit tests for ErrorFormatter class.
+
+Tests error and warning formatting utilities for CLI display.
+"""
+
+import pytest
+from io import StringIO
+
+from rich.console import Console
+
+from mci.utils.error_formatter import (
+    ErrorFormatter,
+    ValidationError,
+    ValidationWarning,
+)
+
+
+@pytest.fixture
+def formatter():
+    """Create an ErrorFormatter with StringIO console for testing."""
+    console = Console(file=StringIO(), force_terminal=True)
+    return ErrorFormatter(console)
+
+
+def test_format_validation_errors_with_single_error(formatter):
+    """Test formatting a single validation error."""
+    errors = [ValidationError(message="Missing required field: name")]
+    formatter.format_validation_errors(errors)
+    
+    output = formatter.console.file.getvalue()  # type: ignore[attr-defined]
+    assert "❌ Validation Errors" in output
+    assert "Missing required field: name" in output
+    assert "Schema Validation Failed" in output
+
+
+def test_format_validation_errors_with_multiple_errors(formatter):
+    """Test formatting multiple validation errors."""
+    errors = [
+        ValidationError(message="Missing required field: name"),
+        ValidationError(message="Invalid type for field: age", location="tools[0]"),
+    ]
+    formatter.format_validation_errors(errors)
+    
+    output = formatter.console.file.getvalue()  # type: ignore[attr-defined]
+    assert "❌ Validation Errors" in output
+    assert "Missing required field: name" in output
+    assert "Invalid type for field: age" in output
+    assert "[tools[0]]" in output
+
+
+def test_format_validation_errors_with_empty_list(formatter):
+    """Test formatting with empty error list (should not output anything)."""
+    formatter.format_validation_errors([])
+    
+    output = formatter.console.file.getvalue()  # type: ignore[attr-defined]
+    assert output == ""
+
+
+def test_format_validation_warnings_with_single_warning(formatter):
+    """Test formatting a single validation warning."""
+    warnings = [
+        ValidationWarning(
+            message="Toolset file not found: weather.mci.json",
+            suggestion="Create the file or update your schema",
+        )
+    ]
+    formatter.format_validation_warnings(warnings)
+    
+    output = formatter.console.file.getvalue()  # type: ignore[attr-defined]
+    assert "⚠️  Validation Warnings" in output
+    assert "Toolset file not found" in output
+    assert "💡 Create the file or update your schema" in output
+
+
+def test_format_validation_warnings_with_no_suggestion(formatter):
+    """Test formatting a warning without suggestion."""
+    warnings = [ValidationWarning(message="Potential issue detected")]
+    formatter.format_validation_warnings(warnings)
+    
+    output = formatter.console.file.getvalue()  # type: ignore[attr-defined]
+    assert "⚠️  Validation Warnings" in output
+    assert "Potential issue detected" in output
+
+
+def test_format_validation_warnings_with_empty_list(formatter):
+    """Test formatting with empty warning list (should not output anything)."""
+    formatter.format_validation_warnings([])
+    
+    output = formatter.console.file.getvalue()  # type: ignore[attr-defined]
+    assert output == ""
+
+
+def test_format_validation_success(formatter):
+    """Test formatting validation success message."""
+    formatter.format_validation_success("mci.json")
+    
+    output = formatter.console.file.getvalue()  # type: ignore[attr-defined]
+    assert "✅ Schema is valid!" in output
+    assert "File: mci.json" in output
+    assert "Validation Successful" in output
+
+
+def test_format_mci_error(formatter):
+    """Test formatting an MCI error message."""
+    formatter.format_mci_error("Failed to load schema: Invalid JSON")
+    
+    output = formatter.console.file.getvalue()  # type: ignore[attr-defined]
+    assert "❌ MCI Error" in output
+    assert "Failed to load schema: Invalid JSON" in output
+
+
+def test_error_formatter_default_console():
+    """Test ErrorFormatter with default console."""
+    formatter = ErrorFormatter()
+    assert formatter.console is not None
+    
+    # Should not raise any errors
+    formatter.format_validation_success("test.json")
+
+
+def test_validation_error_with_location():
+    """Test ValidationError with location."""
+    error = ValidationError(message="Test error", location="tools[0].name")
+    assert error.message == "Test error"
+    assert error.location == "tools[0].name"
+
+
+def test_validation_error_without_location():
+    """Test ValidationError without location."""
+    error = ValidationError(message="Test error")
+    assert error.message == "Test error"
+    assert error.location is None
+
+
+def test_validation_warning_with_suggestion():
+    """Test ValidationWarning with suggestion."""
+    warning = ValidationWarning(message="Test warning", suggestion="Fix it")
+    assert warning.message == "Test warning"
+    assert warning.suggestion == "Fix it"
+
+
+def test_validation_warning_without_suggestion():
+    """Test ValidationWarning without suggestion."""
+    warning = ValidationWarning(message="Test warning")
+    assert warning.message == "Test warning"
+    assert warning.suggestion is None

tests/test_cli_help.py

@@ -0,0 +1,128 @@
+"""
+Unit tests for validation utilities.
+
+Tests file validation functions including path validation,
+file existence checks, and path resolution.
+"""
+
+import os
+import tempfile
+from pathlib import Path
+
+from mci.utils.validation import (
+    file_exists,
+    get_absolute_path,
+    is_readable,
+    is_valid_path,
+)
+
+
+def test_is_valid_path_with_valid_path():
+    """Test is_valid_path with a valid path."""
+    assert is_valid_path("./mci.json") is True
+    assert is_valid_path("/tmp/test.json") is True
+    assert is_valid_path("test.yaml") is True
+
+
+def test_is_valid_path_with_empty_string():
+    """Test is_valid_path with empty string."""
+    assert is_valid_path("") is False
+
+
+def test_is_valid_path_with_none():
+    """Test is_valid_path with None (should handle TypeError)."""
+    # Type ignore because we're testing error handling
+    assert is_valid_path(None) is False  # type: ignore[arg-type]
+
+
+def test_is_valid_path_with_invalid_type():
+    """Test is_valid_path with invalid type."""
+    # Test with number (should handle TypeError)
+    assert is_valid_path(123) is False  # type: ignore[arg-type]
+
+
+def test_file_exists_with_existing_file():
+    """Test file_exists with an existing file."""
+    # Create a temporary file
+    with tempfile.NamedTemporaryFile(delete=False) as tmp:
+        tmp_path = tmp.name
+
+    try:
+        assert file_exists(tmp_path) is True
+    finally:
+        os.unlink(tmp_path)
+
+
+def test_file_exists_with_nonexistent_file():
+    """Test file_exists with a nonexistent file."""
+    temp_path = os.path.join(tempfile.gettempdir(), "nonexistent_file_12345.json")
+    assert file_exists(temp_path) is False
+
+
+def test_file_exists_with_directory():
+    """Test file_exists with a directory (should return False)."""
+    with tempfile.TemporaryDirectory() as tmp_dir:
+        assert file_exists(tmp_dir) is False
+
+
+def test_file_exists_with_invalid_path():
+    """Test file_exists with invalid path."""
+    # Test with empty string
+    assert file_exists("") is False
+
+
+def test_is_readable_with_readable_file():
+    """Test is_readable with a readable file."""
+    # Create a temporary file
+    with tempfile.NamedTemporaryFile(delete=False, mode="w") as tmp:
+        tmp.write("test content")
+        tmp_path = tmp.name
+
+    try:
+        assert is_readable(tmp_path) is True
+    finally:
+        os.unlink(tmp_path)
+
+
+def test_is_readable_with_nonexistent_file():
+    """Test is_readable with a nonexistent file."""
+    temp_path = os.path.join(tempfile.gettempdir(), "nonexistent_file_12345.json")
+    assert is_readable(temp_path) is False
+
+
+def test_is_readable_with_empty_path():
+    """Test is_readable with empty path."""
+    assert is_readable("") is False
+
+
+def test_get_absolute_path_with_relative_path():
+    """Test get_absolute_path with a relative path."""
+    result = get_absolute_path("./test.json")
+    assert Path(result).is_absolute()
+    assert result.endswith("test.json")
+
+
+def test_get_absolute_path_with_absolute_path():
+    """Test get_absolute_path with an absolute path."""
+    result = get_absolute_path("/tmp/test.json")
+    assert result == "/tmp/test.json"
+
+
+def test_get_absolute_path_with_current_dir():
+    """Test get_absolute_path with current directory."""
+    result = get_absolute_path(".")
+    assert Path(result).is_absolute()
+
+
+def test_file_exists_with_oserror_path():
+    """Test file_exists with path that causes OSError."""
+    # Test with extremely long path that might cause OSError
+    long_path = "x" * 5000
+    assert file_exists(long_path) is False
+
+
+def test_is_readable_with_invalid_type():
+    """Test is_readable with invalid type to trigger TypeError."""
+    # Using type ignore since we're testing error handling
+    # os.access raises TypeError for None, which should be caught
+    assert is_readable(None) is False  # type: ignore[arg-type]