diff --git a/tests/backends/test_memory.py b/tests/backends/test_memory.py index bfa9161..9005f6c 100644 --- a/tests/backends/test_memory.py +++ b/tests/backends/test_memory.py @@ -4,6 +4,7 @@ import time from collections.abc import Awaitable from collections.abc import Callable +from typing import Any import pytest import pytest_asyncio @@ -204,8 +205,28 @@ def test_cleanup_restarts_on_a_new_loop_after_the_old_one_closed(): assert task is not stale assert task.get_loop() is second backend.stop_cleanup() # the stale task's loop is closed: must not raise + assert stale is not None + assert not stale.done() # left pending for good: its loop never runs again finally: _close(second) + _silence_pending_task_report(first) + + +def _silence_pending_task_report(loop: asyncio.AbstractEventLoop) -> None: + """Keep ``loop``'s abandoned task from being reported when it is collected. + + A task on a closed loop cannot be cancelled: cancelling schedules a + callback on its loop, which raises. Garbage-collecting it then reports + "Task was destroyed but it is pending!" through the loop's exception + handler, in whichever later test the collector happens to run (#295). + The loop object outlives its close, so its handler can still be set. + """ + + def handler(loop: asyncio.AbstractEventLoop, context: dict[str, Any]) -> None: + if context.get("message") != "Task was destroyed but it is pending!": + loop.default_exception_handler(context) + + loop.set_exception_handler(handler) def test_cleanup_moving_loops_cancels_the_task_on_a_loop_still_open(): diff --git a/tests/conftest.py b/tests/conftest.py index cdfdf3b..42a3b05 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,4 +1,7 @@ import asyncio +import gc +import logging +import os import time from collections.abc import AsyncGenerator from datetime import datetime @@ -24,6 +27,60 @@ from fastapi_cachex.state import models as state_models from fastapi_cachex.state.proxy import StateManagerProxy +_PENDING_TASK_MESSAGE = "Task was destroyed but it is pending!" + + +class _PendingTaskHandler(logging.Handler): + """Record every "Task was destroyed but it is pending!" asyncio logs. + + asyncio reports such a task through its logger, not as a warning, so + `filterwarnings = ["error"]` never fails on it (#295). It is logged when + the garbage collector frees the task, often during an unrelated test. + """ + + def __init__(self) -> None: + super().__init__(logging.ERROR) + self.reports: list[tuple[str, str]] = [] + + def emit(self, record: logging.LogRecord) -> None: + message = record.getMessage() + if message.startswith(_PENDING_TASK_MESSAGE): + running = os.environ.get("PYTEST_CURRENT_TEST", "outside any test") + self.reports.append((running, message)) + + +_pending_tasks = _PendingTaskHandler() + + +def pytest_configure(config: pytest.Config) -> None: + logging.getLogger("asyncio").addHandler(_pending_tasks) + + +def pytest_sessionfinish(session: pytest.Session) -> None: + """Fail the run if any test left a pending task behind. + + Collecting here makes a task that is still waiting for the garbage + collector report now, so the check does not depend on when it runs. + """ + gc.collect() + logging.getLogger("asyncio").removeHandler(_pending_tasks) + if _pending_tasks.reports: + session.exitstatus = pytest.ExitCode.TESTS_FAILED + + +def pytest_terminal_summary(terminalreporter: Any) -> None: + if not _pending_tasks.reports: + return + terminalreporter.section("asyncio tasks destroyed while pending", red=True) + terminalreporter.line( + "A test left an asyncio task pending on a loop that no longer runs it. " + "The test named below is only where the garbage collector freed it; " + "rerun with PYTHONASYNCIODEBUG=1 to see where the task was created." + ) + for running, message in _pending_tasks.reports: + terminalreporter.line(f"\nfreed during: {running}\n{message}") + + # Every proxy is a process-wide singleton, so whatever one test installs is # still installed for the next one. _PROXIES = (CacheManagerProxy, SessionManagerProxy, StateManagerProxy) @@ -38,13 +95,48 @@ async def memory_backend(): @pytest.fixture(autouse=True) -def setup_default_backend(): - """Auto-use fixture to set MemoryBackend as default for all tests.""" +async def setup_default_backend() -> AsyncGenerator[None, None]: + """Auto-use fixture to set MemoryBackend as default for all tests. + + Async so that its teardown runs on the test's loop while that loop is + still open; see `close_memory_backends`. + """ backend = MemoryBackend() backend.start_cleanup() BackendProxy.set(backend) yield - backend.stop_cleanup() + await backend.aclose() + + +@pytest.fixture(autouse=True) +async def close_memory_backends( + monkeypatch: pytest.MonkeyPatch, +) -> AsyncGenerator[None, None]: + """Stop the cleanup task of every `MemoryBackend` a test builds. + + A backend starts its cleanup task on the first operation, on the test's + loop. pytest-asyncio 1.x runs each test in an `asyncio.Runner`, which + cancels leftover tasks before it closes the loop; 0.26 (our floor, run by + the `lowest` tox env) closes the loop without cancelling them. Such a task + stays pending for good and is reported as "Task was destroyed but it is + pending!" when collected (#295). Closing each backend here, on the test's + loop, finishes its task on either version. + + Like `close_network_clients`, this records backends in `__new__`, + including ones the library builds itself, such as the fallback backend. + """ + opened: list[MemoryBackend] = [] + + def record(cls: type[Any], *args: Any, **kwargs: Any) -> Any: + backend = object.__new__(cls) + opened.append(backend) + return backend + + monkeypatch.setattr(MemoryBackend, "__new__", record) + yield + for backend in opened: + if hasattr(backend, "_cleanup_task"): # missing if the constructor raised + await backend.aclose() @pytest.fixture(autouse=True)