From 694aa9ccb57be9a2ba04fdb42fa4da497aaff5ec Mon Sep 17 00:00:00 2001 From: nilsmechtel Date: Sun, 27 Sep 2026 03:29:59 +0200 Subject: [PATCH 1/3] test(apps): stop hand-built proxies leaving an un-awaited destructor behind MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five test modules build ProxyDeployment stand-ins with object.__new__ and drop them. The class carries an `async def __del__` because Ray Serve's only per-replica shutdown hook is the destructor and Serve awaits it; CPython's collector does not, so each dropped stand-in produced a "coroutine 'ProxyDeployment.__del__' was never awaited" RuntimeWarning charged to whichever unrelated test the collector happened to run in — 54 of the suite's 70 warnings. Route those modules through a shared ProxyDouble: the production class minus the Serve-only destructor. Source assertions and behaviour tests keep using the real class. Adds a guard that the production destructor stays a coroutine function, since a synchronous one would return before the deregistration and disconnect it contains ever ran. Co-Authored-By: Claude Opus 5 (1M context) --- tests/apps/_proxy_double.py | 32 ++++++++++++ tests/apps/test_peer_connection_sweep.py | 7 ++- .../test_proxy_entry_saturation_tolerance.py | 6 +-- .../apps/test_proxy_hypha_decoupled_health.py | 6 +-- tests/apps/test_proxy_teardown_hook.py | 51 +++++++++++++++++++ .../apps/test_service_id_registration_gate.py | 15 ++++-- tests/apps/test_usage_ledger.py | 7 ++- 7 files changed, 105 insertions(+), 19 deletions(-) create mode 100644 tests/apps/_proxy_double.py create mode 100644 tests/apps/test_proxy_teardown_hook.py diff --git a/tests/apps/_proxy_double.py b/tests/apps/_proxy_double.py new file mode 100644 index 00000000..d028e75e --- /dev/null +++ b/tests/apps/_proxy_double.py @@ -0,0 +1,32 @@ +"""A ``ProxyDeployment`` stand-in for tests that no collector has to finalise. + +``ProxyDeployment.__del__`` is ``async def`` because a deployment class's only +per-replica shutdown hook in Ray Serve *is* the destructor, and Serve awaits it +(``call_destructor`` in ``ray/serve/_private/replica.py``). CPython's collector +does not await it: it calls ``__del__``, gets a coroutine object back and drops +it. Every hand-built proxy a test leaves behind therefore turns into +``RuntimeWarning: coroutine 'ProxyDeployment.__del__' was never awaited``, +reported against whichever unrelated test the collector happened to run in. + +Tests build their stand-ins from :class:`ProxyDouble`, which is the production +class minus that Serve-only hook. Nothing else differs, so source assertions and +behaviour tests should keep using ``PROXY_CLS``. +""" + +from __future__ import annotations + +from bioengine.apps import proxy_deployment as pd_module + +PROXY_CLS = pd_module.ProxyDeployment.func_or_class + + +class ProxyDouble(PROXY_CLS): + """``ProxyDeployment`` without the async destructor Ray Serve drives.""" + + def __del__(self) -> None: + """Drop the inherited ``async def __del__``. + + No Serve replica owns a test instance, so there is nobody to await the + production destructor — leaving it in place only hands the collector a + coroutine it will discard with a warning. + """ diff --git a/tests/apps/test_peer_connection_sweep.py b/tests/apps/test_peer_connection_sweep.py index c33314a5..64580555 100644 --- a/tests/apps/test_peer_connection_sweep.py +++ b/tests/apps/test_peer_connection_sweep.py @@ -22,9 +22,8 @@ import pytest -from bioengine.apps import proxy_deployment as pd_module - -_ProxyDeployment = pd_module.ProxyDeployment.func_or_class +from tests.apps._proxy_double import PROXY_CLS as _ProxyDeployment +from tests.apps._proxy_double import ProxyDouble class _StubPeerConnection: @@ -38,7 +37,7 @@ async def close(self) -> None: def _make_instance() -> _ProxyDeployment: """Skip ``__init__`` (it wants ~15 constructor args and a Ray Serve context) and stamp on just the attributes the sweep touches.""" - obj = _ProxyDeployment.__new__(_ProxyDeployment) + obj = ProxyDouble.__new__(ProxyDouble) obj.application_id = "test-app" obj._active_peer_connections = {} return obj diff --git a/tests/apps/test_proxy_entry_saturation_tolerance.py b/tests/apps/test_proxy_entry_saturation_tolerance.py index f2f4c9fc..f43da62d 100644 --- a/tests/apps/test_proxy_entry_saturation_tolerance.py +++ b/tests/apps/test_proxy_entry_saturation_tolerance.py @@ -31,8 +31,8 @@ import pytest from bioengine.apps import proxy_deployment as pd_module - -_ProxyCls = pd_module.ProxyDeployment.func_or_class +from tests.apps._proxy_double import PROXY_CLS as _ProxyCls +from tests.apps._proxy_double import ProxyDouble class _WsService: @@ -63,7 +63,7 @@ async def __call__(self, *args, **kwargs): def _bare_proxy(**attrs): - inst = object.__new__(_ProxyCls) + inst = object.__new__(ProxyDouble) inst.application_id = "app" inst._own_deployment_name = "ProxyDeployment" inst.entry_deployment_ready = False diff --git a/tests/apps/test_proxy_hypha_decoupled_health.py b/tests/apps/test_proxy_hypha_decoupled_health.py index eb403d8c..2c2db206 100644 --- a/tests/apps/test_proxy_hypha_decoupled_health.py +++ b/tests/apps/test_proxy_hypha_decoupled_health.py @@ -31,12 +31,12 @@ import pytest from bioengine.apps import proxy_deployment as pd_module - -_ProxyCls = pd_module.ProxyDeployment.func_or_class +from tests.apps._proxy_double import PROXY_CLS as _ProxyCls +from tests.apps._proxy_double import ProxyDouble def _bare_proxy(**attrs): - inst = object.__new__(_ProxyCls) + inst = object.__new__(ProxyDouble) inst.application_id = "app" inst.entry_deployment_ready = True inst.server = None diff --git a/tests/apps/test_proxy_teardown_hook.py b/tests/apps/test_proxy_teardown_hook.py new file mode 100644 index 00000000..94d82bdc --- /dev/null +++ b/tests/apps/test_proxy_teardown_hook.py @@ -0,0 +1,51 @@ +"""Pin how ``ProxyDeployment``'s teardown hook behaves in and out of Ray Serve. + +Serve gives a deployment class exactly one per-replica shutdown hook — the +destructor — and awaits it (``call_destructor`` in +``ray/serve/_private/replica.py``, whose comment says "Make sure to accept +``async def __del__(self)`` as well"). That is why the production destructor is +a coroutine function, and why making it synchronous would silently drop the +Hypha deregistration and client_id release the registration record depends on. + +Nothing outside Serve awaits it, so every proxy a test hand-builds and drops +used to hand CPython's collector a coroutine it discarded with a +``RuntimeWarning``, charged to whichever unrelated test was running at the time. +``ProxyDouble`` exists to keep that off the suite. +""" + +from __future__ import annotations + +import gc +import inspect +import warnings + +from tests.apps._proxy_double import PROXY_CLS, ProxyDouble + + +def _unawaited_destructor_warnings(cls) -> list[str]: + """Build an instance of ``cls``, drop it, and collect the GC's complaints.""" + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + instance = object.__new__(cls) + del instance + gc.collect() + gc.collect() + return [str(w.message) for w in caught if "never awaited" in str(w.message)] + + +def test_the_production_destructor_stays_awaitable() -> None: + """Serve awaits ``__del__``; a synchronous one would return before the + deregistration and disconnect it contains ever ran.""" + assert inspect.iscoroutinefunction(PROXY_CLS.__del__) + + +def test_dropping_the_production_class_warns() -> None: + """Positive control for the test below — without it, a double that stopped + suppressing anything would still look clean.""" + assert _unawaited_destructor_warnings(PROXY_CLS) == [ + "coroutine 'ProxyDeployment.__del__' was never awaited" + ] + + +def test_dropping_the_test_double_is_silent() -> None: + assert _unawaited_destructor_warnings(ProxyDouble) == [] diff --git a/tests/apps/test_service_id_registration_gate.py b/tests/apps/test_service_id_registration_gate.py index cfad3bc1..fb768c9c 100644 --- a/tests/apps/test_service_id_registration_gate.py +++ b/tests/apps/test_service_id_registration_gate.py @@ -36,8 +36,9 @@ from bioengine.apps import proxy_deployment as pd_module from bioengine.apps.manager import AppsManager from bioengine.cluster.proxy_actor import BioEngineProxyActor +from tests.apps._proxy_double import PROXY_CLS as _ProxyCls +from tests.apps._proxy_double import ProxyDouble -_ProxyCls = pd_module.ProxyDeployment.func_or_class _ActorCls = BioEngineProxyActor.__ray_actor_class__ APP_ID = "nuclei-seg" @@ -263,7 +264,7 @@ def _record_claim(self, application_id: str, replica_id=None) -> None: def _bare_proxy(**attrs): - inst = object.__new__(_ProxyCls) + inst = object.__new__(ProxyDouble) inst.application_id = APP_ID inst._replica_id = "replica-0" inst.entry_deployment_ready = True @@ -324,7 +325,11 @@ async def test_deregistering_reports_the_service_as_gone() -> None: def _construct_proxy(monkeypatch, handle: _Handle, replica_tag: str = "replica-0"): - """Build a real ProxyDeployment, with only Ray's two lookups stubbed.""" + """Build a real ProxyDeployment, with only Ray's two lookups stubbed. + + ``ProxyDouble`` is that class minus Ray Serve's async destructor, which no + test has a replica to await; nothing on the claim path differs. + """ monkeypatch.setattr(pd_module.ray, "get_actor", lambda name, namespace: handle) monkeypatch.setattr( pd_module, @@ -333,7 +338,7 @@ def _construct_proxy(monkeypatch, handle: _Handle, replica_tag: str = "replica-0 deployment="ProxyDeployment", replica_tag=replica_tag, app_name=APP_ID ), ) - return _ProxyCls( + return ProxyDouble( application_id=APP_ID, application_name="Nuclei Segmentation", application_description="Segment nuclei.", @@ -381,7 +386,7 @@ def test_a_missing_actor_handle_never_breaks_the_replica() -> None: def test_reporting_survives_a_part_built_replica() -> None: # __del__ -> _deregister_services -> here, reachable before __init__ has # assigned the handle at all. - inst = object.__new__(_ProxyCls) + inst = object.__new__(ProxyDouble) inst._report_service_registration(False) # must not raise AttributeError diff --git a/tests/apps/test_usage_ledger.py b/tests/apps/test_usage_ledger.py index d77f43b1..655a1213 100644 --- a/tests/apps/test_usage_ledger.py +++ b/tests/apps/test_usage_ledger.py @@ -43,8 +43,7 @@ ledger_dir_for_app, read_usage, ) - -_ProxyCls = pd_module.ProxyDeployment.func_or_class +from tests.apps._proxy_double import ProxyDouble # ===== helpers ===== @@ -71,7 +70,7 @@ def __getattr__(self, name): def _bare_proxy(tmp_path: Path, behaviour, *, authorized_users=None, slots=4): - inst = object.__new__(_ProxyCls) + inst = object.__new__(ProxyDouble) inst.application_id = "counted-app" inst.workspace = "host-ws" inst.authorized_users = authorized_users or {"*": ["*"]} @@ -925,7 +924,7 @@ async def _register_rtc(server, service_id, config): def test_an_app_without_durable_storage_still_deploys(tmp_path: Path, monkeypatch) -> None: monkeypatch.delenv("BIOENGINE_APP_DIR", raising=False) - inst = object.__new__(_ProxyCls) + inst = object.__new__(ProxyDouble) inst.application_id = "counted-app" inst.workspace = "host-ws" inst.app_data = {} From 2f2c8ace502b15f09cb50e2ad2175e2346e09031 Mon Sep 17 00:00:00 2001 From: nilsmechtel Date: Sun, 27 Sep 2026 03:57:25 +0200 Subject: [PATCH 2/3] test: fail the session if a proxy is left for the collector to finalise Routing the five modules through ProxyDouble removed the warnings but nothing stopped them coming back: a module regressing to the raw class still passed, because the warning is charged to whichever unrelated test the collector tripped in rather than to the test that built the object. A session-wide pytest_warning_recorded hook collects those records and pytest_sessionfinish fails the run, naming ProxyDouble and the nodeids that were charged. Promoting the warning with -W error::pytest.PytestUnraisableExceptionWarning would also fail the run, but it fails on the misattributed tests, which is the misdirection this is meant to remove. Co-Authored-By: Claude Opus 5 (1M context) --- tests/conftest.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/conftest.py b/tests/conftest.py index d1e7b684..11fe7e03 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -259,5 +259,33 @@ async def artifact_manager(hypha_client: RemoteService) -> ObjectProxy: return artifact_manager_service +_UNAWAITED_DESTRUCTOR = "coroutine 'ProxyDeployment.__del__' was never awaited" +_unawaited_destructor_nodeids: list = [] + + +def pytest_warning_recorded(warning_message, nodeid, **_): + """Record proxies left for the collector to finalise. + + Ray Serve awaits ``ProxyDeployment.__del__``; CPython's collector calls it, + gets a coroutine and discards it. The warning is charged to whichever test + was running when the collector tripped, not to the one that built the + object, so it has to be caught session-wide rather than per test. + """ + if _UNAWAITED_DESTRUCTOR in str(warning_message.message): + _unawaited_destructor_nodeids.append(nodeid) + + +def pytest_sessionfinish(session) -> None: + if not _unawaited_destructor_nodeids: + return + session.exitstatus = 1 + print( + f"\n{len(_unawaited_destructor_nodeids)} un-awaited ProxyDeployment " + f"destructor(s). Build test proxies from tests.apps._proxy_double." + f"ProxyDouble. Charged to: " + f"{', '.join(sorted(set(_unawaited_destructor_nodeids)))}" + ) + + # Configure asyncio for pytest pytest_plugins = ("pytest_asyncio",) From a7732737b4389346938b61f89a9cc58bb844fd00 Mon Sep 17 00:00:00 2001 From: nilsmechtel Date: Sun, 27 Sep 2026 04:11:31 +0200 Subject: [PATCH 3/3] test: let the proxy ratchet keep a more informative exit status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Setting exitstatus unconditionally downgraded INTERRUPTED to TESTS_FAILED: a pytest.exit bail-out — a real Ctrl-C, or this suite's own aiortc gate — came out as 1 instead of 2 whenever the session had also recorded proxy warnings. Only overwrite a passing status; the message still prints either way. Co-Authored-By: Claude Opus 5 (1M context) --- tests/conftest.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index 11fe7e03..db3fe064 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -278,7 +278,9 @@ def pytest_warning_recorded(warning_message, nodeid, **_): def pytest_sessionfinish(session) -> None: if not _unawaited_destructor_nodeids: return - session.exitstatus = 1 + # Only claim a passing run: INTERRUPTED and the error statuses say more. + if session.exitstatus == 0: + session.exitstatus = 1 print( f"\n{len(_unawaited_destructor_nodeids)} un-awaited ProxyDeployment " f"destructor(s). Build test proxies from tests.apps._proxy_double."