diff --git a/launch/conftest.py b/launch/conftest.py index 47348f5e4..c12837230 100644 --- a/launch/conftest.py +++ b/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) diff --git a/launch/launch/event_handlers/on_process_exit.py b/launch/launch/event_handlers/on_process_exit.py index 58b55474d..968e6599a 100644 --- a/launch/launch/event_handlers/on_process_exit.py +++ b/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 diff --git a/launch/launch/logging/__init__.py b/launch/launch/logging/__init__.py index bc058a161..b0bd2b24b 100644 --- a/launch/launch/logging/__init__.py +++ b/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. diff --git a/launch/pytest.ini b/launch/pytest.ini index 354fda3cb..0b30d72a5 100644 --- a/launch/pytest.ini +++ b/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 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..266a2b416 100644 --- a/launch_testing/launch_testing/pytest/hooks.py +++ b/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): diff --git a/launch_testing/launch_testing/pytest/hookspecs.py b/launch_testing/launch_testing/pytest/hookspecs.py index 89249d5ea..c35e5c3b8 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, path, parent, entrypoint): """Make launch test module appropriate for the found test entrypoint.""" pass