Use a daemon-ready sentinel in discovery tests
The discovery `daemon` fixture probed UDS readiness by connecting and immediately closing. That entered Tractor's actor handshake without an `Aid` payload and destabilized the remote registrar on macOS before test roots attempted discovery. Run the child through a small `open_root_actor()` wrapper and publish a filesystem sentinel only after runtime startup completes. Poll that sentinel with process-liveness checks and guaranteed setup-failure cleanup, without touching the transport socket. Cover transport-free readiness and deterministic polling backoff. Caught-during: review remediation Found-via: `/run-tests` discovery daemon fixture consumers Cause: initial sentinel drafts mishandled `pytest.Testdir`, emitted invalid `python -c` syntax, and misplaced return-code logging. Review: PR #480 (goodboy) https://github.com/goodboy/tractor/pull/480 (this patch was generated in some part by `opencode` using `gpt-5.6-sol` (`openai`))wkt/uds_macos_473
parent
5724c0516a
commit
6e424d4696
|
|
@ -13,9 +13,8 @@ under `tests/discovery/` automatically picks this up.
|
||||||
|
|
||||||
'''
|
'''
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
import os
|
from pathlib import Path
|
||||||
import platform
|
import platform
|
||||||
import socket
|
|
||||||
import subprocess
|
import subprocess
|
||||||
import sys
|
import sys
|
||||||
import time
|
import time
|
||||||
|
|
@ -31,27 +30,22 @@ from ..conftest import (
|
||||||
|
|
||||||
|
|
||||||
def _wait_for_daemon_ready(
|
def _wait_for_daemon_ready(
|
||||||
reg_addr: tuple,
|
ready_path: Path,
|
||||||
tpt_proto: str,
|
|
||||||
*,
|
*,
|
||||||
deadline: float = 10.0,
|
deadline: float = 10.0,
|
||||||
poll_interval: float = 0.05,
|
poll_interval: float = 0.05,
|
||||||
proc: subprocess.Popen|None = None,
|
proc: subprocess.Popen|None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
'''
|
'''
|
||||||
Active-poll the daemon's bind address until it
|
Poll until the daemon reports completed actor startup.
|
||||||
accepts a connection (proving it has called
|
|
||||||
`bind() + listen()` and is ready to handle IPC).
|
|
||||||
|
|
||||||
Replaces the historical blind `time.sleep()` in the
|
Replaces the historical blind `time.sleep()` in the
|
||||||
`daemon` fixture which was racy under load — see
|
`daemon` fixture which was racy under load — see
|
||||||
`ai/conc-anal/test_register_duplicate_name_daemon_connect_race_issue.md`.
|
`ai/conc-anal/test_register_duplicate_name_daemon_connect_race_issue.md`.
|
||||||
|
|
||||||
Uses stdlib `socket` directly (no trio runtime
|
The child writes `ready_path` only after entering
|
||||||
bootstrap cost) — sufficient because
|
`open_root_actor()`, which guarantees all transport listeners are
|
||||||
`tractor.run_daemon()` doesn't return from
|
serving without requiring a raw connection probe.
|
||||||
bootstrap until the runtime is fully ready to
|
|
||||||
accept IPC.
|
|
||||||
|
|
||||||
Raises `TimeoutError` on `deadline` exceeded. If
|
Raises `TimeoutError` on `deadline` exceeded. If
|
||||||
`proc` is given, ALSO raises early if the daemon
|
`proc` is given, ALSO raises early if the daemon
|
||||||
|
|
@ -70,43 +64,25 @@ def _wait_for_daemon_ready(
|
||||||
if proc is not None and proc.poll() is not None:
|
if proc is not None and proc.poll() is not None:
|
||||||
raise RuntimeError(
|
raise RuntimeError(
|
||||||
f'Daemon proc exited (rc={proc.returncode}) '
|
f'Daemon proc exited (rc={proc.returncode}) '
|
||||||
f'before becoming ready to accept on '
|
f'before reporting ready at {ready_path!r}'
|
||||||
f'{reg_addr!r}'
|
|
||||||
)
|
)
|
||||||
try:
|
try:
|
||||||
if tpt_proto == 'tcp':
|
if ready_path.is_file():
|
||||||
# `socket.create_connection` does the
|
if proc is not None and proc.poll() is not None:
|
||||||
# `socket() + connect()` dance with a
|
raise RuntimeError(
|
||||||
# builtin timeout — perfect primitive
|
f'Daemon proc exited (rc={proc.returncode}) '
|
||||||
# for a one-shot probe.
|
f'after reporting ready at {ready_path!r}'
|
||||||
with socket.create_connection(
|
)
|
||||||
reg_addr,
|
return
|
||||||
timeout=poll_interval,
|
|
||||||
):
|
|
||||||
return
|
|
||||||
else:
|
|
||||||
# UDS — `reg_addr` is a `(filedir, sockname)`
|
|
||||||
# tuple per `tractor.ipc._uds.UDSAddress.unwrap`.
|
|
||||||
sockpath: str = os.path.join(*reg_addr)
|
|
||||||
sock = socket.socket(socket.AF_UNIX)
|
|
||||||
try:
|
|
||||||
sock.settimeout(poll_interval)
|
|
||||||
sock.connect(sockpath)
|
|
||||||
return
|
|
||||||
finally:
|
|
||||||
sock.close()
|
|
||||||
except (
|
except (
|
||||||
ConnectionRefusedError,
|
|
||||||
FileNotFoundError,
|
FileNotFoundError,
|
||||||
OSError,
|
OSError,
|
||||||
socket.timeout,
|
|
||||||
) as exc:
|
) as exc:
|
||||||
last_exc = exc
|
last_exc = exc
|
||||||
time.sleep(poll_interval)
|
time.sleep(poll_interval)
|
||||||
raise TimeoutError(
|
raise TimeoutError(
|
||||||
f'Daemon never accepted on {reg_addr!r} within '
|
f'Daemon never reported ready at {ready_path!r} within '
|
||||||
f'{deadline}s (last connect-attempt exc: '
|
f'{deadline}s (last sentinel-state exc: {last_exc!r})'
|
||||||
f'{last_exc!r})'
|
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|
@ -136,18 +112,27 @@ def daemon(
|
||||||
)
|
)
|
||||||
loglevel: str = 'info'
|
loglevel: str = 'info'
|
||||||
|
|
||||||
|
ready_path: Path = (
|
||||||
|
Path(str(testdir.tmpdir))
|
||||||
|
/ 'daemon-ready'
|
||||||
|
)
|
||||||
|
ready_path.unlink(missing_ok=True)
|
||||||
code: str = (
|
code: str = (
|
||||||
"import tractor; "
|
f'from pathlib import Path\n'
|
||||||
"tractor.run_daemon([], "
|
f'import tractor\n'
|
||||||
"registry_addrs={reg_addrs}, "
|
f'import trio\n'
|
||||||
"enable_transports={enable_tpts}, "
|
f'\n'
|
||||||
"debug_mode={debug_mode}, "
|
f'async def main():\n'
|
||||||
"loglevel={ll})"
|
f' async with tractor.open_root_actor(\n'
|
||||||
).format(
|
f' registry_addrs={[reg_addr]!r},\n'
|
||||||
reg_addrs=str([reg_addr]),
|
f' enable_transports={[tpt_proto]!r},\n'
|
||||||
enable_tpts=str([tpt_proto]),
|
f' debug_mode={debug_mode!r},\n'
|
||||||
ll="'{}'".format(loglevel) if loglevel else None,
|
f' loglevel={loglevel!r},\n'
|
||||||
debug_mode=debug_mode,
|
f' ):\n'
|
||||||
|
f' Path({str(ready_path)!r}).touch()\n'
|
||||||
|
f' await trio.sleep_forever()\n'
|
||||||
|
f'\n'
|
||||||
|
f'trio.run(main)\n'
|
||||||
)
|
)
|
||||||
cmd: list[str] = [
|
cmd: list[str] = [
|
||||||
sys.executable,
|
sys.executable,
|
||||||
|
|
@ -176,48 +161,49 @@ def daemon(
|
||||||
15.0 if (_non_linux and ci_env)
|
15.0 if (_non_linux and ci_env)
|
||||||
else 10.0
|
else 10.0
|
||||||
)
|
)
|
||||||
_wait_for_daemon_ready(
|
try:
|
||||||
reg_addr=reg_addr,
|
_wait_for_daemon_ready(
|
||||||
tpt_proto=tpt_proto,
|
ready_path=ready_path,
|
||||||
deadline=deadline,
|
deadline=deadline,
|
||||||
proc=proc,
|
proc=proc,
|
||||||
)
|
|
||||||
|
|
||||||
assert not proc.returncode
|
|
||||||
yield proc
|
|
||||||
sig_prog(proc, _INT_SIGNAL)
|
|
||||||
|
|
||||||
# XXX! yeah.. just be reaaal careful with this bc
|
|
||||||
# sometimes it can lock up on the `_io.BufferedReader`
|
|
||||||
# and hang..
|
|
||||||
#
|
|
||||||
# NB, drain happens at TEARDOWN (post-yield), so the
|
|
||||||
# test body has its chance to read `proc.stderr`
|
|
||||||
# FIRST. Reading here AFTER would silently swallow
|
|
||||||
# the daemon's stderr output and break tests that
|
|
||||||
# assert on it (e.g. `test_abort_on_sigint`).
|
|
||||||
stderr: str = proc.stderr.read().decode()
|
|
||||||
stdout: str = proc.stdout.read().decode()
|
|
||||||
if (
|
|
||||||
stderr
|
|
||||||
or
|
|
||||||
stdout
|
|
||||||
):
|
|
||||||
print(
|
|
||||||
f'Daemon actor tree produced output:\n'
|
|
||||||
f'{proc.args}\n'
|
|
||||||
f'\n'
|
|
||||||
f'stderr: {stderr!r}\n'
|
|
||||||
f'stdout: {stdout!r}\n'
|
|
||||||
)
|
)
|
||||||
|
|
||||||
if (rc := proc.returncode) != -2:
|
assert not proc.returncode
|
||||||
msg: str = (
|
yield proc
|
||||||
f'Daemon actor tree was not cancelled !?\n'
|
finally:
|
||||||
f'proc.args: {proc.args!r}\n'
|
if proc.poll() is None:
|
||||||
f'proc.returncode: {rc!r}\n'
|
sig_prog(proc, _INT_SIGNAL)
|
||||||
)
|
|
||||||
if rc < 0:
|
|
||||||
raise RuntimeError(msg)
|
|
||||||
|
|
||||||
test_log.error(msg)
|
# XXX! yeah.. just be reaaal careful with this bc
|
||||||
|
# sometimes it can lock up on the `_io.BufferedReader`
|
||||||
|
# and hang..
|
||||||
|
#
|
||||||
|
# NB, drain happens at TEARDOWN (post-yield), so the
|
||||||
|
# test body has its chance to read `proc.stderr`
|
||||||
|
# FIRST. Reading here AFTER would silently swallow
|
||||||
|
# the daemon's stderr output and break tests that
|
||||||
|
# assert on it (e.g. `test_abort_on_sigint`).
|
||||||
|
stderr: str = proc.stderr.read().decode()
|
||||||
|
stdout: str = proc.stdout.read().decode()
|
||||||
|
if (
|
||||||
|
stderr
|
||||||
|
or
|
||||||
|
stdout
|
||||||
|
):
|
||||||
|
print(
|
||||||
|
f'Daemon actor tree produced output:\n'
|
||||||
|
f'{proc.args}\n'
|
||||||
|
f'\n'
|
||||||
|
f'stderr: {stderr!r}\n'
|
||||||
|
f'stdout: {stdout!r}\n'
|
||||||
|
)
|
||||||
|
|
||||||
|
if (rc := proc.returncode) != -2:
|
||||||
|
msg: str = (
|
||||||
|
f'Daemon actor tree was not cancelled !?\n'
|
||||||
|
f'proc.args: {proc.args!r}\n'
|
||||||
|
f'proc.returncode: {rc!r}\n'
|
||||||
|
)
|
||||||
|
if rc < 0:
|
||||||
|
raise RuntimeError(msg)
|
||||||
|
test_log.error(msg)
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,72 @@
|
||||||
|
'''
|
||||||
|
Discovery daemon fixture regressions.
|
||||||
|
|
||||||
|
'''
|
||||||
|
from unittest.mock import (
|
||||||
|
call,
|
||||||
|
Mock,
|
||||||
|
)
|
||||||
|
|
||||||
|
from .conftest import _wait_for_daemon_ready
|
||||||
|
|
||||||
|
|
||||||
|
def test_daemon_ready_check_does_not_connect(
|
||||||
|
monkeypatch,
|
||||||
|
tmp_path,
|
||||||
|
):
|
||||||
|
'''
|
||||||
|
Detect a listening UDS daemon without creating a raw connection.
|
||||||
|
|
||||||
|
The old UDS readiness helper connected and immediately closed. That
|
||||||
|
entered Tractor's actor-handshake handler with no `Aid` payload and
|
||||||
|
destabilized the remote registrar on macOS before discovery tests
|
||||||
|
started. This test creates the child sentinel, forbids all socket
|
||||||
|
construction and connection helpers, then proves readiness returns
|
||||||
|
without touching the transport layer.
|
||||||
|
|
||||||
|
'''
|
||||||
|
ready_path = tmp_path / 'daemon-ready'
|
||||||
|
ready_path.touch()
|
||||||
|
socket_ctor = Mock(side_effect=AssertionError('socket opened'))
|
||||||
|
connect = Mock(side_effect=AssertionError('socket connected'))
|
||||||
|
monkeypatch.setattr('socket.socket', socket_ctor)
|
||||||
|
monkeypatch.setattr('socket.create_connection', connect)
|
||||||
|
|
||||||
|
_wait_for_daemon_ready(
|
||||||
|
ready_path=ready_path,
|
||||||
|
deadline=.1,
|
||||||
|
poll_interval=.01,
|
||||||
|
)
|
||||||
|
|
||||||
|
socket_ctor.assert_not_called()
|
||||||
|
connect.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
|
def test_daemon_ready_check_backs_off(monkeypatch):
|
||||||
|
'''
|
||||||
|
Back off while waiting for the child startup sentinel.
|
||||||
|
|
||||||
|
The sentinel may appear after several parent polling intervals.
|
||||||
|
A deterministic false/false/true path sequence proves the helper
|
||||||
|
sleeps between unsuccessful observations instead of hot-spinning
|
||||||
|
and starving a booting daemon on constrained CI workers.
|
||||||
|
|
||||||
|
'''
|
||||||
|
ready_path = Mock()
|
||||||
|
ready_path.is_file.side_effect = [False, False, True]
|
||||||
|
sleep = Mock()
|
||||||
|
monotonic = Mock(side_effect=[0, 0, 0, 0])
|
||||||
|
monkeypatch.setattr('time.sleep', sleep)
|
||||||
|
monkeypatch.setattr('time.monotonic', monotonic)
|
||||||
|
|
||||||
|
_wait_for_daemon_ready(
|
||||||
|
ready_path=ready_path,
|
||||||
|
deadline=.2,
|
||||||
|
poll_interval=.01,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert ready_path.is_file.call_count == 3
|
||||||
|
assert sleep.call_args_list == [
|
||||||
|
call(.01),
|
||||||
|
call(.01),
|
||||||
|
]
|
||||||
Loading…
Reference in New Issue