From 0c7e1cb04e20774e6626797296bb88fce8456c2c Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Tue, 18 Aug 2026 15:33:58 +0200 Subject: [PATCH] fix(tests): mock the live DNS service instance, not a dead legacy class (T-55) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test_dns_lookup_returns_result patched src.controllers.tools_controller. DNSService, which is never imported by the request path under test. client wraps src.main.app, which routes /tools/dns/lookup through src.domains.tools.controller.tools_controller — a singleton constructed at import time from src.domains.tools.dns.service.DNSService. The patched class was dead; the mock was never consulted, so the test issued a real DNS query for example.com and asserted on its outcome. With no network the query times out and the assertion fails (D-26). The three sibling tests in the same class patch the same dead class and also run unmocked, but happen not to notice: DNS failures are caught inside DNSService.lookup() and returned as a normal 200 response with success=False, and their assertions only check status_code / DNSQueryError branches that don't depend on resolution actually succeeding. Only this test's `data["success"] is True` assertion is sensitive to the real network outcome, which is why it's the only one D-26's namespace run catches. Not touched here — out of T-55's scope, flagging for the record. Fix patches tools_controller.dns_service, the actual instance attribute the live route calls, via patch.object on the singleton rather than patch() on the constructor class (the instance already exists by the time a class-level patch would apply). Verified: - unshare -rn (lo up): 381 passed, exit 0 (was 1 failed, 380 passed, exit 2) - with network: 381 passed, exit 0 (unchanged from before the fix) - mutation check: retargeted the patch.object to a nonexistent attribute name, confirmed count==1 before editing; namespace run then reproduced the original failure (1 failed, 380 passed); reverted and reconfirmed 381 passed, exit 0. Co-Authored-By: Claude --- tests/test_tools_controller.py | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/tests/test_tools_controller.py b/tests/test_tools_controller.py index a313e1c..806a0b3 100644 --- a/tests/test_tools_controller.py +++ b/tests/test_tools_controller.py @@ -4,6 +4,7 @@ from fastapi.testclient import TestClient from unittest.mock import patch, AsyncMock, MagicMock from src.main import app +from src.domains.tools.controller import tools_controller @pytest.fixture @@ -36,9 +37,21 @@ class TestDNSLookup: ) assert response.status_code == 200 - @patch("src.controllers.tools_controller.DNSService") - def test_dns_lookup_returns_result(self, mock_dns_class, client): - """DNS lookup should return lookup results.""" + def test_dns_lookup_returns_result(self, client): + """DNS lookup should return lookup results. + + Patches the live singleton's `dns_service` attribute, not the + `src.controllers.tools_controller.DNSService` class: that module is + the legacy top-level package (not wired into `src.main`, see + CLAUDE.md "Legacy top-level packages"). `client` exercises + `src.main.app`, which routes through + `src.domains.tools.controller.tools_controller`, a singleton built + at import time — so patching the class there would also miss, + since `tools_controller.dns_service` is already a constructed + instance by the time a test patches the class. Patching the + instance attribute directly is the only patch that actually + intercepts this request path. + """ mock_response = MagicMock() mock_response.success = True mock_response.domain = "example.com" @@ -59,12 +72,12 @@ class TestDNSLookup: mock_service = MagicMock() mock_service.lookup = AsyncMock(return_value=mock_response) - mock_dns_class.return_value = mock_service - response = client.post( - "/tools/dns/lookup", - json={"domain": "example.com", "record_type": "A"} - ) + with patch.object(tools_controller, "dns_service", mock_service): + response = client.post( + "/tools/dns/lookup", + json={"domain": "example.com", "record_type": "A"} + ) data = response.json() assert data["success"] is True