From ac744774585abe91bbbd78004ab6400b22778704 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Sat, 8 Feb 2020 19:22:17 -0500 Subject: [PATCH 1/4] Extract more assertValidPointer() method from assertValidPointer() --- tracetools_test/tracetools_test/case.py | 30 +++++++++++++++++++------ 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/tracetools_test/tracetools_test/case.py b/tracetools_test/tracetools_test/case.py index 49d6722..d1940e8 100644 --- a/tracetools_test/tracetools_test/case.py +++ b/tracetools_test/tracetools_test/case.py @@ -132,17 +132,33 @@ class TraceTestCase(unittest.TestCase): handle_field_names: Union[str, List[str]], ) -> None: """ - Check that the handle associated with a field name is valid. + Check that the handle associated to a field name is valid. :param event: the event which has a handle field :param handle_field_names: the handle field name(s) to check """ - if not isinstance(handle_field_names, list): - handle_field_names = [handle_field_names] - for field_name in handle_field_names: - handle_value = self.get_field(event, field_name) - self.assertIsInstance(handle_value, int, 'handle value not int') - self.assertGreater(handle_value, 0, f'invalid handle value: {field_name}') + self.assertValidPointer( + event, + handle_field_names, + ) + + def assertValidPointer( + self, + event: DictEvent, + pointer_field_names: Union[str, List[str]], + ) -> None: + """ + Check that the pointer associated to a field name is valid. + + :param event: the event which has a pointer field + :param pointer_field_names: the pointer field name(s) to check + """ + if not isinstance(pointer_field_names, list): + pointer_field_names = [pointer_field_names] + for field_name in pointer_field_names: + pointer_value = self.get_field(event, field_name) + self.assertIsInstance(pointer_value, int, 'pointer value not int') + self.assertGreater(pointer_value, 0, f'invalid pointer value: {field_name}') def assertValidQueueDepth( self, From bae71f0cad1bbbeb87ab48e406e4a3194bbf6734 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Sat, 8 Feb 2020 19:22:42 -0500 Subject: [PATCH 2/4] Make order check optional for assertMatchingField() --- tracetools_test/tracetools_test/case.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/tracetools_test/tracetools_test/case.py b/tracetools_test/tracetools_test/case.py index d1940e8..3612aa0 100644 --- a/tracetools_test/tracetools_test/case.py +++ b/tracetools_test/tracetools_test/case.py @@ -251,6 +251,7 @@ class TraceTestCase(unittest.TestCase): field_name: str, matching_event_name: str = None, events: List[DictEvent] = None, + check_order: bool = True, ) -> None: """ Check that the value of a field for a given event has a matching event that follows. @@ -259,6 +260,7 @@ class TraceTestCase(unittest.TestCase): :param field_name: the name of the common field to check :param matching_event_name: the name of the event to check (or `None` to check all) :param events: the events to check (or `None` to check all in trace) + :param check_order: whether to check that the matching event comes after the initial event """ if events is None: events = self._events @@ -276,14 +278,15 @@ class TraceTestCase(unittest.TestCase): len(matches), 1, f'no corresponding {field_name}') - # Check order - # Since matching pairs might repeat, we need to check - # that there is at least one match that comes after - matches_ordered = [e for e in matches if self.are_events_ordered(initial_event, e)] - self.assertGreaterEqual( - len(matches_ordered), - 1, - 'matching field event not after initial event') + if check_order: + # Check order + # Since matching pairs might repeat, we need to check + # that there is at least one match that comes after + matches_ordered = [e for e in matches if self.are_events_ordered(initial_event, e)] + self.assertGreaterEqual( + len(matches_ordered), + 1, + 'matching field event not after initial event') def assertFieldEquals( self, From 1b73e4be94e98c0a88dbe8abff4e133c9d1ca85a Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Sat, 8 Feb 2020 19:23:05 -0500 Subject: [PATCH 3/4] Merge and update service test to cover callback registration --- tracetools_test/CMakeLists.txt | 11 - tracetools_test/test/test_service.py | 208 +++++++++++++++--- tracetools_test/test/test_service_callback.py | 70 ------ 3 files changed, 182 insertions(+), 107 deletions(-) delete mode 100644 tracetools_test/test/test_service_callback.py diff --git a/tracetools_test/CMakeLists.txt b/tracetools_test/CMakeLists.txt index 7add0e6..24211d0 100644 --- a/tracetools_test/CMakeLists.txt +++ b/tracetools_test/CMakeLists.txt @@ -82,15 +82,6 @@ if(BUILD_TESTING) tracetools ) target_link_libraries(test_timer "${RDYNAMIC_FLAG}") - add_executable(test_service - src/test_service.cpp - ) - ament_target_dependencies(test_service - rclcpp - std_srvs - tracetools - ) - target_link_libraries(test_service "${RDYNAMIC_FLAG}") add_executable(test_service_ping src/test_service_ping.cpp ) @@ -115,7 +106,6 @@ if(BUILD_TESTING) test_ping test_pong test_publisher - test_service test_service_ping test_service_pong test_timer @@ -135,7 +125,6 @@ if(BUILD_TESTING) test/test_node.py test/test_publisher.py test/test_service.py - test/test_service_callback.py test/test_subscription.py test/test_timer.py ) diff --git a/tracetools_test/test/test_service.py b/tracetools_test/test/test_service.py index ccc997a..c65b468 100644 --- a/tracetools_test/test/test_service.py +++ b/tracetools_test/test/test_service.py @@ -22,13 +22,16 @@ class TestService(TraceTestCase): def __init__(self, *args) -> None: super().__init__( *args, - session_name_prefix='session-test-service-creation', + session_name_prefix='session-test-service', events_ros=[ 'ros2:rcl_node_init', 'ros2:rcl_service_init', 'ros2:rclcpp_service_callback_added', + 'ros2:rclcpp_callback_register', + 'ros2:callback_start', + 'ros2:callback_end', ], - nodes=['test_service'], + nodes=['test_service_ping', 'test_service_pong'], ) def test_all(self): @@ -38,53 +41,206 @@ class TestService(TraceTestCase): # Check fields srv_init_events = self.get_events_with_name('ros2:rcl_service_init') callback_added_events = self.get_events_with_name('ros2:rclcpp_service_callback_added') + callback_register_events = self.get_events_with_name('ros2:rclcpp_callback_register') + start_events = self.get_events_with_name('ros2:callback_start') + end_events = self.get_events_with_name('ros2:callback_end') for event in srv_init_events: self.assertValidHandle(event, ['service_handle', 'node_handle', 'rmw_service_handle']) self.assertStringFieldNotEmpty(event, 'service_name') for event in callback_added_events: self.assertValidHandle(event, ['service_handle', 'callback']) + for event in callback_register_events: + self.assertValidPointer(event, 'callback') + self.assertStringFieldNotEmpty(event, 'symbol') + for event in start_events: + self.assertValidHandle(event, 'callback') + # Should not be 1 for services (yet) + self.assertFieldEquals( + event, + 'is_intra_process', + 0, + 'invalid value for is_intra_process', + ) + for event in end_events: + self.assertValidHandle(event, 'callback') - # Check that the test service name exists - test_srv_init_events = self.get_events_with_procname('test_service', srv_init_events) - event_service_names = self.get_events_with_field_value( + # Check that the test services names exists + ping_node_srv_init_events = self.get_events_with_procname( + 'test_service_ping', + srv_init_events, + ) + ping_node_test_srv_init_events = self.get_events_with_field_value( 'service_name', - '/the_service', - test_srv_init_events, + '/pong', + ping_node_srv_init_events, + ) + self.assertEqual( + len(ping_node_test_srv_init_events), + 1, + 'none or more than 1 /pong service under the test_service_pong node', + ) + + pong_node_srv_init_events = self.get_events_with_procname( + 'test_service_pong', + srv_init_events, + ) + pong_node_test_srv_init_events = self.get_events_with_field_value( + 'service_name', + '/ping', + pong_node_srv_init_events, ) self.assertGreaterEqual( - len(event_service_names), + len(pong_node_test_srv_init_events), 1, 'cannot find test service name', ) - # Check that the node handle matches the node_init event + # Check that the service init events have a matching node handle (with node_init events) node_init_events = self.get_events_with_name('ros2:rcl_node_init') - test_srv_node_init_events = self.get_events_with_procname( - 'test_service', - node_init_events, - ) - self.assertNumEventsEqual( - test_srv_node_init_events, - 1, - 'none or more than 1 rcl_node_init event', - ) - test_srv_node_init_event = test_srv_node_init_events[0] + + ping_node_test_srv_init_event = ping_node_test_srv_init_events[0] self.assertMatchingField( - test_srv_node_init_event, + ping_node_test_srv_init_event, 'node_handle', - 'ros2:rcl_service_init', - test_srv_init_events, + None, + node_init_events, + False, ) - # Check that the service handles match - test_event_srv_init = event_service_names[0] + pong_node_test_srv_init_event = pong_node_test_srv_init_events[0] self.assertMatchingField( - test_event_srv_init, - 'service_handle', + pong_node_test_srv_init_event, + 'node_handle', None, + node_init_events, + False, + ) + + # Check that there are matching rclcpp_service_callback_added events + ping_node_test_service_handle = self.get_field( + ping_node_test_srv_init_event, + 'service_handle', + ) + pong_node_test_service_handle = self.get_field( + pong_node_test_srv_init_event, + 'service_handle', + ) + + ping_node_srv_callback_added_events = self.get_events_with_procname( + 'test_service_ping', callback_added_events, ) + ping_node_test_srv_callback_added_events = self.get_events_with_field_value( + 'service_handle', + ping_node_test_service_handle, + ping_node_srv_callback_added_events, + ) + self.assertEqual( + len(ping_node_test_srv_callback_added_events), + 1, + 'none or more than 1 matching callback_added events for the test_service_ping node', + ) + + pong_node_srv_callback_added_events = self.get_events_with_procname( + 'test_service_pong', + callback_added_events, + ) + pong_node_test_srv_callback_added_events = self.get_events_with_field_value( + 'service_handle', + pong_node_test_service_handle, + pong_node_srv_callback_added_events, + ) + self.assertEqual( + len(pong_node_test_srv_callback_added_events), + 1, + 'none or more than 1 matching callback_added events for the test_service_pong node', + ) + + # Check that there are matching rclcpp_callback_register events + ping_node_test_callback_ref = self.get_field( + ping_node_test_srv_callback_added_events[0], + 'callback', + ) + pong_node_test_callback_ref = self.get_field( + pong_node_test_srv_callback_added_events[0], + 'callback', + ) + + ping_node_callback_register_events = self.get_events_with_procname( + 'test_service_ping', + callback_register_events, + ) + ping_node_test_callback_register_events = self.get_events_with_field_value( + 'callback', + ping_node_test_callback_ref, + ping_node_callback_register_events, + ) + self.assertEqual( + len(ping_node_test_callback_register_events), + 1, + 'none or more than 1 matching callback_register events for the test_service_ping node', + ) + + pong_node_callback_register_events = self.get_events_with_procname( + 'test_service_pong', + callback_register_events, + ) + pong_node_test_callback_register_events = self.get_events_with_field_value( + 'callback', + pong_node_test_callback_ref, + pong_node_callback_register_events, + ) + self.assertEqual( + len(pong_node_test_callback_register_events), + 1, + 'none or more than 1 matching callback_register events for the test_service_pong node', + ) + + # Check that there are corresponding callback_start/stop pairs + ping_node_test_callback_start_events = self.get_events_with_field_value( + 'callback', + ping_node_test_callback_ref, + start_events, + ) + self.assertEqual( + len(ping_node_test_callback_start_events), + 1, + 'none or more than 1 matching callback_start events for the test_service_ping node', + ) + + pong_node_test_callback_start_events = self.get_events_with_field_value( + 'callback', + pong_node_test_callback_ref, + start_events, + ) + self.assertEqual( + len(pong_node_test_callback_start_events), + 1, + 'none or more than 1 matching callback_start events for the test_service_pong node', + ) + + ping_node_test_callback_end_events = self.get_events_with_field_value( + 'callback', + ping_node_test_callback_ref, + end_events, + ) + self.assertEqual( + len(ping_node_test_callback_end_events), + 1, + 'none or more than 1 matching callback_end events for the test_service_ping node', + ) + + pong_node_test_callback_end_events = self.get_events_with_field_value( + 'callback', + pong_node_test_callback_ref, + end_events, + ) + self.assertEqual( + len(pong_node_test_callback_end_events), + 1, + 'none or more than 1 matching callback_end events for the test_service_pong node', + ) if __name__ == '__main__': diff --git a/tracetools_test/test/test_service_callback.py b/tracetools_test/test/test_service_callback.py deleted file mode 100644 index d78f726..0000000 --- a/tracetools_test/test/test_service_callback.py +++ /dev/null @@ -1,70 +0,0 @@ -# Copyright 2019 Robert Bosch GmbH -# -# 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 unittest - -from tracetools_test.case import TraceTestCase - - -class TestServiceCallback(TraceTestCase): - - def __init__(self, *args) -> None: - super().__init__( - *args, - session_name_prefix='session-test-service-callback', - events_ros=[ - 'ros2:callback_start', - 'ros2:callback_end', - ], - nodes=['test_service_ping', 'test_service_pong'], - ) - - def test_all(self): - # Check events as set - self.assertEventsSet(self._events_ros) - - # Check fields - start_events = self.get_events_with_name('ros2:callback_start') - end_events = self.get_events_with_name('ros2:callback_end') - - for event in start_events: - self.assertValidHandle(event, 'callback') - # Should not be 1 for services (yet) - self.assertFieldEquals( - event, - 'is_intra_process', - 0, - 'invalid value for is_intra_process', - ) - for event in end_events: - self.assertValidHandle(event, 'callback') - - # Check that there is at least 1 start/end pair for each node - for node in self._nodes: - test_start_events = self.get_events_with_procname(node, start_events) - test_end_events = self.get_events_with_procname(node, end_events) - self.assertGreater( - len(test_start_events), - 0, - f'no start_callback events for node: {node}', - ) - self.assertGreater( - len(test_end_events), - 0, - f'no end_callback events for node: {node}', - ) - - -if __name__ == '__main__': - unittest.main() From ef64e4cb18d3d3a09909c17e2d94c427e1e0cb09 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Sat, 8 Feb 2020 19:42:41 -0500 Subject: [PATCH 4/4] Point instrumented rclcpp branch to service callback registration fix --- instrumented.repos | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumented.repos b/instrumented.repos index d453b10..7800d61 100644 --- a/instrumented.repos +++ b/instrumented.repos @@ -5,5 +5,5 @@ repositories: version: master ros2/rclcpp: type: git - url: https://github.com/ros2/rclcpp.git - version: master + url: https://gitlab.com/micro-ROS/ros_tracing/rclcpp.git + version: add-missing-service-callback-registration