From 2aaf948b4caa23931a63cdb548c384c7ff5f4613 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Tue, 22 Dec 2020 19:15:46 -0300 Subject: [PATCH 1/4] Make test_subscription_nominal_string_sequence more reliable Signed-off-by: Ivan Santiago Paunovic --- rcl/test/rcl/test_subscription.cpp | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/rcl/test/rcl/test_subscription.cpp b/rcl/test/rcl/test_subscription.cpp index 14a1863fd..a1da34a79 100644 --- a/rcl/test/rcl/test_subscription.cpp +++ b/rcl/test/rcl/test_subscription.cpp @@ -391,6 +391,7 @@ TEST_F( CLASSNAME( TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription_nominal_string_sequence) { + using namespace std::chrono_literals; rcl_ret_t ret; rcl_publisher_t publisher = rcl_get_zero_initialized_publisher(); const rosidl_message_type_support_t * ts = @@ -526,9 +527,17 @@ TEST_F( test_msgs__msg__Strings__Sequence__destroy(seq); }); - ret = rcl_take_sequence(&subscription, 3, &messages, &message_infos, nullptr); + auto start = std::chrono::steady_clock::now(); + size_t total_messages_taken = 0u; + do { + // `wait_for_subscription_to_be_ready` only ensures there's one message ready, + // so we need to loop to guarantee that we get the three published messages. + ASSERT_TRUE(wait_for_subscription_to_be_ready(&subscription, context_ptr, 1, 100)); + ret = rcl_take_sequence(&subscription, 3, &messages, &message_infos, nullptr); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + total_messages_taken += messages.size; + } while (total_messages_taken < 3 && std::chrono::steady_clock::now() < start + 10s); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ASSERT_EQ(3u, messages.size); ASSERT_EQ(3u, message_infos.size); From f885968372b9586e719a1897794e9c691946e264 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Wed, 23 Dec 2020 09:40:26 -0300 Subject: [PATCH 2/4] Duplicate fix elsewhere Signed-off-by: Ivan Santiago Paunovic --- rcl/test/rcl/test_subscription.cpp | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/rcl/test/rcl/test_subscription.cpp b/rcl/test/rcl/test_subscription.cpp index a1da34a79..9d7492a81 100644 --- a/rcl/test/rcl/test_subscription.cpp +++ b/rcl/test/rcl/test_subscription.cpp @@ -480,8 +480,17 @@ TEST_F( test_msgs__msg__Strings__Sequence__destroy(seq); }); - ret = rcl_take_sequence(&subscription, 5, &messages, &message_infos, nullptr); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + auto start = std::chrono::steady_clock::now(); + size_t total_messages_taken = 0u; + do { + // `wait_for_subscription_to_be_ready` only ensures there's one message ready, + // so we need to loop to guarantee that we get the three published messages. + ASSERT_TRUE(wait_for_subscription_to_be_ready(&subscription, context_ptr, 1, 100)); + ret = rcl_take_sequence(&subscription, 5, &messages, &message_infos, nullptr); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + total_messages_taken += messages.size; + } while (total_messages_taken < 3 && std::chrono::steady_clock::now() < start + 10s); + ASSERT_EQ(3u, messages.size); ASSERT_EQ(3u, message_infos.size); } From 908b7dce0cc97d5904e03faf0c756a4866c73e44 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Wed, 23 Dec 2020 10:46:30 -0300 Subject: [PATCH 3/4] extremely large timeout Signed-off-by: Ivan Santiago Paunovic --- rcl/test/rcl/test_subscription.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rcl/test/rcl/test_subscription.cpp b/rcl/test/rcl/test_subscription.cpp index 9d7492a81..84fba2052 100644 --- a/rcl/test/rcl/test_subscription.cpp +++ b/rcl/test/rcl/test_subscription.cpp @@ -489,7 +489,7 @@ TEST_F( ret = rcl_take_sequence(&subscription, 5, &messages, &message_infos, nullptr); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; total_messages_taken += messages.size; - } while (total_messages_taken < 3 && std::chrono::steady_clock::now() < start + 10s); + } while (total_messages_taken < 3 && std::chrono::steady_clock::now() < start + 30s); ASSERT_EQ(3u, messages.size); ASSERT_EQ(3u, message_infos.size); @@ -545,7 +545,7 @@ TEST_F( ret = rcl_take_sequence(&subscription, 3, &messages, &message_infos, nullptr); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; total_messages_taken += messages.size; - } while (total_messages_taken < 3 && std::chrono::steady_clock::now() < start + 10s); + } while (total_messages_taken < 3 && std::chrono::steady_clock::now() < start + 30s); ASSERT_EQ(3u, messages.size); ASSERT_EQ(3u, message_infos.size); From 19ab89b5e9bd19f172fae7b721b9a2c6d38b6a22 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Wed, 23 Dec 2020 11:22:04 -0300 Subject: [PATCH 4/4] fix bug Signed-off-by: Ivan Santiago Paunovic --- rcl/test/rcl/test_subscription.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/rcl/test/rcl/test_subscription.cpp b/rcl/test/rcl/test_subscription.cpp index 84fba2052..adedd7bb9 100644 --- a/rcl/test/rcl/test_subscription.cpp +++ b/rcl/test/rcl/test_subscription.cpp @@ -489,10 +489,10 @@ TEST_F( ret = rcl_take_sequence(&subscription, 5, &messages, &message_infos, nullptr); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; total_messages_taken += messages.size; - } while (total_messages_taken < 3 && std::chrono::steady_clock::now() < start + 30s); + EXPECT_EQ(messages.size, message_infos.size); + } while (total_messages_taken < 3 && std::chrono::steady_clock::now() < start + 10s); - ASSERT_EQ(3u, messages.size); - ASSERT_EQ(3u, message_infos.size); + EXPECT_EQ(3u, total_messages_taken); } { @@ -545,12 +545,11 @@ TEST_F( ret = rcl_take_sequence(&subscription, 3, &messages, &message_infos, nullptr); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; total_messages_taken += messages.size; - } while (total_messages_taken < 3 && std::chrono::steady_clock::now() < start + 30s); + EXPECT_EQ(messages.size, message_infos.size); + } while (total_messages_taken < 3 && std::chrono::steady_clock::now() < start + 10s); - ASSERT_EQ(3u, messages.size); - ASSERT_EQ(3u, message_infos.size); - - ASSERT_EQ( + EXPECT_EQ(3u, total_messages_taken); + EXPECT_EQ( std::string(test_string), std::string(seq->data[0].string_value.data, seq->data[0].string_value.size)); }