refactor: Skip MCP Memory leak integration test#9358
Conversation
WalkthroughThe test module src/backend/tests/integration/components/mcp/test_mcp_memory_leak.py now applies both a 300-second thread-based timeout and a skip marker at the module level, causing all tests within the module to be skipped while retaining the timeout annotation. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
github-actions
Bot
added
refactor
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/backend/tests/integration/components/mcp/test_mcp_memory_leak.py (3)
20-23: Prefer env-gated skip to allow opt-in runs; optionally addno_blockbuster.Unconditional skip blocks all runs (including local debugging). Consider an environment-gated skip so CI keeps them off by default while developers can opt-in. Adding
no_blockbusteralso avoids plugin interference when enabled.Apply this diff:
pytestmark = [ pytest.mark.timeout(300, method="thread"), - pytest.mark.skip(reason="Skipping all MCP memory leak integration tests for now."), + pytest.mark.skipif( + os.getenv("RUN_MCP_MEMORY_LEAK_TESTS") != "1", + reason="Temporarily disabled (see issue #<ISSUE_ID>). Set RUN_MCP_MEMORY_LEAK_TESTS=1 to enable.", + ), + pytest.mark.no_blockbuster, ]
20-23: Add a tracking reference in the skip reason (issue link/ID) and a TODO with re-enable criteria.Helps prevent “skip rot” and makes it clear when and how to re-enable these tests.
I can add the issue link and TODO, or open a tracking issue if one doesn’t exist. Want me to proceed?
274-415: Module-level skip also suppresses unit-like tests; consider splitting them out.Tests like
test_session_manager_server_key_generation,test_session_manager_connectivity_validation, andtest_session_manager_cleanup_alldon’t require Node/npx and provide valuable coverage. With a module-level skip, they won’t run. Consider moving these into a separate unit test module (e.g., src/backend/tests/unit/components/mcp/test_session_manager.py) so coverage is preserved while integration tests remain skipped.Please confirm if you prefer me to draft the move (and markers) in a follow-up PR.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/backend/tests/integration/components/mcp/test_mcp_memory_leak.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
{src/backend/**/*.py,tests/**/*.py,Makefile}
📄 CodeRabbit Inference Engine (.cursor/rules/backend_development.mdc)
{src/backend/**/*.py,tests/**/*.py,Makefile}: Run make format_backend to format Python code before linting or committing changes
Run make lint to perform linting checks on backend Python code
Files:
src/backend/tests/integration/components/mcp/test_mcp_memory_leak.py
src/backend/tests/**/*.py
📄 CodeRabbit Inference Engine (.cursor/rules/testing.mdc)
src/backend/tests/**/*.py: Unit tests for backend code must be located in the 'src/backend/tests/' directory, with component tests organized by component subdirectory under 'src/backend/tests/unit/components/'.
Test files should use the same filename as the component under test, with an appropriate test prefix or suffix (e.g., 'my_component.py' → 'test_my_component.py').
Use the 'client' fixture (an async httpx.AsyncClient) for API tests in backend Python tests, as defined in 'src/backend/tests/conftest.py'.
When writing component tests, inherit from the appropriate base class in 'src/backend/tests/base.py' (ComponentTestBase, ComponentTestBaseWithClient, or ComponentTestBaseWithoutClient) and provide the required fixtures: 'component_class', 'default_kwargs', and 'file_names_mapping'.
Each test in backend Python test files should have a clear docstring explaining its purpose, and complex setups or mocks should be well-commented.
Test both sync and async code paths in backend Python tests, using '@pytest.mark.asyncio' for async tests.
Mock external dependencies appropriately in backend Python tests to isolate unit tests from external services.
Test error handling and edge cases in backend Python tests, including using 'pytest.raises' and asserting error messages.
Validate input/output behavior and test component initialization and configuration in backend Python tests.
Use the 'no_blockbuster' pytest marker to skip the blockbuster plugin in tests when necessary.
Be aware of ContextVar propagation in async tests; test both direct event loop execution and 'asyncio.to_thread' scenarios to ensure proper context isolation.
Test error handling by mocking internal functions using monkeypatch in backend Python tests.
Test resource cleanup in backend Python tests by using fixtures that ensure proper initialization and cleanup of resources.
Test timeout and performance constraints in backend Python tests using 'asyncio.wait_for' and timing assertions.
Test Langflow's Messag...
Files:
src/backend/tests/integration/components/mcp/test_mcp_memory_leak.py
src/backend/**/components/**/*.py
📄 CodeRabbit Inference Engine (.cursor/rules/icons.mdc)
In your Python component class, set the
iconattribute to a string matching the frontend icon mapping exactly (case-sensitive).
Files:
src/backend/tests/integration/components/mcp/test_mcp_memory_leak.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: langflow-ai/langflow#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-21T14:16:14.125Z
Learning: Applies to src/backend/tests/**/*.py : Test timeout and performance constraints in backend Python tests using 'asyncio.wait_for' and timing assertions.
Learnt from: CR
PR: langflow-ai/langflow#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-21T14:16:14.125Z
Learning: Applies to src/backend/tests/**/*.py : Use the 'no_blockbuster' pytest marker to skip the blockbuster plugin in tests when necessary.
📚 Learning: 2025-07-21T14:16:14.125Z
Learnt from: CR
PR: langflow-ai/langflow#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-07-21T14:16:14.125Z
Learning: Applies to src/backend/tests/**/*.py : Use the 'no_blockbuster' pytest marker to skip the blockbuster plugin in tests when necessary.
Applied to files:
src/backend/tests/integration/components/mcp/test_mcp_memory_leak.py
🔇 Additional comments (1)
src/backend/tests/integration/components/mcp/test_mcp_memory_leak.py (1)
20-23: Change aligns with PR goal: module-level skip correctly disables this suite.The module-level skip is a straightforward way to temporarily disable the MCP memory leak tests and matches the stated PR objective.
Update test_mcp_memory_leak.py
Update test_mcp_memory_leak.py
3 participants
This pull request disables all MCP memory leak integration tests in
test_mcp_memory_leak.pyby adding a skip marker. This is a temporary measure to prevent these tests from running.pytest.mark.skipto thepytestmarklist intest_mcp_memory_leak.py, causing all MCP memory leak integration tests to be skipped for now.Summary by CodeRabbit