modelcontextprotocol/python-sdk
Watch
1
0
Fork
You've already forked python-sdk
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

refactor: drop test classes (#1924)

Browse source
This commit is contained in:
Marcelo Trylesinski 2026-01-22 12:37:52 +01:00 • committed by GitHub
commit d77292fb06
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 1611 additions and 1562 deletions
Download patch file Download diff file Expand all files Collapse all files

105
tests/shared/test_auth.py
View file

@ -3,59 +3,58 @@
from mcp.shared.auth import OAuthMetadata
class TestOAuthMetadata:
"""Tests for OAuthMetadata parsing."""
def test_oauth():
"""Should not throw when parsing OAuth metadata."""
OAuthMetadata.model_validate(
{
"issuer": "https://example.com",
"authorization_endpoint": "https://example.com/oauth2/authorize",
"token_endpoint": "https://example.com/oauth2/token",
"scopes_supported": ["read", "write"],
"response_types_supported": ["code", "token"],
"token_endpoint_auth_methods_supported": ["client_secret_basic", "client_secret_post"],
}
)
def test_oauth(self):
"""Should not throw when parsing OAuth metadata."""
OAuthMetadata.model_validate(
{
"issuer": "https://example.com",
"authorization_endpoint": "https://example.com/oauth2/authorize",
"token_endpoint": "https://example.com/oauth2/token",
"scopes_supported": ["read", "write"],
"response_types_supported": ["code", "token"],
"token_endpoint_auth_methods_supported": ["client_secret_basic", "client_secret_post"],
}
)
def test_oidc(self):
"""Should not throw when parsing OIDC metadata."""
OAuthMetadata.model_validate(
{
"issuer": "https://example.com",
"authorization_endpoint": "https://example.com/oauth2/authorize",
"token_endpoint": "https://example.com/oauth2/token",
"end_session_endpoint": "https://example.com/logout",
"id_token_signing_alg_values_supported": ["RS256"],
"jwks_uri": "https://example.com/.well-known/jwks.json",
"response_types_supported": ["code", "token"],
"revocation_endpoint": "https://example.com/oauth2/revoke",
"scopes_supported": ["openid", "read", "write"],
"subject_types_supported": ["public"],
"token_endpoint_auth_methods_supported": ["client_secret_basic", "client_secret_post"],
"userinfo_endpoint": "https://example.com/oauth2/userInfo",
}
)
def test_oidc():
"""Should not throw when parsing OIDC metadata."""
OAuthMetadata.model_validate(
{
"issuer": "https://example.com",
"authorization_endpoint": "https://example.com/oauth2/authorize",
"token_endpoint": "https://example.com/oauth2/token",
"end_session_endpoint": "https://example.com/logout",
"id_token_signing_alg_values_supported": ["RS256"],
"jwks_uri": "https://example.com/.well-known/jwks.json",
"response_types_supported": ["code", "token"],
"revocation_endpoint": "https://example.com/oauth2/revoke",
"scopes_supported": ["openid", "read", "write"],
"subject_types_supported": ["public"],
"token_endpoint_auth_methods_supported": ["client_secret_basic", "client_secret_post"],
"userinfo_endpoint": "https://example.com/oauth2/userInfo",
}
)
def test_oauth_with_jarm(self):
"""Should not throw when parsing OAuth metadata that includes JARM response modes."""
OAuthMetadata.model_validate(
{
"issuer": "https://example.com",
"authorization_endpoint": "https://example.com/oauth2/authorize",
"token_endpoint": "https://example.com/oauth2/token",
"scopes_supported": ["read", "write"],
"response_types_supported": ["code", "token"],
"response_modes_supported": [
"query",
"fragment",
"form_post",
"query.jwt",
"fragment.jwt",
"form_post.jwt",
"jwt",
],
"token_endpoint_auth_methods_supported": ["client_secret_basic", "client_secret_post"],
}
)
def test_oauth_with_jarm():
"""Should not throw when parsing OAuth metadata that includes JARM response modes."""
OAuthMetadata.model_validate(
{
"issuer": "https://example.com",
"authorization_endpoint": "https://example.com/oauth2/authorize",
"token_endpoint": "https://example.com/oauth2/token",
"scopes_supported": ["read", "write"],
"response_types_supported": ["code", "token"],
"response_modes_supported": [
"query",
"fragment",
"form_post",
"query.jwt",
"fragment.jwt",
"form_post.jwt",
"jwt",
],
"token_endpoint_auth_methods_supported": ["client_secret_basic", "client_secret_post"],
}
)

195
tests/shared/test_auth_utils.py
View file

@ -4,109 +4,120 @@ from pydantic import HttpUrl
from mcp.shared.auth_utils import check_resource_allowed, resource_url_from_server_url
class TestResourceUrlFromServerUrl:
"""Tests for resource_url_from_server_url function."""
def test_removes_fragment(self):
"""Fragment should be removed per RFC 8707."""
assert resource_url_from_server_url("https://example.com/path#fragment") == "https://example.com/path"
assert resource_url_from_server_url("https://example.com/#fragment") == "https://example.com/"
def test_preserves_path(self):
"""Path should be preserved."""
assert (
resource_url_from_server_url("https://example.com/path/to/resource")
== "https://example.com/path/to/resource"
)
assert resource_url_from_server_url("https://example.com/") == "https://example.com/"
assert resource_url_from_server_url("https://example.com") == "https://example.com"
def test_preserves_query(self):
"""Query parameters should be preserved."""
assert resource_url_from_server_url("https://example.com/path?foo=bar") == "https://example.com/path?foo=bar"
assert resource_url_from_server_url("https://example.com/?key=value") == "https://example.com/?key=value"
def test_preserves_port(self):
"""Non-default ports should be preserved."""
assert resource_url_from_server_url("https://example.com:8443/path") == "https://example.com:8443/path"
assert resource_url_from_server_url("http://example.com:8080/") == "http://example.com:8080/"
def test_lowercase_scheme_and_host(self):
"""Scheme and host should be lowercase for canonical form."""
assert resource_url_from_server_url("HTTPS://EXAMPLE.COM/path") == "https://example.com/path"
assert resource_url_from_server_url("Http://Example.Com:8080/") == "http://example.com:8080/"
def test_handles_pydantic_urls(self):
"""Should handle Pydantic URL types."""
url = HttpUrl("https://example.com/path")
assert resource_url_from_server_url(url) == "https://example.com/path"
# Tests for resource_url_from_server_url function
class TestCheckResourceAllowed:
"""Tests for check_resource_allowed function."""
def test_resource_url_from_server_url_removes_fragment():
"""Fragment should be removed per RFC 8707."""
assert resource_url_from_server_url("https://example.com/path#fragment") == "https://example.com/path"
assert resource_url_from_server_url("https://example.com/#fragment") == "https://example.com/"
def test_identical_urls(self):
"""Identical URLs should match."""
assert check_resource_allowed("https://example.com/path", "https://example.com/path") is True
assert check_resource_allowed("https://example.com/", "https://example.com/") is True
assert check_resource_allowed("https://example.com", "https://example.com") is True
def test_different_schemes(self):
"""Different schemes should not match."""
assert check_resource_allowed("https://example.com/path", "http://example.com/path") is False
assert check_resource_allowed("http://example.com/", "https://example.com/") is False
def test_resource_url_from_server_url_preserves_path():
"""Path should be preserved."""
assert (
resource_url_from_server_url("https://example.com/path/to/resource") == "https://example.com/path/to/resource"
)
assert resource_url_from_server_url("https://example.com/") == "https://example.com/"
assert resource_url_from_server_url("https://example.com") == "https://example.com"
def test_different_domains(self):
"""Different domains should not match."""
assert check_resource_allowed("https://example.com/path", "https://example.org/path") is False
assert check_resource_allowed("https://sub.example.com/", "https://example.com/") is False
def test_different_ports(self):
"""Different ports should not match."""
assert check_resource_allowed("https://example.com:8443/path", "https://example.com/path") is False
assert check_resource_allowed("https://example.com:8080/", "https://example.com:8443/") is False
def test_resource_url_from_server_url_preserves_query():
"""Query parameters should be preserved."""
assert resource_url_from_server_url("https://example.com/path?foo=bar") == "https://example.com/path?foo=bar"
assert resource_url_from_server_url("https://example.com/?key=value") == "https://example.com/?key=value"
def test_hierarchical_matching(self):
"""Child paths should match parent paths."""
# Parent resource allows child resources
assert check_resource_allowed("https://example.com/api/v1/users", "https://example.com/api") is True
assert check_resource_allowed("https://example.com/api/v1", "https://example.com/api") is True
assert check_resource_allowed("https://example.com/mcp/server", "https://example.com/mcp") is True
# Exact match
assert check_resource_allowed("https://example.com/api", "https://example.com/api") is True
def test_resource_url_from_server_url_preserves_port():
"""Non-default ports should be preserved."""
assert resource_url_from_server_url("https://example.com:8443/path") == "https://example.com:8443/path"
assert resource_url_from_server_url("http://example.com:8080/") == "http://example.com:8080/"
# Parent cannot use child's token
assert check_resource_allowed("https://example.com/api", "https://example.com/api/v1") is False
assert check_resource_allowed("https://example.com/", "https://example.com/api") is False
def test_path_boundary_matching(self):
"""Path matching should respect boundaries."""
# Should not match partial path segments
assert check_resource_allowed("https://example.com/apiextra", "https://example.com/api") is False
assert check_resource_allowed("https://example.com/api123", "https://example.com/api") is False
def test_resource_url_from_server_url_lowercase_scheme_and_host():
"""Scheme and host should be lowercase for canonical form."""
assert resource_url_from_server_url("HTTPS://EXAMPLE.COM/path") == "https://example.com/path"
assert resource_url_from_server_url("Http://Example.Com:8080/") == "http://example.com:8080/"
# Should match with trailing slash
assert check_resource_allowed("https://example.com/api/", "https://example.com/api") is True
assert check_resource_allowed("https://example.com/api/v1", "https://example.com/api/") is True
def test_trailing_slash_handling(self):
"""Trailing slashes should be handled correctly."""
# With and without trailing slashes
assert check_resource_allowed("https://example.com/api/", "https://example.com/api") is True
assert check_resource_allowed("https://example.com/api", "https://example.com/api/") is False
assert check_resource_allowed("https://example.com/api/v1", "https://example.com/api") is True
assert check_resource_allowed("https://example.com/api/v1", "https://example.com/api/") is True
def test_resource_url_from_server_url_handles_pydantic_urls():
"""Should handle Pydantic URL types."""
url = HttpUrl("https://example.com/path")
assert resource_url_from_server_url(url) == "https://example.com/path"
def test_case_insensitive_origin(self):
"""Origin comparison should be case-insensitive."""
assert check_resource_allowed("https://EXAMPLE.COM/path", "https://example.com/path") is True
assert check_resource_allowed("HTTPS://example.com/path", "https://example.com/path") is True
assert check_resource_allowed("https://Example.Com:8080/api", "https://example.com:8080/api") is True
def test_empty_paths(self):
"""Empty paths should be handled correctly."""
assert check_resource_allowed("https://example.com", "https://example.com") is True
assert check_resource_allowed("https://example.com/", "https://example.com") is True
assert check_resource_allowed("https://example.com/api", "https://example.com") is True
# Tests for check_resource_allowed function
def test_check_resource_allowed_identical_urls():
"""Identical URLs should match."""
assert check_resource_allowed("https://example.com/path", "https://example.com/path") is True
assert check_resource_allowed("https://example.com/", "https://example.com/") is True
assert check_resource_allowed("https://example.com", "https://example.com") is True
def test_check_resource_allowed_different_schemes():
"""Different schemes should not match."""
assert check_resource_allowed("https://example.com/path", "http://example.com/path") is False
assert check_resource_allowed("http://example.com/", "https://example.com/") is False
def test_check_resource_allowed_different_domains():
"""Different domains should not match."""
assert check_resource_allowed("https://example.com/path", "https://example.org/path") is False
assert check_resource_allowed("https://sub.example.com/", "https://example.com/") is False
def test_check_resource_allowed_different_ports():
"""Different ports should not match."""
assert check_resource_allowed("https://example.com:8443/path", "https://example.com/path") is False
assert check_resource_allowed("https://example.com:8080/", "https://example.com:8443/") is False
def test_check_resource_allowed_hierarchical_matching():
"""Child paths should match parent paths."""
# Parent resource allows child resources
assert check_resource_allowed("https://example.com/api/v1/users", "https://example.com/api") is True
assert check_resource_allowed("https://example.com/api/v1", "https://example.com/api") is True
assert check_resource_allowed("https://example.com/mcp/server", "https://example.com/mcp") is True
# Exact match
assert check_resource_allowed("https://example.com/api", "https://example.com/api") is True
# Parent cannot use child's token
assert check_resource_allowed("https://example.com/api", "https://example.com/api/v1") is False
assert check_resource_allowed("https://example.com/", "https://example.com/api") is False
def test_check_resource_allowed_path_boundary_matching():
"""Path matching should respect boundaries."""
# Should not match partial path segments
assert check_resource_allowed("https://example.com/apiextra", "https://example.com/api") is False
assert check_resource_allowed("https://example.com/api123", "https://example.com/api") is False
# Should match with trailing slash
assert check_resource_allowed("https://example.com/api/", "https://example.com/api") is True
assert check_resource_allowed("https://example.com/api/v1", "https://example.com/api/") is True
def test_check_resource_allowed_trailing_slash_handling():
"""Trailing slashes should be handled correctly."""
# With and without trailing slashes
assert check_resource_allowed("https://example.com/api/", "https://example.com/api") is True
assert check_resource_allowed("https://example.com/api", "https://example.com/api/") is False
assert check_resource_allowed("https://example.com/api/v1", "https://example.com/api") is True
assert check_resource_allowed("https://example.com/api/v1", "https://example.com/api/") is True
def test_check_resource_allowed_case_insensitive_origin():
"""Origin comparison should be case-insensitive."""
assert check_resource_allowed("https://EXAMPLE.COM/path", "https://example.com/path") is True
assert check_resource_allowed("HTTPS://example.com/path", "https://example.com/path") is True
assert check_resource_allowed("https://Example.Com:8080/api", "https://example.com:8080/api") is True
def test_check_resource_allowed_empty_paths():
"""Empty paths should be handled correctly."""
assert check_resource_allowed("https://example.com", "https://example.com") is True
assert check_resource_allowed("https://example.com/", "https://example.com") is True
assert check_resource_allowed("https://example.com/api", "https://example.com") is True

269
tests/shared/test_exceptions.py
View file

@ -6,154 +6,159 @@ from mcp.shared.exceptions import McpError, UrlElicitationRequiredError
from mcp.types import URL_ELICITATION_REQUIRED, ElicitRequestURLParams, ErrorData
class TestUrlElicitationRequiredError:
"""Tests for UrlElicitationRequiredError exception class."""
def test_url_elicitation_required_error_create_with_single_elicitation() -> None:
"""Test creating error with a single elicitation."""
elicitation = ElicitRequestURLParams(
mode="url",
message="Auth required",
url="https://example.com/auth",
elicitation_id="test-123",
)
error = UrlElicitationRequiredError([elicitation])
def test_create_with_single_elicitation(self) -> None:
"""Test creating error with a single elicitation."""
elicitation = ElicitRequestURLParams(
assert error.error.code == URL_ELICITATION_REQUIRED
assert error.error.message == "URL elicitation required"
assert len(error.elicitations) == 1
assert error.elicitations[0].elicitation_id == "test-123"
def test_url_elicitation_required_error_create_with_multiple_elicitations() -> None:
"""Test creating error with multiple elicitations uses plural message."""
elicitations = [
ElicitRequestURLParams(
mode="url",
message="Auth required",
url="https://example.com/auth",
elicitation_id="test-123",
)
error = UrlElicitationRequiredError([elicitation])
assert error.error.code == URL_ELICITATION_REQUIRED
assert error.error.message == "URL elicitation required"
assert len(error.elicitations) == 1
assert error.elicitations[0].elicitation_id == "test-123"
def test_create_with_multiple_elicitations(self) -> None:
"""Test creating error with multiple elicitations uses plural message."""
elicitations = [
ElicitRequestURLParams(
mode="url",
message="Auth 1",
url="https://example.com/auth1",
elicitation_id="test-1",
),
ElicitRequestURLParams(
mode="url",
message="Auth 2",
url="https://example.com/auth2",
elicitation_id="test-2",
),
]
error = UrlElicitationRequiredError(elicitations)
assert error.error.message == "URL elicitations required" # Plural
assert len(error.elicitations) == 2
def test_custom_message(self) -> None:
"""Test creating error with a custom message."""
elicitation = ElicitRequestURLParams(
message="Auth 1",
url="https://example.com/auth1",
elicitation_id="test-1",
),
ElicitRequestURLParams(
mode="url",
message="Auth required",
url="https://example.com/auth",
elicitation_id="test-123",
)
error = UrlElicitationRequiredError([elicitation], message="Custom message")
message="Auth 2",
url="https://example.com/auth2",
elicitation_id="test-2",
),
]
error = UrlElicitationRequiredError(elicitations)
assert error.error.message == "Custom message"
assert error.error.message == "URL elicitations required" # Plural
assert len(error.elicitations) == 2
def test_from_error_data(self) -> None:
"""Test reconstructing error from ErrorData."""
error_data = ErrorData(
code=URL_ELICITATION_REQUIRED,
message="URL elicitation required",
data={
"elicitations": [
{
"mode": "url",
"message": "Auth required",
"url": "https://example.com/auth",
"elicitationId": "test-123",
}
]
},
)
error = UrlElicitationRequiredError.from_error(error_data)
def test_url_elicitation_required_error_custom_message() -> None:
"""Test creating error with a custom message."""
elicitation = ElicitRequestURLParams(
mode="url",
message="Auth required",
url="https://example.com/auth",
elicitation_id="test-123",
)
error = UrlElicitationRequiredError([elicitation], message="Custom message")
assert len(error.elicitations) == 1
assert error.elicitations[0].elicitation_id == "test-123"
assert error.elicitations[0].url == "https://example.com/auth"
assert error.error.message == "Custom message"
def test_from_error_data_wrong_code(self) -> None:
"""Test that from_error raises ValueError for wrong error code."""
error_data = ErrorData(
code=-32600, # Wrong code
message="Some other error",
data={},
)
with pytest.raises(ValueError, match="Expected error code"):
UrlElicitationRequiredError.from_error(error_data)
def test_serialization_roundtrip(self) -> None:
"""Test that error can be serialized and reconstructed."""
original = UrlElicitationRequiredError(
[
ElicitRequestURLParams(
mode="url",
message="Auth required",
url="https://example.com/auth",
elicitation_id="test-123",
)
def test_url_elicitation_required_error_from_error_data() -> None:
"""Test reconstructing error from ErrorData."""
error_data = ErrorData(
code=URL_ELICITATION_REQUIRED,
message="URL elicitation required",
data={
"elicitations": [
{
"mode": "url",
"message": "Auth required",
"url": "https://example.com/auth",
"elicitationId": "test-123",
}
]
)
},
)
# Simulate serialization over wire
error_data = original.error
error = UrlElicitationRequiredError.from_error(error_data)
# Reconstruct
reconstructed = UrlElicitationRequiredError.from_error(error_data)
assert len(error.elicitations) == 1
assert error.elicitations[0].elicitation_id == "test-123"
assert error.elicitations[0].url == "https://example.com/auth"
assert reconstructed.elicitations[0].elicitation_id == original.elicitations[0].elicitation_id
assert reconstructed.elicitations[0].url == original.elicitations[0].url
assert reconstructed.elicitations[0].message == original.elicitations[0].message
def test_error_data_contains_elicitations(self) -> None:
"""Test that error data contains properly serialized elicitations."""
elicitation = ElicitRequestURLParams(
mode="url",
message="Please authenticate",
url="https://example.com/oauth",
elicitation_id="oauth-flow-1",
)
error = UrlElicitationRequiredError([elicitation])
def test_url_elicitation_required_error_from_error_data_wrong_code() -> None:
"""Test that from_error raises ValueError for wrong error code."""
error_data = ErrorData(
code=-32600, # Wrong code
message="Some other error",
data={},
)
assert error.error.data is not None
assert "elicitations" in error.error.data
elicit_data = error.error.data["elicitations"][0]
assert elicit_data["mode"] == "url"
assert elicit_data["message"] == "Please authenticate"
assert elicit_data["url"] == "https://example.com/oauth"
assert elicit_data["elicitationId"] == "oauth-flow-1"
with pytest.raises(ValueError, match="Expected error code"):
UrlElicitationRequiredError.from_error(error_data)
def test_inherits_from_mcp_error(self) -> None:
"""Test that UrlElicitationRequiredError inherits from McpError."""
elicitation = ElicitRequestURLParams(
mode="url",
message="Auth required",
url="https://example.com/auth",
elicitation_id="test-123",
)
error = UrlElicitationRequiredError([elicitation])
assert isinstance(error, McpError)
assert isinstance(error, Exception)
def test_url_elicitation_required_error_serialization_roundtrip() -> None:
"""Test that error can be serialized and reconstructed."""
original = UrlElicitationRequiredError(
[
ElicitRequestURLParams(
mode="url",
message="Auth required",
url="https://example.com/auth",
elicitation_id="test-123",
)
]
)
def test_exception_message(self) -> None:
"""Test that exception message is set correctly."""
elicitation = ElicitRequestURLParams(
mode="url",
message="Auth required",
url="https://example.com/auth",
elicitation_id="test-123",
)
error = UrlElicitationRequiredError([elicitation])
# Simulate serialization over wire
error_data = original.error
# The exception's string representation should match the message
assert str(error) == "URL elicitation required"
# Reconstruct
reconstructed = UrlElicitationRequiredError.from_error(error_data)
assert reconstructed.elicitations[0].elicitation_id == original.elicitations[0].elicitation_id
assert reconstructed.elicitations[0].url == original.elicitations[0].url
assert reconstructed.elicitations[0].message == original.elicitations[0].message
def test_url_elicitation_required_error_data_contains_elicitations() -> None:
"""Test that error data contains properly serialized elicitations."""
elicitation = ElicitRequestURLParams(
mode="url",
message="Please authenticate",
url="https://example.com/oauth",
elicitation_id="oauth-flow-1",
)
error = UrlElicitationRequiredError([elicitation])
assert error.error.data is not None
assert "elicitations" in error.error.data
elicit_data = error.error.data["elicitations"][0]
assert elicit_data["mode"] == "url"
assert elicit_data["message"] == "Please authenticate"
assert elicit_data["url"] == "https://example.com/oauth"
assert elicit_data["elicitationId"] == "oauth-flow-1"
def test_url_elicitation_required_error_inherits_from_mcp_error() -> None:
"""Test that UrlElicitationRequiredError inherits from McpError."""
elicitation = ElicitRequestURLParams(
mode="url",
message="Auth required",
url="https://example.com/auth",
elicitation_id="test-123",
)
error = UrlElicitationRequiredError([elicitation])
assert isinstance(error, McpError)
assert isinstance(error, Exception)
def test_url_elicitation_required_error_exception_message() -> None:
"""Test that exception message is set correctly."""
elicitation = ElicitRequestURLParams(
mode="url",
message="Auth required",
url="https://example.com/auth",
elicitation_id="test-123",
)
error = UrlElicitationRequiredError([elicitation])
# The exception's string representation should match the message
assert str(error) == "URL elicitation required"

369
tests/shared/test_tool_name_validation.py
View file

@ -10,190 +10,199 @@ from mcp.shared.tool_name_validation import (
validate_tool_name,
)
class TestValidateToolName:
"""Tests for validate_tool_name function."""
class TestValidNames:
"""Test cases for valid tool names."""
@pytest.mark.parametrize(
"tool_name",
[
"getUser",
"get_user_profile",
"user-profile-update",
"admin.tools.list",
"DATA_EXPORT_v2.1",
"a",
"a" * 128,
],
ids=[
"simple_alphanumeric",
"with_underscores",
"with_dashes",
"with_dots",
"mixed_characters",
"single_character",
"max_length_128",
],
)
def test_accepts_valid_names(self, tool_name: str) -> None:
"""Valid tool names should pass validation with no warnings."""
result = validate_tool_name(tool_name)
assert result.is_valid is True
assert result.warnings == []
class TestInvalidNames:
"""Test cases for invalid tool names."""
def test_rejects_empty_name(self) -> None:
"""Empty names should be rejected."""
result = validate_tool_name("")
assert result.is_valid is False
assert "Tool name cannot be empty" in result.warnings
def test_rejects_name_exceeding_max_length(self) -> None:
"""Names exceeding 128 characters should be rejected."""
result = validate_tool_name("a" * 129)
assert result.is_valid is False
assert any("exceeds maximum length of 128 characters (current: 129)" in w for w in result.warnings)
@pytest.mark.parametrize(
"tool_name,expected_char",
[
("get user profile", "' '"),
("get,user,profile", "','"),
("user/profile/update", "'/'"),
("user@domain.com", "'@'"),
],
ids=[
"with_spaces",
"with_commas",
"with_slashes",
"with_at_symbol",
],
)
def test_rejects_invalid_characters(self, tool_name: str, expected_char: str) -> None:
"""Names with invalid characters should be rejected."""
result = validate_tool_name(tool_name)
assert result.is_valid is False
assert any("invalid characters" in w and expected_char in w for w in result.warnings)
def test_rejects_multiple_invalid_chars(self) -> None:
"""Names with multiple invalid chars should list all of them."""
result = validate_tool_name("user name@domain,com")
assert result.is_valid is False
warning = next(w for w in result.warnings if "invalid characters" in w)
assert "' '" in warning
assert "'@'" in warning
assert "','" in warning
def test_rejects_unicode_characters(self) -> None:
"""Names with unicode characters should be rejected."""
result = validate_tool_name("user-\u00f1ame") # n with tilde
assert result.is_valid is False
class TestWarningsForProblematicPatterns:
"""Test cases for valid names that generate warnings."""
def test_warns_on_leading_dash(self) -> None:
"""Names starting with dash should generate warning but be valid."""
result = validate_tool_name("-get-user")
assert result.is_valid is True
assert any("starts or ends with a dash" in w for w in result.warnings)
def test_warns_on_trailing_dash(self) -> None:
"""Names ending with dash should generate warning but be valid."""
result = validate_tool_name("get-user-")
assert result.is_valid is True
assert any("starts or ends with a dash" in w for w in result.warnings)
def test_warns_on_leading_dot(self) -> None:
"""Names starting with dot should generate warning but be valid."""
result = validate_tool_name(".get.user")
assert result.is_valid is True
assert any("starts or ends with a dot" in w for w in result.warnings)
def test_warns_on_trailing_dot(self) -> None:
"""Names ending with dot should generate warning but be valid."""
result = validate_tool_name("get.user.")
assert result.is_valid is True
assert any("starts or ends with a dot" in w for w in result.warnings)
# Tests for validate_tool_name function - valid names
class TestIssueToolNameWarning:
"""Tests for issue_tool_name_warning function."""
def test_logs_warnings(self, caplog: pytest.LogCaptureFixture) -> None:
"""Warnings should be logged at WARNING level."""
warnings = ["Warning 1", "Warning 2"]
with caplog.at_level(logging.WARNING):
issue_tool_name_warning("test-tool", warnings)
assert 'Tool name validation warning for "test-tool"' in caplog.text
assert "- Warning 1" in caplog.text
assert "- Warning 2" in caplog.text
assert "Tool registration will proceed" in caplog.text
assert "SEP-986" in caplog.text
def test_no_logging_for_empty_warnings(self, caplog: pytest.LogCaptureFixture) -> None:
"""Empty warnings list should not produce any log output."""
with caplog.at_level(logging.WARNING):
issue_tool_name_warning("test-tool", [])
assert caplog.text == ""
@pytest.mark.parametrize(
"tool_name",
[
"getUser",
"get_user_profile",
"user-profile-update",
"admin.tools.list",
"DATA_EXPORT_v2.1",
"a",
"a" * 128,
],
ids=[
"simple_alphanumeric",
"with_underscores",
"with_dashes",
"with_dots",
"mixed_characters",
"single_character",
"max_length_128",
],
)
def test_validate_tool_name_accepts_valid_names(tool_name: str) -> None:
"""Valid tool names should pass validation with no warnings."""
result = validate_tool_name(tool_name)
assert result.is_valid is True
assert result.warnings == []
class TestValidateAndWarnToolName:
"""Tests for validate_and_warn_tool_name function."""
def test_returns_true_for_valid_name(self) -> None:
"""Valid names should return True."""
assert validate_and_warn_tool_name("valid-tool-name") is True
def test_returns_false_for_invalid_name(self) -> None:
"""Invalid names should return False."""
assert validate_and_warn_tool_name("") is False
assert validate_and_warn_tool_name("a" * 129) is False
assert validate_and_warn_tool_name("invalid name") is False
def test_logs_warnings_for_invalid_name(self, caplog: pytest.LogCaptureFixture) -> None:
"""Invalid names should trigger warning logs."""
with caplog.at_level(logging.WARNING):
validate_and_warn_tool_name("invalid name")
assert "Tool name validation warning" in caplog.text
def test_no_warnings_for_clean_valid_name(self, caplog: pytest.LogCaptureFixture) -> None:
"""Clean valid names should not produce any log output."""
with caplog.at_level(logging.WARNING):
result = validate_and_warn_tool_name("clean-tool-name")
assert result is True
assert caplog.text == ""
# Tests for validate_tool_name function - invalid names
class TestEdgeCases:
"""Test edge cases and robustness."""
def test_validate_tool_name_rejects_empty_name() -> None:
"""Empty names should be rejected."""
result = validate_tool_name("")
assert result.is_valid is False
assert "Tool name cannot be empty" in result.warnings
@pytest.mark.parametrize(
"tool_name,is_valid,expected_warning_fragment",
[
("...", True, "starts or ends with a dot"),
("---", True, "starts or ends with a dash"),
("///", False, "invalid characters"),
("user@name123", False, "invalid characters"),
],
ids=[
"only_dots",
"only_dashes",
"only_slashes",
"mixed_valid_invalid",
],
)
def test_edge_cases(self, tool_name: str, is_valid: bool, expected_warning_fragment: str) -> None:
"""Various edge cases should be handled correctly."""
result = validate_tool_name(tool_name)
assert result.is_valid is is_valid
assert any(expected_warning_fragment in w for w in result.warnings)
def test_validate_tool_name_rejects_name_exceeding_max_length() -> None:
"""Names exceeding 128 characters should be rejected."""
result = validate_tool_name("a" * 129)
assert result.is_valid is False
assert any("exceeds maximum length of 128 characters (current: 129)" in w for w in result.warnings)
@pytest.mark.parametrize(
"tool_name,expected_char",
[
("get user profile", "' '"),
("get,user,profile", "','"),
("user/profile/update", "'/'"),
("user@domain.com", "'@'"),
],
ids=[
"with_spaces",
"with_commas",
"with_slashes",
"with_at_symbol",
],
)
def test_validate_tool_name_rejects_invalid_characters(tool_name: str, expected_char: str) -> None:
"""Names with invalid characters should be rejected."""
result = validate_tool_name(tool_name)
assert result.is_valid is False
assert any("invalid characters" in w and expected_char in w for w in result.warnings)
def test_validate_tool_name_rejects_multiple_invalid_chars() -> None:
"""Names with multiple invalid chars should list all of them."""
result = validate_tool_name("user name@domain,com")
assert result.is_valid is False
warning = next(w for w in result.warnings if "invalid characters" in w)
assert "' '" in warning
assert "'@'" in warning
assert "','" in warning
def test_validate_tool_name_rejects_unicode_characters() -> None:
"""Names with unicode characters should be rejected."""
result = validate_tool_name("user-\u00f1ame") # n with tilde
assert result.is_valid is False
# Tests for validate_tool_name function - warnings for problematic patterns
def test_validate_tool_name_warns_on_leading_dash() -> None:
"""Names starting with dash should generate warning but be valid."""
result = validate_tool_name("-get-user")
assert result.is_valid is True
assert any("starts or ends with a dash" in w for w in result.warnings)
def test_validate_tool_name_warns_on_trailing_dash() -> None:
"""Names ending with dash should generate warning but be valid."""
result = validate_tool_name("get-user-")
assert result.is_valid is True
assert any("starts or ends with a dash" in w for w in result.warnings)
def test_validate_tool_name_warns_on_leading_dot() -> None:
"""Names starting with dot should generate warning but be valid."""
result = validate_tool_name(".get.user")
assert result.is_valid is True
assert any("starts or ends with a dot" in w for w in result.warnings)
def test_validate_tool_name_warns_on_trailing_dot() -> None:
"""Names ending with dot should generate warning but be valid."""
result = validate_tool_name("get.user.")
assert result.is_valid is True
assert any("starts or ends with a dot" in w for w in result.warnings)
# Tests for issue_tool_name_warning function
def test_issue_tool_name_warning_logs_warnings(caplog: pytest.LogCaptureFixture) -> None:
"""Warnings should be logged at WARNING level."""
warnings = ["Warning 1", "Warning 2"]
with caplog.at_level(logging.WARNING):
issue_tool_name_warning("test-tool", warnings)
assert 'Tool name validation warning for "test-tool"' in caplog.text
assert "- Warning 1" in caplog.text
assert "- Warning 2" in caplog.text
assert "Tool registration will proceed" in caplog.text
assert "SEP-986" in caplog.text
def test_issue_tool_name_warning_no_logging_for_empty_warnings(caplog: pytest.LogCaptureFixture) -> None:
"""Empty warnings list should not produce any log output."""
with caplog.at_level(logging.WARNING):
issue_tool_name_warning("test-tool", [])
assert caplog.text == ""
# Tests for validate_and_warn_tool_name function
def test_validate_and_warn_tool_name_returns_true_for_valid_name() -> None:
"""Valid names should return True."""
assert validate_and_warn_tool_name("valid-tool-name") is True
def test_validate_and_warn_tool_name_returns_false_for_invalid_name() -> None:
"""Invalid names should return False."""
assert validate_and_warn_tool_name("") is False
assert validate_and_warn_tool_name("a" * 129) is False
assert validate_and_warn_tool_name("invalid name") is False
def test_validate_and_warn_tool_name_logs_warnings_for_invalid_name(caplog: pytest.LogCaptureFixture) -> None:
"""Invalid names should trigger warning logs."""
with caplog.at_level(logging.WARNING):
validate_and_warn_tool_name("invalid name")
assert "Tool name validation warning" in caplog.text
def test_validate_and_warn_tool_name_no_warnings_for_clean_valid_name(caplog: pytest.LogCaptureFixture) -> None:
"""Clean valid names should not produce any log output."""
with caplog.at_level(logging.WARNING):
result = validate_and_warn_tool_name("clean-tool-name")
assert result is True
assert caplog.text == ""
# Tests for edge cases
@pytest.mark.parametrize(
"tool_name,is_valid,expected_warning_fragment",
[
("...", True, "starts or ends with a dot"),
("---", True, "starts or ends with a dash"),
("///", False, "invalid characters"),
("user@name123", False, "invalid characters"),
],
ids=[
"only_dots",
"only_dashes",
"only_slashes",
"mixed_valid_invalid",
],
)
def test_edge_cases(tool_name: str, is_valid: bool, expected_warning_fragment: str) -> None:
"""Various edge cases should be handled correctly."""
result = validate_tool_name(tool_name)
assert result.is_valid is is_valid
assert any(expected_warning_fragment in w for w in result.warnings)