test: address final 3 CodeRabbit review threads

- http_layer/test_request_construction.py: tighten JSON error match from
  "invalid response" to "invalid response.*not valid JSON" to prevent
  false positives
- safety/test_destructive_guards.py: add test_docker_update_all_with_confirm
  to TestConfirmAllowsExecution (was missing positive coverage for update_all)
- safety/test_destructive_guards.py: expand conftest import comment to explain
  why the direct conftest import is intentional and correct
This commit is contained in:
Jacob Magar
2026-02-19 02:25:21 -05:00
parent 1751bc2984
commit 2a5b19c42f
2 changed files with 14 additions and 3 deletions

View File

@@ -10,8 +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
# conftest.py is the shared test-helper module for this project.
# pytest automatically adds tests/ to sys.path, making it importable here
# without a package __init__.py. Do NOT add tests/__init__.py — it breaks
# conftest.py's fixture auto-discovery.
from conftest import make_tool_fn
from unraid_mcp.core.exceptions import ToolError
@@ -271,6 +273,15 @@ class TestConfirmationGuards:
class TestConfirmAllowsExecution:
"""Destructive actions with confirm=True should reach the GraphQL layer."""
async def test_docker_update_all_with_confirm(self, _mock_docker_graphql: AsyncMock) -> None:
_mock_docker_graphql.return_value = {
"docker": {"updateAllContainers": [{"id": "c1", "names": ["app"], "state": "running", "status": "Up"}]}
}
tool_fn = make_tool_fn("unraid_mcp.tools.docker", "register_docker_tool", "unraid_docker")
result = await tool_fn(action="update_all", confirm=True)
assert result["success"] is True
assert result["action"] == "update_all"
async def test_docker_remove_with_confirm(self, _mock_docker_graphql: AsyncMock) -> None:
cid = "a" * 64 + ":local"
_mock_docker_graphql.side_effect = [