From ce0c39a24d8a8f9b3dc39a935baf811576588f53 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Tue, 12 Nov 2019 18:06:42 -0800 Subject: [PATCH 01/20] Update test_intra after new intraprocess communications --- tracetools_test/CMakeLists.txt | 2 +- tracetools_test/test/test_intra.py | 47 +++++++----------------------- 2 files changed, 12 insertions(+), 37 deletions(-) diff --git a/tracetools_test/CMakeLists.txt b/tracetools_test/CMakeLists.txt index 9de0467..e744477 100644 --- a/tracetools_test/CMakeLists.txt +++ b/tracetools_test/CMakeLists.txt @@ -141,7 +141,7 @@ if(BUILD_TESTING) # Run each test in its own pytest invocation set(_tracetools_test_pytest_tests - #test/test_intra.py + test/test_intra.py test/test_node.py test/test_publisher.py test/test_service.py diff --git a/tracetools_test/test/test_intra.py b/tracetools_test/test/test_intra.py index 413b5d3..4b4fedb 100644 --- a/tracetools_test/test/test_intra.py +++ b/tracetools_test/test/test_intra.py @@ -36,28 +36,19 @@ class TestIntra(TraceTestCase): # Check events order as set (e.g. node_init before pub_init) self.assertEventsOrderSet(self._events_ros) - # Check sub_init for normal and intraprocess events + # Check sub_init sub_init_events = self.get_events_with_name('ros2:rcl_subscription_init') sub_init_normal_events = self.get_events_with_field_value( 'topic_name', '/the_topic', sub_init_events) - sub_init_intra_events = self.get_events_with_field_value( - 'topic_name', - '/the_topic/_intra', - sub_init_events) self.assertNumEventsEqual( sub_init_normal_events, 1, - 'none or more than 1 sub init event for normal sub') - self.assertNumEventsEqual( - sub_init_intra_events, - 1, - 'none or more than 1 sub init event for intra sub') + 'none or more than 1 sub init event') - # Get subscription handles for normal & intra subscriptions + # Get subscription handle sub_init_normal_event = sub_init_normal_events[0] - # Note: sub handle linked to "normal" topic is the one actually linked to intra callback sub_handle_intra = self.get_field(sub_init_normal_event, 'subscription_handle') print(f'sub_handle_intra: {sub_handle_intra}') @@ -73,27 +64,27 @@ class TestIntra(TraceTestCase): 1, 'none or more than 1 callback added event') callback_added_event = callback_added_events[0] - callback_handle_intra = self.get_field(callback_added_event, 'callback') + callback_handle = self.get_field(callback_added_event, 'callback') # Get corresponding callback start/end pairs start_events = self.get_events_with_name('ros2:callback_start') end_events = self.get_events_with_name('ros2:callback_end') - # Should still have at least two start:end pairs (1 normal + 1 intra) + # Should have at least one start:end pair self.assertNumEventsGreaterEqual( start_events, - 2, - 'does not have at least 2 callback start events') + 1, + 'does not have at least 1 callback start event') self.assertNumEventsGreaterEqual( end_events, - 2, - 'does not have at least 2 callback end events') + 1, + 'does not have at least 1 callback end event') start_events_intra = self.get_events_with_field_value( 'callback', - callback_handle_intra, + callback_handle, start_events) end_events_intra = self.get_events_with_field_value( 'callback', - callback_handle_intra, + callback_handle, end_events) self.assertNumEventsGreaterEqual( start_events_intra, @@ -112,22 +103,6 @@ class TestIntra(TraceTestCase): 1, 'is_intra_process field value not valid for intra callback') - # Also check that the other callback_start event (normal one) has the right field value - start_events_not_intra = self.get_events_with_field_not_value( - 'callback', - callback_handle_intra, - start_events) - self.assertNumEventsGreaterEqual( - start_events_not_intra, - 1, - 'no normal start event') - start_event_not_intra = start_events_not_intra[0] - self.assertFieldEquals( - start_event_not_intra, - 'is_intra_process', - 0, - 'is_intra_process field value not valid for normal callback') - if __name__ == '__main__': unittest.main() From 5fbc899941e3fed2e0aa742840e04869095a8db6 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Tue, 12 Nov 2019 18:06:51 -0800 Subject: [PATCH 02/20] Cleanup test --- tracetools_test/test/test_intra.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tracetools_test/test/test_intra.py b/tracetools_test/test/test_intra.py index 4b4fedb..f08616d 100644 --- a/tracetools_test/test/test_intra.py +++ b/tracetools_test/test/test_intra.py @@ -50,7 +50,6 @@ class TestIntra(TraceTestCase): # Get subscription handle sub_init_normal_event = sub_init_normal_events[0] sub_handle_intra = self.get_field(sub_init_normal_event, 'subscription_handle') - print(f'sub_handle_intra: {sub_handle_intra}') # Get corresponding callback handle # Callback handle From 44166ff48fea9af630cfb10412e24063d6cf797d Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Thu, 14 Nov 2019 14:07:36 -0800 Subject: [PATCH 03/20] Add new rclcpp_subscription_init tracepoint as an intermediate --- tracetools/include/tracetools/tp_call.h | 17 +++++++++++++++-- tracetools/include/tracetools/tracetools.h | 10 +++++++++- tracetools/src/tracetools.c | 16 ++++++++++++++-- 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/tracetools/include/tracetools/tp_call.h b/tracetools/include/tracetools/tp_call.h index a1aaa13..5f30460 100644 --- a/tracetools/include/tracetools/tp_call.h +++ b/tracetools/include/tracetools/tp_call.h @@ -100,13 +100,26 @@ TRACEPOINT_EVENT( TRACEPOINT_EVENT( TRACEPOINT_PROVIDER, - rclcpp_subscription_callback_added, + rclcpp_subscription_init, TP_ARGS( const void *, subscription_handle_arg, - const void *, callback_arg + const void *, subscription_arg ), TP_FIELDS( ctf_integer_hex(const void *, subscription_handle, subscription_handle_arg) + ctf_integer_hex(const void *, subscription, subscription_arg) + ) +) + +TRACEPOINT_EVENT( + TRACEPOINT_PROVIDER, + rclcpp_subscription_callback_added, + TP_ARGS( + const void *, subscription_arg, + const void *, callback_arg + ), + TP_FIELDS( + ctf_integer_hex(const void *, subscription, subscription_arg) ctf_integer_hex(const void *, callback, callback_arg) ) ) diff --git a/tracetools/include/tracetools/tracetools.h b/tracetools/include/tracetools/tracetools.h index 141bfb8..9c768e2 100644 --- a/tracetools/include/tracetools/tracetools.h +++ b/tracetools/include/tracetools/tracetools.h @@ -80,12 +80,20 @@ DECLARE_TRACEPOINT( const char * topic_name, const size_t queue_depth) +/** + * tp: rclcpp_subscription_init + */ +DECLARE_TRACEPOINT( + rclcpp_subscription_init, + const void * subscription_handle, + const void * subscription) + /** * tp: rclcpp_subscription_callback_added */ DECLARE_TRACEPOINT( rclcpp_subscription_callback_added, - const void * subscription_handle, + const void * subscription, const void * callback) /** diff --git a/tracetools/src/tracetools.c b/tracetools/src/tracetools.c index dcb713e..75c8037 100644 --- a/tracetools/src/tracetools.c +++ b/tracetools/src/tracetools.c @@ -105,14 +105,26 @@ void TRACEPOINT( } void TRACEPOINT( - rclcpp_subscription_callback_added, + rclcpp_subscription_init, const void * subscription_handle, + const void * subscription) +{ + CONDITIONAL_TP( + ros2, + rclcpp_subscription_init, + subscription_handle, + subscription); +} + +void TRACEPOINT( + rclcpp_subscription_callback_added, + const void * subscription, const void * callback) { CONDITIONAL_TP( ros2, rclcpp_subscription_callback_added, - subscription_handle, + subscription, callback); } From 2740d40b0f6ec7580a708cca753d6d5af747948e Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Thu, 14 Nov 2019 14:24:18 -0800 Subject: [PATCH 04/20] Move tracepoint provider name to tracepoint macro --- tracetools/src/tracetools.c | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/tracetools/src/tracetools.c b/tracetools/src/tracetools.c index 75c8037..f5ca06f 100644 --- a/tracetools/src/tracetools.c +++ b/tracetools/src/tracetools.c @@ -19,7 +19,7 @@ #if defined(TRACETOOLS_LTTNG_ENABLED) # include "tracetools/tp_call.h" # define CONDITIONAL_TP(...) \ - tracepoint(__VA_ARGS__) + tracepoint(TRACEPOINT_PROVIDER, __VA_ARGS__) #else # define CONDITIONAL_TP(...) #endif @@ -46,7 +46,6 @@ void TRACEPOINT( const void * context_handle) { CONDITIONAL_TP( - ros2, rcl_init, context_handle, tracetools_VERSION); @@ -60,7 +59,6 @@ void TRACEPOINT( const char * node_namespace) { CONDITIONAL_TP( - ros2, rcl_node_init, node_handle, rmw_handle, @@ -77,7 +75,6 @@ void TRACEPOINT( const size_t queue_depth) { CONDITIONAL_TP( - ros2, rcl_publisher_init, publisher_handle, node_handle, @@ -95,7 +92,6 @@ void TRACEPOINT( const size_t queue_depth) { CONDITIONAL_TP( - ros2, rcl_subscription_init, subscription_handle, node_handle, @@ -110,7 +106,6 @@ void TRACEPOINT( const void * subscription) { CONDITIONAL_TP( - ros2, rclcpp_subscription_init, subscription_handle, subscription); @@ -122,7 +117,6 @@ void TRACEPOINT( const void * callback) { CONDITIONAL_TP( - ros2, rclcpp_subscription_callback_added, subscription, callback); @@ -136,7 +130,6 @@ void TRACEPOINT( const char * service_name) { CONDITIONAL_TP( - ros2, rcl_service_init, service_handle, node_handle, @@ -150,7 +143,6 @@ void TRACEPOINT( const void * callback) { CONDITIONAL_TP( - ros2, rclcpp_service_callback_added, service_handle, callback); @@ -164,7 +156,6 @@ void TRACEPOINT( const char * service_name) { CONDITIONAL_TP( - ros2, rcl_client_init, client_handle, node_handle, @@ -178,7 +169,6 @@ void TRACEPOINT( int64_t period) { CONDITIONAL_TP( - ros2, rcl_timer_init, timer_handle, period); @@ -190,7 +180,6 @@ void TRACEPOINT( const void * callback) { CONDITIONAL_TP( - ros2, rclcpp_timer_callback_added, timer_handle, callback); @@ -202,7 +191,6 @@ void TRACEPOINT( const char * function_symbol) { CONDITIONAL_TP( - ros2, rclcpp_callback_register, callback, function_symbol); @@ -214,7 +202,6 @@ void TRACEPOINT( const bool is_intra_process) { CONDITIONAL_TP( - ros2, callback_start, callback, (is_intra_process ? 1 : 0)); @@ -225,7 +212,6 @@ void TRACEPOINT( const void * callback) { CONDITIONAL_TP( - ros2, callback_end, callback); } From d0eb897ab050e844c928b77a7855ce58bd4a7195 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Thu, 14 Nov 2019 14:35:07 -0800 Subject: [PATCH 05/20] Add new rclcpp_subscription_init tracepoint to list --- tracetools_trace/tracetools_trace/tools/names.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tracetools_trace/tracetools_trace/tools/names.py b/tracetools_trace/tracetools_trace/tools/names.py index 43ad7b0..4e02a43 100644 --- a/tracetools_trace/tracetools_trace/tools/names.py +++ b/tracetools_trace/tracetools_trace/tools/names.py @@ -60,6 +60,7 @@ DEFAULT_EVENTS_ROS = [ 'ros2:rcl_node_init', 'ros2:rcl_publisher_init', 'ros2:rcl_subscription_init', + 'ros2:rclcpp_subscription_init', 'ros2:rclcpp_subscription_callback_added', 'ros2:rcl_service_init', 'ros2:rclcpp_service_callback_added', From 8f3a4582bb372322ec8533679a857dd2e31636d7 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Thu, 14 Nov 2019 16:15:55 -0800 Subject: [PATCH 06/20] Fix events set assertion method --- tracetools_test/test/test_intra.py | 4 ++-- tracetools_test/test/test_node.py | 4 ++-- tracetools_test/test/test_publisher.py | 4 ++-- tracetools_test/test/test_service.py | 4 ++-- tracetools_test/test/test_service_callback.py | 4 ++-- tracetools_test/test/test_subscription.py | 4 ++-- tracetools_test/test/test_subscription_callback.py | 4 ++-- tracetools_test/test/test_timer.py | 4 ++-- tracetools_test/tracetools_test/case.py | 4 ++-- 9 files changed, 18 insertions(+), 18 deletions(-) diff --git a/tracetools_test/test/test_intra.py b/tracetools_test/test/test_intra.py index f08616d..e4f20d6 100644 --- a/tracetools_test/test/test_intra.py +++ b/tracetools_test/test/test_intra.py @@ -33,8 +33,8 @@ class TestIntra(TraceTestCase): ) def test_all(self): - # Check events order as set (e.g. node_init before pub_init) - self.assertEventsOrderSet(self._events_ros) + # Check events as set + self.assertEventsSet(self._events_ros) # Check sub_init sub_init_events = self.get_events_with_name('ros2:rcl_subscription_init') diff --git a/tracetools_test/test/test_node.py b/tracetools_test/test/test_node.py index bd731b2..189a937 100644 --- a/tracetools_test/test/test_node.py +++ b/tracetools_test/test/test_node.py @@ -34,8 +34,8 @@ class TestNode(TraceTestCase): ) def test_all(self): - # Check events order as set (e.g. init before node_init) - self.assertEventsOrderSet(self._events_ros) + # Check events as set + self.assertEventsSet(self._events_ros) # Check fields rcl_init_events = self.get_events_with_name('ros2:rcl_init') diff --git a/tracetools_test/test/test_publisher.py b/tracetools_test/test/test_publisher.py index 595de47..ad2a3de 100644 --- a/tracetools_test/test/test_publisher.py +++ b/tracetools_test/test/test_publisher.py @@ -31,8 +31,8 @@ class TestPublisher(TraceTestCase): ) def test_all(self): - # Check events order as set (e.g. node_init before pub_init) - self.assertEventsOrderSet(self._events_ros) + # Check events as set + self.assertEventsSet(self._events_ros) # Check fields pub_init_events = self.get_events_with_name('ros2:rcl_publisher_init') diff --git a/tracetools_test/test/test_service.py b/tracetools_test/test/test_service.py index f4a315c..856a873 100644 --- a/tracetools_test/test/test_service.py +++ b/tracetools_test/test/test_service.py @@ -32,8 +32,8 @@ class TestService(TraceTestCase): ) def test_all(self): - # Check events order as set (e.g. service_init before callback_added) - self.assertEventsOrderSet(self._events_ros) + # Check events as set + self.assertEventsSet(self._events_ros) # Check fields srv_init_events = self.get_events_with_name('ros2:rcl_service_init') diff --git a/tracetools_test/test/test_service_callback.py b/tracetools_test/test/test_service_callback.py index 97412ef..b77336a 100644 --- a/tracetools_test/test/test_service_callback.py +++ b/tracetools_test/test/test_service_callback.py @@ -31,8 +31,8 @@ class TestServiceCallback(TraceTestCase): ) def test_all(self): - # Check events order as set (e.g. start before end) - self.assertEventsOrderSet(self._events_ros) + # Check events as set + self.assertEventsSet(self._events_ros) # Check fields start_events = self.get_events_with_name('ros2:callback_start') diff --git a/tracetools_test/test/test_subscription.py b/tracetools_test/test/test_subscription.py index c7ba776..9237c63 100644 --- a/tracetools_test/test/test_subscription.py +++ b/tracetools_test/test/test_subscription.py @@ -32,8 +32,8 @@ class TestSubscription(TraceTestCase): ) def test_all(self): - # Check events order as set (e.g. sub_init before callback_added) - self.assertEventsOrderSet(self._events_ros) + # Check events as set + self.assertEventsSet(self._events_ros) # Check fields sub_init_events = self.get_events_with_name('ros2:rcl_subscription_init') diff --git a/tracetools_test/test/test_subscription_callback.py b/tracetools_test/test/test_subscription_callback.py index 1f057da..5312c64 100644 --- a/tracetools_test/test/test_subscription_callback.py +++ b/tracetools_test/test/test_subscription_callback.py @@ -31,8 +31,8 @@ class TestSubscriptionCallback(TraceTestCase): ) def test_all(self): - # Check events order as set (e.g. start before end) - self.assertEventsOrderSet(self._events_ros) + # Check events as set + self.assertEventsSet(self._events_ros) # Check fields start_events = self.get_events_with_name('ros2:callback_start') diff --git a/tracetools_test/test/test_timer.py b/tracetools_test/test/test_timer.py index 308e7df..99f9e37 100644 --- a/tracetools_test/test/test_timer.py +++ b/tracetools_test/test/test_timer.py @@ -33,8 +33,8 @@ class TestTimer(TraceTestCase): ) def test_all(self): - # Check events order as set (e.g. init, callback added, start, end) - self.assertEventsOrderSet(self._events_ros) + # Check events as set + self.assertEventsSet(self._events_ros) # Check fields init_events = self.get_events_with_name('ros2:rcl_timer_init') diff --git a/tracetools_test/tracetools_test/case.py b/tracetools_test/tracetools_test/case.py index 9e6a899..353edb3 100644 --- a/tracetools_test/tracetools_test/case.py +++ b/tracetools_test/tracetools_test/case.py @@ -99,13 +99,13 @@ class TraceTestCase(unittest.TestCase): def tearDown(self): cleanup_trace(self._full_path) - def assertEventsOrderSet(self, event_names: List[str]): + def assertEventsSet(self, event_names: List[str]): """ Compare given event names to trace events names as sets. :param event_names: the list of event names to compare to (as a set) """ - self.assertSetEqual(set(self._event_names), set(event_names), 'wrong events order') + self.assertSetEqual(set(self._event_names), set(event_names), 'wrong events') def assertProcessNamesExist(self, names: List[str]): """ From fc1ce31504ed11f565e7352bec37d6ff0e24adcf Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Thu, 14 Nov 2019 16:16:55 -0800 Subject: [PATCH 07/20] Make get_events_with_field_*value take a single field value or a list --- tracetools_test/tracetools_test/case.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/tracetools_test/tracetools_test/case.py b/tracetools_test/tracetools_test/case.py index 353edb3..a530ddf 100644 --- a/tracetools_test/tracetools_test/case.py +++ b/tracetools_test/tracetools_test/case.py @@ -320,38 +320,42 @@ class TraceTestCase(unittest.TestCase): def get_events_with_field_value( self, field_name: str, - field_value: Any, + field_values: Any, events: List[DictEvent] = None ) -> List[DictEvent]: """ Get all events with the given field:value. :param field_name: the name of the field to check - :param field_value: the value of the field to check + :param field_values: the value(s) of the field to check :param events: the events to check (or `None` to check all events) :return: the events with the given field:value pair """ + if not isinstance(field_values, list): + field_values = [field_values] if events is None: events = self._events - return [e for e in events if get_field(e, field_name, None) == field_value] + return [e for e in events if get_field(e, field_name, None) in field_values] def get_events_with_field_not_value( self, field_name: str, - field_value: Any, + field_values: Any, events: List[DictEvent] = None ) -> List[DictEvent]: """ Get all events with the given field but not the value. :param field_name: the name of the field to check - :param field_value: the value of the field to check + :param field_values: the value(s) of the field to check :param events: the events to check (or `None` to check all events) :return: the events with the given field:value pair """ + if not isinstance(field_values, list): + field_values = [field_values] if events is None: events = self._events - return [e for e in events if get_field(e, field_name, None) != field_value] + return [e for e in events if get_field(e, field_name, None) not in field_values] def are_events_ordered(self, first_event: DictEvent, second_event: DictEvent): """ From 3c4a386003a5bcd6ecad2b30e53d9644431e0aff Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Thu, 14 Nov 2019 16:17:18 -0800 Subject: [PATCH 08/20] Update test_intra test after new intra-process communications --- tracetools_test/test/test_intra.py | 104 ++++++++++++++++++++--------- 1 file changed, 72 insertions(+), 32 deletions(-) diff --git a/tracetools_test/test/test_intra.py b/tracetools_test/test/test_intra.py index e4f20d6..e7f447a 100644 --- a/tracetools_test/test/test_intra.py +++ b/tracetools_test/test/test_intra.py @@ -25,45 +25,68 @@ class TestIntra(TraceTestCase): session_name_prefix='session-test-intra', events_ros=[ 'ros2:rcl_subscription_init', + 'ros2:rclcpp_subscription_init', 'ros2:rclcpp_subscription_callback_added', 'ros2:callback_start', 'ros2:callback_end', ], - nodes=['test_intra'] + nodes=['test_intra'], ) def test_all(self): # Check events as set self.assertEventsSet(self._events_ros) - # Check sub_init - sub_init_events = self.get_events_with_name('ros2:rcl_subscription_init') - sub_init_normal_events = self.get_events_with_field_value( + print('EVENTS: ', self._events) + + # Check rcl_subscription_init events + rcl_sub_init_events = self.get_events_with_name('ros2:rcl_subscription_init') + rcl_sub_init_topic_events = self.get_events_with_field_value( 'topic_name', '/the_topic', - sub_init_events) + rcl_sub_init_events, + ) + # Only 1 for our given topic self.assertNumEventsEqual( - sub_init_normal_events, + rcl_sub_init_topic_events, 1, - 'none or more than 1 sub init event') + 'none or more than 1 rcl_sub_init event for the topic', + ) # Get subscription handle - sub_init_normal_event = sub_init_normal_events[0] - sub_handle_intra = self.get_field(sub_init_normal_event, 'subscription_handle') + rcl_sub_init_topic_event = rcl_sub_init_topic_events[0] + sub_handle_intra = self.get_field(rcl_sub_init_topic_event, 'subscription_handle') - # Get corresponding callback handle - # Callback handle - callback_added_events = self.get_events_with_field_value( + # Check rclcpp_subscription_init events + rclcpp_sub_init_events = self.get_events_with_name('ros2:rclcpp_subscription_init') + rclcpp_sub_init_topic_events = self.get_events_with_field_value( 'subscription_handle', sub_handle_intra, - self.get_events_with_name( - 'ros2:rclcpp_subscription_callback_added')) + rclcpp_sub_init_events, + ) + # Should have 2 events for the given subscription handle + # (Subscription and SubscriptionIntraProcess) self.assertNumEventsEqual( - callback_added_events, - 1, - 'none or more than 1 callback added event') - callback_added_event = callback_added_events[0] - callback_handle = self.get_field(callback_added_event, 'callback') + rclcpp_sub_init_topic_events, + 2, + 'number of rclcpp_sub_init events for the topic not equal to 2', + ) + + # Get the 2 subscription pointers + events = rclcpp_sub_init_topic_events + subscription_pointers = [self.get_field(e, 'subscription') for e in events] + + # Get the corresponding callback pointers + rclcpp_sub_callback_added_events = self.get_events_with_name( + 'ros2:rclcpp_subscription_callback_added', + ) + rclcpp_sub_callback_added_topic_events = self.get_events_with_field_value( + 'subscription', + subscription_pointers, + rclcpp_sub_callback_added_events, + ) + events = rclcpp_sub_callback_added_topic_events + callback_pointers = [self.get_field(e, 'callback') for e in events] # Get corresponding callback start/end pairs start_events = self.get_events_with_name('ros2:callback_start') @@ -77,30 +100,47 @@ class TestIntra(TraceTestCase): end_events, 1, 'does not have at least 1 callback end event') - start_events_intra = self.get_events_with_field_value( + start_events_topic = self.get_events_with_field_value( 'callback', - callback_handle, + callback_pointers, start_events) - end_events_intra = self.get_events_with_field_value( + end_events_topic = self.get_events_with_field_value( 'callback', - callback_handle, + callback_pointers, end_events) self.assertNumEventsGreaterEqual( - start_events_intra, + start_events_topic, 1, - 'no intra start event') + 'no callback_start event found for topic') self.assertNumEventsGreaterEqual( - end_events_intra, + end_events_topic, 1, - 'no intra end event') + 'no callback_end found event for topic') - # Check is_intra_process field value - start_event_intra = start_events_intra[0] - self.assertFieldEquals( - start_event_intra, + # Check for is_intra_process field value of 1/true + start_events_intra = self.get_events_with_field_value( 'is_intra_process', 1, - 'is_intra_process field value not valid for intra callback') + start_events_topic, + ) + # Should have one event + self.assertNumEventsEqual( + start_events_intra, + 1, + 'none or more than 1 callback_start event for intra-process topic', + ) + # As a sanity check, make sure its callback pointer has a corresponding callback_end event + callback_pointer_intra = self.get_field(start_events_intra[0], 'callback') + end_events_intra = self.get_events_with_field_value( + 'callback', + callback_pointer_intra, + end_events_topic, + ) + self.assertNumEventsEqual( + end_events_intra, + 1, + 'none or more than 1 callback_end event for intra-process topic', + ) if __name__ == '__main__': From 66750e4fbbcb8bdf79ce34e2c584175f5a2a58b2 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Sat, 16 Nov 2019 15:03:42 -0800 Subject: [PATCH 09/20] Update style in test_node --- tracetools_test/test/test_node.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tracetools_test/test/test_node.py b/tracetools_test/test/test_node.py index 189a937..563ad6d 100644 --- a/tracetools_test/test/test_node.py +++ b/tracetools_test/test/test_node.py @@ -56,7 +56,8 @@ class TestNode(TraceTestCase): for node_name in self._nodes: self.assertTrue( node_name in node_name_fields, - f'cannot find node_init event for node name: {node_name} ({node_name_fields})') + f'cannot find node_init event for node name: {node_name} ({node_name_fields})', + ) if __name__ == '__main__': From 3c3a575c80af5e23f12689694832ea46932a3dcc Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Sat, 16 Nov 2019 15:09:10 -0800 Subject: [PATCH 10/20] Refactor handling of str vs. list(str) in assertValidHandle --- tracetools_test/tracetools_test/case.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tracetools_test/tracetools_test/case.py b/tracetools_test/tracetools_test/case.py index a530ddf..f783c4e 100644 --- a/tracetools_test/tracetools_test/case.py +++ b/tracetools_test/tracetools_test/case.py @@ -119,15 +119,15 @@ class TraceTestCase(unittest.TestCase): name_trimmed = name[:15] self.assertTrue(name_trimmed in procnames, 'node name not found in tracepoints') - def assertValidHandle(self, event: DictEvent, handle_field_name: Union[str, List[str]]): + def assertValidHandle(self, event: DictEvent, handle_field_names: Union[str, List[str]]): """ Check that the handle associated with a field name is valid. :param event: the event which has a handle field - :param handle_field_name: the field name(s) of the handle to check + :param handle_field_names: the handle field name(s) to check """ - is_list = isinstance(handle_field_name, list) - handle_field_names = handle_field_name if is_list else [handle_field_name] + 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') From ff4155ab840d1820c78577e58624306659e173e5 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Sat, 16 Nov 2019 15:09:26 -0800 Subject: [PATCH 11/20] Update style in test_publisher --- tracetools_test/test/test_publisher.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/tracetools_test/test/test_publisher.py b/tracetools_test/test/test_publisher.py index ad2a3de..8f320b7 100644 --- a/tracetools_test/test/test_publisher.py +++ b/tracetools_test/test/test_publisher.py @@ -39,7 +39,8 @@ class TestPublisher(TraceTestCase): for event in pub_init_events: self.assertValidHandle( event, - ['publisher_handle', 'node_handle', 'rmw_publisher_handle']) + ['publisher_handle', 'node_handle', 'rmw_publisher_handle'], + ) self.assertValidQueueDepth(event, 'queue_depth') self.assertStringFieldNotEmpty(event, 'topic_name') @@ -48,11 +49,13 @@ class TestPublisher(TraceTestCase): test_pub_init_topic_events = self.get_events_with_field_value( 'topic_name', '/the_topic', - test_pub_init_events) + test_pub_init_events, + ) self.assertNumEventsEqual( test_pub_init_topic_events, 1, - 'none or more than 1 pub_init even for test topic') + 'none or more than 1 rcl_pub_init even for test topic', + ) # Check queue_depth value test_pub_init_topic_event = test_pub_init_topic_events[0] @@ -60,23 +63,27 @@ class TestPublisher(TraceTestCase): test_pub_init_topic_event, 'queue_depth', 10, - 'pub_init event does not have expected queue depth value') + 'pub_init event does not have expected queue depth value', + ) # Check that the node handle matches with the node_init event node_init_events = self.get_events_with_name('ros2:rcl_node_init') test_pub_node_init_events = self.get_events_with_procname( 'test_publisher', - node_init_events) + node_init_events, + ) self.assertNumEventsEqual( test_pub_node_init_events, 1, - 'none or more than 1 node_init event') + 'none or more than 1 node_init event', + ) test_pub_node_init_event = test_pub_node_init_events[0] self.assertMatchingField( test_pub_node_init_event, 'node_handle', None, - test_pub_init_events) + test_pub_init_events, + ) if __name__ == '__main__': From 5b5731dc84c9d3df469440032d12626a30ec9e4f Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Sat, 16 Nov 2019 15:17:15 -0800 Subject: [PATCH 12/20] Update style for test_service_callback --- tracetools_test/test/test_service_callback.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tracetools_test/test/test_service_callback.py b/tracetools_test/test/test_service_callback.py index b77336a..8bfcddb 100644 --- a/tracetools_test/test/test_service_callback.py +++ b/tracetools_test/test/test_service_callback.py @@ -36,6 +36,8 @@ class TestServiceCallback(TraceTestCase): # 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) @@ -43,24 +45,25 @@ class TestServiceCallback(TraceTestCase): event, 'is_intra_process', 0, - 'invalid value for is_intra_process') - - end_events = self.get_events_with_name('ros2:callback_end') + 'invalid value for is_intra_process', + ) for event in end_events: self.assertValidHandle(event, 'callback') - # Check that there is at least a start/end pair for each node + # 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}') + f'no start_callback events for node: {node}', + ) self.assertGreater( len(test_end_events), 0, - f'no end_callback events for node: {node}') + f'no end_callback events for node: {node}', + ) if __name__ == '__main__': From 40ebb9d09a63adbc3c45a45eca7c8fbe9e765140 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Sat, 16 Nov 2019 15:18:00 -0800 Subject: [PATCH 13/20] Add note about procname getting truncated in get_events_with_procname() --- tracetools_test/tracetools_test/case.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tracetools_test/tracetools_test/case.py b/tracetools_test/tracetools_test/case.py index f783c4e..9ea38f6 100644 --- a/tracetools_test/tracetools_test/case.py +++ b/tracetools_test/tracetools_test/case.py @@ -309,6 +309,9 @@ class TraceTestCase(unittest.TestCase): """ Get all events with the given procname. + Note: the given procname value will be truncated to the same max length as the procname + field. + :param procname: the procname :param events: the events to check (or `None` to check all events) :return: the events with the given procname From 481dbea21bad41a4225812be434f0f11f000fd2f Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Sat, 16 Nov 2019 15:18:25 -0800 Subject: [PATCH 14/20] Update style in TraceTestCase file --- tracetools_test/tracetools_test/case.py | 81 ++++++++++++++++++------- 1 file changed, 59 insertions(+), 22 deletions(-) diff --git a/tracetools_test/tracetools_test/case.py b/tracetools_test/tracetools_test/case.py index 9ea38f6..b7763c5 100644 --- a/tracetools_test/tracetools_test/case.py +++ b/tracetools_test/tracetools_test/case.py @@ -99,7 +99,10 @@ class TraceTestCase(unittest.TestCase): def tearDown(self): cleanup_trace(self._full_path) - def assertEventsSet(self, event_names: List[str]): + def assertEventsSet( + self, + event_names: List[str], + ) -> None: """ Compare given event names to trace events names as sets. @@ -107,7 +110,10 @@ class TraceTestCase(unittest.TestCase): """ self.assertSetEqual(set(self._event_names), set(event_names), 'wrong events') - def assertProcessNamesExist(self, names: List[str]): + def assertProcessNamesExist( + self, + names: List[str], + ) -> None: """ Check that the given processes exist. @@ -119,7 +125,11 @@ class TraceTestCase(unittest.TestCase): name_trimmed = name[:15] self.assertTrue(name_trimmed in procnames, 'node name not found in tracepoints') - def assertValidHandle(self, event: DictEvent, handle_field_names: Union[str, List[str]]): + def assertValidHandle( + self, + event: DictEvent, + handle_field_names: Union[str, List[str]], + ) -> None: """ Check that the handle associated with a field name is valid. @@ -133,7 +143,11 @@ class TraceTestCase(unittest.TestCase): self.assertIsInstance(handle_value, int, 'handle value not int') self.assertGreater(handle_value, 0, f'invalid handle value: {field_name}') - def assertValidQueueDepth(self, event: DictEvent, queue_depth_field_name: str = 'queue_depth'): + def assertValidQueueDepth( + self, + event: DictEvent, + queue_depth_field_name: str = 'queue_depth', + ) -> None: """ Check that the queue depth value is valid. @@ -144,7 +158,11 @@ class TraceTestCase(unittest.TestCase): self.assertIsInstance(queue_depth_value, int, 'invalid queue depth type') self.assertGreater(queue_depth_value, 0, 'invalid queue depth') - def assertStringFieldNotEmpty(self, event: DictEvent, string_field_name: str): + def assertStringFieldNotEmpty( + self, + event: DictEvent, + string_field_name: str, + ) -> None: """ Check that a string field is not empty. @@ -154,7 +172,11 @@ class TraceTestCase(unittest.TestCase): string_field = self.get_field(event, string_field_name) self.assertGreater(len(string_field), 0, 'empty string') - def assertEventAfterTimestamp(self, event: DictEvent, timestamp: int): + def assertEventAfterTimestamp( + self, + event: DictEvent, + timestamp: int, + ) -> None: """ Check that the event happens after the given timestamp. @@ -163,7 +185,11 @@ class TraceTestCase(unittest.TestCase): """ self.assertGreater(get_event_timestamp(event), timestamp, 'event not after timestamp') - def assertEventOrder(self, first_event: DictEvent, second_event: DictEvent): + def assertEventOrder( + self, + first_event: DictEvent, + second_event: DictEvent, + ) -> None: """ Check that the first event was generated before the second event. @@ -176,8 +202,8 @@ class TraceTestCase(unittest.TestCase): self, events: List[DictEvent], expected_number: int, - msg: str = 'wrong number of events' - ): + msg: str = 'wrong number of events', + ) -> None: """ Check number of events. @@ -191,8 +217,8 @@ class TraceTestCase(unittest.TestCase): self, events: List[DictEvent], min_expected_number: int, - msg: str = 'wrong number of events' - ): + msg: str = 'wrong number of events', + ) -> None: """ Check that the number of events is greater of equal. @@ -207,8 +233,8 @@ class TraceTestCase(unittest.TestCase): initial_event: DictEvent, field_name: str, matching_event_name: str = None, - events: List[DictEvent] = None - ): + events: List[DictEvent] = None, + ) -> None: """ Check that the value of a field for a given event has a matching event that follows. @@ -247,8 +273,8 @@ class TraceTestCase(unittest.TestCase): event: DictEvent, field_name: str, value: Any, - msg: str = 'wrong field value' - ): + msg: str = 'wrong field value', + ) -> None: """ Check the value of a field. @@ -260,7 +286,11 @@ class TraceTestCase(unittest.TestCase): actual_value = self.get_field(event, field_name) self.assertEqual(actual_value, value, msg) - def get_field(self, event: DictEvent, field_name: str) -> Any: + def get_field( + self, + event: DictEvent, + field_name: str, + ) -> Any: """ Get field value; will fail test if not found. @@ -276,7 +306,10 @@ class TraceTestCase(unittest.TestCase): else: return value - def get_procname(self, event: DictEvent) -> str: + def get_procname( + self, + event: DictEvent, + ) -> str: """ Get procname. @@ -288,7 +321,7 @@ class TraceTestCase(unittest.TestCase): def get_events_with_name( self, event_name: str, - events: List[DictEvent] = None + events: List[DictEvent] = None, ) -> List[DictEvent]: """ Get all events with the given name. @@ -304,7 +337,7 @@ class TraceTestCase(unittest.TestCase): def get_events_with_procname( self, procname: str, - events: List[DictEvent] = None + events: List[DictEvent] = None, ) -> List[DictEvent]: """ Get all events with the given procname. @@ -324,7 +357,7 @@ class TraceTestCase(unittest.TestCase): self, field_name: str, field_values: Any, - events: List[DictEvent] = None + events: List[DictEvent] = None, ) -> List[DictEvent]: """ Get all events with the given field:value. @@ -344,7 +377,7 @@ class TraceTestCase(unittest.TestCase): self, field_name: str, field_values: Any, - events: List[DictEvent] = None + events: List[DictEvent] = None, ) -> List[DictEvent]: """ Get all events with the given field but not the value. @@ -360,7 +393,11 @@ class TraceTestCase(unittest.TestCase): events = self._events return [e for e in events if get_field(e, field_name, None) not in field_values] - def are_events_ordered(self, first_event: DictEvent, second_event: DictEvent): + def are_events_ordered( + self, + first_event: DictEvent, + second_event: DictEvent, + ) -> bool: """ Check that the first event was generated before the second event. From 3fd400d039ad4a7156886794a134a4fc2669cc2b Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Sat, 16 Nov 2019 15:20:55 -0800 Subject: [PATCH 15/20] Update style in test_service --- tracetools_test/test/test_service.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/tracetools_test/test/test_service.py b/tracetools_test/test/test_service.py index 856a873..61a2f0a 100644 --- a/tracetools_test/test/test_service.py +++ b/tracetools_test/test/test_service.py @@ -37,11 +37,11 @@ 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') + for event in srv_init_events: self.assertValidHandle(event, ['service_handle', 'node_handle', 'rmw_service_handle']) self.assertStringFieldNotEmpty(event, 'service_name') - - callback_added_events = self.get_events_with_name('ros2:rclcpp_service_callback_added') for event in callback_added_events: self.assertValidHandle(event, ['service_handle', 'callback']) @@ -50,27 +50,32 @@ class TestService(TraceTestCase): event_service_names = self.get_events_with_field_value( 'service_name', '/the_service', - test_srv_init_events) + test_srv_init_events, + ) self.assertGreaterEqual( len(event_service_names), 1, - 'cannot find test service name') + 'cannot find test service name', + ) # Check that the node handle matches the node_init event 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) + node_init_events, + ) self.assertNumEventsEqual( test_srv_node_init_events, 1, - 'none or more than 1 node_init event') + 'none or more than 1 rcl_node_init event', + ) test_srv_node_init_event = test_srv_node_init_events[0] self.assertMatchingField( test_srv_node_init_event, 'node_handle', 'ros2:rcl_service_init', - test_srv_init_events) + test_srv_init_events, + ) # Check that the service handles match test_event_srv_init = event_service_names[0] @@ -78,7 +83,8 @@ class TestService(TraceTestCase): test_event_srv_init, 'service_handle', None, - callback_added_events) + callback_added_events, + ) if __name__ == '__main__': From a5739a099c6bb99144ec0f88007316a5ecf17454 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Sat, 16 Nov 2019 15:25:52 -0800 Subject: [PATCH 16/20] Add comma --- tracetools_test/test/test_node.py | 2 +- tracetools_test/test/test_publisher.py | 2 +- tracetools_test/test/test_service.py | 2 +- tracetools_test/test/test_service_callback.py | 2 +- tracetools_test/test/test_subscription_callback.py | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tracetools_test/test/test_node.py b/tracetools_test/test/test_node.py index 563ad6d..b1f672d 100644 --- a/tracetools_test/test/test_node.py +++ b/tracetools_test/test/test_node.py @@ -30,7 +30,7 @@ class TestNode(TraceTestCase): 'ros2:rcl_init', 'ros2:rcl_node_init', ], - nodes=['test_publisher'] + nodes=['test_publisher'], ) def test_all(self): diff --git a/tracetools_test/test/test_publisher.py b/tracetools_test/test/test_publisher.py index 8f320b7..89b4c87 100644 --- a/tracetools_test/test/test_publisher.py +++ b/tracetools_test/test/test_publisher.py @@ -27,7 +27,7 @@ class TestPublisher(TraceTestCase): 'ros2:rcl_node_init', 'ros2:rcl_publisher_init', ], - nodes=['test_publisher'] + nodes=['test_publisher'], ) def test_all(self): diff --git a/tracetools_test/test/test_service.py b/tracetools_test/test/test_service.py index 61a2f0a..ccc997a 100644 --- a/tracetools_test/test/test_service.py +++ b/tracetools_test/test/test_service.py @@ -28,7 +28,7 @@ class TestService(TraceTestCase): 'ros2:rcl_service_init', 'ros2:rclcpp_service_callback_added', ], - nodes=['test_service'] + nodes=['test_service'], ) def test_all(self): diff --git a/tracetools_test/test/test_service_callback.py b/tracetools_test/test/test_service_callback.py index 8bfcddb..d78f726 100644 --- a/tracetools_test/test/test_service_callback.py +++ b/tracetools_test/test/test_service_callback.py @@ -27,7 +27,7 @@ class TestServiceCallback(TraceTestCase): 'ros2:callback_start', 'ros2:callback_end', ], - nodes=['test_service_ping', 'test_service_pong'] + nodes=['test_service_ping', 'test_service_pong'], ) def test_all(self): diff --git a/tracetools_test/test/test_subscription_callback.py b/tracetools_test/test/test_subscription_callback.py index 5312c64..cfe463f 100644 --- a/tracetools_test/test/test_subscription_callback.py +++ b/tracetools_test/test/test_subscription_callback.py @@ -27,7 +27,7 @@ class TestSubscriptionCallback(TraceTestCase): 'ros2:callback_start', 'ros2:callback_end', ], - nodes=['test_ping', 'test_pong'] + nodes=['test_ping', 'test_pong'], ) def test_all(self): From bb509e672cee1e11525f1bf29a2208d157969f51 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Sat, 16 Nov 2019 15:27:08 -0800 Subject: [PATCH 17/20] Update style for test_subscription_callback --- tracetools_test/test/test_subscription_callback.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tracetools_test/test/test_subscription_callback.py b/tracetools_test/test/test_subscription_callback.py index cfe463f..e1e2483 100644 --- a/tracetools_test/test/test_subscription_callback.py +++ b/tracetools_test/test/test_subscription_callback.py @@ -42,14 +42,14 @@ class TestSubscriptionCallback(TraceTestCase): self.assertIsInstance(is_intra_process_value, int, 'is_intra_process not int') self.assertTrue( is_intra_process_value in [0, 1], - f'invalid value for is_intra_process: {is_intra_process_value}') + f'invalid value for is_intra_process: {is_intra_process_value}', + ) end_events = self.get_events_with_name('ros2:callback_end') for event in end_events: self.assertValidHandle(event, 'callback') # Check that a start:end pair has a common callback handle - # Note: might be unstable if tracing is disabled too early ping_events = self.get_events_with_procname('test_ping') pong_events = self.get_events_with_procname('test_pong') ping_events_start = self.get_events_with_name('ros2:callback_start', ping_events) @@ -59,13 +59,15 @@ class TestSubscriptionCallback(TraceTestCase): ping_start, 'callback', 'ros2:callback_end', - ping_events) + ping_events, + ) for pong_start in pong_events_start: self.assertMatchingField( pong_start, 'callback', 'ros2:callback_end', - pong_events) + pong_events, + ) if __name__ == '__main__': From 841bc50f65e1723c289de78104f8be0cc217b86a Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Sat, 16 Nov 2019 15:52:15 -0800 Subject: [PATCH 18/20] Update style in test_timer --- tracetools_test/test/test_timer.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tracetools_test/test/test_timer.py b/tracetools_test/test/test_timer.py index 99f9e37..ae52a6c 100644 --- a/tracetools_test/test/test_timer.py +++ b/tracetools_test/test/test_timer.py @@ -29,7 +29,7 @@ class TestTimer(TraceTestCase): 'ros2:callback_start', 'ros2:callback_end', ], - nodes=['test_timer'] + nodes=['test_timer'], ) def test_all(self): @@ -56,7 +56,8 @@ class TestTimer(TraceTestCase): event, 'is_intra_process', 0, - 'invalid value for is_intra_process') + 'invalid value for is_intra_process', + ) end_events = self.get_events_with_name('ros2:callback_end') for event in end_events: @@ -73,7 +74,8 @@ class TestTimer(TraceTestCase): test_init_event, 'timer_handle', 'ros2:rclcpp_timer_callback_added', - callback_added_events) + callback_added_events, + ) # Check that a callback start:end pair has a common callback handle for start_event in start_events: @@ -81,7 +83,8 @@ class TestTimer(TraceTestCase): start_event, 'callback', None, - end_events) + end_events, + ) if __name__ == '__main__': From 7c0e5712b15fb2d2609c995f400653ef56c160ce Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Sat, 16 Nov 2019 15:56:50 -0800 Subject: [PATCH 19/20] Update test_subscription after new intra-process communications --- tracetools_test/test/test_subscription.py | 79 ++++++++++++++++------- 1 file changed, 57 insertions(+), 22 deletions(-) diff --git a/tracetools_test/test/test_subscription.py b/tracetools_test/test/test_subscription.py index 9237c63..e9f5ebd 100644 --- a/tracetools_test/test/test_subscription.py +++ b/tracetools_test/test/test_subscription.py @@ -26,9 +26,10 @@ class TestSubscription(TraceTestCase): events_ros=[ 'ros2:rcl_node_init', 'ros2:rcl_subscription_init', + 'ros2:rclcpp_subscription_init', 'ros2:rclcpp_subscription_callback_added', ], - nodes=['test_subscription'] + nodes=['test_subscription'], ) def test_all(self): @@ -36,56 +37,90 @@ class TestSubscription(TraceTestCase): self.assertEventsSet(self._events_ros) # Check fields - sub_init_events = self.get_events_with_name('ros2:rcl_subscription_init') - for event in sub_init_events: + rcl_sub_init_events = self.get_events_with_name('ros2:rcl_subscription_init') + rclcpp_sub_init_events = self.get_events_with_name('ros2:rclcpp_subscription_init') + callback_added_events = self.get_events_with_name( + 'ros2:rclcpp_subscription_callback_added', + ) + + for event in rcl_sub_init_events: self.assertValidHandle( event, - ['subscription_handle', 'node_handle', 'rmw_subscription_handle']) + ['subscription_handle', 'node_handle', 'rmw_subscription_handle'], + ) self.assertValidQueueDepth(event, 'queue_depth') self.assertStringFieldNotEmpty(event, 'topic_name') - - callback_added_events = self.get_events_with_name( - 'ros2:rclcpp_subscription_callback_added') + for event in rclcpp_sub_init_events: + self.assertValidHandle( + event, + ['subscription_handle', 'subscription'], + ) for event in callback_added_events: - self.assertValidHandle(event, ['subscription_handle', 'callback']) + self.assertValidHandle(event, ['subscription', 'callback']) # Check that the test topic name exists - test_sub_init_events = self.get_events_with_field_value( + test_rcl_sub_init_events = self.get_events_with_field_value( 'topic_name', '/the_topic', - sub_init_events) - self.assertNumEventsEqual(test_sub_init_events, 1, 'cannot find test topic name') - test_sub_init_event = test_sub_init_events[0] + rcl_sub_init_events, + ) + self.assertNumEventsEqual(test_rcl_sub_init_events, 1, 'cannot find test topic name') + test_rcl_sub_init_event = test_rcl_sub_init_events[0] # Check queue_depth value self.assertFieldEquals( - test_sub_init_event, + test_rcl_sub_init_event, 'queue_depth', 10, - 'sub_init event does not have expected queue depth value') + 'sub_init event does not have expected queue depth value', + ) # Check that the node handle matches the node_init event node_init_events = self.get_events_with_name('ros2:rcl_node_init') test_sub_node_init_events = self.get_events_with_procname( 'test_subscription', - node_init_events) + node_init_events, + ) self.assertNumEventsEqual( test_sub_node_init_events, 1, - 'none or more than 1 node_init event') + 'none or more than 1 node_init event', + ) test_sub_node_init_event = test_sub_node_init_events[0] self.assertMatchingField( test_sub_node_init_event, 'node_handle', 'ros2:rcl_subscription_init', - sub_init_events) + rcl_sub_init_events, + ) - # Check that subscription handle matches with callback_added event - self.assertMatchingField( - test_sub_init_event, + # Check that subscription handle matches between rcl_sub_init and rclcpp_sub_init + subscription_handle = self.get_field(test_rcl_sub_init_event, 'subscription_handle') + rclcpp_sub_init_matching_events = self.get_events_with_field_value( 'subscription_handle', - None, - callback_added_events) + subscription_handle, + rclcpp_sub_init_events, + ) + # Should only have 1 rclcpp_sub_init event, since intra-process is not enabled + self.assertNumEventsEqual( + rclcpp_sub_init_matching_events, + 1, + 'none or more than 1 rclcpp_sub_init event for topic', + ) + + # Check that subscription pointer matches between rclcpp_sub_init and sub_callback_added + rclcpp_sub_init_matching_event = rclcpp_sub_init_matching_events[0] + subscription_pointer = self.get_field(rclcpp_sub_init_matching_event, 'subscription') + callback_added_matching_events = self.get_events_with_field_value( + 'subscription', + subscription_pointer, + callback_added_events, + ) + self.assertNumEventsEqual( + callback_added_matching_events, + 1, + 'none or more than 1 rclcpp_sub_callback_added event for topic', + ) if __name__ == '__main__': From 3a8f30e710620f472d1d37822bbca622e0caea99 Mon Sep 17 00:00:00 2001 From: Christophe Bedard Date: Sat, 16 Nov 2019 16:43:53 -0800 Subject: [PATCH 20/20] Merge test_subscription* into test_subscription --- tracetools_test/CMakeLists.txt | 11 --- tracetools_test/src/test_ping.cpp | 5 +- tracetools_test/src/test_subscription.cpp | 59 --------------- tracetools_test/test/test_subscription.py | 72 ++++++++++++++++-- .../test/test_subscription_callback.py | 74 ------------------- 5 files changed, 70 insertions(+), 151 deletions(-) delete mode 100644 tracetools_test/src/test_subscription.cpp delete mode 100644 tracetools_test/test/test_subscription_callback.py diff --git a/tracetools_test/CMakeLists.txt b/tracetools_test/CMakeLists.txt index e744477..e6a7e44 100644 --- a/tracetools_test/CMakeLists.txt +++ b/tracetools_test/CMakeLists.txt @@ -47,15 +47,6 @@ if(BUILD_TESTING) tracetools ) target_link_libraries(test_publisher "${RDYNAMIC_FLAG}") - add_executable(test_subscription - src/test_subscription.cpp - ) - ament_target_dependencies(test_subscription - rclcpp - std_msgs - tracetools - ) - target_link_libraries(test_subscription "${RDYNAMIC_FLAG}") add_executable(test_intra src/test_intra.cpp ) @@ -127,7 +118,6 @@ if(BUILD_TESTING) test_service test_service_ping test_service_pong - test_subscription test_timer DESTINATION lib/${PROJECT_NAME} ) @@ -147,7 +137,6 @@ if(BUILD_TESTING) test/test_service.py test/test_service_callback.py test/test_subscription.py - test/test_subscription_callback.py test/test_timer.py ) diff --git a/tracetools_test/src/test_ping.cpp b/tracetools_test/src/test_ping.cpp index b419e72..ec62227 100644 --- a/tracetools_test/src/test_ping.cpp +++ b/tracetools_test/src/test_ping.cpp @@ -23,6 +23,7 @@ using namespace std::chrono_literals; #define NODE_NAME "test_ping" #define SUB_TOPIC_NAME "pong" #define PUB_TOPIC_NAME "ping" +#define QUEUE_DEPTH 10 class PingNode : public rclcpp::Node { @@ -32,11 +33,11 @@ public: { sub_ = this->create_subscription( SUB_TOPIC_NAME, - rclcpp::QoS(10), + rclcpp::QoS(QUEUE_DEPTH), std::bind(&PingNode::callback, this, std::placeholders::_1)); pub_ = this->create_publisher( PUB_TOPIC_NAME, - rclcpp::QoS(10)); + rclcpp::QoS(QUEUE_DEPTH)); timer_ = this->create_wall_timer( 500ms, std::bind(&PingNode::timer_callback, this)); diff --git a/tracetools_test/src/test_subscription.cpp b/tracetools_test/src/test_subscription.cpp deleted file mode 100644 index a13e37d..0000000 --- a/tracetools_test/src/test_subscription.cpp +++ /dev/null @@ -1,59 +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. - -#include - -#include "rclcpp/rclcpp.hpp" -#include "std_msgs/msg/string.hpp" - -#define NODE_NAME "test_subscription" -#define TOPIC_NAME "the_topic" -#define QUEUE_DEPTH 10 - -class SubNode : public rclcpp::Node -{ -public: - explicit SubNode(rclcpp::NodeOptions options) - : Node(NODE_NAME, options) - { - sub_ = this->create_subscription( - TOPIC_NAME, - rclcpp::QoS(QUEUE_DEPTH), - std::bind(&SubNode::callback, this, std::placeholders::_1)); - } - -private: - void callback(const std_msgs::msg::String::SharedPtr msg) - { - // Nothing - (void)msg; - } - - rclcpp::Subscription::SharedPtr sub_; -}; - -int main(int argc, char * argv[]) -{ - rclcpp::init(argc, argv); - - rclcpp::executors::SingleThreadedExecutor exec; - auto sub_node = std::make_shared(rclcpp::NodeOptions()); - exec.add_node(sub_node); - - printf("spinning once\n"); - exec.spin_once(); - - rclcpp::shutdown(); - return 0; -} diff --git a/tracetools_test/test/test_subscription.py b/tracetools_test/test/test_subscription.py index e9f5ebd..9b5eac1 100644 --- a/tracetools_test/test/test_subscription.py +++ b/tracetools_test/test/test_subscription.py @@ -1,4 +1,5 @@ # Copyright 2019 Robert Bosch GmbH +# Copyright 2019 Apex.AI, Inc. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -22,14 +23,16 @@ class TestSubscription(TraceTestCase): def __init__(self, *args) -> None: super().__init__( *args, - session_name_prefix='session-test-subscription-creation', + session_name_prefix='session-test-subscription', events_ros=[ 'ros2:rcl_node_init', 'ros2:rcl_subscription_init', 'ros2:rclcpp_subscription_init', 'ros2:rclcpp_subscription_callback_added', + 'ros2:callback_start', + 'ros2:callback_end', ], - nodes=['test_subscription'], + nodes=['test_ping', 'test_pong'], ) def test_all(self): @@ -42,6 +45,8 @@ class TestSubscription(TraceTestCase): callback_added_events = self.get_events_with_name( 'ros2:rclcpp_subscription_callback_added', ) + start_events = self.get_events_with_name('ros2:callback_start') + end_events = self.get_events_with_name('ros2:callback_end') for event in rcl_sub_init_events: self.assertValidHandle( @@ -57,11 +62,22 @@ class TestSubscription(TraceTestCase): ) for event in callback_added_events: self.assertValidHandle(event, ['subscription', 'callback']) + for event in start_events: + self.assertValidHandle(event, 'callback') + is_intra_process_value = self.get_field(event, 'is_intra_process') + self.assertIsInstance(is_intra_process_value, int, 'is_intra_process not int') + self.assertTrue( + is_intra_process_value in [0, 1], + f'invalid value for is_intra_process: {is_intra_process_value}', + ) + for event in end_events: + self.assertValidHandle(event, 'callback') - # Check that the test topic name exists + # Check that the pong test topic name exists + # Note: using the ping node test_rcl_sub_init_events = self.get_events_with_field_value( 'topic_name', - '/the_topic', + '/pong', rcl_sub_init_events, ) self.assertNumEventsEqual(test_rcl_sub_init_events, 1, 'cannot find test topic name') @@ -78,7 +94,7 @@ class TestSubscription(TraceTestCase): # Check that the node handle matches the node_init event node_init_events = self.get_events_with_name('ros2:rcl_node_init') test_sub_node_init_events = self.get_events_with_procname( - 'test_subscription', + 'test_ping', node_init_events, ) self.assertNumEventsEqual( @@ -122,6 +138,52 @@ class TestSubscription(TraceTestCase): 'none or more than 1 rclcpp_sub_callback_added event for topic', ) + # Check that each start:end pair has a common callback handle + ping_events = self.get_events_with_procname('test_ping') + pong_events = self.get_events_with_procname('test_pong') + ping_events_start = self.get_events_with_name('ros2:callback_start', ping_events) + pong_events_start = self.get_events_with_name('ros2:callback_start', pong_events) + for ping_start in ping_events_start: + self.assertMatchingField( + ping_start, + 'callback', + 'ros2:callback_end', + ping_events, + ) + for pong_start in pong_events_start: + self.assertMatchingField( + pong_start, + 'callback', + 'ros2:callback_end', + pong_events, + ) + + # Check that callback pointer matches between sub_callback_added and callback_start/end + # There is only one callback for /pong topic in ping node + callback_added_matching_event = callback_added_matching_events[0] + callback_pointer = self.get_field(callback_added_matching_event, 'callback') + callback_start_matching_events = self.get_events_with_field_value( + 'callback', + callback_pointer, + ping_events_start, + ) + self.assertNumEventsEqual( + callback_start_matching_events, + 1, + 'none or more than 1 callback_start event for topic callback', + ) + ping_events_end = self.get_events_with_name('ros2:callback_end', ping_events) + callback_end_matching_events = self.get_events_with_field_value( + 'callback', + callback_pointer, + ping_events_end, + ) + self.assertNumEventsEqual( + callback_end_matching_events, + 1, + 'none or more than 1 callback_end event for topic callback', + ) + if __name__ == '__main__': unittest.main() diff --git a/tracetools_test/test/test_subscription_callback.py b/tracetools_test/test/test_subscription_callback.py deleted file mode 100644 index e1e2483..0000000 --- a/tracetools_test/test/test_subscription_callback.py +++ /dev/null @@ -1,74 +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 TestSubscriptionCallback(TraceTestCase): - - def __init__(self, *args) -> None: - super().__init__( - *args, - session_name_prefix='session-test-subscription-callback', - events_ros=[ - 'ros2:callback_start', - 'ros2:callback_end', - ], - nodes=['test_ping', 'test_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') - for event in start_events: - self.assertValidHandle(event, 'callback') - is_intra_process_value = self.get_field(event, 'is_intra_process') - self.assertIsInstance(is_intra_process_value, int, 'is_intra_process not int') - self.assertTrue( - is_intra_process_value in [0, 1], - f'invalid value for is_intra_process: {is_intra_process_value}', - ) - - end_events = self.get_events_with_name('ros2:callback_end') - for event in end_events: - self.assertValidHandle(event, 'callback') - - # Check that a start:end pair has a common callback handle - ping_events = self.get_events_with_procname('test_ping') - pong_events = self.get_events_with_procname('test_pong') - ping_events_start = self.get_events_with_name('ros2:callback_start', ping_events) - pong_events_start = self.get_events_with_name('ros2:callback_start', pong_events) - for ping_start in ping_events_start: - self.assertMatchingField( - ping_start, - 'callback', - 'ros2:callback_end', - ping_events, - ) - for pong_start in pong_events_start: - self.assertMatchingField( - pong_start, - 'callback', - 'ros2:callback_end', - pong_events, - ) - - -if __name__ == '__main__': - unittest.main()