From 6bc2557336c08fc8445fbbdb1cd624c559209eea Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Fri, 12 Jun 2020 10:14:22 -0300 Subject: [PATCH 1/7] Improve rcl timer test coverage. Signed-off-by: Michel Hidalgo --- rcl/test/CMakeLists.txt | 20 ++++--- rcl/test/rcl/test_timer.cpp | 108 ++++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 7 deletions(-) diff --git a/rcl/test/CMakeLists.txt b/rcl/test/CMakeLists.txt index 1c1a92ec6..771ae26df 100644 --- a/rcl/test/CMakeLists.txt +++ b/rcl/test/CMakeLists.txt @@ -50,6 +50,19 @@ function(test_target_function) AMENT_DEPENDENCIES ${rmw_implementation} ) + set(timer_test_timeout 60) + if(rmw_implementation STREQUAL "rmw_connext_cpp") + set(timer_test_timeout 90) # Bump timeout for RTI Connext + endif() + rcl_add_custom_gtest(test_timer${target_suffix} + SRCS rcl/test_timer.cpp + ENV ${rmw_implementation_env_var} + TIMEOUT ${timer_test_timeout} + APPEND_LIBRARY_DIRS ${extra_lib_dirs} + LIBRARIES ${PROJECT_NAME} osrf_testing_tools_cpp::memory_tools + AMENT_DEPENDENCIES ${rmw_implementation} + ) + rcl_add_custom_gtest(test_context${target_suffix} SRCS rcl/test_context.cpp ENV ${rmw_implementation_env_var} ${memory_tools_ld_preload_env_var} @@ -350,13 +363,6 @@ rcl_add_custom_gtest(test_expand_topic_name LIBRARIES ${PROJECT_NAME} ) -rcl_add_custom_gtest(test_timer${target_suffix} - SRCS rcl/test_timer.cpp - APPEND_LIBRARY_DIRS ${extra_lib_dirs} - LIBRARIES ${PROJECT_NAME} - AMENT_DEPENDENCIES "osrf_testing_tools_cpp" -) - rcl_add_custom_gtest(test_security SRCS rcl/test_security.cpp APPEND_LIBRARY_DIRS ${extra_lib_dirs} diff --git a/rcl/test/rcl/test_timer.cpp b/rcl/test/rcl/test_timer.cpp index 4801e24da..b19f7fd76 100644 --- a/rcl/test/rcl/test_timer.cpp +++ b/rcl/test/rcl/test_timer.cpp @@ -274,6 +274,104 @@ TEST_F(TestTimerFixture, test_timer_not_ready) { EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; } +TEST_F(TestTimerFixture, test_timer_overrun) { + rcl_clock_t clock; + rcl_allocator_t allocator = rcl_get_default_allocator(); + rcl_ret_t ret = rcl_clock_init(RCL_STEADY_TIME, &clock, &allocator); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + rcl_ret_t ret = rcl_clock_fini(&clock); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + rcl_reset_error(); + }); + + rcl_timer_t timer = rcl_get_zero_initialized_timer(); + ret = rcl_timer_init( + &timer, &clock, this->context_ptr, RCL_MS_TO_NS(100), nullptr, rcl_get_default_allocator()); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + rcl_ret_t ret = rcl_timer_fini(&timer); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + rcl_reset_error(); + }); + + rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set(); + ret = rcl_wait_set_init(&wait_set, 0, 0, 1, 0, 0, 0, context_ptr, rcl_get_default_allocator()); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + rcl_ret_t ret = rcl_wait_set_fini(&wait_set); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + rcl_reset_error(); + }); + + // Force multiple timer timeouts. + ret = rcl_wait(&wait_set, RCL_MS_TO_NS(500)); + EXPECT_EQ(RCL_RET_TIMEOUT, ret) << rcl_get_error_string().str; + rcl_reset_error(); + + bool is_ready = false; + ret = rcl_timer_is_ready(&timer, &is_ready); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + EXPECT_TRUE(is_ready); + rcl_reset_error(); + + EXPECT_EQ(RCL_RET_OK, rcl_timer_call(&timer)) << rcl_get_error_string().str; + rcl_reset_error(); + + ret = rcl_wait_set_add_timer(&wait_set, &timer, NULL); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + rcl_reset_error(); + + // Ensure period is re-aligned. + ret = rcl_wait(&wait_set, RCL_MS_TO_NS(50)); + EXPECT_EQ(RCL_RET_TIMEOUT, ret) << rcl_get_error_string().str; + rcl_reset_error(); + + ret = rcl_timer_is_ready(&timer, &is_ready); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + EXPECT_FALSE(is_ready); + rcl_reset_error(); +} + +TEST_F(TestTimerFixture, test_timer_with_zero_period) { + rcl_clock_t clock; + rcl_allocator_t allocator = rcl_get_default_allocator(); + rcl_ret_t ret = rcl_clock_init(RCL_STEADY_TIME, &clock, &allocator); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + rcl_ret_t ret = rcl_clock_fini(&clock); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + }); + + rcl_timer_t timer = rcl_get_zero_initialized_timer(); + ret = rcl_timer_init( + &timer, &clock, this->context_ptr, 0, nullptr, rcl_get_default_allocator()); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + rcl_ret_t ret = rcl_timer_fini(&timer); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + }); + + bool is_ready = false; + ret = rcl_timer_is_ready(&timer, &is_ready); + EXPECT_TRUE(is_ready) << rcl_get_error_string().str; + rcl_reset_error(); + + int64_t time_until_next_call = 0; + ret = rcl_timer_get_time_until_next_call(&timer, &time_until_next_call); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + EXPECT_LE(time_until_next_call, 0); + rcl_reset_error(); + + EXPECT_EQ(RCL_RET_OK, rcl_timer_call(&timer)) << rcl_get_error_string().str; + rcl_reset_error(); +} + TEST_F(TestTimerFixture, test_canceled_timer) { rcl_ret_t ret; @@ -567,6 +665,7 @@ TEST_F(TestPreInitTimer, test_timer_get_allocator) { EXPECT_TRUE(rcutils_allocator_is_valid(allocator_returned)); EXPECT_EQ(NULL, rcl_timer_get_allocator(nullptr)); + rcl_reset_error(); } TEST_F(TestPreInitTimer, test_timer_clock) { @@ -601,6 +700,7 @@ TEST_F(TestPreInitTimer, test_timer_call) { EXPECT_EQ(RCL_RET_OK, rcl_timer_cancel(&timer)) << rcl_get_error_string().str; EXPECT_EQ(RCL_RET_TIMER_CANCELED, rcl_timer_call(&timer)); + rcl_reset_error(); EXPECT_EQ(times_called, 4); } @@ -650,17 +750,21 @@ TEST_F(TestPreInitTimer, test_invalid_init_fini) { rcl_allocator_t bad_allocator = get_failing_allocator(); rcl_timer_t timer_fail = rcl_get_zero_initialized_timer(); + rcl_reset_error(); EXPECT_EQ( RCL_RET_ALREADY_INIT, rcl_timer_init( &timer, &clock, this->context_ptr, 500, nullptr, rcl_get_default_allocator())) << rcl_get_error_string().str; + rcl_reset_error(); ASSERT_EQ( RCL_RET_BAD_ALLOC, rcl_timer_init( &timer_fail, &clock, this->context_ptr, RCL_S_TO_NS(1), timer_callback_test, bad_allocator)) << rcl_get_error_string().str; + rcl_reset_error(); EXPECT_EQ(RCL_RET_OK, rcl_timer_fini(nullptr)); + rcl_reset_error(); } TEST_F(TestPreInitTimer, test_timer_get_period) { @@ -669,7 +773,9 @@ TEST_F(TestPreInitTimer, test_timer_get_period) { EXPECT_EQ(RCL_S_TO_NS(1), period); EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_timer_get_period(nullptr, &period)); + rcl_reset_error(); EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_timer_get_period(&timer, nullptr)); + rcl_reset_error(); } TEST_F(TestPreInitTimer, test_time_since_last_call) { @@ -677,6 +783,8 @@ TEST_F(TestPreInitTimer, test_time_since_last_call) { rcl_time_point_value_t time_sice_next_call_end = 0u; ASSERT_EQ(RCL_RET_OK, rcl_timer_get_time_since_last_call(&timer, &time_sice_next_call_start)); + // Cope with coarse system time resolution. + std::this_thread::sleep_for(std::chrono::milliseconds(1)); ASSERT_EQ(RCL_RET_OK, rcl_timer_get_time_since_last_call(&timer, &time_sice_next_call_end)); EXPECT_GT(time_sice_next_call_end, time_sice_next_call_start); } From dc69a75b42164bee88849ad8b0570460c00f87b7 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Fri, 12 Jun 2020 14:28:24 -0300 Subject: [PATCH 2/7] Fix flaky waits on Windows. Signed-off-by: Michel Hidalgo --- rcl/test/rcl/test_timer.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rcl/test/rcl/test_timer.cpp b/rcl/test/rcl/test_timer.cpp index b19f7fd76..96b4fc66c 100644 --- a/rcl/test/rcl/test_timer.cpp +++ b/rcl/test/rcl/test_timer.cpp @@ -183,7 +183,7 @@ TEST_F(TestTimerFixture, test_two_timers_ready_before_timeout) { rcl_timer_t timer2 = rcl_get_zero_initialized_timer(); ret = rcl_timer_init( - &timer, &clock, this->context_ptr, RCL_MS_TO_NS(50), nullptr, rcl_get_default_allocator()); + &timer, &clock, this->context_ptr, RCL_MS_TO_NS(10), nullptr, rcl_get_default_allocator()); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ret = rcl_timer_init( @@ -288,7 +288,7 @@ TEST_F(TestTimerFixture, test_timer_overrun) { rcl_timer_t timer = rcl_get_zero_initialized_timer(); ret = rcl_timer_init( - &timer, &clock, this->context_ptr, RCL_MS_TO_NS(100), nullptr, rcl_get_default_allocator()); + &timer, &clock, this->context_ptr, RCL_MS_TO_NS(200), nullptr, rcl_get_default_allocator()); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( { @@ -326,7 +326,7 @@ TEST_F(TestTimerFixture, test_timer_overrun) { rcl_reset_error(); // Ensure period is re-aligned. - ret = rcl_wait(&wait_set, RCL_MS_TO_NS(50)); + ret = rcl_wait(&wait_set, RCL_MS_TO_NS(10)); EXPECT_EQ(RCL_RET_TIMEOUT, ret) << rcl_get_error_string().str; rcl_reset_error(); From b19b6f078ac8da5d7f3a0086e9d7dc4b31c17b0e Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Mon, 15 Jun 2020 17:36:46 -0300 Subject: [PATCH 3/7] Address peer review comments. Signed-off-by: Michel Hidalgo --- rcl/test/rcl/test_timer.cpp | 87 +++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/rcl/test/rcl/test_timer.cpp b/rcl/test/rcl/test_timer.cpp index 96b4fc66c..f3fca6dab 100644 --- a/rcl/test/rcl/test_timer.cpp +++ b/rcl/test/rcl/test_timer.cpp @@ -114,6 +114,86 @@ class TestPreInitTimer : public TestTimerFixture } }; +TEST_F(TestTimerFixture, test_timer_init_with_invalid_arguments) { + rcl_clock_t clock; + rcl_allocator_t allocator = rcl_get_default_allocator(); + rcl_ret_t ret = rcl_clock_init(RCL_STEADY_TIME, &clock, &allocator); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + rcl_timer_t timer = rcl_get_zero_initialized_timer(); + + ret = rcl_timer_init( + nullptr, &clock, this->context_ptr, RCL_MS_TO_NS(50), nullptr, allocator); + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret); + rcl_reset_error(); + + ret = rcl_timer_init( + &timer, nullptr, this->context_ptr, RCL_MS_TO_NS(50), nullptr, allocator); + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret); + rcl_reset_error(); + + ret = rcl_timer_init( + &timer, &clock, nullptr, RCL_MS_TO_NS(50), nullptr, allocator); + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret); + rcl_reset_error(); + + ret = rcl_timer_init( + &timer, &clock, this->context_ptr, -1, nullptr, allocator); + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret); + rcl_reset_error(); + + rcl_allocator_t invalid_allocator = rcutils_get_zero_initialized_allocator(); + ret = rcl_timer_init( + &timer, &clock, this->context_ptr, RCL_MS_TO_NS(50), nullptr, invalid_allocator); + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret); + rcl_reset_error(); +} + +TEST_F(TestTimerFixture, test_timer_with_invalid_clock) { + rcl_clock_t clock; + rcl_allocator_t allocator = rcl_get_default_allocator(); + rcl_ret_t ret = rcl_clock_init(RCL_STEADY_TIME, &clock, &allocator); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + rcl_ret_t ret = rcl_clock_fini(&clock); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + rcl_reset_error(); + }); + + rcl_timer_t timer = rcl_get_zero_initialized_timer(); + ret = rcl_timer_init( + &timer, &clock, this->context_ptr, 0, nullptr, allocator); + ASSERT_EQ(RCL_RET_OK, ret); + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + rcl_ret_t ret = rcl_timer_fini(&timer); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + rcl_reset_error(); + }); + + rcl_clock_t * timer_clock; + ret = rcl_timer_clock(&timer, &timer_clock); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + timer_clock->get_now = nullptr; + + ret = rcl_timer_call(&timer); + EXPECT_EQ(RCL_RET_ERROR, ret); + rcl_reset_error(); + + int64_t time_until_next_call; + ret = rcl_timer_get_time_until_next_call(&timer, &time_until_next_call); + EXPECT_EQ(RCL_RET_ERROR, ret); + rcl_reset_error(); + + bool ready; + ret = rcl_timer_is_ready(&timer, &ready); + EXPECT_EQ(RCL_RET_ERROR, ret); + rcl_reset_error(); + + rcl_time_point_value_t time_since_last_call; + ret = rcl_timer_get_time_since_last_call(&timer, &time_since_last_call); + EXPECT_EQ(RCL_RET_ERROR, ret); + rcl_reset_error(); +} + TEST_F(TestTimerFixture, test_two_timers) { rcl_ret_t ret; @@ -182,6 +262,7 @@ TEST_F(TestTimerFixture, test_two_timers_ready_before_timeout) { rcl_timer_t timer = rcl_get_zero_initialized_timer(); rcl_timer_t timer2 = rcl_get_zero_initialized_timer(); + // Keep the first timer period low enough so that rcl_wait() doesn't timeout too early. ret = rcl_timer_init( &timer, &clock, this->context_ptr, RCL_MS_TO_NS(10), nullptr, rcl_get_default_allocator()); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; @@ -698,6 +779,12 @@ TEST_F(TestPreInitTimer, test_timer_call) { EXPECT_EQ(RCL_RET_OK, rcl_timer_get_time_until_next_call(&timer, &next_call_end)); EXPECT_GT(next_call_start, next_call_end); + EXPECT_EQ(RCL_RET_OK, rcl_enable_ros_time_override(&this->clock)) << rcl_get_error_string().str; + EXPECT_EQ(RCL_RET_OK, rcl_set_ros_time_override(&this->clock, -1)) << rcl_get_error_string().str; + EXPECT_EQ(RCL_RET_ERROR, rcl_timer_call(&timer)); + rcl_reset_error(); + EXPECT_EQ(times_called, 4); + EXPECT_EQ(RCL_RET_OK, rcl_timer_cancel(&timer)) << rcl_get_error_string().str; EXPECT_EQ(RCL_RET_TIMER_CANCELED, rcl_timer_call(&timer)); rcl_reset_error(); From fd44afb92333a8b3497737100ec0783df18b9dea Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 16 Jun 2020 14:26:33 -0300 Subject: [PATCH 4/7] Please linter. Signed-off-by: Michel Hidalgo --- rcl/test/rcl/test_timer.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rcl/test/rcl/test_timer.cpp b/rcl/test/rcl/test_timer.cpp index f3fca6dab..05b154f0d 100644 --- a/rcl/test/rcl/test_timer.cpp +++ b/rcl/test/rcl/test_timer.cpp @@ -153,7 +153,8 @@ TEST_F(TestTimerFixture, test_timer_with_invalid_clock) { rcl_allocator_t allocator = rcl_get_default_allocator(); rcl_ret_t ret = rcl_clock_init(RCL_STEADY_TIME, &clock, &allocator); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { rcl_ret_t ret = rcl_clock_fini(&clock); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; rcl_reset_error(); @@ -163,7 +164,8 @@ TEST_F(TestTimerFixture, test_timer_with_invalid_clock) { ret = rcl_timer_init( &timer, &clock, this->context_ptr, 0, nullptr, allocator); ASSERT_EQ(RCL_RET_OK, ret); - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { rcl_ret_t ret = rcl_timer_fini(&timer); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; rcl_reset_error(); From 4041baedaf2f98005395bb2e08b8b240b46527cf Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 16 Jun 2020 17:05:33 -0300 Subject: [PATCH 5/7] Cover a few more lines using a faulty clock. Signed-off-by: Michel Hidalgo --- rcl/test/rcl/test_timer.cpp | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/rcl/test/rcl/test_timer.cpp b/rcl/test/rcl/test_timer.cpp index 05b154f0d..7727c43f4 100644 --- a/rcl/test/rcl/test_timer.cpp +++ b/rcl/test/rcl/test_timer.cpp @@ -151,7 +151,16 @@ TEST_F(TestTimerFixture, test_timer_init_with_invalid_arguments) { TEST_F(TestTimerFixture, test_timer_with_invalid_clock) { rcl_clock_t clock; rcl_allocator_t allocator = rcl_get_default_allocator(); - rcl_ret_t ret = rcl_clock_init(RCL_STEADY_TIME, &clock, &allocator); + rcl_ret_t ret = rcl_clock_init(RCL_CLOCK_UNINITIALIZED, &clock, &allocator); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + + rcl_timer_t timer = rcl_get_zero_initialized_timer(); + ret = rcl_timer_init( + &timer, &clock, this->context_ptr, 0, nullptr, allocator); + EXPECT_EQ(RCL_RET_ERROR, ret); + rcl_reset_error(); + + ret = rcl_clock_init(RCL_ROS_TIME, &clock, &allocator); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( { @@ -160,7 +169,6 @@ TEST_F(TestTimerFixture, test_timer_with_invalid_clock) { rcl_reset_error(); }); - rcl_timer_t timer = rcl_get_zero_initialized_timer(); ret = rcl_timer_init( &timer, &clock, this->context_ptr, 0, nullptr, allocator); ASSERT_EQ(RCL_RET_OK, ret); @@ -176,6 +184,11 @@ TEST_F(TestTimerFixture, test_timer_with_invalid_clock) { ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; timer_clock->get_now = nullptr; + // Trigger clock jump callbacks + ret = rcl_enable_ros_time_override(timer_clock); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + rcl_reset_error(); + ret = rcl_timer_call(&timer); EXPECT_EQ(RCL_RET_ERROR, ret); rcl_reset_error(); @@ -194,6 +207,10 @@ TEST_F(TestTimerFixture, test_timer_with_invalid_clock) { ret = rcl_timer_get_time_since_last_call(&timer, &time_since_last_call); EXPECT_EQ(RCL_RET_ERROR, ret); rcl_reset_error(); + + ret = rcl_timer_reset(&timer); + EXPECT_EQ(RCL_RET_ERROR, ret); + rcl_reset_error(); } TEST_F(TestTimerFixture, test_two_timers) { From d2bd754c201c9658025167e57f524f9e5003fb04 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 16 Jun 2020 17:05:50 -0300 Subject: [PATCH 6/7] Skip rcl timer tests for rmw_connext_cpp. Signed-off-by: Michel Hidalgo --- rcl/test/CMakeLists.txt | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/rcl/test/CMakeLists.txt b/rcl/test/CMakeLists.txt index 771ae26df..73e4eda70 100644 --- a/rcl/test/CMakeLists.txt +++ b/rcl/test/CMakeLists.txt @@ -50,17 +50,21 @@ function(test_target_function) AMENT_DEPENDENCIES ${rmw_implementation} ) - set(timer_test_timeout 60) + # TODO(hidmic): re-enable timer tests against RTI Connext once + # https://github.com/ros2/rcl/issues/687 is resolved + set(AMENT_GTEST_ARGS "") if(rmw_implementation STREQUAL "rmw_connext_cpp") - set(timer_test_timeout 90) # Bump timeout for RTI Connext + message(STATUS "Skipping test_timer${target_suffix} test.") + set(AMENT_GTEST_ARGS "SKIP_TEST") endif() + rcl_add_custom_gtest(test_timer${target_suffix} SRCS rcl/test_timer.cpp ENV ${rmw_implementation_env_var} - TIMEOUT ${timer_test_timeout} APPEND_LIBRARY_DIRS ${extra_lib_dirs} LIBRARIES ${PROJECT_NAME} osrf_testing_tools_cpp::memory_tools AMENT_DEPENDENCIES ${rmw_implementation} + ${AMENT_GTEST_ARGS} ) rcl_add_custom_gtest(test_context${target_suffix} From 6aa785ecfbad493be355f805101b27d1a719f954 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Fri, 19 Jun 2020 11:11:37 -0300 Subject: [PATCH 7/7] Drop unnecessary rcl_reset_error() calls. Signed-off-by: Michel Hidalgo --- rcl/test/rcl/test_timer.cpp | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/rcl/test/rcl/test_timer.cpp b/rcl/test/rcl/test_timer.cpp index 7727c43f4..ae3984bed 100644 --- a/rcl/test/rcl/test_timer.cpp +++ b/rcl/test/rcl/test_timer.cpp @@ -166,7 +166,6 @@ TEST_F(TestTimerFixture, test_timer_with_invalid_clock) { { rcl_ret_t ret = rcl_clock_fini(&clock); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - rcl_reset_error(); }); ret = rcl_timer_init( @@ -176,7 +175,6 @@ TEST_F(TestTimerFixture, test_timer_with_invalid_clock) { { rcl_ret_t ret = rcl_timer_fini(&timer); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - rcl_reset_error(); }); rcl_clock_t * timer_clock; @@ -187,7 +185,6 @@ TEST_F(TestTimerFixture, test_timer_with_invalid_clock) { // Trigger clock jump callbacks ret = rcl_enable_ros_time_override(timer_clock); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - rcl_reset_error(); ret = rcl_timer_call(&timer); EXPECT_EQ(RCL_RET_ERROR, ret); @@ -383,7 +380,6 @@ TEST_F(TestTimerFixture, test_timer_overrun) { { rcl_ret_t ret = rcl_clock_fini(&clock); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - rcl_reset_error(); }); rcl_timer_t timer = rcl_get_zero_initialized_timer(); @@ -394,7 +390,6 @@ TEST_F(TestTimerFixture, test_timer_overrun) { { rcl_ret_t ret = rcl_timer_fini(&timer); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - rcl_reset_error(); }); rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set(); @@ -404,7 +399,6 @@ TEST_F(TestTimerFixture, test_timer_overrun) { { rcl_ret_t ret = rcl_wait_set_fini(&wait_set); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - rcl_reset_error(); }); // Force multiple timer timeouts. @@ -416,14 +410,11 @@ TEST_F(TestTimerFixture, test_timer_overrun) { ret = rcl_timer_is_ready(&timer, &is_ready); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; EXPECT_TRUE(is_ready); - rcl_reset_error(); EXPECT_EQ(RCL_RET_OK, rcl_timer_call(&timer)) << rcl_get_error_string().str; - rcl_reset_error(); ret = rcl_wait_set_add_timer(&wait_set, &timer, NULL); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - rcl_reset_error(); // Ensure period is re-aligned. ret = rcl_wait(&wait_set, RCL_MS_TO_NS(10)); @@ -433,7 +424,6 @@ TEST_F(TestTimerFixture, test_timer_overrun) { ret = rcl_timer_is_ready(&timer, &is_ready); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; EXPECT_FALSE(is_ready); - rcl_reset_error(); } TEST_F(TestTimerFixture, test_timer_with_zero_period) { @@ -459,17 +449,15 @@ TEST_F(TestTimerFixture, test_timer_with_zero_period) { bool is_ready = false; ret = rcl_timer_is_ready(&timer, &is_ready); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; EXPECT_TRUE(is_ready) << rcl_get_error_string().str; - rcl_reset_error(); int64_t time_until_next_call = 0; ret = rcl_timer_get_time_until_next_call(&timer, &time_until_next_call); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; EXPECT_LE(time_until_next_call, 0); - rcl_reset_error(); EXPECT_EQ(RCL_RET_OK, rcl_timer_call(&timer)) << rcl_get_error_string().str; - rcl_reset_error(); } TEST_F(TestTimerFixture, test_canceled_timer) { @@ -856,7 +844,6 @@ TEST_F(TestPreInitTimer, test_invalid_init_fini) { rcl_allocator_t bad_allocator = get_failing_allocator(); rcl_timer_t timer_fail = rcl_get_zero_initialized_timer(); - rcl_reset_error(); EXPECT_EQ( RCL_RET_ALREADY_INIT, rcl_timer_init( &timer, &clock, this->context_ptr, 500, nullptr, @@ -869,8 +856,7 @@ TEST_F(TestPreInitTimer, test_invalid_init_fini) { bad_allocator)) << rcl_get_error_string().str; rcl_reset_error(); - EXPECT_EQ(RCL_RET_OK, rcl_timer_fini(nullptr)); - rcl_reset_error(); + EXPECT_EQ(RCL_RET_OK, rcl_timer_fini(nullptr)) << rcl_get_error_string().str; } TEST_F(TestPreInitTimer, test_timer_get_period) {