Skip to content

fix: Fix n8n integration test mocking issues #21

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.

Sign up for GitHub

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

Merged
MervinPraison merged 1 commit into from
Apr 16, 2026
Merged

fix: Fix n8n integration test mocking issues #21

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions tests/test_n8n_integration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,22 @@ class TestN8nWorkflowTool:

@pytest.fixture
def mock_httpx(self):
# Mock httpx in sys.modules to catch lazy imports
with patch.dict('sys.modules', {'httpx': MagicMock()}) as mock_modules:
mock_httpx = mock_modules['httpx']
yield mock_httpx
"""Mock httpx module for testing."""
mock_module = MagicMock()

# Create proper exception classes
mock_module.TimeoutException = type('TimeoutException', (Exception,), {})

class MockHTTPStatusError(Exception):
def __init__(self, message, request=None, response=None):
super().__init__(message)
self.request = request
self.response = response

mock_module.HTTPStatusError = MockHTTPStatusError

with patch.dict('sys.modules', {'httpx': mock_module}) as mock_modules:
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

patch.dict('sys.modules', {'httpx': mock_module}) as mock_modules binds mock_modules but it’s never used, which adds noise (and can trip linters). Drop the as mock_modules binding (or use it) to keep the fixture minimal.

Suggested change
with patch.dict('sys.modules', {'httpx': mock_module}) as mock_modules:
with patch.dict('sys.modules', {'httpx': mock_module}):

Copilot uses AI. Check for mistakes.
yield mock_module

def test_import_n8n_tools(self):
"""Test that n8n tools can be imported."""
Expand Down Expand Up @@ -69,7 +81,8 @@ def test_n8n_workflow_missing_httpx(self):
from praisonai_tools.n8n import N8nWorkflowTool

tool = N8nWorkflowTool()
with patch('praisonai_tools.n8n.n8n_workflow.httpx', None):
# Mock the import failure using sys.modules approach
with patch.dict('sys.modules', {'httpx': None}):
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The patch on builtins.__import__ (line 86) is redundant when sys.modules['httpx'] is set to None. Setting a module to None in sys.modules is the standard way to simulate a missing module in Python and will cause an ImportError immediately upon an import httpx statement. Removing the redundant patch simplifies the test and avoids the potential side effects of patching a core builtin like __import__.

with patch('builtins.__import__', side_effect=ImportError("No module named 'httpx'")):
result = tool.run(workflow_id="test-workflow")
Comment on lines +85 to 87
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patching builtins.__import__ with a blanket ImportError makes this test overly broad: if tool.run() (or anything it calls) adds any other import, the test will still pass even though the failure wasn’t specifically due to missing httpx. Prefer relying on the sys.modules['httpx']=None behavior alone, or make the __import__ side effect conditional so it only fails when importing httpx.

Copilot uses AI. Check for mistakes.
assert "httpx not installed" in result["error"]
Expand Down
Loading