fix(tests): mock the live DNS service instance, not a dead legacy class (T-55)
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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,8 +72,8 @@ class TestDNSLookup:
|
||||
|
||||
mock_service = MagicMock()
|
||||
mock_service.lookup = AsyncMock(return_value=mock_response)
|
||||
mock_dns_class.return_value = mock_service
|
||||
|
||||
with patch.object(tools_controller, "dns_service", mock_service):
|
||||
response = client.post(
|
||||
"/tools/dns/lookup",
|
||||
json={"domain": "example.com", "record_type": "A"}
|
||||
|
||||
Reference in New Issue
Block a user