-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: make MCP package import optional to prevent errors when not installed #1096
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| """ | ||
| Test that MCP module imports are optional and don't break basic functionality. | ||
| This test verifies the fix for issue #1091. | ||
| """ | ||
| import sys | ||
| import pytest | ||
|
|
||
|
|
||
| class TestMCPOptionalImport: | ||
| """Test that MCP is properly optional.""" | ||
|
|
||
| def test_praisonaiagents_import_without_mcp(self): | ||
| """Test that praisonaiagents can be imported without mcp installed.""" | ||
| # This should not raise any ImportError | ||
| import praisonaiagents | ||
| assert praisonaiagents is not None | ||
|
|
||
| def test_agent_import_without_mcp(self): | ||
| """Test that Agent can be imported without mcp installed.""" | ||
| from praisonaiagents import Agent | ||
| assert Agent is not None | ||
|
|
||
| def test_mcp_lazy_loading(self): | ||
| """Test that MCP is lazily loaded.""" | ||
| from praisonaiagents import MCP | ||
| # MCP should either be available or None (not raise ImportError on access) | ||
| # The actual ImportError should only be raised when trying to instantiate | ||
| # if mcp package is not installed | ||
|
|
||
|
Comment on lines
+23
to
+29
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The def test_mcp_instantiation_raises_error_if_mcp_unavailable(self):
"""Test that instantiating MCP classes raises ImportError if mcp is not installed."""
from unittest.mock import patch
with patch.dict('sys.modules', {'mcp': None, 'mcp.client.stdio': None, 'mcp.client.sse': None}):
import importlib
# Test MCP class
from praisonaiagents.mcp import mcp
importlib.reload(mcp)
assert not mcp.MCP_AVAILABLE
with pytest.raises(ImportError, match="MCP .* not installed"):
mcp.MCP("some command")
importlib.reload(mcp) # Restore
# Test SSEMCPClient class
from praisonaiagents.mcp import mcp_sse
importlib.reload(mcp_sse)
assert not mcp_sse.MCP_AVAILABLE
with pytest.raises(ImportError, match="MCP .* not installed"):
mcp_sse.SSEMCPClient("http://localhost/sse")
importlib.reload(mcp_sse) # Restore
# Test HTTPStreamMCPClient class for MCP
from praisonaiagents.mcp import mcp_http_stream
importlib.reload(mcp_http_stream)
assert not mcp_http_stream.MCP_AVAILABLE
with pytest.raises(ImportError, match="MCP .* not installed"):
mcp_http_stream.HTTPStreamMCPClient("http://localhost/stream")
importlib.reload(mcp_http_stream) # Restore
def test_http_stream_instantiation_raises_error_if_aiohttp_unavailable(self):
"""Test that instantiating HTTPStreamMCPClient raises ImportError if aiohttp is not installed."""
from unittest.mock import patch
with patch.dict('sys.modules', {'aiohttp': None}):
import importlib
from praisonaiagents.mcp import mcp_http_stream
importlib.reload(mcp_http_stream)
with pytest.raises(ImportError, match="aiohttp is required"):
mcp_http_stream.HTTPStreamMCPClient("http://localhost/stream")
importlib.reload(mcp_http_stream) # Restore |
||
| def test_mcp_module_import_no_crash(self): | ||
| """Test that mcp module can be imported without crashing.""" | ||
| # This tests that the try/except in mcp.py works correctly | ||
| try: | ||
| from praisonaiagents.mcp import mcp as mcp_module | ||
| assert hasattr(mcp_module, 'MCP_AVAILABLE') | ||
| except ImportError: | ||
| # If import fails, it should be because of missing mcp, | ||
| # not because of a syntax error | ||
| pass | ||
|
|
||
| def test_mcp_sse_module_import_no_crash(self): | ||
| """Test that mcp_sse module can be imported without crashing.""" | ||
| try: | ||
| from praisonaiagents.mcp import mcp_sse | ||
| assert hasattr(mcp_sse, 'MCP_AVAILABLE') | ||
| except ImportError: | ||
| pass | ||
|
|
||
| def test_mcp_http_stream_module_import_no_crash(self): | ||
| """Test that mcp_http_stream module can be imported without crashing.""" | ||
| try: | ||
| from praisonaiagents.mcp import mcp_http_stream | ||
| assert hasattr(mcp_http_stream, 'MCP_AVAILABLE') | ||
| except ImportError: | ||
| pass | ||
|
|
||
| def test_basic_agent_creation_without_mcp_tools(self): | ||
| """Test that agents can be created without MCP tools.""" | ||
| from praisonaiagents import Agent | ||
|
|
||
| # Creating an agent without MCP tools should work | ||
| agent = Agent( | ||
| name="Test Agent", | ||
| instructions="You are a test agent.", | ||
| llm="gpt-4o-mini" | ||
| ) | ||
| assert agent is not None | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| pytest.main([__file__, "-v"]) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To properly test for missing optional dependencies by mocking imports, we need to use
patchfromunittest.mock. Please add this import.