Address peer review comments.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
ros2 · Jun 15, 2020 · b19b6f0 · b19b6f0
1 parent dc69a75
commit b19b6f0
Showing 1 changed file with 87 additions and 0 deletions.
diff --git 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();