diff --git a/tests/http_layer/test_request_construction.py b/tests/http_layer/test_request_construction.py index 8ac7ad1..766fb80 100644 --- a/tests/http_layer/test_request_construction.py +++ b/tests/http_layer/test_request_construction.py @@ -19,6 +19,7 @@ from tests.conftest import make_tool_fn from unraid_mcp.core.client import DEFAULT_TIMEOUT, DISK_TIMEOUT, make_graphql_request from unraid_mcp.core.exceptions import ToolError + # --------------------------------------------------------------------------- # Shared fixtures # --------------------------------------------------------------------------- @@ -582,7 +583,7 @@ class TestVMToolRequests: return_value=_graphql_response({"vm": {"stop": True}}) ) tool = self._get_tool() - result = await tool(action="stop", vm_id="vm-456") + await tool(action="stop", vm_id="vm-456") body = _extract_request_body(route.calls.last.request) assert "StopVM" in body["query"] assert body["variables"] == {"id": "vm-456"} diff --git a/tests/safety/test_destructive_guards.py b/tests/safety/test_destructive_guards.py index 43bf230..115849f 100644 --- a/tests/safety/test_destructive_guards.py +++ b/tests/safety/test_destructive_guards.py @@ -10,6 +10,10 @@ from unittest.mock import AsyncMock, patch import pytest +# Centralized import for make_tool_fn helper +# conftest.py sits in tests/ and is importable without __init__.py +from conftest import make_tool_fn + from unraid_mcp.core.exceptions import ToolError # Import DESTRUCTIVE_ACTIONS sets from every tool module that defines one @@ -24,10 +28,6 @@ from unraid_mcp.tools.rclone import MUTATIONS as RCLONE_MUTATIONS from unraid_mcp.tools.virtualization import DESTRUCTIVE_ACTIONS as VM_DESTRUCTIVE from unraid_mcp.tools.virtualization import MUTATIONS as VM_MUTATIONS -# Centralized import for make_tool_fn helper -# conftest.py sits in tests/ and is importable without __init__.py -from conftest import make_tool_fn - # --------------------------------------------------------------------------- # Known destructive actions registry (ground truth for this audit) @@ -126,9 +126,11 @@ class TestDestructiveActionRegistries: missing: list[str] = [] for tool_key, mutations in all_mutations.items(): destructive = all_destructive[tool_key] - for action_name in mutations: - if ("delete" in action_name or "remove" in action_name) and action_name not in destructive: - missing.append(f"{tool_key}/{action_name}") + missing.extend( + f"{tool_key}/{action_name}" + for action_name in mutations + if ("delete" in action_name or "remove" in action_name) and action_name not in destructive + ) assert not missing, ( f"Mutations with 'delete'/'remove' not in DESTRUCTIVE_ACTIONS: {missing}" ) diff --git a/tests/test_array.py b/tests/test_array.py index 5f22c6a..8717d78 100644 --- a/tests/test_array.py +++ b/tests/test_array.py @@ -84,7 +84,7 @@ class TestArrayActions: async def test_generic_exception_wraps(self, _mock_graphql: AsyncMock) -> None: _mock_graphql.side_effect = RuntimeError("disk error") tool_fn = _make_tool() - with pytest.raises(ToolError, match="disk error"): + with pytest.raises(ToolError, match="Failed to execute array/parity_status"): await tool_fn(action="parity_status") diff --git a/tests/test_keys.py b/tests/test_keys.py index 3d7ab5e..2236fbe 100644 --- a/tests/test_keys.py +++ b/tests/test_keys.py @@ -100,5 +100,5 @@ class TestKeysActions: async def test_generic_exception_wraps(self, _mock_graphql: AsyncMock) -> None: _mock_graphql.side_effect = RuntimeError("connection lost") tool_fn = _make_tool() - with pytest.raises(ToolError, match="connection lost"): + with pytest.raises(ToolError, match="Failed to execute keys/list"): await tool_fn(action="list") diff --git a/tests/test_notifications.py b/tests/test_notifications.py index 946fed7..40dae42 100644 --- a/tests/test_notifications.py +++ b/tests/test_notifications.py @@ -149,7 +149,7 @@ class TestNotificationsActions: async def test_generic_exception_wraps(self, _mock_graphql: AsyncMock) -> None: _mock_graphql.side_effect = RuntimeError("boom") tool_fn = _make_tool() - with pytest.raises(ToolError, match="boom"): + with pytest.raises(ToolError, match="Failed to execute notifications/overview"): await tool_fn(action="overview") diff --git a/tests/test_storage.py b/tests/test_storage.py index eac4c36..f86e720 100644 --- a/tests/test_storage.py +++ b/tests/test_storage.py @@ -93,6 +93,14 @@ class TestStorageValidation: result = await tool_fn(action="logs", log_path="/var/log/syslog", tail_lines=10_000) assert result["content"] == "ok" + async def test_non_logs_action_ignores_tail_lines_validation( + self, _mock_graphql: AsyncMock + ) -> None: + _mock_graphql.return_value = {"shares": []} + tool_fn = _make_tool() + result = await tool_fn(action="shares", tail_lines=0) + assert result["shares"] == [] + class TestFormatKb: def test_none_returns_na(self) -> None: @@ -102,7 +110,7 @@ class TestFormatKb: assert format_kb("not-a-number") == "N/A" def test_kilobytes_range(self) -> None: - assert format_kb(512) == "512 KB" + assert format_kb(512) == "512.00 KB" def test_megabytes_range(self) -> None: assert format_kb(2048) == "2.00 MB" diff --git a/unraid_mcp/core/utils.py b/unraid_mcp/core/utils.py index 1db6dc4..5b5ec9b 100644 --- a/unraid_mcp/core/utils.py +++ b/unraid_mcp/core/utils.py @@ -13,6 +13,7 @@ def safe_get(data: dict[str, Any], *keys: str, default: Any = None) -> Any: Returns: The value at the end of the key chain, or default if unreachable. + Explicit ``None`` values at the final key also return ``default``. """ current = data for key in keys: @@ -65,4 +66,4 @@ def format_kb(k: Any) -> str: return f"{k / (1024 * 1024):.2f} GB" if k >= 1024: return f"{k / 1024:.2f} MB" - return f"{k} KB" + return f"{k:.2f} KB"