From e90d2cd5b313cc968fa1e0cc564c1a86b2afc241 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Wed, 1 Jul 2020 18:55:33 -0300 Subject: [PATCH 1/7] Add test subscriber lost_event Signed-off-by: Jorge Perez --- rcl/test/rcl/test_events.cpp | 70 +++++++++++++++++++++++++++++++++++- 1 file changed, 69 insertions(+), 1 deletion(-) diff --git a/rcl/test/rcl/test_events.cpp b/rcl/test/rcl/test_events.cpp index 351356c2b..a708860fe 100644 --- a/rcl/test/rcl/test_events.cpp +++ b/rcl/test/rcl/test_events.cpp @@ -721,7 +721,7 @@ TEST_F(TestEventFixture, test_bad_event_ini) &subscription, unknown_sub_type); EXPECT_EQ(ret, RCL_RET_INVALID_ARGUMENT); - + tear_down_publisher_subscriber(); } @@ -773,6 +773,74 @@ TEST_F(TestEventFixture, test_event_is_valid) tear_down_publisher_subscriber(); } +/* + * Basic test subscriber event message lost + */ +TEST_P(TestEventFixture, test_sub_message_lost_event) +{ + const auto & input = GetParam(); + const auto & subscription_qos_profile = input.subscription_qos_profile; + const auto & error_msg = input.error_msg; + + rcl_ret_t ret = setup_subscriber(subscription_qos_profile); + ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; + + // Check if supported + subscription_event = rcl_get_zero_initialized_event(); + ret = rcl_subscription_event_init( + &subscription_event, + &subscription, + RCL_SUBSCRIPTION_MESSAGE_LOST); + EXPECT_TRUE(ret == RCL_RET_OK || ret == RCL_RET_UNSUPPORTED); + + if (ret == RCL_RET_UNSUPPORTED) { + // clean up and exit test early + ret = rcl_event_fini(&subscription_event); + EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; + ret = rcl_subscription_fini(&subscription, this->node_ptr); + EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; + return; + } + + // (TODO blast545): Check reason test, timeout failing + // conditional_wait_for_msgs_and_events returns Timeout for rmw_cyclonedds_cpp + + WaitConditionPredicate events_ready = []( + const bool & /*msg_persist_ready*/, + const bool & subscription_persist_ready, + const bool & /*publisher_persist_ready*/) { + return subscription_persist_ready; + }; + bool msg_persist_ready, subscription_persist_ready, publisher_persist_ready; + rcl_ret_t wait_res = conditional_wait_for_msgs_and_events( + context_ptr, MAX_WAIT_PER_TESTCASE, events_ready, + &subscription, &subscription_event, nullptr, + &msg_persist_ready, &subscription_persist_ready, &publisher_persist_ready); + EXPECT_EQ(wait_res, RCL_RET_OK); + + // test that the subscriber/datareader discovered a lost message event + EXPECT_TRUE(subscription_persist_ready); + if (subscription_persist_ready) { + rmw_message_lost_status_t message_lost_status; + rcl_ret_t ret = rcl_take_event(&subscription_event, &message_lost_status); + EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; + const auto & sub_total_count = message_lost_status.total_count; + const auto & sub_total_count_change = message_lost_status.total_count_change; + if (sub_total_count != 0 && sub_total_count_change != 0) { + EXPECT_EQ(sub_total_count, 1u) << error_msg; + EXPECT_EQ(sub_total_count_change, 1u) << error_msg; + } else { + ADD_FAILURE() << "Subscription incompatible message lost event timed out for: " << error_msg; + } + } + + // clean up + ret = rcl_event_fini(&subscription_event); + EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; + ret = rcl_subscription_fini(&subscription, this->node_ptr); + EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; +} + static std::array get_test_pubsub_incompatible_qos_inputs() From 7049acd8a303e561e7874bdab182e3e59392d0b1 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Thu, 16 Jul 2020 19:58:48 -0300 Subject: [PATCH 2/7] Fix lint warning Signed-off-by: Jorge Perez --- rcl/test/rcl/test_events.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rcl/test/rcl/test_events.cpp b/rcl/test/rcl/test_events.cpp index a708860fe..cd8625f1b 100644 --- a/rcl/test/rcl/test_events.cpp +++ b/rcl/test/rcl/test_events.cpp @@ -721,7 +721,7 @@ TEST_F(TestEventFixture, test_bad_event_ini) &subscription, unknown_sub_type); EXPECT_EQ(ret, RCL_RET_INVALID_ARGUMENT); - + tear_down_publisher_subscriber(); } From 30cc49b2166533917ce4783a8dea89706c125e4f Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Fri, 17 Jul 2020 19:36:57 -0300 Subject: [PATCH 3/7] Remove test wait for lost event Signed-off-by: Jorge Perez --- rcl/test/rcl/test_events.cpp | 42 ++++++------------------------------ 1 file changed, 7 insertions(+), 35 deletions(-) diff --git a/rcl/test/rcl/test_events.cpp b/rcl/test/rcl/test_events.cpp index cd8625f1b..ff740d3cb 100644 --- a/rcl/test/rcl/test_events.cpp +++ b/rcl/test/rcl/test_events.cpp @@ -776,11 +776,9 @@ TEST_F(TestEventFixture, test_event_is_valid) /* * Basic test subscriber event message lost */ -TEST_P(TestEventFixture, test_sub_message_lost_event) +TEST_F(TestEventFixture, test_sub_message_lost_event) { - const auto & input = GetParam(); - const auto & subscription_qos_profile = input.subscription_qos_profile; - const auto & error_msg = input.error_msg; + const rmw_qos_profile_t subscription_qos_profile = default_qos_profile; rcl_ret_t ret = setup_subscriber(subscription_qos_profile); ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; @@ -802,37 +800,11 @@ TEST_P(TestEventFixture, test_sub_message_lost_event) return; } - // (TODO blast545): Check reason test, timeout failing - // conditional_wait_for_msgs_and_events returns Timeout for rmw_cyclonedds_cpp - - WaitConditionPredicate events_ready = []( - const bool & /*msg_persist_ready*/, - const bool & subscription_persist_ready, - const bool & /*publisher_persist_ready*/) { - return subscription_persist_ready; - }; - bool msg_persist_ready, subscription_persist_ready, publisher_persist_ready; - rcl_ret_t wait_res = conditional_wait_for_msgs_and_events( - context_ptr, MAX_WAIT_PER_TESTCASE, events_ready, - &subscription, &subscription_event, nullptr, - &msg_persist_ready, &subscription_persist_ready, &publisher_persist_ready); - EXPECT_EQ(wait_res, RCL_RET_OK); - - // test that the subscriber/datareader discovered a lost message event - EXPECT_TRUE(subscription_persist_ready); - if (subscription_persist_ready) { - rmw_message_lost_status_t message_lost_status; - rcl_ret_t ret = rcl_take_event(&subscription_event, &message_lost_status); - EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; - const auto & sub_total_count = message_lost_status.total_count; - const auto & sub_total_count_change = message_lost_status.total_count_change; - if (sub_total_count != 0 && sub_total_count_change != 0) { - EXPECT_EQ(sub_total_count, 1u) << error_msg; - EXPECT_EQ(sub_total_count_change, 1u) << error_msg; - } else { - ADD_FAILURE() << "Subscription incompatible message lost event timed out for: " << error_msg; - } - } + // Can't reproduce reliably this event + // Test that take_event is able to read the configured event + rmw_message_lost_status_t message_lost_status; + ret = rcl_take_event(&subscription_event, &message_lost_status); + EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; // clean up ret = rcl_event_fini(&subscription_event); From a93d662b6e0c77941fcf53280bb1fb893bb0f373 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 20 Jul 2020 16:47:27 -0300 Subject: [PATCH 4/7] Check for fastrtps as unsupported Signed-off-by: Jorge Perez --- rcl/test/rcl/test_events.cpp | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/rcl/test/rcl/test_events.cpp b/rcl/test/rcl/test_events.cpp index ff740d3cb..57ed9b65f 100644 --- a/rcl/test/rcl/test_events.cpp +++ b/rcl/test/rcl/test_events.cpp @@ -783,21 +783,28 @@ TEST_F(TestEventFixture, test_sub_message_lost_event) rcl_ret_t ret = setup_subscriber(subscription_qos_profile); ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; - // Check if supported - subscription_event = rcl_get_zero_initialized_event(); - ret = rcl_subscription_event_init( - &subscription_event, - &subscription, - RCL_SUBSCRIPTION_MESSAGE_LOST); - EXPECT_TRUE(ret == RCL_RET_OK || ret == RCL_RET_UNSUPPORTED); + if (is_fastrtps) { + // Check not supported + subscription_event = rcl_get_zero_initialized_event(); + ret = rcl_subscription_event_init( + &subscription_event, + &subscription, + RCL_SUBSCRIPTION_MESSAGE_LOST); + EXPECT_EQ(ret, RCL_RET_UNSUPPORTED); - if (ret == RCL_RET_UNSUPPORTED) { // clean up and exit test early ret = rcl_event_fini(&subscription_event); EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; ret = rcl_subscription_fini(&subscription, this->node_ptr); EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; return; + } else { + subscription_event = rcl_get_zero_initialized_event(); + ret = rcl_subscription_event_init( + &subscription_event, + &subscription, + RCL_SUBSCRIPTION_MESSAGE_LOST); + EXPECT_EQ(ret, RCL_RET_OK); } // Can't reproduce reliably this event From 6de0b1c7e53b13a7bdb823f0d667c99fa0d2cb82 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 20 Jul 2020 17:00:39 -0300 Subject: [PATCH 5/7] Add check for event count returned Signed-off-by: Jorge Perez --- rcl/test/rcl/test_events.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rcl/test/rcl/test_events.cpp b/rcl/test/rcl/test_events.cpp index 57ed9b65f..4f283507d 100644 --- a/rcl/test/rcl/test_events.cpp +++ b/rcl/test/rcl/test_events.cpp @@ -812,6 +812,8 @@ TEST_F(TestEventFixture, test_sub_message_lost_event) rmw_message_lost_status_t message_lost_status; ret = rcl_take_event(&subscription_event, &message_lost_status); EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; + EXPECT_EQ(message_lost_status.total_count, 0u); + EXPECT_EQ(message_lost_status.total_count_change, 0u); // clean up ret = rcl_event_fini(&subscription_event); From 21c5577f767d583027b51607d6b9aca15dc17900 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Tue, 21 Jul 2020 18:31:21 -0300 Subject: [PATCH 6/7] Reformat to delete extra code Signed-off-by: Jorge Perez --- rcl/test/rcl/test_events.cpp | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/rcl/test/rcl/test_events.cpp b/rcl/test/rcl/test_events.cpp index 4f283507d..3ee2c6ea9 100644 --- a/rcl/test/rcl/test_events.cpp +++ b/rcl/test/rcl/test_events.cpp @@ -783,27 +783,26 @@ TEST_F(TestEventFixture, test_sub_message_lost_event) rcl_ret_t ret = setup_subscriber(subscription_qos_profile); ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; - if (is_fastrtps) { - // Check not supported - subscription_event = rcl_get_zero_initialized_event(); + subscription_event = rcl_get_zero_initialized_event(); ret = rcl_subscription_event_init( &subscription_event, &subscription, RCL_SUBSCRIPTION_MESSAGE_LOST); - EXPECT_EQ(ret, RCL_RET_UNSUPPORTED); - - // clean up and exit test early + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { ret = rcl_event_fini(&subscription_event); EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; ret = rcl_subscription_fini(&subscription, this->node_ptr); EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; + }); + + if (is_fastrtps) { + // Check not supported + EXPECT_EQ(ret, RCL_RET_UNSUPPORTED); + + // clean up and exit test early return; } else { - subscription_event = rcl_get_zero_initialized_event(); - ret = rcl_subscription_event_init( - &subscription_event, - &subscription, - RCL_SUBSCRIPTION_MESSAGE_LOST); EXPECT_EQ(ret, RCL_RET_OK); } @@ -814,12 +813,6 @@ TEST_F(TestEventFixture, test_sub_message_lost_event) EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; EXPECT_EQ(message_lost_status.total_count, 0u); EXPECT_EQ(message_lost_status.total_count_change, 0u); - - // clean up - ret = rcl_event_fini(&subscription_event); - EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; - ret = rcl_subscription_fini(&subscription, this->node_ptr); - EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; } static From 87e7df7316fe45fc0bff745557542da37096decd Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Tue, 21 Jul 2020 18:37:16 -0300 Subject: [PATCH 7/7] Fix linter spacing Signed-off-by: Jorge Perez --- rcl/test/rcl/test_events.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rcl/test/rcl/test_events.cpp b/rcl/test/rcl/test_events.cpp index 3ee2c6ea9..568d13022 100644 --- a/rcl/test/rcl/test_events.cpp +++ b/rcl/test/rcl/test_events.cpp @@ -784,10 +784,10 @@ TEST_F(TestEventFixture, test_sub_message_lost_event) ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; subscription_event = rcl_get_zero_initialized_event(); - ret = rcl_subscription_event_init( - &subscription_event, - &subscription, - RCL_SUBSCRIPTION_MESSAGE_LOST); + ret = rcl_subscription_event_init( + &subscription_event, + &subscription, + RCL_SUBSCRIPTION_MESSAGE_LOST); OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( { ret = rcl_event_fini(&subscription_event);