test(infra): fixture swaps real functions via monkeypatch (CI-robust)
CI on Python 3.11 still failed test_allow_outbound_network_fixture_* because the previous module-global toggle (_ALLOW_OUTBOUND=True/False) was unreliable on the runner — the wrapper's global lookup at call time sometimes saw False even after the fixture's True assignment. Switch to monkeypatch-based fixture: instead of toggling a global that the wrapper checks, restore socket.create_connection and socket.socket.connect to their REAL captured implementations for the duration of the test. Pytest's monkeypatch fixture handles teardown so the wrappers are reinstalled automatically. Rewrote the two paired tests to check function identity (socket.create_connection is _hermes_blocked_create_connection vs. is _REAL_CREATE_CONNECTION) instead of attempting a live outbound to 8.8.8.8:53 — direct identity check is hermetic and doesn't depend on whether the CI runner has any outbound network access at all.
This commit is contained in:
parent
6d83d16016
commit
d9bc8360a4
2 changed files with 40 additions and 44 deletions
|
|
@ -205,8 +205,6 @@ import socket as _hermes_test_socket
|
|||
_REAL_CREATE_CONNECTION = _hermes_test_socket.create_connection
|
||||
_REAL_SOCKET_CONNECT = _hermes_test_socket.socket.connect
|
||||
|
||||
_ALLOW_OUTBOUND = False # toggled by the allow_outbound_network fixture
|
||||
|
||||
|
||||
def _hermes_addr_is_local(host: str) -> bool:
|
||||
"""Return True for loopback / RFC1918 / link-local / reserved-TLD hosts."""
|
||||
|
|
@ -258,8 +256,6 @@ def _hermes_addr_is_local(host: str) -> bool:
|
|||
|
||||
|
||||
def _hermes_blocked_create_connection(address, *a, **kw):
|
||||
if _ALLOW_OUTBOUND:
|
||||
return _REAL_CREATE_CONNECTION(address, *a, **kw)
|
||||
try:
|
||||
host = address[0]
|
||||
except (TypeError, IndexError):
|
||||
|
|
@ -274,8 +270,6 @@ def _hermes_blocked_create_connection(address, *a, **kw):
|
|||
|
||||
|
||||
def _hermes_blocked_socket_connect(self, address):
|
||||
if _ALLOW_OUTBOUND:
|
||||
return _REAL_SOCKET_CONNECT(self, address)
|
||||
try:
|
||||
host = address[0]
|
||||
except (TypeError, IndexError):
|
||||
|
|
@ -292,19 +286,22 @@ _hermes_test_socket.socket.connect = _hermes_blocked_socket_connect
|
|||
|
||||
|
||||
@pytest.fixture
|
||||
def allow_outbound_network():
|
||||
def allow_outbound_network(monkeypatch):
|
||||
"""Opt-in to real outbound network for the duration of one test.
|
||||
|
||||
Use sparingly. Today only `test_dns_resolution_failure` needs this —
|
||||
it intentionally exercises a real DNS lookup on a reserved-TLD hostname
|
||||
to verify the probe maps `socket.gaierror` to `error='dns'`.
|
||||
Swaps `socket.create_connection` and `socket.socket.connect` back to the
|
||||
real (unwrapped) implementations for this test only, then monkeypatch
|
||||
teardown restores the wrapped versions. Direct swap is more reliable
|
||||
than a module-global toggle on CI runners where wrapper-closure
|
||||
lookup semantics can surprise.
|
||||
|
||||
Use sparingly. Today zero tests in the repo call this — the previous
|
||||
test_dns_resolution_failure case was rewritten to mock socket.getaddrinfo
|
||||
instead, which is fully hermetic.
|
||||
"""
|
||||
global _ALLOW_OUTBOUND
|
||||
_ALLOW_OUTBOUND = True
|
||||
try:
|
||||
yield
|
||||
finally:
|
||||
_ALLOW_OUTBOUND = False
|
||||
monkeypatch.setattr(_hermes_test_socket, "create_connection", _REAL_CREATE_CONNECTION)
|
||||
monkeypatch.setattr(_hermes_test_socket.socket, "connect", _REAL_SOCKET_CONNECT)
|
||||
yield
|
||||
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -97,38 +97,37 @@ def test_public_ipv4_is_blocked():
|
|||
assert _conftest._hermes_addr_is_local("204.0.113.0") is False # outside
|
||||
|
||||
|
||||
def test_allow_outbound_network_fixture_disables_the_block_for_a_public_ip(allow_outbound_network):
|
||||
"""When a test opts in to the fixture, the block does NOT fire on a
|
||||
destination that is otherwise blocked.
|
||||
def test_allow_outbound_network_fixture_unswaps_the_wrappers(allow_outbound_network):
|
||||
"""When a test opts in to the fixture, socket.create_connection and
|
||||
socket.socket.connect are restored to their real (unwrapped) implementations
|
||||
for this test only.
|
||||
|
||||
Uses 8.8.8.8 (Google DNS, a real public IPv4 not in any allow-list)
|
||||
so we can prove the fixture actually disabled the wrapper. Without
|
||||
the fixture, the wrapper would raise OSError("hermes test network
|
||||
isolation: ..."). With the fixture, the real socket.create_connection
|
||||
runs and we either succeed (port 53 is genuinely open) or fail with
|
||||
a *real* connect error — never with our wrapper's message.
|
||||
Direct identity check is safer than a behavioral test: it doesn't depend
|
||||
on whether the CI runner has outbound network access, only on whether the
|
||||
fixture's monkeypatch actually swapped the symbols.
|
||||
"""
|
||||
err_msg = ""
|
||||
try:
|
||||
sock = socket.create_connection(("8.8.8.8", 53), timeout=2)
|
||||
sock.close()
|
||||
# Success is fine — proves the wrapper got out of the way.
|
||||
return
|
||||
except OSError as e:
|
||||
err_msg = str(e)
|
||||
|
||||
# If we did get an OSError, it must NOT be from our wrapper.
|
||||
assert "hermes test network isolation" not in err_msg, (
|
||||
f"allow_outbound_network fixture should disable the block, but got: {err_msg}"
|
||||
import tests.conftest as _conftest
|
||||
# Inside the fixture, socket.create_connection should be the REAL one
|
||||
# (the one captured before the module-level wrap), not the blocked wrapper.
|
||||
assert socket.create_connection is _conftest._REAL_CREATE_CONNECTION, (
|
||||
"allow_outbound_network fixture did not restore the real create_connection"
|
||||
)
|
||||
assert socket.socket.connect is _conftest._REAL_SOCKET_CONNECT, (
|
||||
"allow_outbound_network fixture did not restore the real socket.connect"
|
||||
)
|
||||
|
||||
|
||||
def test_block_is_active_outside_the_fixture():
|
||||
"""Sanity: a test that does NOT request the fixture has the block active.
|
||||
"""Sanity: a test that does NOT request the fixture has the wrapped
|
||||
socket.create_connection installed (not the real one).
|
||||
|
||||
Pairs with the test above to prove the fixture toggle is real — without
|
||||
this paired test the fixture test would self-pass even if the toggle
|
||||
didn't work (since the block is on by default and the wrapper-or-real
|
||||
distinction is what matters)."""
|
||||
with pytest.raises(OSError, match="hermes test network isolation"):
|
||||
socket.create_connection(("8.8.8.8", 53), timeout=1)
|
||||
Pairs with the test above to prove the fixture swap is real — the
|
||||
fixture test confirms the unswap; this test confirms the default-on
|
||||
state."""
|
||||
import tests.conftest as _conftest
|
||||
assert socket.create_connection is _conftest._hermes_blocked_create_connection, (
|
||||
"default state should have the blocked wrapper installed on socket.create_connection"
|
||||
)
|
||||
assert socket.socket.connect is _conftest._hermes_blocked_socket_connect, (
|
||||
"default state should have the blocked wrapper installed on socket.socket.connect"
|
||||
)
|
||||
|
|
|
|||
Loading…
Reference in a new issue