From 2a5b19c42f78c582af6219f3fa407e3eab8a0b7b Mon Sep 17 00:00:00 2001 From: Jacob Magar Date: Thu, 19 Feb 2026 02:25:21 -0500 Subject: [PATCH] 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 --- tests/http_layer/test_request_construction.py | 2 +- tests/safety/test_destructive_guards.py | 15 +++++++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/tests/http_layer/test_request_construction.py b/tests/http_layer/test_request_construction.py index 766fb80..e588c77 100644 --- a/tests/http_layer/test_request_construction.py +++ b/tests/http_layer/test_request_construction.py @@ -195,7 +195,7 @@ class TestHttpErrorHandling: @respx.mock async def test_invalid_json_response(self) -> None: respx.post(API_URL).mock(return_value=httpx.Response(200, text="not json")) - with pytest.raises(ToolError, match="invalid response"): + with pytest.raises(ToolError, match=r"invalid response.*not valid JSON"): await make_graphql_request("query { online }") diff --git a/tests/safety/test_destructive_guards.py b/tests/safety/test_destructive_guards.py index 115849f..1512906 100644 --- a/tests/safety/test_destructive_guards.py +++ b/tests/safety/test_destructive_guards.py @@ -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 = [