Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add time API coverage tests #1347

Merged
merged 11 commits into from
Sep 30, 2020
Merged

Add time API coverage tests #1347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
e3c6677
Change value used as max representation
Blast545 Sep 29, 2020
fc172ff
Add coverage tests time
Blast545 Sep 29, 2020
107ea49
Add call to detach clock
Blast545 Sep 29, 2020
5273325
Add tests time
Blast545 Sep 29, 2020
699197e
Add duration construction tests
Blast545 Sep 29, 2020
7ba92dd
Revert change
Blast545 Sep 29, 2020
095bd0f
Add const qualifier to constants
Blast545 Sep 29, 2020
5d54798
Add check clock stays the same
Blast545 Sep 29, 2020
b29c102
Make operator RCLCPP_PUBLIC
Blast545 Sep 30, 2020
049ba01
Add tests exceptions duration
Blast545 Sep 30, 2020
a546ce5
Fix division by 0 on windows
Blast545 Sep 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions rclcpp/include/rclcpp/time.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ class Time
/**
* \throws std::overflow_error if addition leads to overflow
*/
RCLCPP_PUBLIC
Time
operator+(const rclcpp::Duration & lhs, const rclcpp::Time & rhs);

Expand Down
24 changes: 24 additions & 0 deletions rclcpp/test/rclcpp/test_duration.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "rclcpp/rclcpp.hpp"
#include "rclcpp/duration.hpp"

#include "../utils/rclcpp_gtest_macros.hpp"

using namespace std::chrono_literals;

Expand Down Expand Up @@ -237,3 +238,26 @@ TEST_F(TestDuration, conversions) {
EXPECT_EQ(chrono_duration.count(), -ONE_AND_HALF_SEC_IN_NS);
}
}

TEST_F(TestDuration, test_some_constructors) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to add coverage for the exceptions thrown during the operators - (which calls bounds_check_duration_difference) and * (which just needs a check that multiplying by inf catches the appropriate exception)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add them before merging! checking Windows CI first

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, see 049ba01

builtin_interfaces::msg::Duration duration_msg;
duration_msg.sec = 1;
duration_msg.nanosec = 1000;
rclcpp::Duration duration_from_msg(duration_msg);
EXPECT_EQ(RCL_S_TO_NS(1) + 1000, duration_from_msg.nanoseconds());

rcl_duration_t duration_struct;
duration_struct.nanoseconds = 4000;
rclcpp::Duration duration_from_struct(duration_struct);
EXPECT_EQ(4000, duration_from_struct.nanoseconds());
}

TEST_F(TestDuration, test_some_exceptions) {
rclcpp::Duration test_duration(0u);
RCLCPP_EXPECT_THROW_EQ(
test_duration = rclcpp::Duration(INT64_MAX) - rclcpp::Duration(-1),
std::overflow_error("duration subtraction leads to int64_t overflow"));
RCLCPP_EXPECT_THROW_EQ(
test_duration = test_duration * (std::numeric_limits<double>::infinity()),
std::runtime_error("abnormal scale in rclcpp::Duration"));
}
92 changes: 92 additions & 0 deletions rclcpp/test/rclcpp/test_time.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
#include "rclcpp/time.hpp"
#include "rclcpp/utilities.hpp"

#include "../utils/rclcpp_gtest_macros.hpp"

namespace
{

Expand Down Expand Up @@ -352,3 +354,93 @@ TEST_F(TestTime, seconds) {
EXPECT_DOUBLE_EQ(4.5, rclcpp::Time(4, 500000000).seconds());
EXPECT_DOUBLE_EQ(2.5, rclcpp::Time(0, 2500000000).seconds());
}

TEST_F(TestTime, test_max) {
const rclcpp::Time time_max = rclcpp::Time::max();
const rclcpp::Time max_time(std::numeric_limits<int32_t>::max(), 999999999);
EXPECT_DOUBLE_EQ(max_time.seconds(), time_max.seconds());
EXPECT_EQ(max_time.nanoseconds(), time_max.nanoseconds());
}

TEST_F(TestTime, test_constructor_from_rcl_time_point) {
const rcl_time_point_value_t test_nano_seconds = 555;
const rcl_clock_type_t test_clock_type = RCL_ROS_TIME;
rcl_time_point_t test_time_point;
test_time_point.nanoseconds = test_nano_seconds;
test_time_point.clock_type = test_clock_type;

const rclcpp::Time time_max = rclcpp::Time(test_time_point);

EXPECT_EQ(test_nano_seconds, time_max.nanoseconds());
EXPECT_EQ(test_nano_seconds, test_time_point.nanoseconds);
EXPECT_EQ(test_clock_type, time_max.get_clock_type());
EXPECT_EQ(test_clock_type, test_time_point.clock_type);
}

TEST_F(TestTime, test_assignment_operator_from_builtin_msg_time) {
rclcpp::Clock ros_clock(RCL_ROS_TIME);
const builtin_interfaces::msg::Time ros_now = ros_clock.now();
EXPECT_NE(0, ros_now.sec);
EXPECT_NE(0u, ros_now.nanosec);

rclcpp::Time test_time(0u, RCL_CLOCK_UNINITIALIZED);
EXPECT_EQ(0u, test_time.nanoseconds());
EXPECT_EQ(RCL_CLOCK_UNINITIALIZED, test_time.get_clock_type());

test_time = ros_now;
EXPECT_NE(0, test_time.nanoseconds());
// The clock type is hardcoded internally
EXPECT_EQ(RCL_ROS_TIME, test_time.get_clock_type());
}

TEST_F(TestTime, test_sum_operator) {
const rclcpp::Duration one(1);
const rclcpp::Time test_time(0u);
EXPECT_EQ(0u, test_time.nanoseconds());

const rclcpp::Time new_time = one + test_time;
EXPECT_EQ(1, new_time.nanoseconds());
}

TEST_F(TestTime, test_overflow_underflow_throws) {
rclcpp::Time test_time(0u);

RCLCPP_EXPECT_THROW_EQ(
test_time = rclcpp::Time(INT64_MAX) + rclcpp::Duration(1),
std::overflow_error("addition leads to int64_t overflow"));
RCLCPP_EXPECT_THROW_EQ(
test_time = rclcpp::Time(INT64_MIN) + rclcpp::Duration(-1),
std::underflow_error("addition leads to int64_t underflow"));

RCLCPP_EXPECT_THROW_EQ(
test_time = rclcpp::Time(INT64_MAX) - rclcpp::Duration(-1),
std::overflow_error("time subtraction leads to int64_t overflow"));
RCLCPP_EXPECT_THROW_EQ(
test_time = rclcpp::Time(INT64_MIN) - rclcpp::Duration(1),
std::underflow_error("time subtraction leads to int64_t underflow"));

test_time = rclcpp::Time(INT64_MAX);
RCLCPP_EXPECT_THROW_EQ(
test_time += rclcpp::Duration(1),
std::overflow_error("addition leads to int64_t overflow"));
test_time = rclcpp::Time(INT64_MIN);
RCLCPP_EXPECT_THROW_EQ(
test_time += rclcpp::Duration(-1),
std::underflow_error("addition leads to int64_t underflow"));

test_time = rclcpp::Time(INT64_MAX);
RCLCPP_EXPECT_THROW_EQ(
test_time -= rclcpp::Duration(-1),
std::overflow_error("time subtraction leads to int64_t overflow"));
test_time = rclcpp::Time(INT64_MIN);
RCLCPP_EXPECT_THROW_EQ(
test_time -= rclcpp::Duration(1),
std::underflow_error("time subtraction leads to int64_t underflow"));

RCLCPP_EXPECT_THROW_EQ(
test_time = rclcpp::Duration(INT64_MAX) + rclcpp::Time(1),
std::overflow_error("addition leads to int64_t overflow"));
RCLCPP_EXPECT_THROW_EQ(
test_time = rclcpp::Duration(INT64_MIN) + rclcpp::Time(-1),
std::underflow_error("addition leads to int64_t underflow"));
}
3 changes: 3 additions & 0 deletions rclcpp/test/rclcpp/test_time_source.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,9 @@ TEST_F(TestTimeSource, ROS_time_valid_attach_detach) {

ts.attachNode(node);
EXPECT_FALSE(ros_clock->ros_time_is_active());

ts.detachClock(ros_clock);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there anything you can check to make sure detachClock worked as expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a check similar to the rest of the test scenario here: 5d54798

I didn't see any straightforward way to test it out, the enable_ros_time and disable_ros_time could be great for that, but those are private. Let me know if you have any clue how to test this out.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't see any straightforward way to test it out, the enable_ros_time and disable_ros_time could be great for that, but those are private. Let me know if you have any clue how to test this out.

We could consider making those protected and deriving the test class from it, but it's not required.

EXPECT_FALSE(ros_clock->ros_time_is_active());
}

TEST_F(TestTimeSource, ROS_time_valid_wall_time) {
Expand Down