Merge branch 'rolling' into feat/argument_check_skipping

launch/CHANGELOG.rst

@@ -2,6 +2,27 @@
 Changelog for package launch
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+3.4.1 (2024-03-28)
+------------------
+* Small fixes for modern flake8. (`#772 <https://github.com/ros2/launch/issues/772>`_)
+* Cleanup some type annotations.
+* Contributors: Chris Lalancette
+
+3.4.0 (2024-02-07)
+------------------
+* Rework task exceptions loop. (`#755 <https://github.com/ros2/launch/issues/755>`_)
+* add format overriding by environment variables (`#722 <https://github.com/ros2/launch/issues/722>`_)
+* Add exception type to error output (`#753 <https://github.com/ros2/launch/issues/753>`_)
+* Contributors: Chris Lalancette, David Yackzan, Marc Bestmann
+
+3.3.0 (2024-01-24)
+------------------
+* Let XML executables/nodes be "required" (like in ROS 1) (`#751 <https://github.com/ros2/launch/issues/751>`_)
+* Contributors: Matthew Elwin
+
+3.2.1 (2023-12-26)
+------------------
+
 3.2.0 (2023-10-04)
 ------------------
 * Add conditional substitution (`#734 <https://github.com/ros2/launch/issues/734>`_)

launch/launch/actions/execute_process.py

@@ -22,7 +22,7 @@
 from typing import Text
 
 from .execute_local import ExecuteLocal
-
+from .shutdown_action import Shutdown
 from ..descriptions import Executable
 from ..frontend import Entity
 from ..frontend import expose_action
@@ -331,6 +331,16 @@ def parse(
             if name is not None:
                 kwargs['name'] = parser.parse_substitution(name)
 
+        if 'on_exit' not in ignore:
+            on_exit = entity.get_attr('on_exit', optional=True)
+            if on_exit is not None:
+                if on_exit == 'shutdown':
+                    kwargs['on_exit'] = [Shutdown()]
+                else:
+                    raise ValueError(
+                        'Attribute on_exit of Entity node expected to be shutdown but got `{}`'
+                        'Other on_exit actions not yet supported'.format(on_exit))
+
         if 'prefix' not in ignore:
             prefix = entity.get_attr('launch-prefix', optional=True)
             if prefix is not None:

launch/launch/invalid_launch_file_error.py

@@ -32,7 +32,7 @@ def __init__(self, extension='', *, likely_errors=None):
             ).format('multiple exceptions' if len(self._likely_errors) > 1 else 'exception',
                      self._extension)
             for error in self._likely_errors:
-                self._error_message += '\n - {}'.format(error)
+                self._error_message += '\n - {}: {}'.format(type(error).__name__, error)
 
             self.__cause__ = self._likely_errors[0]
 

launch/launch/launch_context.py

@@ -52,24 +52,24 @@ def __init__(
         self.__argv = argv if argv is not None else []
         self.__noninteractive = noninteractive
 
-        self._event_queue = asyncio.Queue()  # type: asyncio.Queue
-        self._event_handlers = collections.deque()  # type: collections.deque
-        self._completion_futures = []  # type: List[asyncio.Future]
+        self._event_queue: asyncio.Queue = asyncio.Queue()
+        self._event_handlers: collections.deque = collections.deque()
+        self._completion_futures: List[asyncio.Future] = []
 
-        self.__globals = {}  # type: Dict[Text, Any]
-        self.__locals_stack = []  # type: List[Dict[Text, Any]]
-        self.__locals = {}  # type: Dict[Text, Any]
-        self.__combined_locals_cache = None  # type: Optional[Dict[Text, Any]]
+        self.__globals: Dict[Text, Any] = {}
+        self.__locals_stack: List[Dict[Text, Any]] = []
+        self.__locals: Dict[Text, Any] = {}
+        self.__combined_locals_cache: Optional[Dict[Text, Any]] = None
 
-        self.__launch_configurations_stack = []  # type: List[Dict[Text, Text]]
-        self.__launch_configurations = {}  # type: Dict[Text, Text]
+        self.__launch_configurations_stack: List[Dict[Text, Text]] = []
+        self.__launch_configurations: Dict[Text, Text] = {}
 
-        self.__environment_stack = []  # type: List[Mapping[Text, Text]]
+        self.__environment_stack: List[Mapping[Text, Text]] = []
         # We will reset to this copy when "reset_environment" is called
-        self.__environment_reset = os.environ.copy()  # type: Mapping[Text, Text]
+        self.__environment_reset: Mapping[Text, Text] = os.environ.copy()
 
         self.__is_shutdown = False
-        self.__asyncio_loop = None  # type: Optional[asyncio.AbstractEventLoop]
+        self.__asyncio_loop: Optional[asyncio.AbstractEventLoop] = None
 
         self.__logger = launch.logging.get_logger(__name__)
 

launch/launch/launch_service.py

@@ -327,14 +327,20 @@ def _on_exception(loop, context):
                         return_when=asyncio.FIRST_COMPLETED
                     )
                     # Propagate exception from completed tasks
-                    completed_tasks_exceptions = [task.exception() for task in completed_tasks]
-                    completed_tasks_exceptions = list(filter(None, completed_tasks_exceptions))
-                    if completed_tasks_exceptions:
-                        self.__logger.debug('An exception was raised in an async action/event')
-                        # in case there is more than one completed_task, log other exceptions
-                        for completed_tasks_exception in completed_tasks_exceptions[1:]:
-                            self.__logger.error(completed_tasks_exception)
-                        raise completed_tasks_exceptions[0]
+                    exception_to_raise = None
+                    for task in completed_tasks:
+                        exc = task.exception()
+                        if exc is None:
+                            continue
+
+                        if exception_to_raise is None:
+                            self.__logger.debug('An exception was raised in an async action/event')
+                            exception_to_raise = exc
+                        else:
+                            self.__logger.error(exc)
+
+                    if exception_to_raise is not None:
+                        raise exception_to_raise
 
                 except KeyboardInterrupt:
                     continue

launch/launch/logging/__init__.py

@@ -206,10 +206,21 @@ def set_screen_format(self, screen_format, *, screen_style=None):
         :param screen_format: format specification used when logging to the screen,
             as expected by the `logging.Formatter` constructor.
             Alternatively, aliases for common formats are available, see above.
+            This format can also be overridden by the environment variable
+            'OVERRIDE_LAUNCH_SCREEN_FORMAT'.
         :param screen_style: the screen style used if no alias is used for
             screen_format.
             No style can be provided if a format alias is given.
         """
+        # Check if the environment variable is set
+        screen_format_env = os.environ.get('OVERRIDE_LAUNCH_SCREEN_FORMAT')
+        # If the environment variable is set override the given format
+        if screen_format_env not in [None, '']:
+            # encoded escape characters correctly
+            screen_format = screen_format_env.encode(
+                'latin1').decode('unicode_escape')
+            # Set the style correspondingly
+            screen_style = '{'
         if screen_format is not None:
             if screen_format == 'default':
                 screen_format = '[{levelname}] [{name}]: {msg}'
@@ -258,9 +269,20 @@ def set_log_format(self, log_format, *, log_style=None):
             as expected by the `logging.Formatter` constructor.
             Alternatively, the 'default' alias can be given to log verbosity level,
             logger name and logged message.
+            This format can also be overridden by the environment variable
+            'OVERRIDE_LAUNCH_LOG_FORMAT'.
         :param log_style: the log style used if no alias is given for log_format.
             No style can be provided if a format alias is given.
         """
+        # Check if the environment variable is set
+        log_format_env = os.environ.get('OVERRIDE_LAUNCH_LOG_FORMAT')
+        # If the environment variable is set override the given format
+        if log_format_env not in [None, '']:
+            # encoded escape characters correctly
+            log_format = log_format_env.encode(
+                'latin1').decode('unicode_escape')
+            # Set the style correspondingly
+            log_style = '{'
         if log_format is not None:
             if log_format == 'default':
                 log_format = '{created:.7f} [{levelname}] [{name}]: {msg}'
@@ -489,7 +511,7 @@ def get_output_loggers(process_name, output_config):
 
 # Mypy does not support dynamic base classes, so workaround by typing the base
 # class as Any
-_Base = logging.getLoggerClass()  # type: Any
+_Base: Any = logging.getLoggerClass()
 
 
 # Track all loggers to support module resets

launch/package.xml

@@ -2,7 +2,7 @@
 <?xml-model href="http://download.ros.org/schema/package_format2.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
 <package format="2">
   <name>launch</name>
-  <version>3.2.0</version>
+  <version>3.4.1</version>
   <description>The ROS launch tool.</description>
 
   <maintainer email="[email protected]">Aditya Pande</maintainer>

launch/setup.py

@@ -5,7 +5,7 @@
 
 setup(
     name=package_name,
-    version='3.2.0',
+    version='3.4.1',
     packages=find_packages(exclude=['test']),
     data_files=[
         ('share/' + package_name, ['package.xml']),

launch/test/launch/actions/test_push_and_pop_environment.py

@@ -33,7 +33,7 @@ def test_push_and_pop_environment_constructors():
 @sandbox_environment_variables
 def test_push_and_pop_environment_execute():
     """Test the execute() of the PopEnvironment and PushEnvironment classes."""
-    assert type(os.environ) == os._Environ
+    assert isinstance(os.environ, os._Environ)
 
     context = LaunchContext()
 
@@ -89,4 +89,4 @@ def test_push_and_pop_environment_execute():
     assert context.environment['foo'] == 'FOO'
 
     # Pushing and popping the environment should not change the type of os.environ
-    assert type(os.environ) == os._Environ
+    assert isinstance(os.environ, os._Environ)

launch/test/launch/frontend/test_substitutions.py

@@ -21,6 +21,7 @@
 from launch import SomeSubstitutionsType
 from launch import Substitution
 from launch.actions import ExecuteProcess
+from launch.frontend import Parser
 from launch.frontend.expose import expose_substitution
 from launch.frontend.parse_substitution import parse_if_substitutions
 from launch.frontend.parse_substitution import parse_substitution
@@ -317,13 +318,13 @@ def test_parse_if_substitutions():
         parse_if_substitutions(['$(test asd)', 1, 1.0])
 
 
-class MockParser:
+class MockParser(Parser):
 
-    def parse_substitution(self, value: Text) -> SomeSubstitutionsType:
+    def parse_substitution(self, value: Text) -> List[Substitution]:
         return parse_substitution(value)
 
 
-def test_execute_process_parse_cmd_line():
+def test_execute_process_parse_cmd_line() -> None:
     """Test ExecuteProcess._parse_cmd_line."""
     parser = MockParser()
 

launch/test/launch/test_invalid_launch_file_error.py

@@ -0,0 +1,33 @@
+# Copyright 2024 Open Source Robotics Foundation, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from launch.invalid_launch_file_error import InvalidLaunchFileError
+
+
+def test_invalid_launch_file_error():
+    try:
+        exception = KeyError('Test')
+        raise InvalidLaunchFileError(extension='.py', likely_errors=[exception])
+    except InvalidLaunchFileError as ex:
+        assert 'KeyError' in ex.__str__()
+
+
+def test_invalid_launch_file_errors():
+    try:
+        exceptions = [ValueError('Test1'), AttributeError('Test2'), BufferError('Test3')]
+        raise InvalidLaunchFileError(extension='.py', likely_errors=exceptions)
+    except InvalidLaunchFileError as ex:
+        assert 'ValueError' in ex.__str__()
+        assert 'AttributeError' in ex.__str__()
+        assert 'BufferError' in ex.__str__()

launch/test/launch/test_logging.py

@@ -31,7 +31,13 @@ def log_dir(tmpdir_factory):
     return str(tmpdir_factory.mktemp('logs'))
 
 
-def test_bad_logging_launch_config():
+@pytest.fixture
+def mock_clean_env(monkeypatch):
+    monkeypatch.delenv('OVERRIDE_LAUNCH_SCREEN_FORMAT', raising=False)
+    monkeypatch.delenv('OVERRIDE_LAUNCH_LOG_FORMAT', raising=False)
+
+
+def test_bad_logging_launch_config(mock_clean_env):
     """Tests that setup throws at bad configuration."""
     launch.logging.reset()
 
@@ -83,7 +89,7 @@ def test_output_loggers_bad_configuration(log_dir):
         },
     )
 ])
-def test_output_loggers_configuration(capsys, log_dir, config, checks):
+def test_output_loggers_configuration(capsys, log_dir, config, checks, mock_clean_env):
     checks = {'stdout': set(), 'stderr': set(), 'both': set(), **checks}
     launch.logging.reset()
     launch.logging.launch_config.log_dir = log_dir
@@ -162,7 +168,7 @@ def test_output_loggers_configuration(capsys, log_dir, config, checks):
         assert (not os.path.exists(own_log_path) or 0 == os.stat(own_log_path).st_size)
 
 
-def test_screen_default_format_with_timestamps(capsys, log_dir):
+def test_screen_default_format_with_timestamps(capsys, log_dir, mock_clean_env):
     """Test screen logging when using the default logs format with timestamps."""
     launch.logging.reset()
     launch.logging.launch_config.level = logging.DEBUG
@@ -181,7 +187,7 @@ def test_screen_default_format_with_timestamps(capsys, log_dir):
     assert 0 == len(capture.err)
 
 
-def test_screen_default_format(capsys):
+def test_screen_default_format(capsys, mock_clean_env):
     """Test screen logging when using the default logs format."""
     launch.logging.reset()
 
@@ -218,6 +224,25 @@ def test_log_default_format(log_dir):
         assert re.match(r'[0-9]+\.[0-9]+ \[ERROR\] \[some-proc\]: baz', lines[0]) is not None
 
 
+def test_logging_env_var_format(capsys, monkeypatch):
+    monkeypatch.setenv('OVERRIDE_LAUNCH_SCREEN_FORMAT', 'TESTSCREEN {message} {name} TESTSCREEN')
+    monkeypatch.setenv('OVERRIDE_LAUNCH_LOG_FORMAT', 'TESTLOG {message} {name} TESTLOG')
+    launch.logging.reset()
+
+    logger = launch.logging.get_logger('some-proc')
+    logger.addHandler(launch.logging.launch_config.get_screen_handler())
+
+    logger.info('bar')
+    capture = capsys.readouterr()
+    lines = capture.out.splitlines()
+    assert 'TESTSCREEN bar some-proc TESTSCREEN' == lines.pop()
+
+    launch.logging.launch_config.get_log_file_handler().flush()
+    with open(launch.logging.launch_config.get_log_file_path(), 'r') as f:
+        lines = f.readlines()
+        assert 'TESTLOG bar some-proc TESTLOG\n' == lines[0]
+
+
 def test_log_handler_factory(log_dir):
     """Test logging using a custom log handlers."""
     class TestStreamHandler(launch.logging.handlers.Handler):

launch_pytest/CHANGELOG.rst

@@ -2,6 +2,20 @@
 Changelog for package launch_pytest
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+3.4.1 (2024-03-28)
+------------------
+* Switch tryfirst/trylast to hookimpl.
+* Contributors: Chris Lalancette
+
+3.4.0 (2024-02-07)
+------------------
+
+3.3.0 (2024-01-24)
+------------------
+
+3.2.1 (2023-12-26)
+------------------
+
 3.2.0 (2023-10-04)
 ------------------
 

launch_pytest/launch_pytest/plugin.py

@@ -213,7 +213,7 @@ def from_parent(cls, *args, **kwargs):
 # Part of this function was adapted from
 # https://github.com/pytest-dev/pytest-asyncio/blob/f21e0da345f877755b89ff87b6dcea70815b4497/pytest_asyncio/plugin.py#L37-L50.
 # See their license https://github.com/pytest-dev/pytest-asyncio/blob/master/LICENSE.
-@pytest.mark.tryfirst
+@pytest.hookimpl(tryfirst=True)
 def pytest_pycollect_makeitem(collector, name, obj):
     """Collect coroutine based launch tests."""
     if collector.funcnamefilter(name) and is_valid_test_item(obj):
@@ -275,7 +275,7 @@ def get_fixture_params(item):
     return get_fixture_params(left_item) == get_fixture_params(right_item)
 
 
-@pytest.mark.trylast
+@pytest.hookimpl(trylast=True)
 def pytest_collection_modifyitems(session, config, items):
     """Move shutdown tests after normal tests."""
     def enumerate_reversed(sequence):

launch_pytest/package.xml

@@ -2,7 +2,7 @@
 <?xml-model href="http://download.ros.org/schema/package_format2.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
 <package format="2">
   <name>launch_pytest</name>
-  <version>3.2.0</version>
+  <version>3.4.1</version>
   <description>A package to create tests which involve launch files and multiple processes.</description>
 
   <maintainer email="[email protected]">Aditya Pande</maintainer>

launch_pytest/setup.py

@@ -9,7 +9,7 @@
 
 setup(
     name=package_name,
-    version='3.2.0',
+    version='3.4.1',
     packages=find_packages(exclude=['test']),
     data_files=[
         ('share/ament_index/resource_index/packages', [f'resource/{package_name}']),