From 6d7358bc365d830069ca74ecd93409bcd88a7018 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Mon, 4 May 2026 17:41:20 -0500 Subject: [PATCH] Fix Pytest 8/9 compatibility and coroutine leaks in launch_pytest This commit addresses two issues: 1. Renames deprecated path argument to module_path/collection_path in pytest hooks to support Pytest 8/9. 2. Ensures LaunchService run_async task is always awaited by registering the finalizer earlier and passing the task explicitly. Assisted-by: Gemini CLI:2.0-Flash [run_shell_command, replace, git, gh] Signed-off-by: Michael Carroll --- launch/conftest.py | 4 +++- launch_pytest/launch_pytest/fixture.py | 9 ++++++--- launch_pytest/launch_pytest/plugin.py | 15 ++++++++++----- launch_testing/launch_testing/pytest/hooks.py | 18 +++++++++++++----- .../launch_testing/pytest/hookspecs.py | 2 +- 5 files changed, 33 insertions(+), 15 deletions(-) diff --git a/launch/conftest.py b/launch/conftest.py index 47348f5e4..879f46712 100644 --- a/launch/conftest.py +++ b/launch/conftest.py @@ -15,7 +15,9 @@ from pathlib import PurePath -def pytest_ignore_collect(path): +def pytest_ignore_collect(collection_path=None, path=None): + if collection_path is not None: + path = collection_path # pytest doctest messes up when trying to import .launch.py packages, ignore them. # It also messes up when trying to import launch.logging.handlers due to conflicts with # logging.handlers, ignore that as well. diff --git a/launch_pytest/launch_pytest/fixture.py b/launch_pytest/launch_pytest/fixture.py index aec03c6ba..93b8c4f7c 100644 --- a/launch_pytest/launch_pytest/fixture.py +++ b/launch_pytest/launch_pytest/fixture.py @@ -27,13 +27,16 @@ def scope_gt(scope1, scope2): from _pytest.fixtures import scopemismatch as scope_gt -def finalize_launch_service(launch_service, eprefix='', auto_shutdown=True): +def finalize_launch_service(launch_service, eprefix='', auto_shutdown=True, task=None): if auto_shutdown: launch_service.shutdown(force_sync=True) loop = launch_service.event_loop if loop is not None and not loop.is_closed(): - rc = loop.run_until_complete(launch_service.task) - assert rc == 0, f"{eprefix} launch service failed when finishing, return code '{rc}'" + if task is None: + task = launch_service.task + if task is not None: + rc = loop.run_until_complete(task) + assert rc == 0, f"{eprefix} launch service failed when finishing, return code '{rc}'" def get_launch_service_fixture(*, scope='function', overridable=True): diff --git a/launch_pytest/launch_pytest/plugin.py b/launch_pytest/launch_pytest/plugin.py index 3dd66d05c..16f6a6d81 100644 --- a/launch_pytest/launch_pytest/plugin.py +++ b/launch_pytest/launch_pytest/plugin.py @@ -119,12 +119,17 @@ def pytest_fixture_setup(fixturedef, request): run_async_task = event_loop.create_task(ls.run_async( shutdown_when_idle=options['shutdown_when_idle'] )) + fixturedef.addfinalizer(functools.partial( + finalize_launch_service, + ls, + eprefix=eprefix, + auto_shutdown=options['auto_shutdown'], + task=run_async_task + )) ready = get_ready_to_test_action(ld) asyncio.set_event_loop(event_loop) event = asyncio.Event() ready._add_callback(lambda: event.set()) - fixturedef.addfinalizer(functools.partial( - finalize_launch_service, ls, eprefix=eprefix, auto_shutdown=options['auto_shutdown'])) run_until_complete(event_loop, event.wait()) # this is guaranteed by the current run_async() implementation, let's check it just in case # it changes in the future @@ -361,7 +366,7 @@ def pytest_pyfunc_call(pyfuncitem): wrap_generator(func, event_loop, on_shutdown) ) shutdown_item._fixtureinfo = shutdown_item.session._fixturemanager.getfixtureinfo( - shutdown_item, shutdown_item.obj, shutdown_item.cls, funcargs=True) + shutdown_item, shutdown_item.obj, shutdown_item.cls) else: pyfuncitem.obj = wrap_generator_fscope(func, event_loop, on_shutdown) elif inspect.isasyncgenfunction(func): @@ -371,7 +376,7 @@ def pytest_pyfunc_call(pyfuncitem): wrap_asyncgen(func, event_loop, on_shutdown) ) shutdown_item._fixtureinfo = shutdown_item.session._fixturemanager.getfixtureinfo( - shutdown_item, shutdown_item.obj, shutdown_item.cls, funcargs=True) + shutdown_item, shutdown_item.obj, shutdown_item.cls) else: pyfuncitem.obj = wrap_asyncgen_fscope(func, event_loop, on_shutdown) elif not getattr(pyfuncitem.obj, '_launch_pytest_wrapped', False): @@ -417,7 +422,7 @@ def wrap_generator(func, event_loop, on_shutdown): """Return wrappers for the normal test and the teardown test for a generator function.""" gen = None - def shutdown(): + def shutdown(**kwargs): if gen is None: skip('shutdown test skipped because the test failed before') on_shutdown() diff --git a/launch_testing/launch_testing/pytest/hooks.py b/launch_testing/launch_testing/pytest/hooks.py index 174c4c476..3726d9570 100644 --- a/launch_testing/launch_testing/pytest/hooks.py +++ b/launch_testing/launch_testing/pytest/hooks.py @@ -189,12 +189,20 @@ def find_launch_test_entrypoint(path): return None -def pytest_pycollect_makemodule(path, parent): +if _pytest_version_ge(8): + def pytest_pycollect_makemodule(module_path, parent): + return _pytest_pycollect_makemodule(module_path, parent) +else: + def pytest_pycollect_makemodule(path, parent): + return _pytest_pycollect_makemodule(path, parent) + + +def _pytest_pycollect_makemodule(path, parent): entrypoint = find_launch_test_entrypoint(path) if entrypoint is not None: ihook = parent.session.gethookproxy(path) module = ihook.pytest_launch_collect_makemodule( - path=path, parent=parent, entrypoint=entrypoint + module_path=path, parent=parent, entrypoint=entrypoint ) if module is not None: return module @@ -216,14 +224,14 @@ def pytest_pycollect_makemodule(path, parent): @pytest.hookimpl(trylast=True) -def pytest_launch_collect_makemodule(path, parent, entrypoint): +def pytest_launch_collect_makemodule(module_path, parent, entrypoint): marks = getattr(entrypoint, 'pytestmark', []) if marks and any(m.name == 'launch_test' for m in marks): if _pytest_version_ge(7): - path = pathlib.Path(path) + path = pathlib.Path(module_path) module = LaunchTestModule.from_parent(parent=parent, path=path) else: - module = LaunchTestModule.from_parent(parent=parent, fspath=path) + module = LaunchTestModule.from_parent(parent=parent, fspath=module_path) for mark in marks: decorator = getattr(pytest.mark, mark.name) decorator = decorator.with_args(*mark.args, **mark.kwargs) diff --git a/launch_testing/launch_testing/pytest/hookspecs.py b/launch_testing/launch_testing/pytest/hookspecs.py index 89249d5ea..52912415e 100644 --- a/launch_testing/launch_testing/pytest/hookspecs.py +++ b/launch_testing/launch_testing/pytest/hookspecs.py @@ -16,6 +16,6 @@ @pytest.hookspec(firstresult=True) -def pytest_launch_collect_makemodule(path, parent, entrypoint): +def pytest_launch_collect_makemodule(module_path, parent, entrypoint): """Make launch test module appropriate for the found test entrypoint.""" pass