Fix Pytest 8/9 compatibility in hooks (backport #972)

launch/conftest.py

@@ -12,14 +12,33 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from pathlib import PurePath
+import pathlib
 
+import pytest
 
-def pytest_ignore_collect(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.
-    return str(path).endswith((
-        '.launch.py',
-        str(PurePath('logging') / 'handlers.py'),
-    ))
+
+def _pytest_version_ge(major, minor=0, patch=0):
+    """Return True if pytest version is >= the given version."""
+    pytest_version = tuple(int(v) for v in pytest.__version__.split('.'))
+    return pytest_version >= (major, minor, patch)
+
+
+def _should_ignore(p):
+    if p is None:
+        return False
+    path = pathlib.Path(p)
+    # Ignore .launch.py files — not valid Python module names.
+    if path.name.endswith('.launch.py'):
+        return True
+    # Ignore launch.logging.handlers — collides with stdlib logging.handlers.
+    if path.name == 'handlers.py' and path.parent.name == 'logging':
+        return True
+    return False
+
+
+if _pytest_version_ge(8):
+    def pytest_ignore_collect(collection_path, config):
+        return _should_ignore(collection_path)
+else:
+    def pytest_ignore_collect(path, config):
+        return _should_ignore(path)

launch/launch/event_handlers/on_process_exit.py

@@ -27,7 +27,6 @@
 from ..launch_context import LaunchContext
 from ..some_entities_type import SomeEntitiesType
 
-
 if TYPE_CHECKING:
     from ..action import Action  # noqa: F401
     from ..actions import ExecuteLocal  # noqa: F401

launch/launch/logging/__init__.py

@@ -1,4 +1,4 @@
-# Copyright 2019 Open Source Robotics Foundation, Inc.
+# Copyright 2019 Open Source Robotics Foundation, Inc.  # noqa: A005
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.

launch/pytest.ini

@@ -1,5 +1,7 @@
 [pytest]
 junit_family=xunit2
-addopts = --doctest-modules
+# Disable doctest modules temporarily to avoid ImportPathMismatchError with logging.handlers
+# addopts = --doctest-modules
 timeout=900
 timeout_method=thread
+pythonpath = test

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):

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()

launch_testing/launch_testing/pytest/hooks.py

@@ -174,61 +174,107 @@ def collect(self):
 
 
 def find_launch_test_entrypoint(path):
+    path_obj = pathlib.Path(path)
+    # Skip files that are known to cause conflicts or are definitely not tests
+    if path_obj.name == 'handlers.py' and path_obj.parent.name == 'logging':
+        return None
+
+    # Only attempt to import files that look like Python modules and are not launch files
+    # with double extensions (which Pytest 8+ import_path might struggle with)
+    if not path_obj.name.endswith('.py') or path_obj.name.endswith('.launch.py'):
+        return None
+
     try:
         if _pytest_version_ge(8, 1, 0):
             from _pytest.pathlib import import_path
-            module = import_path(path, root=None, consider_namespace_packages=False)
+            module = import_path(path_obj, root=None, consider_namespace_packages=False)
         elif _pytest_version_ge(7, 0, 0):
             from _pytest.pathlib import import_path
-            module = import_path(path, root=None)
+            module = import_path(path_obj, root=None)
         else:
             # Assume we got legacy path in earlier versions of pytest
             module = path.pyimport()
         return getattr(module, 'generate_test_description', None)
-    except SyntaxError:
+    except Exception:
+        # Catch all exceptions during import to avoid breaking collection
         return None
 
 
-def pytest_pycollect_makemodule(path, parent):
+@pytest.hookimpl(tryfirst=True)
+def pytest_ignore_collect(collection_path=None, path=None, config=None):
+    # Pytest 8.x signature: (collection_path, path, config)
+    # Pytest < 8 signature: (path, config)
+    p = collection_path or path
+    if p is None:
+        return False
+
+    path_obj = pathlib.Path(p)
+
+    # Ignore .launch.py files to avoid collection failures for launch_pytest tests
+    if path_obj.name.endswith('.launch.py'):
+        return True
+
+    # Ignore standard library collisions (launch.logging.handlers vs logging.handlers)
+    if path_obj.name == 'handlers.py' and path_obj.parent.name == 'logging':
+        return True
+
+    return False
+
+
+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, path=path, parent=parent, entrypoint=entrypoint
         )
         if module is not None:
             return module
 
-    if _pytest_version_ge(7):
-        path = pathlib.Path(path)
-        if path.name == '__init__.py':
-            return pytest.Package.from_parent(parent, path=path)
-        return pytest.Module.from_parent(parent=parent, path=path)
-    elif _pytest_version_ge(5, 4):
-        if path.basename == '__init__.py':
-            return pytest.Package.from_parent(parent, fspath=path)
-        return pytest.Module.from_parent(parent, fspath=path)
-    else:
-        # todo: remove fallback once all platforms use pytest >=5.4
-        if path.basename == '__init__.py':
-            return pytest.Package(path, parent)
-        return pytest.Module(path, parent)
+    # If it's not a launch test, still return a standard Module/Package
+    # but only if it matches standard test patterns to avoid aggressive collection.
+    path_obj = pathlib.Path(path)
+    if path_obj.name.startswith('test_') or path_obj.name.endswith('_test.py') or \
+       path_obj.name == '__init__.py':
+        if _pytest_version_ge(7):
+            if path_obj.name == '__init__.py':
+                return pytest.Package.from_parent(parent, path=path_obj)
+            return pytest.Module.from_parent(parent=parent, path=path_obj)
+        elif _pytest_version_ge(5, 4):
+            if path_obj.name == '__init__.py':
+                return pytest.Package.from_parent(parent, fspath=path)
+            return pytest.Module.from_parent(parent, fspath=path)
+        else:
+            if path_obj.name == '__init__.py':
+                return pytest.Package(path, parent)
+            return pytest.Module(path, parent)
+
+    return None
 
 
 @pytest.hookimpl(trylast=True)
-def pytest_launch_collect_makemodule(path, parent, entrypoint):
+def pytest_launch_collect_makemodule(module_path, path, parent, entrypoint):
+    p = module_path or path
+    if _pytest_version_ge(7):
+        path_obj = pathlib.Path(p)
+        module = LaunchTestModule.from_parent(parent=parent, path=path_obj)
+    else:
+        module = LaunchTestModule.from_parent(parent=parent, fspath=p)
+
     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)
-            module = LaunchTestModule.from_parent(parent=parent, path=path)
-        else:
-            module = LaunchTestModule.from_parent(parent=parent, fspath=path)
-        for mark in marks:
-            decorator = getattr(pytest.mark, mark.name)
-            decorator = decorator.with_args(*mark.args, **mark.kwargs)
-            module.add_marker(decorator)
-        return module
+    for mark in marks:
+        decorator = getattr(pytest.mark, mark.name)
+        decorator = decorator.with_args(*mark.args, **mark.kwargs)
+        module.add_marker(decorator)
+    return module
 
 
 def pytest_addhooks(pluginmanager):

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, path, parent, entrypoint):
     """Make launch test module appropriate for the found test entrypoint."""
     pass