From 2e954cea2106e349a3cdbe6feb5fac9d69503730 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Mon, 28 Jan 2019 17:54:23 -0300 Subject: [PATCH 01/17] Emits events.ExecutionComplete on Action completion. Signed-off-by: Michel Hidalgo --- launch/launch/action.py | 16 ++- launch/launch/actions/execute_process.py | 2 +- launch/launch/event_handlers/__init__.py | 2 + .../event_handlers/on_execution_complete.py | 122 ++++++++++++++++++ launch/launch/events/__init__.py | 6 +- launch/launch/events/execution_complete.py | 32 +++++ launch/launch/events/matchers.py | 24 ++++ launch/launch/events/process/__init__.py | 2 - .../launch/events/process/process_matchers.py | 5 - .../events/process/process_targeted_event.py | 2 +- 10 files changed, 202 insertions(+), 11 deletions(-) create mode 100644 launch/launch/event_handlers/on_execution_complete.py create mode 100644 launch/launch/events/execution_complete.py create mode 100644 launch/launch/events/matchers.py diff --git a/launch/launch/action.py b/launch/launch/action.py index e136e33f0..85d987bb3 100644 --- a/launch/launch/action.py +++ b/launch/launch/action.py @@ -56,7 +56,21 @@ def describe(self) -> Text: def visit(self, context: LaunchContext) -> Optional[List[LaunchDescriptionEntity]]: """Override visit from LaunchDescriptionEntity so that it executes.""" if self.__condition is None or self.__condition.evaluate(context): - return cast(Optional[List[LaunchDescriptionEntity]], self.execute(context)) + try: + return cast(Optional[List[LaunchDescriptionEntity]], self.execute(context)) + finally: + from .events import ExecutionComplete # noqa + future = self.get_asyncio_future() + if future is not None: + future.add_done_callback( + lambda _: context.emit_event_sync( + ExecutionComplete(action=self) + ) + ) + else: + context.emit_event_sync( + ExecutionComplete(action=self) + ) return None def execute(self, context: LaunchContext) -> Optional[List['Action']]: diff --git a/launch/launch/actions/execute_process.py b/launch/launch/actions/execute_process.py index ad11f3321..0e42da5f4 100644 --- a/launch/launch/actions/execute_process.py +++ b/launch/launch/actions/execute_process.py @@ -42,7 +42,7 @@ from ..event_handler import EventHandler from ..event_handlers import OnShutdown from ..events import Shutdown -from ..events.process import matches_action +from ..events import matches_action from ..events.process import ProcessExited from ..events.process import ProcessStarted from ..events.process import ProcessStderr diff --git a/launch/launch/event_handlers/__init__.py b/launch/launch/event_handlers/__init__.py index e6753c3fa..ff3f68355 100644 --- a/launch/launch/event_handlers/__init__.py +++ b/launch/launch/event_handlers/__init__.py @@ -15,6 +15,7 @@ """Package for event_handlers.""" from .event_named import event_named +from .on_execution_complete import OnExecutionComplete from .on_include_launch_description import OnIncludeLaunchDescription from .on_process_exit import OnProcessExit from .on_process_io import OnProcessIO @@ -22,6 +23,7 @@ __all__ = [ 'event_named', + 'OnExecutionComplete', 'OnIncludeLaunchDescription', 'OnProcessExit', 'OnProcessIO', diff --git a/launch/launch/event_handlers/on_execution_complete.py b/launch/launch/event_handlers/on_execution_complete.py new file mode 100644 index 000000000..2d6cce8f9 --- /dev/null +++ b/launch/launch/event_handlers/on_execution_complete.py @@ -0,0 +1,122 @@ +# Copyright 2019 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. + +import collections.abc +from typing import Callable +from typing import cast +from typing import List # noqa +from typing import Optional +from typing import overload +from typing import Text + +from ..event import Event +from ..events import ExecutionComplete +from ..event_handler import EventHandler +from ..launch_context import LaunchContext +from ..launch_description_entity import LaunchDescriptionEntity +from ..some_actions_type import SomeActionsType + + +class OnExecutionComplete(EventHandler): + """ + Convenience class for handling an action completion event. + + It may be configured to only handle the completion of a specific action, + or to handle them all. + """ + + @overload + def __init__( + self, *, + target_action: Optional['Action'] = None, + on_completion: SomeActionsType, + **kwargs + ) -> None: + """Overload which takes just actions.""" + ... + + @overload # noqa: F811 + def __init__( + self, + *, + target_action: Optional['Action'] = None, + on_completion: Callable[[int], Optional[SomeActionsType]], + **kwargs + ) -> None: + """Overload which takes a callable to handle completion.""" + ... + + def __init__(self, *, target_action=None, on_completion, **kwargs) -> None: # noqa: F811 + """Constructor.""" + from ..action import Action # noqa + if not isinstance(target_action, (Action, type(None))): + raise RuntimeError("OnExecutionComplete requires an 'Action' as the target") + super().__init__( + matcher=( + lambda event: ( + isinstance(event, ExecutionComplete) and ( + target_action is None or + event.action == target_action + ) + ) + ), + entities=None, + **kwargs, + ) + self.__target_action = target_action + # TODO(wjwwood) check that it is not only callable, but also a callable that matches + # the correct signature for a handler in this case + self.__on_completion = on_completion + self.__actions_on_completion = [] # type: List[LaunchDescriptionEntity] + if callable(on_completion): + # Then on_completion is a function or lambda, so we can just call it, but + # we don't put anything in self.__actions_on_completion because we cannot + # know what the function will return. + pass + else: + # Otherwise, setup self.__actions_on_completion + if isinstance(on_completion, collections.abc.Iterable): + for entity in on_completion: + if not isinstance(entity, LaunchDescriptionEntity): + raise ValueError( + "expected all items in 'on_completion' iterable to be of type " + "'LaunchDescriptionEntity' but got '{}'".format(type(entity))) + self.__actions_on_completion = list(on_completion) + else: + self.__actions_on_completion = [on_completion] + # Then return it from a lambda and use that as the self.__on_completion callback. + self.__on_completion = lambda event, context: self.__actions_on_completion + + def handle(self, event: Event, context: LaunchContext) -> Optional[SomeActionsType]: + """Handle the given event.""" + return self.__on_completion(cast(ExecutionComplete, event), context) + + @property + def handler_description(self) -> Text: + """Return the string description of the handler.""" + # TODO(jacobperron): revisit how to describe known actions that are passed in. + # It would be nice if the parent class could output their description + # via the 'entities' property. + if self.__actions_on_completion: + return '' + return '{}'.format(self.__on_exit) + + @property + def matcher_description(self) -> Text: + """Return the string description of the matcher.""" + if self.__target_action is None: + return 'event == ExecutionComplete' + return 'event == ExecutionComplete and event.action == Action({})'.format( + hex(id(self.__target_action)) + ) diff --git a/launch/launch/events/__init__.py b/launch/launch/events/__init__.py index 3c37d73a2..39e4c148b 100644 --- a/launch/launch/events/__init__.py +++ b/launch/launch/events/__init__.py @@ -1,4 +1,4 @@ -# Copyright 2018 Open Source Robotics Foundation, Inc. +# Copyright 2018-2019 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. @@ -15,12 +15,16 @@ """Package for events.""" from . import process +from .execution_complete import ExecutionComplete from .include_launch_description import IncludeLaunchDescription +from .matchers import matches_action from .shutdown import Shutdown from .timer_event import TimerEvent __all__ = [ + 'matches_action', 'process', + 'ExecutionComplete', 'IncludeLaunchDescription', 'Shutdown', 'TimerEvent', diff --git a/launch/launch/events/execution_complete.py b/launch/launch/events/execution_complete.py new file mode 100644 index 000000000..161fd443d --- /dev/null +++ b/launch/launch/events/execution_complete.py @@ -0,0 +1,32 @@ +# Copyright 2019 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. + +"""Module for ExecutionComplete event.""" + +from ..event import Event + + +class ExecutionComplete(Event): + """Event that is emitted on action execution completion.""" + + name = 'launch.events.ExecutionComplete' + + def __init__(self, *, action: 'Action') -> None: + """Constructor.""" + self.__action = action + + @property + def action(self): + """Getter for action.""" + return self.__action diff --git a/launch/launch/events/matchers.py b/launch/launch/events/matchers.py new file mode 100644 index 000000000..a7e8ebd25 --- /dev/null +++ b/launch/launch/events/matchers.py @@ -0,0 +1,24 @@ +# Copyright 2018 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. + +"""Module for standard "matchers", which are used with Events.""" + +from typing import Callable + +from ..action import Action + + +def matches_action(target_action: 'Action') -> Callable[['Action'], bool]: + """Return a matcher which matches based on an exact given ExecuteProcess action.""" + return lambda action: action == target_action diff --git a/launch/launch/events/process/__init__.py b/launch/launch/events/process/__init__.py index 9f7f00d5b..dbdd3a520 100644 --- a/launch/launch/events/process/__init__.py +++ b/launch/launch/events/process/__init__.py @@ -16,7 +16,6 @@ from .process_exited import ProcessExited from .process_io import ProcessIO -from .process_matchers import matches_action from .process_matchers import matches_executable from .process_matchers import matches_name from .process_matchers import matches_pid @@ -30,7 +29,6 @@ from .signal_process import SignalProcess __all__ = [ - 'matches_action', 'matches_executable', 'matches_name', 'matches_pid', diff --git a/launch/launch/events/process/process_matchers.py b/launch/launch/events/process/process_matchers.py index 54ce352bc..f80f50a1a 100644 --- a/launch/launch/events/process/process_matchers.py +++ b/launch/launch/events/process/process_matchers.py @@ -22,11 +22,6 @@ from ...actions import ExecuteProcess # noqa -def matches_action(execute_process_action: 'ExecuteProcess') -> Callable[['ExecuteProcess'], bool]: - """Return a matcher which matches based on an exact given ExecuteProcess action.""" - return lambda action: action == execute_process_action - - def matches_pid(pid: int) -> Callable[['ExecuteProcess'], bool]: """Return a matcher which matches based on the pid of the process.""" def matcher(action: 'ExecuteProcess') -> bool: diff --git a/launch/launch/events/process/process_targeted_event.py b/launch/launch/events/process/process_targeted_event.py index 946990960..617bf8e5d 100644 --- a/launch/launch/events/process/process_targeted_event.py +++ b/launch/launch/events/process/process_targeted_event.py @@ -34,7 +34,7 @@ def __init__(self, *, process_matcher: Callable[['ExecuteProcess'], bool]) -> No Some standard matchers are also available, like: - - :func:`launch.events.process.matches_action()` + - :func:`launch.events.matches_action()` - :func:`launch.events.process.matches_pid()` - :func:`launch.events.process.matches_name()` - :func:`launch.events.process.matches_executable()` From 9398abae70ddba337e7f8b14081106c6586a7a2f Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 29 Jan 2019 12:39:41 -0300 Subject: [PATCH 02/17] Makes LaunchTestService handle non-process actions. Signed-off-by: Michel Hidalgo --- launch_testing/launch_testing/__init__.py | 61 +++++++++++++---------- 1 file changed, 35 insertions(+), 26 deletions(-) diff --git a/launch_testing/launch_testing/__init__.py b/launch_testing/launch_testing/__init__.py index a553616b7..99296a808 100644 --- a/launch_testing/launch_testing/__init__.py +++ b/launch_testing/launch_testing/__init__.py @@ -18,6 +18,7 @@ from launch.actions import ExecuteProcess from launch.actions import RegisterEventHandler from launch.event_handlers import OnProcessExit +from launch.event_handlers import OnExecutionComplete from launch.events import Shutdown # for backward compatibility @@ -32,37 +33,48 @@ class LaunchTestService(): def __init__(self): - self.__test_processes = [] - self.__test_returncodes = OrderedDict() + self.__test_action_complete = OrderedDict() + self.__test_action_rc = OrderedDict() def add_test_action(self, launch_description, action): """ Add action used for testing. - If either all test actions exited with a return code of zero or any - test action exited with a non-zero return code a shutdown event is - emitted. + If either all test actions have completed or a process action has + exited with a non-zero return code, a shutdown event is emitted. """ - assert isinstance(action, ExecuteProcess), \ - 'The passed action must be a ExecuteProcess action' + launch_description.add_action(action) + self.__test_action_complete[action] = False + if isinstance(action, ExecuteProcess): + def on_test_process_exit(event, context): + self.__test_action_rc[event.action] = event.returncode + if event.returncode != 0: + return EmitEvent(Shutdown( + reason='{} test action failed!'.format( + event.action.process_details['name'] + ) + )) - self.__test_processes.append(action) + self.__test_action_complete[event.action] = True + if all(self.__test_action_complete.values()): + return EmitEvent(Shutdown(reason='all test actions finished')) - def on_test_process_exit(event, context): - nonlocal action - nonlocal self - self.__test_returncodes[action] = event.returncode + launch_description.add_action( + RegisterEventHandler(OnProcessExit( + target_action=action, on_exit=on_test_process_exit + )) + ) + else: + def on_test_action_complete(event, context): + self.__test_action_complete[event.action] = True + if all(self.__test_action_complete.values()): + return EmitEvent(Shutdown(reason='all test actions finished')) - if len(self.__test_returncodes) == len(self.__test_processes): - shutdown_event = Shutdown( - reason='all test processes finished') - return EmitEvent(event=shutdown_event) - - launch_description.add_action( - RegisterEventHandler(OnProcessExit( - target_action=action, on_exit=on_test_process_exit, - )) - ) + launch_description.add_action( + RegisterEventHandler(OnExecutionComplete( + target_action=action, on_completion=on_test_action_complete + )) + ) def run(self, launch_service, *args, **kwargs): """ @@ -74,8 +86,5 @@ def run(self, launch_service, *args, **kwargs): """ rc = launch_service.run(*args, **kwargs) if not rc: - for test_process_rc in self.__test_returncodes.values(): - if test_process_rc: - rc = test_process_rc - break + rc = next((rc for rc in self.__test_action_rc.values() if rc), rc) return rc From b9f3bae795d4bd53ba7e6ae258f36e00fa46c854 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 29 Jan 2019 13:21:45 -0300 Subject: [PATCH 03/17] Makes LaunchTestService handle fixture process actions. Signed-off-by: Michel Hidalgo --- launch_testing/launch_testing/__init__.py | 40 +++++++++++++++++++---- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/launch_testing/launch_testing/__init__.py b/launch_testing/launch_testing/__init__.py index 99296a808..2d08fa488 100644 --- a/launch_testing/launch_testing/__init__.py +++ b/launch_testing/launch_testing/__init__.py @@ -34,7 +34,29 @@ class LaunchTestService(): def __init__(self): self.__test_action_complete = OrderedDict() - self.__test_action_rc = OrderedDict() + self.__test_processes_rc = OrderedDict() + + def add_fixture_action(self, launch_description, action): + """ + Add action used as testing fixture. + + If a process action and it exits, a shutdown event is emitted. + """ + + launch_description.add_action(action) + if isinstance(action, ExecuteProcess): + def on_fixture_process_exit(event, context): + self.__test_processes_rc[event.action] = event.returncode + return EmitEvent(event=Shutdown( + reason='{} fixture process died!'.format( + event.action.process_details['name'] + ) + )) + launch_description.add_action( + RegisterEventHandler(OnProcessExit( + target_action=action, on_exit=on_fixture_process_exit + )) + ) def add_test_action(self, launch_description, action): """ @@ -47,9 +69,9 @@ def add_test_action(self, launch_description, action): self.__test_action_complete[action] = False if isinstance(action, ExecuteProcess): def on_test_process_exit(event, context): - self.__test_action_rc[event.action] = event.returncode if event.returncode != 0: - return EmitEvent(Shutdown( + self.__test_processes_rc[event.action] = event.returncode + return EmitEvent(event=Shutdown( reason='{} test action failed!'.format( event.action.process_details['name'] ) @@ -57,7 +79,9 @@ def on_test_process_exit(event, context): self.__test_action_complete[event.action] = True if all(self.__test_action_complete.values()): - return EmitEvent(Shutdown(reason='all test actions finished')) + return EmitEvent(event=Shutdown( + reason='all test actions finished' + )) launch_description.add_action( RegisterEventHandler(OnProcessExit( @@ -68,7 +92,9 @@ def on_test_process_exit(event, context): def on_test_action_complete(event, context): self.__test_action_complete[event.action] = True if all(self.__test_action_complete.values()): - return EmitEvent(Shutdown(reason='all test actions finished')) + return EmitEvent(event=Shutdown( + reason='all test actions finished' + )) launch_description.add_action( RegisterEventHandler(OnExecutionComplete( @@ -85,6 +111,6 @@ def run(self, launch_service, *args, **kwargs): the first failed test process is returned. """ rc = launch_service.run(*args, **kwargs) - if not rc: - rc = next((rc for rc in self.__test_action_rc.values() if rc), rc) + if rc == 0: + rc = next((rc for rc in self.__test_processes_rc.values() if rc), rc) return rc From f4faf9184d3a6607c2974a5f193868d30c6d8a92 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 29 Jan 2019 15:06:54 -0300 Subject: [PATCH 04/17] Adds output checks to LaunchTestService. Signed-off-by: Michel Hidalgo --- launch_testing/launch_testing/__init__.py | 114 +++++++++++++++------- launch_testing/launch_testing/output.py | 105 ++++++++++++++++++++ 2 files changed, 185 insertions(+), 34 deletions(-) create mode 100644 launch_testing/launch_testing/output.py diff --git a/launch_testing/launch_testing/__init__.py b/launch_testing/launch_testing/__init__.py index 2d08fa488..12f4711b9 100644 --- a/launch_testing/launch_testing/__init__.py +++ b/launch_testing/launch_testing/__init__.py @@ -18,23 +18,31 @@ from launch.actions import ExecuteProcess from launch.actions import RegisterEventHandler from launch.event_handlers import OnProcessExit +from launch.event_handlers import OnProcessIO from launch.event_handlers import OnExecutionComplete from launch.events import Shutdown -# for backward compatibility -from launch_testing.legacy import create_handler # noqa: F401 -from launch_testing.legacy import get_default_filtered_patterns # noqa: F401 -from launch_testing.legacy import get_default_filtered_prefixes # noqa: F401 -from launch_testing.legacy import get_rmw_output_filter # noqa: F401 -from launch_testing.legacy import InMemoryHandler # noqa: F401 -from launch_testing.legacy import UnmatchedOutputError # noqa: F401 +from .output import create_output_check class LaunchTestService(): def __init__(self): - self.__test_action_complete = OrderedDict() - self.__test_processes_rc = OrderedDict() + self.__tests = OrderedDict() + self.__processes_rc = OrderedDict() + + def _arm(self, test_name): + assert test_name not in self.__tests + self.__tests[test_name] = 'armed' + + def _fail(self, test_name, reason): + self.__tests[test_name] = 'failed' + return EmitEvent(event=Shutdown(reason=reason)) + + def _succeed(self, test_name): + self.__tests[test_name] = 'succeeded' + if all(status == 'succeeded' for status in self.__tests.values()): + return EmitEvent(event=Shutdown(reason='all tests finished')) def add_fixture_action(self, launch_description, action): """ @@ -44,19 +52,20 @@ def add_fixture_action(self, launch_description, action): """ launch_description.add_action(action) + fixture_name = 'fixture_{}'.format(id(action)) if isinstance(action, ExecuteProcess): def on_fixture_process_exit(event, context): - self.__test_processes_rc[event.action] = event.returncode + process_name = event.action.process_details['name'] + self.__processes_rc[process_name] = event.returncode return EmitEvent(event=Shutdown( - reason='{} fixture process died!'.format( - event.action.process_details['name'] - ) + reason='{} fixture process died!'.format(process_name) )) launch_description.add_action( RegisterEventHandler(OnProcessExit( target_action=action, on_exit=on_fixture_process_exit )) ) + return action def add_test_action(self, launch_description, action): """ @@ -66,22 +75,18 @@ def add_test_action(self, launch_description, action): exited with a non-zero return code, a shutdown event is emitted. """ launch_description.add_action(action) - self.__test_action_complete[action] = False + test_name = 'test_{}'.format(id(action)) if isinstance(action, ExecuteProcess): def on_test_process_exit(event, context): if event.returncode != 0: - self.__test_processes_rc[event.action] = event.returncode - return EmitEvent(event=Shutdown( - reason='{} test action failed!'.format( - event.action.process_details['name'] + process_name = event.action.process_details['name'] + self._processes_rc[process_name] = event.returncode + return self._fail( + test_name, reason='{} test failed!'.format( + process_name ) - )) - - self.__test_action_complete[event.action] = True - if all(self.__test_action_complete.values()): - return EmitEvent(event=Shutdown( - reason='all test actions finished' - )) + ) + return self._succeed(test_name) launch_description.add_action( RegisterEventHandler(OnProcessExit( @@ -89,18 +94,59 @@ def on_test_process_exit(event, context): )) ) else: - def on_test_action_complete(event, context): - self.__test_action_complete[event.action] = True - if all(self.__test_action_complete.values()): - return EmitEvent(event=Shutdown( - reason='all test actions finished' - )) - launch_description.add_action( RegisterEventHandler(OnExecutionComplete( - target_action=action, on_completion=on_test_action_complete + target_action=action, on_completion=( + lambda *args: self._succeed(test_name) + ) )) ) + self._arm(test_name) + return action + + def add_output_test(self, launch_description, action, output_file, + filtered_prefixes=None, filtered_patterns=None, + filtered_rmw_implementation=None): + """ + Tests an action process' output against text or regular expressions. + + :param launch_description: test launch description that owns the given action. + :param action: launch action to test whose output is to be tested. + :param output_file: basename (i.e. w/o extension) of either a .txt file containing the + lines to be matched or a .regex file containing patterns to be searched for. + :param filtered_prefixes: A list of byte strings representing prefixes that will cause output + lines to be ignored if they start with one of the prefixes. By default lines starting with + the process ID (`b'pid'`) and return code (`b'rc'`) will be ignored. + :param filtered_patterns: A list of byte strings representing regexes that will cause output + lines to be ignored if they match one of the regexes. + :param filtered_rmw_implementation: RMW implementation for which the output will be ignored + in addition to the `filtered_prefixes`/`filtered_patterns`. + """ + assert isinstance(action, ExecuteProcess) + test_name = 'test_{}_output'.format(id(action)) + + output, collate_output, match_output, match_patterns = create_output_check( + output_file, filtered_prefixes, filtered_patterns, filtered_rmw_implementation + ) + + def on_process_stdout(event): + nonlocal output + nonlocal match_patterns + output = collate_output(output, event.text) + match_patterns = [ + pattern for pattern in match_patterns + if not match_output(output, pattern) + ] + if not any(match_patterns): + return self._succeed(test_name) + + launch_description.add_action( + RegisterEventHandler(OnProcessIO( + target_action=action, on_stdout=on_process_stdout + )) + ) + self._arm(test_name) + return action def run(self, launch_service, *args, **kwargs): """ @@ -112,5 +158,5 @@ def run(self, launch_service, *args, **kwargs): """ rc = launch_service.run(*args, **kwargs) if rc == 0: - rc = next((rc for rc in self.__test_processes_rc.values() if rc), rc) + rc = next((rc for rc in self.__processes_rc.values() if rc), rc) return rc diff --git a/launch_testing/launch_testing/output.py b/launch_testing/launch_testing/output.py new file mode 100644 index 000000000..e658ad7f4 --- /dev/null +++ b/launch_testing/launch_testing/output.py @@ -0,0 +1,105 @@ +import io +import os +import re + +import ament_index_python + + +def get_default_filtered_prefixes(): + return [ + b'pid', b'rc', + ] + + +def get_default_filtered_patterns(): + return [] + + +def get_rmw_output_filter(rmw_implementation, filter_type): + supported_filter_types = ['prefixes', 'patterns'] + if filter_type not in supported_filter_types: + raise TypeError( + 'Unsupported filter_type "{0}". Supported types: {1}' + .format(filter_type, supported_filter_types)) + resource_name = 'rmw_output_' + filter_type + prefix_with_resource = ament_index_python.has_resource( + resource_name, rmw_implementation) + if not prefix_with_resource: + return [] + + # Treat each line of the resource as an independent filter. + rmw_output_filter, _ = ament_index_python.get_resource(resource_name, rmw_implementation) + return [str.encode(l) for l in rmw_output_filter.splitlines()] + + +def get_expected_output(output_file): + literal_file = output_file + '.txt' + if os.path.isfile(literal_file): + with open(literal_file, 'rb') as f: + return f.read().splitlines() + regex_file = output_file + '.regex' + if os.path.isfile(regex_file): + with open(regex_file, 'rb') as f: + return f.read().splitlines() + + +def create_output_lines_filter(filtered_prefixes, filtered_patterns, + filtered_rmw_implementation): + filtered_prefixes = filtered_prefixes or get_default_filtered_prefixes() + filtered_patterns = filtered_patterns or get_default_filtered_patterns() + if filtered_rmw_implementation: + filtered_prefixes.extend(get_rmw_output_filter( + filtered_rmw_implementation, 'prefixes' + )) + filtered_patterns.extend(get_rmw_output_filter( + filtered_rmw_implementation, 'patterns' + )) + filtered_patterns = map(re.compile, filtered_patterns) + + def _filter(output): + for line in output.splitlines(): + # Filter out stdout that comes from underlying DDS implementation + # Note: we do not currently support matching filters across multiple stdout lines. + if any(line.startswith(prefix) for prefix in filtered_prefixes): + continue + if any(pattern.match(line) for pattern in filtered_patterns): + continue + yield line + return _filter + + +def create_output_check(output_file, filtered_prefixes, filtered_patterns, + filtered_rmw_implementation=None): + filter_output_lines = create_output_lines_filter( + filtered_prefixes, filtered_patterns, filtered_rmw_implementation + ) + + literal_file = output_file + '.txt' + if os.path.isfile(literal_file): + def _collate(output, addendum): + output.extend(filter_output_lines(addendum)) + return output + + def _match(output, pattern): + return pattern in output + + with open(literal_file, 'rb') as f: + expected_output = f.read().splitlines() + return [], _collate, _match, expected_output + + regex_file = output_file + '.regex' + if os.path.isfile(regex_file): + def _collate(output, addendum): + output.write(b'\n'.join( + filter_output_lines(addendum) + )) + return output + + def _match(output, pattern): + return bool(pattern.search(output)) + + with open(regex_file, 'rb') as f: + patterns = map(re.compile, f.read().splitlines()) + return io.BytesIO(), _collate, _match, patterns + + raise RuntimeError('could not find output check file: {}'.format(output_file)) From 78a567d7da2d679c960b4196fe74f00405351f87 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 29 Jan 2019 15:22:54 -0300 Subject: [PATCH 05/17] Updates launch_testing package tests. Signed-off-by: Michel Hidalgo --- launch_testing/launch_testing/__init__.py | 51 +++++++++++++------- launch_testing/launch_testing/output.py | 2 +- launch_testing/test/test_env_passing.py | 25 +++++----- launch_testing/test/test_matching.py | 57 +++++++++-------------- 4 files changed, 68 insertions(+), 67 deletions(-) diff --git a/launch_testing/launch_testing/__init__.py b/launch_testing/launch_testing/__init__.py index 12f4711b9..3cb915c6d 100644 --- a/launch_testing/launch_testing/__init__.py +++ b/launch_testing/launch_testing/__init__.py @@ -1,4 +1,4 @@ -# Copyright 2018 Open Source Robotics Foundation, Inc. +# Copyright 2018-2019 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. @@ -17,9 +17,9 @@ from launch.actions import EmitEvent from launch.actions import ExecuteProcess from launch.actions import RegisterEventHandler +from launch.event_handlers import OnExecutionComplete from launch.event_handlers import OnProcessExit from launch.event_handlers import OnProcessIO -from launch.event_handlers import OnExecutionComplete from launch.events import Shutdown from .output import create_output_check @@ -37,6 +37,9 @@ def _arm(self, test_name): def _fail(self, test_name, reason): self.__tests[test_name] = 'failed' + for test_name in self.__tests: + if self.__tests[test_name] == 'armed': + self.__tests[test_name] = 'dropped' return EmitEvent(event=Shutdown(reason=reason)) def _succeed(self, test_name): @@ -44,22 +47,22 @@ def _succeed(self, test_name): if all(status == 'succeeded' for status in self.__tests.values()): return EmitEvent(event=Shutdown(reason='all tests finished')) - def add_fixture_action(self, launch_description, action): + def add_fixture_action(self, launch_description, action, required=False): """ Add action used as testing fixture. If a process action and it exits, a shutdown event is emitted. """ - launch_description.add_action(action) - fixture_name = 'fixture_{}'.format(id(action)) if isinstance(action, ExecuteProcess): def on_fixture_process_exit(event, context): process_name = event.action.process_details['name'] - self.__processes_rc[process_name] = event.returncode - return EmitEvent(event=Shutdown( - reason='{} fixture process died!'.format(process_name) - )) + if required or event.returncode != 0: + rc = event.returncode if event.returncode else 1 + self.__processes_rc[process_name] = rc + return EmitEvent(event=Shutdown( + reason='{} fixture process died!'.format(process_name) + )) launch_description.add_action( RegisterEventHandler(OnProcessExit( target_action=action, on_exit=on_fixture_process_exit @@ -108,19 +111,19 @@ def add_output_test(self, launch_description, action, output_file, filtered_prefixes=None, filtered_patterns=None, filtered_rmw_implementation=None): """ - Tests an action process' output against text or regular expressions. + Test an action process' output against text or regular expressions. :param launch_description: test launch description that owns the given action. :param action: launch action to test whose output is to be tested. :param output_file: basename (i.e. w/o extension) of either a .txt file containing the lines to be matched or a .regex file containing patterns to be searched for. - :param filtered_prefixes: A list of byte strings representing prefixes that will cause output - lines to be ignored if they start with one of the prefixes. By default lines starting with - the process ID (`b'pid'`) and return code (`b'rc'`) will be ignored. - :param filtered_patterns: A list of byte strings representing regexes that will cause output - lines to be ignored if they match one of the regexes. - :param filtered_rmw_implementation: RMW implementation for which the output will be ignored - in addition to the `filtered_prefixes`/`filtered_patterns`. + :param filtered_prefixes: A list of byte strings representing prefixes that will cause + output lines to be ignored if they start with one of the prefixes. By default lines + starting with the process ID (`b'pid'`) and return code (`b'rc'`) will be ignored. + :param filtered_patterns: A list of byte strings representing regexes that will cause + output lines to be ignored if they match one of the regexes. + :param filtered_rmw_implementation: RMW implementation for which the output will be + ignored in addition to the `filtered_prefixes`/`filtered_patterns`. """ assert isinstance(action, ExecuteProcess) test_name = 'test_{}_output'.format(id(action)) @@ -129,6 +132,11 @@ def add_output_test(self, launch_description, action, output_file, output_file, filtered_prefixes, filtered_patterns, filtered_rmw_implementation ) + def on_process_exit(event): + nonlocal match_patterns + if any(match_patterns): + return self._fail(test_name) + def on_process_stdout(event): nonlocal output nonlocal match_patterns @@ -140,12 +148,18 @@ def on_process_stdout(event): if not any(match_patterns): return self._succeed(test_name) + launch_description.add_action( + RegisterEventHandler(OnProcessExit( + target_action=action, on_exit=on_process_exit + )) + ) launch_description.add_action( RegisterEventHandler(OnProcessIO( target_action=action, on_stdout=on_process_stdout )) ) self._arm(test_name) + return action def run(self, launch_service, *args, **kwargs): @@ -158,5 +172,6 @@ def run(self, launch_service, *args, **kwargs): """ rc = launch_service.run(*args, **kwargs) if rc == 0: - rc = next((rc for rc in self.__processes_rc.values() if rc), rc) + default_rc = 1 if 'failed' in self.__tests.values() else 0 + rc = next((rc for rc in self.__processes_rc.values() if rc != 0), default_rc) return rc diff --git a/launch_testing/launch_testing/output.py b/launch_testing/launch_testing/output.py index e658ad7f4..35b0c1349 100644 --- a/launch_testing/launch_testing/output.py +++ b/launch_testing/launch_testing/output.py @@ -96,7 +96,7 @@ def _collate(output, addendum): return output def _match(output, pattern): - return bool(pattern.search(output)) + return pattern.search(output.getvalue()) is not None with open(regex_file, 'rb') as f: patterns = map(re.compile, f.read().splitlines()) diff --git a/launch_testing/test/test_env_passing.py b/launch_testing/test/test_env_passing.py index a37716826..fd86a37c3 100644 --- a/launch_testing/test/test_env_passing.py +++ b/launch_testing/test/test_env_passing.py @@ -16,18 +16,20 @@ import os import sys -from launch.legacy import LaunchDescriptor -from launch.legacy.exit_handler import primary_exit_handler -from launch.legacy.launcher import DefaultLauncher +from launch import LaunchDescription +from launch import LaunchService +from launch.actions import ExecuteProcess +from launch_testing import LaunchTestService def test_env(): - ld = LaunchDescriptor() + ld = LaunchDescription() + launch_test = LaunchTestService() sub_env = copy.deepcopy(os.environ) sub_env['testenv1'] = 'testval1' os.environ['testenv2'] = 'testval2' - ld.add_process( + launch_test.add_test_action(ld, ExecuteProcess( cmd=[ sys.executable, os.path.join( @@ -36,14 +38,11 @@ def test_env(): 'check_env.py')], name='test_env', env=sub_env, - exit_handler=primary_exit_handler, - ) - launcher = DefaultLauncher() - launcher.add_launch_descriptor(ld) - rc = launcher.launch() - - assert rc == 0, \ - "The launch file failed with exit code '" + str(rc) + "'. " + )) + launch_service = LaunchService() + launch_service.include_launch_description(ld) + return_code = launch_test.run(launch_service) + assert return_code == 0, 'Launch failed with exit code %r' % (return_code,) if __name__ == '__main__': diff --git a/launch_testing/test/test_matching.py b/launch_testing/test/test_matching.py index dd6f5d75b..6439cb65e 100644 --- a/launch_testing/test/test_matching.py +++ b/launch_testing/test/test_matching.py @@ -16,51 +16,38 @@ import sys import tempfile -from launch.legacy import LaunchDescriptor -from launch.legacy.exit_handler import ignore_exit_handler -from launch.legacy.launcher import DefaultLauncher -from launch_testing import create_handler +from launch import LaunchDescription +from launch import LaunchService +from launch.actions import ExecuteProcess +from launch_testing import LaunchTestService def test_matching(): - output_handlers = [] - - launch_descriptor = LaunchDescriptor() - - # This temporary directory and files contained in it will be deleted when the process ends. + # This temporary directory and files contained in it + # will be deleted when the process ends. tempdir = tempfile.mkdtemp() output_file = tempdir + os.sep + 'testfile' full_output_file = output_file + '.regex' with open(full_output_file, 'w+') as f: f.write(r'this is line \d\nthis is line [a-z]') - name = 'test_executable_0' - - handler = create_handler(name, launch_descriptor, output_file) - - assert handler, 'Cannot find appropriate handler for %s' % output_file - - output_handlers.append(handler) - executable_command = [ - sys.executable, - os.path.join(os.path.abspath(os.path.dirname(__file__)), 'matching.py')] - - launch_descriptor.add_process( - cmd=executable_command, - name=name, - exit_handler=ignore_exit_handler, - output_handlers=output_handlers) - - launcher = DefaultLauncher() - launcher.add_launch_descriptor(launch_descriptor) - rc = launcher.launch() - - assert rc == 0, \ - "The launch file failed with exit code '" + str(rc) + "'. " - - for handler in output_handlers: - handler.check() + sys.executable, os.path.join( + os.path.abspath(os.path.dirname(__file__)), 'matching.py' + ) + ] + + ld = LaunchDescription() + launch_test = LaunchTestService() + action = launch_test.add_fixture_action( + ld, ExecuteProcess(cmd=executable_command) + ) + launch_test.add_output_test(ld, action, output_file) + + launch_service = LaunchService() + launch_service.include_launch_description(ld) + return_code = launch_test.run(launch_service) + assert return_code == 0, 'Launch failed with exit code %r' % (return_code,) if __name__ == '__main__': From 3a65f6593c9ec5682da18be57673be6fd839c659 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 29 Jan 2019 16:24:26 -0300 Subject: [PATCH 06/17] Applies fixes after launch_testing refactor. Signed-off-by: Michel Hidalgo --- launch_testing/launch_testing/__init__.py | 8 +++++--- launch_testing/launch_testing/output.py | 4 ++-- launch_testing/test/test_matching.py | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/launch_testing/launch_testing/__init__.py b/launch_testing/launch_testing/__init__.py index 3cb915c6d..5c2f43b23 100644 --- a/launch_testing/launch_testing/__init__.py +++ b/launch_testing/launch_testing/__init__.py @@ -127,15 +127,17 @@ def add_output_test(self, launch_description, action, output_file, """ assert isinstance(action, ExecuteProcess) test_name = 'test_{}_output'.format(id(action)) - output, collate_output, match_output, match_patterns = create_output_check( output_file, filtered_prefixes, filtered_patterns, filtered_rmw_implementation ) + assert any(match_patterns) - def on_process_exit(event): + def on_process_exit(event, context): nonlocal match_patterns if any(match_patterns): - return self._fail(test_name) + process_name = event.action.process_details['name'] + reason = 'not all {} output matched!'.format(process_name) + return self._fail(test_name, reason) def on_process_stdout(event): nonlocal output diff --git a/launch_testing/launch_testing/output.py b/launch_testing/launch_testing/output.py index 35b0c1349..cbc5df0d5 100644 --- a/launch_testing/launch_testing/output.py +++ b/launch_testing/launch_testing/output.py @@ -69,7 +69,7 @@ def _filter(output): def create_output_check(output_file, filtered_prefixes, filtered_patterns, - filtered_rmw_implementation=None): + filtered_rmw_implementation): filter_output_lines = create_output_lines_filter( filtered_prefixes, filtered_patterns, filtered_rmw_implementation ) @@ -99,7 +99,7 @@ def _match(output, pattern): return pattern.search(output.getvalue()) is not None with open(regex_file, 'rb') as f: - patterns = map(re.compile, f.read().splitlines()) + patterns = [re.compile(regex) for regex in f.read().splitlines()] return io.BytesIO(), _collate, _match, patterns raise RuntimeError('could not find output check file: {}'.format(output_file)) diff --git a/launch_testing/test/test_matching.py b/launch_testing/test/test_matching.py index 6439cb65e..b3be1e07d 100644 --- a/launch_testing/test/test_matching.py +++ b/launch_testing/test/test_matching.py @@ -40,7 +40,7 @@ def test_matching(): ld = LaunchDescription() launch_test = LaunchTestService() action = launch_test.add_fixture_action( - ld, ExecuteProcess(cmd=executable_command) + ld, ExecuteProcess(cmd=executable_command, output='screen') ) launch_test.add_output_test(ld, action, output_file) From a1f7fb7fb7d950c3a6e2fbfe32aa2aae7143f693 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Mon, 28 Jan 2019 11:13:27 -0300 Subject: [PATCH 07/17] Adds OpaqueCoroutine action subclass. Signed-off-by: Michel Hidalgo --- launch/launch/actions/__init__.py | 2 + launch/launch/actions/opaque_coroutine.py | 89 +++++++++++++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 launch/launch/actions/opaque_coroutine.py diff --git a/launch/launch/actions/__init__.py b/launch/launch/actions/__init__.py index 6bbcb4bbc..e73a6ea8e 100644 --- a/launch/launch/actions/__init__.py +++ b/launch/launch/actions/__init__.py @@ -20,6 +20,7 @@ from .group_action import GroupAction from .include_launch_description import IncludeLaunchDescription from .log_info import LogInfo +from .opaque_coroutine import OpaqueCoroutine from .opaque_function import OpaqueFunction from .pop_launch_configurations import PopLaunchConfigurations from .push_launch_configurations import PushLaunchConfigurations @@ -36,6 +37,7 @@ 'GroupAction', 'IncludeLaunchDescription', 'LogInfo', + 'OpaqueCoroutine', 'OpaqueFunction', 'PopLaunchConfigurations', 'PushLaunchConfigurations', diff --git a/launch/launch/actions/opaque_coroutine.py b/launch/launch/actions/opaque_coroutine.py new file mode 100644 index 000000000..4c9fa321f --- /dev/null +++ b/launch/launch/actions/opaque_coroutine.py @@ -0,0 +1,89 @@ +# Copyright 2019 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. + +"""Module for the OpaqueCoroutine action.""" + +import asyncio +import collections.abc +from typing import Any +from typing import Coroutine +from typing import Dict +from typing import Iterable +from typing import List +from typing import Optional +from typing import Text + +from ..action import Action +from ..launch_context import LaunchContext +from ..utilities import ensure_argument_type + + +class OpaqueCoroutine(Action): + """ + Action that adds a Python coroutine to the launch run loop. + + The signature of a coroutine should be: + + .. code-block:: python + + async def coroutine( + context: LaunchContext, # iff ignore_context is False + *args, + **kwargs + ): + ... + """ + + def __init__( + self, *, + coroutine: Coroutine, + args: Optional[Iterable[Any]] = None, + kwargs: Optional[Dict[Text, Any]] = None, + ignore_context: bool = False, + **left_over_kwargs + ) -> None: + """Constructor.""" + super().__init__(**left_over_kwargs) + if not asyncio.iscoroutinefunction(coroutine): + raise TypeError("OpaqueCoroutine expected a couroutine function for 'couroutine', got '{}'".format( + type(function) + )) + ensure_argument_type( + args, (collections.abc.Iterable, type(None)), 'args', 'OpaqueCoroutine' + ) + ensure_argument_type(kwargs, (dict, type(None)), 'kwargs', 'OpaqueCoroutine') + ensure_argument_type(ignore_context, bool, 'ignore_context', 'OpaqueCoroutine') + self.__coroutine = coroutine + self.__args = [] # type: Iterable + if args is not None: + self.__args = args + self.__kwargs = {} # type: Dict[Text, Any] + if kwargs is not None: + self.__kwargs = kwargs + self.__ignore_context = ignore_context # type: bool + self.__future = None # type: Optional[asyncio.Future] + + def execute(self, context: LaunchContext) -> Optional[List[Action]]: + """Execute the action.""" + args = self.__args + if not self.__ignore_context: + args = [context, *self.__args] + self.__future = context.asyncio_loop.create_task( + self.__coroutine(*args, **self.__kwargs) + ) + return None + + def get_asyncio_future(self) -> Optional[asyncio.Future]: + """Return an asyncio Future, used to let the launch system know when we're done.""" + return self.__future From 7a0ac0ae8a576d16ca68fe27d06f5004d608ec4d Mon Sep 17 00:00:00 2001 From: William Woodall Date: Mon, 4 Feb 2019 10:50:57 -0300 Subject: [PATCH 08/17] Apply suggestions from code review Co-Authored-By: hidmic --- launch/launch/events/__init__.py | 2 +- launch_testing/launch_testing/__init__.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/launch/launch/events/__init__.py b/launch/launch/events/__init__.py index 39e4c148b..088382233 100644 --- a/launch/launch/events/__init__.py +++ b/launch/launch/events/__init__.py @@ -1,4 +1,4 @@ -# Copyright 2018-2019 Open Source Robotics Foundation, Inc. +# Copyright 2018 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. diff --git a/launch_testing/launch_testing/__init__.py b/launch_testing/launch_testing/__init__.py index 5c2f43b23..368208d89 100644 --- a/launch_testing/launch_testing/__init__.py +++ b/launch_testing/launch_testing/__init__.py @@ -1,4 +1,4 @@ -# Copyright 2018-2019 Open Source Robotics Foundation, Inc. +# Copyright 2018 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. From 33d15c2d54b22955d8d2f5ebbf40c10522101558 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Mon, 4 Feb 2019 14:40:58 -0300 Subject: [PATCH 09/17] Addresses peer review comments. - Emit ExecutionComplete events on demand. - Shutdown OpaqueCoroutine gracefully. - Fix failing test cases. Signed-off-by: Michel Hidalgo --- launch/launch/action.py | 18 +++---- launch/launch/actions/execute_process.py | 2 +- launch/launch/actions/opaque_coroutine.py | 20 +++++-- .../event_handlers/on_execution_complete.py | 2 +- launch/launch/events/execution_complete.py | 1 + launch/launch/launch_context.py | 4 ++ launch/test/launch/test_action.py | 3 +- launch/test/launch/test_launch_service.py | 3 +- launch_testing/launch_testing/__init__.py | 53 +++++++++++++++---- 9 files changed, 79 insertions(+), 27 deletions(-) diff --git a/launch/launch/action.py b/launch/launch/action.py index 85d987bb3..112c953e2 100644 --- a/launch/launch/action.py +++ b/launch/launch/action.py @@ -60,17 +60,15 @@ def visit(self, context: LaunchContext) -> Optional[List[LaunchDescriptionEntity return cast(Optional[List[LaunchDescriptionEntity]], self.execute(context)) finally: from .events import ExecutionComplete # noqa - future = self.get_asyncio_future() - if future is not None: - future.add_done_callback( - lambda _: context.emit_event_sync( - ExecutionComplete(action=self) + event = ExecutionComplete(action=self) + if context.would_handle_event(event): + future = self.get_asyncio_future() + if future is not None: + future.add_done_callback( + lambda _: context.emit_event_sync(event) ) - ) - else: - context.emit_event_sync( - ExecutionComplete(action=self) - ) + else: + context.emit_event_sync(event) return None def execute(self, context: LaunchContext) -> Optional[List['Action']]: diff --git a/launch/launch/actions/execute_process.py b/launch/launch/actions/execute_process.py index 0e42da5f4..e98ed7e29 100644 --- a/launch/launch/actions/execute_process.py +++ b/launch/launch/actions/execute_process.py @@ -41,8 +41,8 @@ from ..event import Event from ..event_handler import EventHandler from ..event_handlers import OnShutdown -from ..events import Shutdown from ..events import matches_action +from ..events import Shutdown from ..events.process import ProcessExited from ..events.process import ProcessStarted from ..events.process import ProcessStderr diff --git a/launch/launch/actions/opaque_coroutine.py b/launch/launch/actions/opaque_coroutine.py index 4c9fa321f..4448d030a 100644 --- a/launch/launch/actions/opaque_coroutine.py +++ b/launch/launch/actions/opaque_coroutine.py @@ -25,7 +25,10 @@ from typing import Text from ..action import Action +from ..event import Event +from ..event_handlers import OnShutdown from ..launch_context import LaunchContext +from ..some_actions_type import SomeActionsType from ..utilities import ensure_argument_type @@ -56,9 +59,11 @@ def __init__( """Constructor.""" super().__init__(**left_over_kwargs) if not asyncio.iscoroutinefunction(coroutine): - raise TypeError("OpaqueCoroutine expected a couroutine function for 'couroutine', got '{}'".format( - type(function) - )) + raise TypeError( + "OpaqueCoroutine expected a couroutine for 'couroutine', got '{}'".format( + type(coroutine) + ) + ) ensure_argument_type( args, (collections.abc.Iterable, type(None)), 'args', 'OpaqueCoroutine' ) @@ -74,6 +79,12 @@ def __init__( self.__ignore_context = ignore_context # type: bool self.__future = None # type: Optional[asyncio.Future] + def __on_shutdown(self, event: Event, context: LaunchContext) -> Optional[SomeActionsType]: + """Cancel ongoing coroutine upon shutdown.""" + if self.__future is not None: + self.__future.cancel() + return None + def execute(self, context: LaunchContext) -> Optional[List[Action]]: """Execute the action.""" args = self.__args @@ -82,6 +93,9 @@ def execute(self, context: LaunchContext) -> Optional[List[Action]]: self.__future = context.asyncio_loop.create_task( self.__coroutine(*args, **self.__kwargs) ) + context.register_event_handler( + OnShutdown(on_shutdown=self.__on_shutdown) + ) return None def get_asyncio_future(self) -> Optional[asyncio.Future]: diff --git a/launch/launch/event_handlers/on_execution_complete.py b/launch/launch/event_handlers/on_execution_complete.py index 2d6cce8f9..28c254812 100644 --- a/launch/launch/event_handlers/on_execution_complete.py +++ b/launch/launch/event_handlers/on_execution_complete.py @@ -21,8 +21,8 @@ from typing import Text from ..event import Event -from ..events import ExecutionComplete from ..event_handler import EventHandler +from ..events import ExecutionComplete from ..launch_context import LaunchContext from ..launch_description_entity import LaunchDescriptionEntity from ..some_actions_type import SomeActionsType diff --git a/launch/launch/events/execution_complete.py b/launch/launch/events/execution_complete.py index 161fd443d..f5198f0cc 100644 --- a/launch/launch/events/execution_complete.py +++ b/launch/launch/events/execution_complete.py @@ -14,6 +14,7 @@ """Module for ExecutionComplete event.""" +from ..action import Action from ..event import Event diff --git a/launch/launch/launch_context.py b/launch/launch/launch_context.py index 84b23762d..4b5cc956f 100644 --- a/launch/launch/launch_context.py +++ b/launch/launch/launch_context.py @@ -156,6 +156,10 @@ def launch_configurations(self) -> Dict[Text, Text]: """Getter for launch_configurations dictionary.""" return self.__launch_configurations + def would_handle_event(self, event: Event) -> bool: + """Check whether an event would be handled or not.""" + return any(handler.matches(event) for handler in self._event_handlers) + def register_event_handler(self, event_handler: EventHandler) -> None: """Register a event handler.""" self._event_handlers.appendleft(event_handler) diff --git a/launch/test/launch/test_action.py b/launch/test/launch/test_action.py index a56b3ab03..0d9a942b7 100644 --- a/launch/test/launch/test_action.py +++ b/launch/test/launch/test_action.py @@ -16,6 +16,7 @@ from launch import Action from launch import Condition +from launch import LaunchContext def test_action_constructors(): @@ -26,7 +27,7 @@ def test_action_constructors(): def test_action_methods(): """Test the methods of the Action class.""" - class MockLaunchContext: + class MockLaunchContext(LaunchContext): ... action = Action() diff --git a/launch/test/launch/test_launch_service.py b/launch/test/launch/test_launch_service.py index f58d9500e..4c60c7c4f 100644 --- a/launch/test/launch/test_launch_service.py +++ b/launch/test/launch/test_launch_service.py @@ -19,6 +19,7 @@ from launch import LaunchDescription from launch import LaunchService +from launch.events import ExecutionComplete from launch.utilities import install_signal_handlers # Install the signal handlers here, in the hope that this is executed in the @@ -55,7 +56,7 @@ def test_launch_service_emit_event(): handled_events = queue.Queue() ld = LaunchDescription([ RegisterEventHandler(EventHandler( - matcher=lambda event: True, + matcher=lambda event: not isinstance(event, ExecutionComplete), entities=OpaqueFunction( function=lambda context: handled_events.put(context.locals.event), ), diff --git a/launch_testing/launch_testing/__init__.py b/launch_testing/launch_testing/__init__.py index 368208d89..de29825c6 100644 --- a/launch_testing/launch_testing/__init__.py +++ b/launch_testing/launch_testing/__init__.py @@ -31,23 +31,41 @@ def __init__(self): self.__tests = OrderedDict() self.__processes_rc = OrderedDict() - def _arm(self, test_name): + def _arm( + self, + test_name + ): + """Prepare for test execution.""" assert test_name not in self.__tests self.__tests[test_name] = 'armed' - def _fail(self, test_name, reason): + def _fail( + self, + test_name, + reason + ): + """Mark test as a failure and shutdown, dropping the tests that are still ongoing.""" self.__tests[test_name] = 'failed' for test_name in self.__tests: if self.__tests[test_name] == 'armed': self.__tests[test_name] = 'dropped' return EmitEvent(event=Shutdown(reason=reason)) - def _succeed(self, test_name): + def _succeed( + self, + test_name + ): + """Mark test as a success and shutdown if all other tests have succeeded too.""" self.__tests[test_name] = 'succeeded' if all(status == 'succeeded' for status in self.__tests.values()): return EmitEvent(event=Shutdown(reason='all tests finished')) - def add_fixture_action(self, launch_description, action, required=False): + def add_fixture_action( + self, + launch_description, + action, + required=False + ): """ Add action used as testing fixture. @@ -70,14 +88,17 @@ def on_fixture_process_exit(event, context): ) return action - def add_test_action(self, launch_description, action): + def add_test_action( + self, + launch_description, + action + ): """ Add action used for testing. If either all test actions have completed or a process action has exited with a non-zero return code, a shutdown event is emitted. """ - launch_description.add_action(action) test_name = 'test_{}'.format(id(action)) if isinstance(action, ExecuteProcess): def on_test_process_exit(event, context): @@ -104,12 +125,19 @@ def on_test_process_exit(event, context): ) )) ) + launch_description.add_action(action) self._arm(test_name) return action - def add_output_test(self, launch_description, action, output_file, - filtered_prefixes=None, filtered_patterns=None, - filtered_rmw_implementation=None): + def add_output_test( + self, + launch_description, + action, + output_file, + filtered_prefixes=None, + filtered_patterns=None, + filtered_rmw_implementation=None + ): """ Test an action process' output against text or regular expressions. @@ -164,7 +192,12 @@ def on_process_stdout(event): return action - def run(self, launch_service, *args, **kwargs): + def run( + self, + launch_service, + *args, + **kwargs + ): """ Invoke the `run` method of the launch service. From 07d55f84fe03c496d99e803654b171e7ffb71893 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Fri, 8 Feb 2019 16:47:59 -0300 Subject: [PATCH 10/17] Refactors launch_testing API a bit. To cope with more test cases. Signed-off-by: Michel Hidalgo --- launch/launch/actions/execute_process.py | 13 ++- launch_testing/launch_testing/__init__.py | 93 +++++++++++++--------- launch_testing/launch_testing/output.py | 96 ++++++++++++++--------- launch_testing/test/test_matching.py | 6 +- 4 files changed, 130 insertions(+), 78 deletions(-) diff --git a/launch/launch/actions/execute_process.py b/launch/launch/actions/execute_process.py index e98ed7e29..4461155e0 100644 --- a/launch/launch/actions/execute_process.py +++ b/launch/launch/actions/execute_process.py @@ -68,6 +68,14 @@ _global_process_counter = 0 # in Python3, this number is unbounded (no rollover) +def _is_process_running(pid): + try: + os.kill(pid, 0) + return True + except OSError: + return False + + class ExecuteProcess(Action): """Action that begins executing a process and sets up event handlers for the process.""" @@ -246,7 +254,10 @@ def __on_signal_process_event( raise RuntimeError('Signal event received before execution.') if self._subprocess_transport is None: raise RuntimeError('Signal event received before subprocess transport available.') - if self._subprocess_protocol.complete.done(): + # if self._subprocess_protocol.complete.done(): + # disable above's check as this handler may get called *after* the process has + # terminated but *before* the asyncio future has been resolved. + if not _is_process_running(self._subprocess_transport.get_pid()): # the process is done or is cleaning up, no need to signal _logger.debug("signal '{}' not set to '{}' because it is already closing".format( typed_event.signal_name, self.process_details['name'] diff --git a/launch_testing/launch_testing/__init__.py b/launch_testing/launch_testing/__init__.py index de29825c6..ee3ecea29 100644 --- a/launch_testing/launch_testing/__init__.py +++ b/launch_testing/launch_testing/__init__.py @@ -17,13 +17,12 @@ from launch.actions import EmitEvent from launch.actions import ExecuteProcess from launch.actions import RegisterEventHandler +from launch.actions import UnregisterEventHandler from launch.event_handlers import OnExecutionComplete from launch.event_handlers import OnProcessExit from launch.event_handlers import OnProcessIO from launch.events import Shutdown -from .output import create_output_check - class LaunchTestService(): @@ -49,22 +48,27 @@ def _fail( for test_name in self.__tests: if self.__tests[test_name] == 'armed': self.__tests[test_name] = 'dropped' - return EmitEvent(event=Shutdown(reason=reason)) + return [EmitEvent(event=Shutdown(reason=reason))] def _succeed( self, - test_name + test_name, + side_effect=None ): """Mark test as a success and shutdown if all other tests have succeeded too.""" self.__tests[test_name] = 'succeeded' if all(status == 'succeeded' for status in self.__tests.values()): - return EmitEvent(event=Shutdown(reason='all tests finished')) + return [EmitEvent(event=Shutdown(reason='all tests finished'))] + if side_effect == 'shutdown': + return [EmitEvent(event=Shutdown(reason='shutdown after test'))] + return [] def add_fixture_action( self, launch_description, action, - required=False + exit_allowed=[0], + ignore_returncode=False ): """ Add action used as testing fixture. @@ -75,9 +79,13 @@ def add_fixture_action( if isinstance(action, ExecuteProcess): def on_fixture_process_exit(event, context): process_name = event.action.process_details['name'] - if required or event.returncode != 0: - rc = event.returncode if event.returncode else 1 - self.__processes_rc[process_name] = rc + allowed_to_exit = exit_allowed + if isinstance(exit_allowed, list): + allowed_to_exit = event.returncode in exit_allowed + if not allowed_to_exit: + if not ignore_returncode: + rc = event.returncode if event.returncode else 1 + self.__processes_rc[process_name] = rc return EmitEvent(event=Shutdown( reason='{} fixture process died!'.format(process_name) )) @@ -133,31 +141,28 @@ def add_output_test( self, launch_description, action, - output_file, - filtered_prefixes=None, - filtered_patterns=None, - filtered_rmw_implementation=None + output_test, + test_suffix='output', + output_filter=None, + side_effect=None, ): """ - Test an action process' output against text or regular expressions. + Test an action process' output against a given test. :param launch_description: test launch description that owns the given action. :param action: launch action to test whose output is to be tested. - :param output_file: basename (i.e. w/o extension) of either a .txt file containing the - lines to be matched or a .regex file containing patterns to be searched for. - :param filtered_prefixes: A list of byte strings representing prefixes that will cause - output lines to be ignored if they start with one of the prefixes. By default lines - starting with the process ID (`b'pid'`) and return code (`b'rc'`) will be ignored. - :param filtered_patterns: A list of byte strings representing regexes that will cause - output lines to be ignored if they match one of the regexes. - :param filtered_rmw_implementation: RMW implementation for which the output will be - ignored in addition to the `filtered_prefixes`/`filtered_patterns`. + :param output_test: test tuple as returned by launch_testing.output.create_* functions. + :param test_suffix: an optional test suffix to disambiguate multiple test instances, defaults + to 'output'. + :param output_filter: an optional function to filter out i.e. ignore output lines for the test. + :param side_effect: an optional side effect of a passing test, currently only 'shutdown' + is supported. """ assert isinstance(action, ExecuteProcess) - test_name = 'test_{}_output'.format(id(action)) - output, collate_output, match_output, match_patterns = create_output_check( - output_file, filtered_prefixes, filtered_patterns, filtered_rmw_implementation - ) + test_name = 'test_{}_{}'.format(id(action), test_suffix) + output, collate_output, match_output, match_patterns = output_test + if not output_filter: + output_filter = (lambda x: x) assert any(match_patterns) def on_process_exit(event, context): @@ -165,28 +170,40 @@ def on_process_exit(event, context): if any(match_patterns): process_name = event.action.process_details['name'] reason = 'not all {} output matched!'.format(process_name) - return self._fail(test_name, reason) + return [ + UnregisterEventHandler(on_output), + UnregisterEventHandler(on_exit), + *self._fail(test_name, reason) + ] + + on_exit = OnProcessExit( + target_action=action, on_exit=on_process_exit + ) + + launch_description.add_action( + RegisterEventHandler(on_exit) + ) def on_process_stdout(event): nonlocal output nonlocal match_patterns - output = collate_output(output, event.text) + output = collate_output(output, output_filter(event.text)) match_patterns = [ pattern for pattern in match_patterns if not match_output(output, pattern) ] if not any(match_patterns): - return self._succeed(test_name) - - launch_description.add_action( - RegisterEventHandler(OnProcessExit( - target_action=action, on_exit=on_process_exit - )) + return [ + UnregisterEventHandler(on_output), + UnregisterEventHandler(on_exit), + *self._succeed(test_name, side_effect) + ] + + on_output = OnProcessIO( + target_action=action, on_stdout=on_process_stdout ) launch_description.add_action( - RegisterEventHandler(OnProcessIO( - target_action=action, on_stdout=on_process_stdout - )) + RegisterEventHandler(on_output) ) self._arm(test_name) diff --git a/launch_testing/launch_testing/output.py b/launch_testing/launch_testing/output.py index cbc5df0d5..6e0b09f9c 100644 --- a/launch_testing/launch_testing/output.py +++ b/launch_testing/launch_testing/output.py @@ -32,19 +32,22 @@ def get_rmw_output_filter(rmw_implementation, filter_type): return [str.encode(l) for l in rmw_output_filter.splitlines()] -def get_expected_output(output_file): - literal_file = output_file + '.txt' - if os.path.isfile(literal_file): - with open(literal_file, 'rb') as f: - return f.read().splitlines() - regex_file = output_file + '.regex' - if os.path.isfile(regex_file): - with open(regex_file, 'rb') as f: - return f.read().splitlines() - - -def create_output_lines_filter(filtered_prefixes, filtered_patterns, - filtered_rmw_implementation): +def create_output_lines_filter( + filtered_prefixes=None, + filtered_patterns=None, + filtered_rmw_implementation=None +): + """ + Create a line filtering function to help output testing. + + :param filtered_prefixes: A list of byte strings representing prefixes that will cause + output lines to be ignored if they start with one of the prefixes. By default lines + starting with the process ID (`b'pid'`) and return code (`b'rc'`) will be ignored. + :param filtered_patterns: A list of byte strings representing regexes that will cause + output lines to be ignored if they match one of the regexes. + :param filtered_rmw_implementation: RMW implementation for which the output will be + ignored in addition to the `filtered_prefixes`/`filtered_patterns`. + """ filtered_prefixes = filtered_prefixes or get_default_filtered_prefixes() filtered_patterns = filtered_patterns or get_default_filtered_patterns() if filtered_rmw_implementation: @@ -57,6 +60,7 @@ def create_output_lines_filter(filtered_prefixes, filtered_patterns, filtered_patterns = map(re.compile, filtered_patterns) def _filter(output): + filtered_output = [] for line in output.splitlines(): # Filter out stdout that comes from underlying DDS implementation # Note: we do not currently support matching filters across multiple stdout lines. @@ -64,42 +68,60 @@ def _filter(output): continue if any(pattern.match(line) for pattern in filtered_patterns): continue - yield line + filtered_output.append(line) + if output.endswith(b'\n'): + filtered_output.append(b'\n') + return b'\n'.join(filtered_output) return _filter -def create_output_check(output_file, filtered_prefixes, filtered_patterns, - filtered_rmw_implementation): - filter_output_lines = create_output_lines_filter( - filtered_prefixes, filtered_patterns, filtered_rmw_implementation - ) +def create_output_lines_test(expected_lines): + """ + Create output test given a list of expected lines. + """ + def _collate(output, addendum): + output.extend(addendum.splitlines()) + return output - literal_file = output_file + '.txt' - if os.path.isfile(literal_file): - def _collate(output, addendum): - output.extend(filter_output_lines(addendum)) - return output + def _match(output, pattern): + print(output, pattern, pattern in output) + return any(pattern in line for line in output) - def _match(output, pattern): - return pattern in output + return [], _collate, _match, expected_lines + +def create_output_regex_test(expected_patterns): + """ + Create output test given a list of expected matching regular + expressions. + """ + def _collate(output, addendum): + output.write(addendum) + return output + + def _match(output, pattern): + return pattern.search(output.getvalue()) is not None + + return io.BytesIO(), _collate, _match, expected_patterns + + +def create_output_test_from_file(output_file): + """ + Create output test using the given file content. + + :param output_file: basename (i.e. w/o extension) of either a .txt file containing the + lines to be matched or a .regex file containing patterns to be searched for. + """ + literal_file = output_file + '.txt' + if os.path.isfile(literal_file): with open(literal_file, 'rb') as f: expected_output = f.read().splitlines() - return [], _collate, _match, expected_output + return create_output_lines_test(expected_output) regex_file = output_file + '.regex' if os.path.isfile(regex_file): - def _collate(output, addendum): - output.write(b'\n'.join( - filter_output_lines(addendum) - )) - return output - - def _match(output, pattern): - return pattern.search(output.getvalue()) is not None - with open(regex_file, 'rb') as f: patterns = [re.compile(regex) for regex in f.read().splitlines()] - return io.BytesIO(), _collate, _match, patterns + return create_output_regex_test(patterns) raise RuntimeError('could not find output check file: {}'.format(output_file)) diff --git a/launch_testing/test/test_matching.py b/launch_testing/test/test_matching.py index b3be1e07d..abf5f0849 100644 --- a/launch_testing/test/test_matching.py +++ b/launch_testing/test/test_matching.py @@ -20,7 +20,7 @@ from launch import LaunchService from launch.actions import ExecuteProcess from launch_testing import LaunchTestService - +from launch_testing.output import create_output_test_from_file def test_matching(): # This temporary directory and files contained in it @@ -42,7 +42,9 @@ def test_matching(): action = launch_test.add_fixture_action( ld, ExecuteProcess(cmd=executable_command, output='screen') ) - launch_test.add_output_test(ld, action, output_file) + launch_test.add_output_test( + ld, action, create_output_test_from_file(output_file) + ) launch_service = LaunchService() launch_service.include_launch_description(ld) From 34da363c393a8027e34dff5cfe923f70bc49e084 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 12 Feb 2019 12:10:46 -0300 Subject: [PATCH 11/17] Applies style fixes to launch_testing. Signed-off-by: Michel Hidalgo --- launch_testing/launch_testing/__init__.py | 7 ++++--- launch_testing/launch_testing/output.py | 9 ++------- launch_testing/test/test_matching.py | 1 + 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/launch_testing/launch_testing/__init__.py b/launch_testing/launch_testing/__init__.py index ee3ecea29..f66153f8d 100644 --- a/launch_testing/launch_testing/__init__.py +++ b/launch_testing/launch_testing/__init__.py @@ -152,9 +152,10 @@ def add_output_test( :param launch_description: test launch description that owns the given action. :param action: launch action to test whose output is to be tested. :param output_test: test tuple as returned by launch_testing.output.create_* functions. - :param test_suffix: an optional test suffix to disambiguate multiple test instances, defaults - to 'output'. - :param output_filter: an optional function to filter out i.e. ignore output lines for the test. + :param test_suffix: an optional test suffix to disambiguate multiple test instances, + defaults to 'output'. + :param output_filter: an optional function to filter out i.e. ignore output lines for + the test. :param side_effect: an optional side effect of a passing test, currently only 'shutdown' is supported. """ diff --git a/launch_testing/launch_testing/output.py b/launch_testing/launch_testing/output.py index 6e0b09f9c..3a2c14ec1 100644 --- a/launch_testing/launch_testing/output.py +++ b/launch_testing/launch_testing/output.py @@ -76,9 +76,7 @@ def _filter(output): def create_output_lines_test(expected_lines): - """ - Create output test given a list of expected lines. - """ + """Create output test given a list of expected lines.""" def _collate(output, addendum): output.extend(addendum.splitlines()) return output @@ -91,10 +89,7 @@ def _match(output, pattern): def create_output_regex_test(expected_patterns): - """ - Create output test given a list of expected matching regular - expressions. - """ + """Create output test given a list of expected matching regular expressions.""" def _collate(output, addendum): output.write(addendum) return output diff --git a/launch_testing/test/test_matching.py b/launch_testing/test/test_matching.py index abf5f0849..c3aecfc18 100644 --- a/launch_testing/test/test_matching.py +++ b/launch_testing/test/test_matching.py @@ -22,6 +22,7 @@ from launch_testing import LaunchTestService from launch_testing.output import create_output_test_from_file + def test_matching(): # This temporary directory and files contained in it # will be deleted when the process ends. From 6d04a9f99f7e71be84278912c7c154291d70fb6f Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 12 Feb 2019 15:33:01 -0300 Subject: [PATCH 12/17] Deal with non zero exit on shutdown. Signed-off-by: Michel Hidalgo --- launch/launch/launch_service.py | 1 + launch_testing/launch_testing/__init__.py | 8 +++----- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/launch/launch/launch_service.py b/launch/launch/launch_service.py index 25352af06..2c6dba7f6 100644 --- a/launch/launch/launch_service.py +++ b/launch/launch/launch_service.py @@ -375,6 +375,7 @@ def _on_sigquit(signum, frame): def __on_shutdown(self, event: Event, context: LaunchContext) -> Optional[SomeActionsType]: self.__shutting_down = True + self.__context._set_is_shutdown(True) return None def _shutdown(self, *, reason, due_to_sigint, force_sync=False): diff --git a/launch_testing/launch_testing/__init__.py b/launch_testing/launch_testing/__init__.py index f66153f8d..4aee6978d 100644 --- a/launch_testing/launch_testing/__init__.py +++ b/launch_testing/launch_testing/__init__.py @@ -68,7 +68,6 @@ def add_fixture_action( launch_description, action, exit_allowed=[0], - ignore_returncode=False ): """ Add action used as testing fixture. @@ -82,10 +81,9 @@ def on_fixture_process_exit(event, context): allowed_to_exit = exit_allowed if isinstance(exit_allowed, list): allowed_to_exit = event.returncode in exit_allowed - if not allowed_to_exit: - if not ignore_returncode: - rc = event.returncode if event.returncode else 1 - self.__processes_rc[process_name] = rc + if not context.is_shutdown and not allowed_to_exit: + rc = event.returncode if event.returncode else 1 + self.__processes_rc[process_name] = rc return EmitEvent(event=Shutdown( reason='{} fixture process died!'.format(process_name) )) From aec02bfc8e4868b87be508030a4eab2220cd54e2 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Wed, 13 Feb 2019 15:43:12 -0300 Subject: [PATCH 13/17] Avoids output tests' races. Signed-off-by: Michel Hidalgo --- launch_testing/launch_testing/__init__.py | 64 ++++++++++++++--------- 1 file changed, 39 insertions(+), 25 deletions(-) diff --git a/launch_testing/launch_testing/__init__.py b/launch_testing/launch_testing/__init__.py index 4aee6978d..e9f2cffd3 100644 --- a/launch_testing/launch_testing/__init__.py +++ b/launch_testing/launch_testing/__init__.py @@ -17,10 +17,10 @@ from launch.actions import EmitEvent from launch.actions import ExecuteProcess from launch.actions import RegisterEventHandler -from launch.actions import UnregisterEventHandler from launch.event_handlers import OnExecutionComplete from launch.event_handlers import OnProcessExit from launch.event_handlers import OnProcessIO +from launch.event_handlers import OnShutdown from launch.events import Shutdown @@ -38,12 +38,23 @@ def _arm( assert test_name not in self.__tests self.__tests[test_name] = 'armed' + def _finish( + self, + test_name + ): + """Mark test as finished and shutdown if all other tests have finished too.""" + assert test_name in self.__tests + self.__tests[test_name] = 'finished' + if all(status != 'armed' for status in self.__tests.values()): + return [EmitEvent(event=Shutdown(reason='all tests finished'))] + def _fail( self, test_name, reason ): - """Mark test as a failure and shutdown, dropping the tests that are still ongoing.""" + """Mark test as a failure and shutdown, dropping the tests that are still in progress.""" + assert test_name in self.__tests self.__tests[test_name] = 'failed' for test_name in self.__tests: if self.__tests[test_name] == 'armed': @@ -55,9 +66,10 @@ def _succeed( test_name, side_effect=None ): - """Mark test as a success and shutdown if all other tests have succeeded too.""" + """Mark test as a success and shutdown if all other tests have finished too.""" + assert test_name in self.__tests self.__tests[test_name] = 'succeeded' - if all(status == 'succeeded' for status in self.__tests.values()): + if all(status != 'armed' for status in self.__tests.values()): return [EmitEvent(event=Shutdown(reason='all tests finished'))] if side_effect == 'shutdown': return [EmitEvent(event=Shutdown(reason='shutdown after test'))] @@ -167,20 +179,26 @@ def add_output_test( def on_process_exit(event, context): nonlocal match_patterns if any(match_patterns): - process_name = event.action.process_details['name'] - reason = 'not all {} output matched!'.format(process_name) - return [ - UnregisterEventHandler(on_output), - UnregisterEventHandler(on_exit), - *self._fail(test_name, reason) - ] - - on_exit = OnProcessExit( - target_action=action, on_exit=on_process_exit + # Finish test instead of failing to prevent process exit + # and process output event handlers from racing. + return self._finish(test_name) + + launch_description.add_action( + RegisterEventHandler(OnProcessExit( + target_action=action, on_exit=on_process_exit + )) ) + def on_shutdown(event, context): + nonlocal match_patterns + if any(match_patterns): + process_name = action.process_details['name'] + reason = 'not all {} output matched!'.format(process_name) + self._fail(test_name, reason) + self._succeed(test_name, side_effect) + launch_description.add_action( - RegisterEventHandler(on_exit) + RegisterEventHandler(OnShutdown(on_shutdown=on_shutdown)) ) def on_process_stdout(event): @@ -192,17 +210,13 @@ def on_process_stdout(event): if not match_output(output, pattern) ] if not any(match_patterns): - return [ - UnregisterEventHandler(on_output), - UnregisterEventHandler(on_exit), - *self._succeed(test_name, side_effect) - ] - - on_output = OnProcessIO( - target_action=action, on_stdout=on_process_stdout - ) + return self._succeed(test_name, side_effect) + return None + launch_description.add_action( - RegisterEventHandler(on_output) + RegisterEventHandler(OnProcessIO( + target_action=action, on_stdout=on_process_stdout + )) ) self._arm(test_name) From ea034a275b8ef26f52d3dd18d20389446f606497 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Mon, 18 Feb 2019 14:08:18 -0800 Subject: [PATCH 14/17] Applies misc fixes after Windows triaging. Signed-off-by: Michel Hidalgo --- launch/launch/actions/execute_process.py | 28 ++++++++----------- .../event_handlers/on_execution_complete.py | 2 +- launch/launch/launch_introspector.py | 2 +- launch_testing/launch_testing/__init__.py | 24 +++++++++++++--- launch_testing/launch_testing/output.py | 14 +++++----- 5 files changed, 40 insertions(+), 30 deletions(-) diff --git a/launch/launch/actions/execute_process.py b/launch/launch/actions/execute_process.py index 4461155e0..1c435b4cc 100644 --- a/launch/launch/actions/execute_process.py +++ b/launch/launch/actions/execute_process.py @@ -68,14 +68,6 @@ _global_process_counter = 0 # in Python3, this number is unbounded (no rollover) -def _is_process_running(pid): - try: - os.kill(pid, 0) - return True - except OSError: - return False - - class ExecuteProcess(Action): """Action that begins executing a process and sets up event handlers for the process.""" @@ -254,12 +246,9 @@ def __on_signal_process_event( raise RuntimeError('Signal event received before execution.') if self._subprocess_transport is None: raise RuntimeError('Signal event received before subprocess transport available.') - # if self._subprocess_protocol.complete.done(): - # disable above's check as this handler may get called *after* the process has - # terminated but *before* the asyncio future has been resolved. - if not _is_process_running(self._subprocess_transport.get_pid()): + if self._subprocess_protocol.complete.done(): # the process is done or is cleaning up, no need to signal - _logger.debug("signal '{}' not set to '{}' because it is already closing".format( + _logger.debug("signal '{}' not sent to '{}' because it is already closing".format( typed_event.signal_name, self.process_details['name'] )) return None @@ -274,11 +263,16 @@ def __on_signal_process_event( _logger.info("sending signal '{}' to process[{}]".format( typed_event.signal_name, self.process_details['name'] )) - if typed_event.signal_name == 'SIGKILL': - self._subprocess_transport.kill() # works on both Windows and POSIX + try: + if typed_event.signal_name == 'SIGKILL': + self._subprocess_transport.kill() # works on both Windows and POSIX + return None + self._subprocess_transport.send_signal(typed_event.signal) return None - self._subprocess_transport.send_signal(typed_event.signal) - return None + except ProcessLookupError: + _logger.debug("signal '{}' not sent to '{}' because it has closed already".format( + typed_event.signal_name, self.process_details['name'] + )) def __on_process_stdin_event( self, diff --git a/launch/launch/event_handlers/on_execution_complete.py b/launch/launch/event_handlers/on_execution_complete.py index 28c254812..5fe6dc29d 100644 --- a/launch/launch/event_handlers/on_execution_complete.py +++ b/launch/launch/event_handlers/on_execution_complete.py @@ -110,7 +110,7 @@ def handler_description(self) -> Text: # via the 'entities' property. if self.__actions_on_completion: return '' - return '{}'.format(self.__on_exit) + return '{}'.format(self.__on_completion) @property def matcher_description(self) -> Text: diff --git a/launch/launch/launch_introspector.py b/launch/launch/launch_introspector.py index e2079019f..e927e2ddd 100644 --- a/launch/launch/launch_introspector.py +++ b/launch/launch/launch_introspector.py @@ -110,7 +110,7 @@ def format_action(action: Action) -> List[Text]: ), typed_action.env if typed_action.env is None else '{' + ', '.join( ['{}: {}'.format(format_substitutions(k), format_substitutions(v)) - for k, v in typed_action.env.items()]) + '}', + for k, v in typed_action.env]) + '}', typed_action.shell, ) return [msg] diff --git a/launch_testing/launch_testing/__init__.py b/launch_testing/launch_testing/__init__.py index e9f2cffd3..5a5338849 100644 --- a/launch_testing/launch_testing/__init__.py +++ b/launch_testing/launch_testing/__init__.py @@ -48,6 +48,14 @@ def _finish( if all(status != 'armed' for status in self.__tests.values()): return [EmitEvent(event=Shutdown(reason='all tests finished'))] + def _drop( + self, + test_name + ): + """Mark test as dropped.""" + assert test_name in self.__tests + self.__tests[test_name] = 'dropped' + def _fail( self, test_name, @@ -122,7 +130,7 @@ def add_test_action( def on_test_process_exit(event, context): if event.returncode != 0: process_name = event.action.process_details['name'] - self._processes_rc[process_name] = event.returncode + self.__processes_rc[process_name] = event.returncode return self._fail( test_name, reason='{} test failed!'.format( process_name @@ -136,11 +144,19 @@ def on_test_process_exit(event, context): )) ) else: + def on_test_completion(event, context): + future = event.action.get_asyncio_future() + if future is not None: + if future.cancelled(): + return self._drop(test_name) + exc = future.exception() + if exc is not None: + return self._fail(test_name, str(exc)) + return self._succeed(test_name) + launch_description.add_action( RegisterEventHandler(OnExecutionComplete( - target_action=action, on_completion=( - lambda *args: self._succeed(test_name) - ) + target_action=action, on_completion=on_test_completion )) ) launch_description.add_action(action) diff --git a/launch_testing/launch_testing/output.py b/launch_testing/launch_testing/output.py index 3a2c14ec1..735c79bd3 100644 --- a/launch_testing/launch_testing/output.py +++ b/launch_testing/launch_testing/output.py @@ -58,6 +58,7 @@ def create_output_lines_filter( filtered_rmw_implementation, 'patterns' )) filtered_patterns = map(re.compile, filtered_patterns) + encoded_line_sep = os.linesep.encode('ascii') def _filter(output): filtered_output = [] @@ -69,23 +70,22 @@ def _filter(output): if any(pattern.match(line) for pattern in filtered_patterns): continue filtered_output.append(line) - if output.endswith(b'\n'): - filtered_output.append(b'\n') - return b'\n'.join(filtered_output) + if output.endswith(encoded_line_sep): + filtered_output.append(encoded_line_sep) + return encoded_line_sep.join(filtered_output) return _filter def create_output_lines_test(expected_lines): """Create output test given a list of expected lines.""" def _collate(output, addendum): - output.extend(addendum.splitlines()) + output.write(addendum) return output def _match(output, pattern): - print(output, pattern, pattern in output) - return any(pattern in line for line in output) + return any(pattern in line for line in output.getvalue().splitlines()) - return [], _collate, _match, expected_lines + return io.BytesIO(), _collate, _match, expected_lines def create_output_regex_test(expected_patterns): From 39f123400c2e9a6d30c54cfdc5305c153ea27c8a Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 19 Feb 2019 01:50:25 -0800 Subject: [PATCH 15/17] Applies more fixes after Windows triaging. Signed-off-by: Michel Hidalgo --- launch_testing/launch_testing/__init__.py | 3 ++- launch_testing/launch_testing/output.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/launch_testing/launch_testing/__init__.py b/launch_testing/launch_testing/__init__.py index 5a5338849..7214e1191 100644 --- a/launch_testing/launch_testing/__init__.py +++ b/launch_testing/launch_testing/__init__.py @@ -151,7 +151,7 @@ def on_test_completion(event, context): return self._drop(test_name) exc = future.exception() if exc is not None: - return self._fail(test_name, str(exc)) + return self._fail(test_name, str(exc)) return self._succeed(test_name) launch_description.add_action( @@ -221,6 +221,7 @@ def on_process_stdout(event): nonlocal output nonlocal match_patterns output = collate_output(output, output_filter(event.text)) + print(output.getvalue()) match_patterns = [ pattern for pattern in match_patterns if not match_output(output, pattern) diff --git a/launch_testing/launch_testing/output.py b/launch_testing/launch_testing/output.py index 735c79bd3..7e2ff85b8 100644 --- a/launch_testing/launch_testing/output.py +++ b/launch_testing/launch_testing/output.py @@ -91,7 +91,7 @@ def _match(output, pattern): def create_output_regex_test(expected_patterns): """Create output test given a list of expected matching regular expressions.""" def _collate(output, addendum): - output.write(addendum) + output.write(b'\n'.join(addendum.splitlines())) return output def _match(output, pattern): From 2d9305a75a6867d87d05f2453969a1faf9b1bc25 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Fri, 22 Feb 2019 16:38:57 -0300 Subject: [PATCH 16/17] Fixes linter issue. Signed-off-by: Michel Hidalgo --- launch/launch/actions/execute_process.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/launch/launch/actions/execute_process.py b/launch/launch/actions/execute_process.py index 1c435b4cc..80f8eee5f 100644 --- a/launch/launch/actions/execute_process.py +++ b/launch/launch/actions/execute_process.py @@ -80,9 +80,9 @@ def __init__( env: Optional[Dict[SomeSubstitutionsType, SomeSubstitutionsType]] = None, shell: bool = False, sigterm_timeout: SomeSubstitutionsType = LaunchConfiguration( - 'sigterm_timeout', default = 5), + 'sigterm_timeout', default=5), sigkill_timeout: SomeSubstitutionsType = LaunchConfiguration( - 'sigkill_timeout', default = 5), + 'sigkill_timeout', default=5), prefix: Optional[SomeSubstitutionsType] = None, output: Optional[Text] = None, log_cmd: bool = False, From d05379a5607c749fb27c44bc656d48a659f5c8bc Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Mon, 25 Feb 2019 14:20:56 -0300 Subject: [PATCH 17/17] Addresses peer review comments. - Improved OpaqueCoroutine documentation. - Added test for launch.event_handlers.OnExecutionComplete. Signed-off-by: Michel Hidalgo --- launch/launch/actions/opaque_coroutine.py | 16 ++++- .../event_handlers/on_execution_complete.py | 2 +- .../test_on_execution_complete.py | 60 +++++++++++++++++++ 3 files changed, 75 insertions(+), 3 deletions(-) create mode 100644 launch/test/launch/event_handlers/test_on_execution_complete.py diff --git a/launch/launch/actions/opaque_coroutine.py b/launch/launch/actions/opaque_coroutine.py index 4448d030a..e9f46b9af 100644 --- a/launch/launch/actions/opaque_coroutine.py +++ b/launch/launch/actions/opaque_coroutine.py @@ -41,11 +41,23 @@ class OpaqueCoroutine(Action): .. code-block:: python async def coroutine( - context: LaunchContext, # iff ignore_context is False + context: LaunchContext, *args, **kwargs ): ... + + if ignore_context is False on construction (currently the default), or + + .. code-block:: python + + async def coroutine( + *args, + **kwargs + ): + ... + + if ignore_context is True on construction. """ def __init__( @@ -60,7 +72,7 @@ def __init__( super().__init__(**left_over_kwargs) if not asyncio.iscoroutinefunction(coroutine): raise TypeError( - "OpaqueCoroutine expected a couroutine for 'couroutine', got '{}'".format( + "OpaqueCoroutine expected a coroutine for 'coroutine', got '{}'".format( type(coroutine) ) ) diff --git a/launch/launch/event_handlers/on_execution_complete.py b/launch/launch/event_handlers/on_execution_complete.py index 5fe6dc29d..c1de83833 100644 --- a/launch/launch/event_handlers/on_execution_complete.py +++ b/launch/launch/event_handlers/on_execution_complete.py @@ -61,7 +61,7 @@ def __init__(self, *, target_action=None, on_completion, **kwargs) -> None: # n """Constructor.""" from ..action import Action # noqa if not isinstance(target_action, (Action, type(None))): - raise RuntimeError("OnExecutionComplete requires an 'Action' as the target") + raise ValueError("OnExecutionComplete requires an 'Action' as the target") super().__init__( matcher=( lambda event: ( diff --git a/launch/test/launch/event_handlers/test_on_execution_complete.py b/launch/test/launch/event_handlers/test_on_execution_complete.py new file mode 100644 index 000000000..f1cce0409 --- /dev/null +++ b/launch/test/launch/event_handlers/test_on_execution_complete.py @@ -0,0 +1,60 @@ +# Copyright 2018 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. + +"""Tests for the OnExecutionComplete class.""" + +from launch.actions import LogInfo +from launch.event_handlers import OnExecutionComplete +from launch.events import ExecutionComplete + + +import pytest + + +def test_bad_construction(): + """Test bad construction parameters.""" + with pytest.raises(ValueError): + OnExecutionComplete( + target_action='not-an-action', + on_completion=lambda *args: None + ) + + with pytest.raises(ValueError): + OnExecutionComplete( + target_action=LogInfo(msg='some message'), + on_completion='not-a-callable-nor-an-action-iterable' + ) + + +def test_single_action_is_matched(): + """Test that only the target action execution complete event is matched.""" + an_action = LogInfo(msg='some message') + event_handler = OnExecutionComplete( + target_action=an_action, + on_completion=lambda *args: None + ) + other_action = LogInfo(msg='other message') + assert event_handler.matches(ExecutionComplete(action=an_action)) + assert not event_handler.matches(ExecutionComplete(action=other_action)) + + +def test_all_actions_are_matched(): + """Test that all execution complete events are matched.""" + an_action = LogInfo(msg='some message') + other_action = LogInfo(msg='other message') + event_handler = OnExecutionComplete( + on_completion=lambda *args: None + ) + assert event_handler.matches(ExecutionComplete(action=an_action)) + assert event_handler.matches(ExecutionComplete(action=other_action))