chore: add test logic for server #65
Conversation
|
Hi @ahujasid , It might be better to add Action CI to the project. |
|
Hello @ahujasid , i have added a CI commit and tested the Action using |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis update introduces a new GitHub Actions workflow for Continuous Integration that triggers on pushes and pull requests to the Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub as GitHub Actions
participant Repo as Repository
participant Runner as CI Runner
participant Codecov
GitHub->>Repo: Trigger on push/PR to main
Repo->>Runner: Checkout repository & set up environment
Runner->>Runner: Execute Test Job
Runner->>Runner: Setup Python (3.10-3.13) & cache dependencies on various OS
Runner->>Runner: Run tests using pytest
Runner->>Runner: Execute Coverage Job on Ubuntu (Python 3.13)
Runner->>Runner: Run tests with coverage tracking (pytest-cov)
Runner->>Codecov: Upload coverage report
Codecov-->>Runner: Acknowledge upload
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (9)
.github/workflows/ci.yml (2)
16-17: Consider the maturity of Python 3.13 in your CI matrix.Your CI configuration includes Python 3.13, which is currently in development/alpha stage. While it's forward-looking to test against future versions, be aware this might cause CI failures until 3.13 stabilizes.
You might want to add a comment in the workflow file explaining this is intentional for future-proofing.
86-91: Consider handling Codecov upload failures gracefully.Using
fail_ci_if_error: truewill cause CI to fail if there's an issue uploading to Codecov. This might block PRs if Codecov is experiencing downtime.Consider changing to
fail_ci_if_error: falseto make the coverage reporting non-blocking:- name: Upload coverage to Codecov uses: codecov/codecov-action@v4 with: file: ./coverage.xml - fail_ci_if_error: true + fail_ci_if_error: falsepyproject.toml (1)
8-12: Add back the license classifier for consistency.The license field is specified as "MIT", but the corresponding classifier "License :: OSI Approved :: MIT License" has been removed. This classifier helps package indexes like PyPI properly categorize your package.
Add back the license classifier:
classifiers = [ "Programming Language :: Python :: 3", "Operating System :: OS Independent", + "License :: OSI Approved :: MIT License", ]tests/test_blender_mcp_server.py (6)
1-7: Fix unused imports and optimize imports.The static analysis identified unused imports that should be removed.
import pytest import socket import json - from unittest.mock import patch, MagicMock, call, AsyncMock + from unittest.mock import patch, MagicMock from blender_mcp.server import BlenderConnection, get_blender_connection, server_lifespan, mcp import asyncio🧰 Tools
🪛 Ruff (0.8.2)
4-4:
unittest.mock.callimported but unusedRemove unused import
(F401)
4-4:
unittest.mock.AsyncMockimported but unusedRemove unused import
(F401)
20-23: Simplify boolean comparison.Replace direct comparisons to boolean literals with idiomatic Python.
# Test connection - assert blender_connection.connect() == True + assert blender_connection.connect() mock_socket.assert_called_once_with(socket.AF_INET, socket.SOCK_STREAM) mock_socket_instance.connect.assert_called_once_with(('127.0.0.1', 1234))🧰 Tools
🪛 Ruff (0.8.2)
21-21: Avoid equality comparisons to
True; useif blender_connection.connect():for truth checksReplace with
blender_connection.connect()(E712)
25-28: Simplify boolean comparison.Replace direct comparisons to boolean literals with idiomatic Python.
# Test repeated connection - assert blender_connection.connect() == True + assert blender_connection.connect() # Ensure no new socket is created mock_socket.assert_called_once()🧰 Tools
🪛 Ruff (0.8.2)
26-26: Avoid equality comparisons to
True; useif blender_connection.connect():for truth checksReplace with
blender_connection.connect()(E712)
42-44: Simplify boolean comparison.Replace direct comparisons to boolean literals with idiomatic Python.
- assert blender_connection.connect() == False + assert not blender_connection.connect() assert blender_connection.sock is None🧰 Tools
🪛 Ruff (0.8.2)
43-43: Avoid equality comparisons to
False; useif not blender_connection.connect():for false checksReplace with
not blender_connection.connect()(E712)
399-401: Simplify boolean comparison.Replace direct comparisons to boolean literals with idiomatic Python.
with patch.object(mcp, "tool", return_value=mock_tool): result = mcp.tool("get_polyhaven_status")(ctx) - assert json.loads(result)["enabled"] == True + assert json.loads(result)["enabled"]🧰 Tools
🪛 Ruff (0.8.2)
400-400: Avoid equality comparisons to
True; useif json.loads(result)["enabled"]:for truth checksReplace with
json.loads(result)["enabled"](E712)
446-448: Simplify boolean comparison.Replace direct comparisons to boolean literals with idiomatic Python.
with patch.object(mcp, "tool", return_value=mock_tool): result = mcp.tool("get_hyper3d_status")(ctx) - assert json.loads(result)["enabled"] == True + assert json.loads(result)["enabled"]🧰 Tools
🪛 Ruff (0.8.2)
447-447: Avoid equality comparisons to
True; useif json.loads(result)["enabled"]:for truth checksReplace with
json.loads(result)["enabled"](E712)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
.github/workflows/ci.yml(1 hunks)pyproject.toml(2 hunks)tests/test_blender_mcp_server.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
tests/test_blender_mcp_server.py
4-4: unittest.mock.call imported but unused
Remove unused import
(F401)
4-4: unittest.mock.AsyncMock imported but unused
Remove unused import
(F401)
21-21: Avoid equality comparisons to True; use if blender_connection.connect(): for truth checks
Replace with blender_connection.connect()
(E712)
26-26: Avoid equality comparisons to True; use if blender_connection.connect(): for truth checks
Replace with blender_connection.connect()
(E712)
43-43: Avoid equality comparisons to False; use if not blender_connection.connect(): for false checks
Replace with not blender_connection.connect()
(E712)
400-400: Avoid equality comparisons to True; use if json.loads(result)["enabled"]: for truth checks
Replace with json.loads(result)["enabled"]
(E712)
447-447: Avoid equality comparisons to True; use if json.loads(result)["enabled"]: for truth checks
Replace with json.loads(result)["enabled"]
(E712)
🔇 Additional comments (2)
pyproject.toml (1)
32-34: Good configuration for pytest asyncio support.The asyncio mode configuration is correctly set up for proper async/await testing. Setting
asyncio_mode = "strict"enforces correct async patterns, which is a best practice.tests/test_blender_mcp_server.py (1)
8-483: Excellent test coverage for the server module.The test suite is comprehensive and well-structured with:
- Thorough testing of connection lifecycle and error handling
- Proper mocking of socket interactions
- Coverage of both successful and error paths
- Testing of all tool functions
- Integration tests for external services (PolyHaven and Hyper3D)
The tests follow good practices for setup, execution, and verification.
🧰 Tools
🪛 Ruff (0.8.2)
21-21: Avoid equality comparisons to
True; useif blender_connection.connect():for truth checksReplace with
blender_connection.connect()(E712)
26-26: Avoid equality comparisons to
True; useif blender_connection.connect():for truth checksReplace with
blender_connection.connect()(E712)
43-43: Avoid equality comparisons to
False; useif not blender_connection.connect():for false checksReplace with
not blender_connection.connect()(E712)
400-400: Avoid equality comparisons to
True; useif json.loads(result)["enabled"]:for truth checksReplace with
json.loads(result)["enabled"](E712)
447-447: Avoid equality comparisons to
True; useif json.loads(result)["enabled"]:for truth checksReplace with
json.loads(result)["enabled"](E712)
|
@qodo-merge-pro /review |
|
/describe |
|
PR Description updated to latest commit (e2cb741)
|
|
/review |
|
/improve |
PR Reviewer Guide 🔍(Review updated until commit 7ff360f)Here are some key observations to aid the review process:
|
|
Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here. PR Code Suggestions ✨
|
|||||||||
|
/review |
|
Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here. Persistent review updated to latest commit 7ff360f |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (13)
tests/test_blender_mcp_server.py (13)
1-6: Remove unused imports to improve code cleanliness.There are several imports that are not used directly in this file.
import pytest import socket import json -from unittest.mock import patch, MagicMock, call, AsyncMock +from unittest.mock import patch, MagicMock from blender_mcp.server import BlenderConnection, get_blender_connection, server_lifespan, mcp -import asyncio🧰 Tools
🪛 Ruff (0.8.2)
4-4:
unittest.mock.callimported but unusedRemove unused import
(F401)
4-4:
unittest.mock.AsyncMockimported but unusedRemove unused import
(F401)
6-6:
asyncioimported but unusedRemove unused import:
asyncio(F401)
21-21: Simplify boolean comparison.Use Python's native boolean checking syntax for better readability.
- assert blender_connection.connect() == True + assert blender_connection.connect()🧰 Tools
🪛 Ruff (0.8.2)
21-21: Avoid equality comparisons to
True; useif blender_connection.connect():for truth checksReplace with
blender_connection.connect()(E712)
26-26: Simplify boolean comparison.Use Python's native boolean checking syntax for better readability.
- assert blender_connection.connect() == True + assert blender_connection.connect()🧰 Tools
🪛 Ruff (0.8.2)
26-26: Avoid equality comparisons to
True; useif blender_connection.connect():for truth checksReplace with
blender_connection.connect()(E712)
43-43: Simplify boolean comparison.Use Python's native boolean checking syntax for better readability.
- assert blender_connection.connect() == False + assert not blender_connection.connect()🧰 Tools
🪛 Ruff (0.8.2)
43-43: Avoid equality comparisons to
False; useif not blender_connection.connect():for false checksReplace with
not blender_connection.connect()(E712)
268-276: Use assert_called_once_with for more precise verification.For better test accuracy, use assert_called_once_with instead of assert_called_with to verify that the method is called exactly once with the correct parameters.
- mock_connection.send_command.assert_called_with( + mock_connection.send_command.assert_called_once_with( "create_object", { "type": "CUBE", "location": [0, 0, 0], "rotation": [0, 0, 0], "scale": [1, 1, 1] } )
301-310: Use assert_called_once_with for more precise verification.For better test accuracy, use assert_called_once_with instead of assert_called_with to verify that the method is called exactly once with the correct parameters.
- mock_connection.send_command.assert_called_with( + mock_connection.send_command.assert_called_once_with( "modify_object", { "name": "Cube", "location": [1, 1, 1], "rotation": [0, 0, 0], "scale": [2, 2, 2], "visible": True } )
328-331: Use assert_called_once_with for more precise verification.For better test accuracy, use assert_called_once_with instead of assert_called_with to verify that the method is called exactly once with the correct parameters.
- mock_connection.send_command.assert_called_with( + mock_connection.send_command.assert_called_once_with( "delete_object", {"name": "Cube"} )
354-361: Use assert_called_once_with for more precise verification.For better test accuracy, use assert_called_once_with instead of assert_called_with to verify that the method is called exactly once with the correct parameters.
- mock_connection.send_command.assert_called_with( + mock_connection.send_command.assert_called_once_with( "set_material", { "object_name": "Cube", "material_name": "Red", "color": [1, 0, 0] } )
380-383: Use assert_called_once_with for more precise verification.For better test accuracy, use assert_called_once_with instead of assert_called_with to verify that the method is called exactly once with the correct parameters.
- mock_connection.send_command.assert_called_with( + mock_connection.send_command.assert_called_once_with( "execute_code", {"code": test_code} )
399-399: Simplify boolean comparison.Use Python's native boolean checking syntax for better readability.
- assert json.loads(result)["enabled"] == True + assert json.loads(result)["enabled"]🧰 Tools
🪛 Ruff (0.8.2)
399-399: Avoid equality comparisons to
True; useif json.loads(result)["enabled"]:for truth checksReplace with
json.loads(result)["enabled"](E712)
386-431: Add verification for command arguments in PolyHaven integration tests.While the tests verify the result processing, they don't check that the correct commands and parameters are being sent to the Blender server.
For each subtest, add verification like this after the result assertions:
# Example for get_polyhaven_status mock_connection.send_command.assert_called_with("get_polyhaven_status") # Example for download_polyhaven_asset mock_connection.send_command.assert_called_with( "download_polyhaven_asset", { "asset_id": "test_asset", "asset_type": "hdris" } )🧰 Tools
🪛 Ruff (0.8.2)
399-399: Avoid equality comparisons to
True; useif json.loads(result)["enabled"]:for truth checksReplace with
json.loads(result)["enabled"](E712)
446-446: Simplify boolean comparison.Use Python's native boolean checking syntax for better readability.
- assert json.loads(result)["enabled"] == True + assert json.loads(result)["enabled"]🧰 Tools
🪛 Ruff (0.8.2)
446-446: Avoid equality comparisons to
True; useif json.loads(result)["enabled"]:for truth checksReplace with
json.loads(result)["enabled"](E712)
433-486: Add verification for command arguments in Hyper3D integration tests.Similar to the PolyHaven tests, these tests don't verify that the correct commands and parameters are being sent to the Blender server.
For each subtest, add verification like this after the result assertions:
# Example for generate_hyper3d_model_via_text mock_connection.send_command.assert_called_with( "generate_hyper3d_model_via_text", {"text_prompt": "a red cube"} ) # Example for import_generated_asset mock_connection.send_command.assert_called_with( "import_generated_asset", { "name": "generated_model", "task_uuid": "test_uuid" } )🧰 Tools
🪛 Ruff (0.8.2)
446-446: Avoid equality comparisons to
True; useif json.loads(result)["enabled"]:for truth checksReplace with
json.loads(result)["enabled"](E712)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/test_blender_mcp_server.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
tests/test_blender_mcp_server.py
4-4: unittest.mock.call imported but unused
Remove unused import
(F401)
4-4: unittest.mock.AsyncMock imported but unused
Remove unused import
(F401)
6-6: asyncio imported but unused
Remove unused import: asyncio
(F401)
21-21: Avoid equality comparisons to True; use if blender_connection.connect(): for truth checks
Replace with blender_connection.connect()
(E712)
26-26: Avoid equality comparisons to True; use if blender_connection.connect(): for truth checks
Replace with blender_connection.connect()
(E712)
43-43: Avoid equality comparisons to False; use if not blender_connection.connect(): for false checks
Replace with not blender_connection.connect()
(E712)
399-399: Avoid equality comparisons to True; use if json.loads(result)["enabled"]: for truth checks
Replace with json.loads(result)["enabled"]
(E712)
446-446: Avoid equality comparisons to True; use if json.loads(result)["enabled"]: for truth checks
Replace with json.loads(result)["enabled"]
(E712)
🔇 Additional comments (4)
tests/test_blender_mcp_server.py (4)
47-62: Well-structured test for chunked data responses.This test effectively handles the real-world scenario of receiving data in chunks, which is common in socket programming.
64-71: Good error handling test for timeout scenarios.Well-written test that verifies proper exception handling during socket timeouts.
73-83: Good test for handling incomplete JSON responses.This test properly checks that the system correctly rejects and handles malformed JSON data.
1-486: Excellent comprehensive test suite for the Blender server integration.Overall, this is a well-structured and thorough test file that covers a wide range of functionality:
- Connection management (establishing, reconnecting, disconnecting)
- Various error scenarios (timeouts, connection failures, incomplete responses)
- Command execution and response processing
- Integration with PolyHaven and Hyper3D
The tests effectively use mocking to simulate the Blender server and avoid relying on actual external connections, making them reliable and fast to run. The test coverage is comprehensive, which will help maintain the stability of the codebase as it evolves.
🧰 Tools
🪛 Ruff (0.8.2)
4-4:
unittest.mock.callimported but unusedRemove unused import
(F401)
4-4:
unittest.mock.AsyncMockimported but unusedRemove unused import
(F401)
6-6:
asyncioimported but unusedRemove unused import:
asyncio(F401)
21-21: Avoid equality comparisons to
True; useif blender_connection.connect():for truth checksReplace with
blender_connection.connect()(E712)
26-26: Avoid equality comparisons to
True; useif blender_connection.connect():for truth checksReplace with
blender_connection.connect()(E712)
43-43: Avoid equality comparisons to
False; useif not blender_connection.connect():for false checksReplace with
not blender_connection.connect()(E712)
399-399: Avoid equality comparisons to
True; useif json.loads(result)["enabled"]:for truth checksReplace with
json.loads(result)["enabled"](E712)
446-446: Avoid equality comparisons to
True; useif json.loads(result)["enabled"]:for truth checksReplace with
json.loads(result)["enabled"](E712)
|
Hi @ahujasid , could you please review it? |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/test_blender_mcp_server.py (1)
8-10: Consider parameterizing the host and port.
Theblender_connectionfixture statically sets host and port. If your test environment or CI needs different settings, you could read them from environment variables or pytest parameters. For now, it’s fine, but keep this in mind for future extensibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/test_blender_mcp_server.py(1 hunks)
🔇 Additional comments (11)
tests/test_blender_mcp_server.py (11)
1-6: Excellent use of imports and mocking utilities.
All relevant libraries (pytest,unittest.mock, and built-ins) are neatly imported. This organization aligns well with pytest's structure and shows clear separation of testing libraries.
13-33: Robust connection lifecycle test.
This test not only ensures a successful connection but also verifies no duplicate socket creation on repeated attempts and checks disconnection logic. This thorough coverage is excellent.
35-44: Effective negative test for connection failure.
Patchingconnect.side_effectto simulate refusal ensures that the code handles failures gracefully.
46-81: Comprehensive chunked data reception coverage.
The tests forreceive_full_responsehandle partial data, timeouts, and incomplete JSON. This is crucial for robust network I/O. Good job covering all these cases!
84-153: Extensive command sending tests.
You thoroughly validate success scenarios, error responses, connection issues, and timeouts. This ensures your command interface is reliable and well-guarded against edge cases.
155-178: Global connection management is well-tested.
You verify the creation of a new connection, reuse of existing ones, and appropriate error handling. This thorough approach reduces the likelihood of subtle connection bugs.
180-199: Async server lifespan test is well-structured.
Usingpytest.mark.asyncioto handle async context managers is best practice. The test properly checks that resources are cleaned up (disconnect) after the lifespan context.
201-309: Solid coverage of scene/object creation, retrieval, and modification.
Tests liketest_get_scene_info,test_get_object_info,test_create_object, andtest_modify_objectvalidate both happy-path and error scenarios. The structured approach ensures each tool function is well-verified.
312-361: Well-designed material management tests.
test_delete_objectandtest_set_materialconfirm correct commands, parameters, and response handling. This is ideal for preventing regressions in asset or material operations.
363-383: Great coverage of remote code execution flow.
test_execute_blender_codeensures that arbitrary Blender Python commands are sent correctly and that successful execution is verified. This is crucial for advanced usage.
385-494: Thorough integration testing for PolyHaven and Hyper3D.
You verify enabling statuses, searching assets, downloading, job polling, and final import. Combining real commands with mock responses ensures robust coverage of complex workflows.
|
@watonyweng hey this looks good, there has been an update to the server and addon where I have removed functions like create, modify, delete and render. Will approve once that reflects |
okay, thank you for your reply. I will keep an eye on the progress in the future. |
2 participants
User description
It is my great honor to contribute to this project. First of all, I would like to thank the author for open-sourcing this project. Secondly, I have added some test scripts to the server.
pytestdependencyuv.lockfileSummary by CodeRabbit
Chores
pytestandpytest-asyncio.Tests
PR Type
Tests, Enhancement, Configuration changes
Description
Added comprehensive test suite for
BlenderConnectionand related functions.Introduced GitHub Actions CI pipeline for multi-platform testing.
Updated
pyproject.tomlto includepytestandpytest-asyncioas dev dependencies.Enhanced project configuration for streamlined dependency management.
Changes walkthrough 📝
test_blender_mcp_server.py
Comprehensive test suite for Blender MCP servertests/test_blender_mcp_server.py
BlenderConnectionlifecycle and error handling.get_scene_info,create_object,and integrations.
ci.yml
GitHub Actions CI pipeline for testing.github/workflows/ci.yml
pyproject.toml
Update dependencies and pytest configurationpyproject.toml
pytestandpytest-asyncioas optional dev dependencies.pytestoptions for asyncio mode.