Skip to content

Commit

Permalink
Refactor parameter test fixtures
Browse files Browse the repository at this point in the history 
Renamed functions for consistency and reduced code-smell

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
  • Loading branch information
@jacobperron
jacobperron committed May 14, 2019
1 parent 274fe12 commit 57b1da7
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 66 deletions.
114 changes: 63 additions & 51 deletions test_rclcpp/test/parameter_fixtures.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,88 +27,100 @@

const double test_epsilon = 1e-6;

void declare_test_parameters(std::shared_ptr<rclcpp::Node> node)
std::vector<rclcpp::Parameter>
get_test_parameters()
{
node->declare_parameter("foo");
node->declare_parameter("bar");
node->declare_parameter("barstr");
node->declare_parameter("baz");
node->declare_parameter("foo.first");
node->declare_parameter("foo.second");
node->declare_parameter("foobar");
}

void set_test_parameters(
std::shared_ptr<rclcpp::SyncParametersClient> parameters_client,
bool expected_result = true)
{
printf("Setting parameters\n");
// Set several differnet types of parameters.
auto set_parameters_results = parameters_client->set_parameters({
return {
rclcpp::Parameter("foo", 2),
rclcpp::Parameter("bar", "hello"),
rclcpp::Parameter("barstr", std::string("hello_str")),
rclcpp::Parameter("baz", 1.45),
rclcpp::Parameter("foo.first", 8),
rclcpp::Parameter("foo.second", 42),
rclcpp::Parameter("foobar", true),
});
};
}

void declare_test_parameters(std::shared_ptr<rclcpp::Node> node, int declare_up_to = -1)
{
auto parameters = get_test_parameters();

if (declare_up_to < 0 || declare_up_to > static_cast<int>(parameters.size())) {
declare_up_to = static_cast<int>(parameters.size());
}

for (int i = 0u; i < declare_up_to; ++i) {
node->declare_parameter(parameters[i].get_name());
}
}

void test_set_parameters_sync(
std::shared_ptr<rclcpp::SyncParametersClient> parameters_client,
int successful_up_to = -1)
{
printf("Setting parameters\n");
std::vector<rclcpp::Parameter> parameters = get_test_parameters();
auto set_parameters_results = parameters_client->set_parameters(parameters);
printf("Got set_parameters result\n");

ASSERT_EQ(set_parameters_results.size(), parameters.size());

// Check to see if they were set.
for (auto & result : set_parameters_results) {
ASSERT_EQ(result.successful, expected_result);
if (successful_up_to < 0 || successful_up_to > static_cast<int>(parameters.size())) {
successful_up_to = static_cast<int>(parameters.size());
}

// Check success values
for (int i = 0u; i < successful_up_to; ++i) {
const auto & result = set_parameters_results[i];
ASSERT_TRUE(result.successful);
}
for (int i = successful_up_to; i < static_cast<int>(parameters.size()); ++i) {
const auto & result = set_parameters_results[i];
ASSERT_FALSE(result.successful);
}
}

void set_test_parameters_atomically(
void test_set_parameters_atomically_sync(
std::shared_ptr<rclcpp::SyncParametersClient> parameters_client,
bool expected_result = true)
bool expect_result_successful = true)
{
printf("Setting parameters atomically\n");
// Set several differnet types of parameters.
auto set_parameters_result = parameters_client->set_parameters_atomically({
rclcpp::Parameter("foo", 2),
rclcpp::Parameter("bar", "hello"),
rclcpp::Parameter("barstr", std::string("hello_str")),
rclcpp::Parameter("baz", 1.45),
rclcpp::Parameter("foo.first", 8),
rclcpp::Parameter("foo.second", 42),
rclcpp::Parameter("foobar", true),
});
std::vector<rclcpp::Parameter> parameters = get_test_parameters();
auto set_parameters_result = parameters_client->set_parameters_atomically(parameters);
printf("Got set_parameters_atomically result\n");

// Check to see if they were set.
ASSERT_EQ(set_parameters_result.successful, expected_result);
ASSERT_EQ(set_parameters_result.successful, expect_result_successful);
}

void verify_set_parameters_async(
void test_set_parameters_async(
std::shared_ptr<rclcpp::Node> node,
std::shared_ptr<rclcpp::AsyncParametersClient> parameters_client,
bool expected_result = true)
int successful_up_to = -1)
{
printf("Setting parameters\n");
// Set several differnet types of parameters.
auto set_parameters_results = parameters_client->set_parameters({
rclcpp::Parameter("foo", 2),
rclcpp::Parameter("bar", "hello"),
rclcpp::Parameter("barstr", std::string("hello_str")),
rclcpp::Parameter("baz", 1.45),
rclcpp::Parameter("foo.first", 8),
rclcpp::Parameter("foo.second", 42),
rclcpp::Parameter("foobar", true),
});
std::vector<rclcpp::Parameter> parameters = get_test_parameters();
auto set_parameters_results = parameters_client->set_parameters(parameters);
rclcpp::spin_until_future_complete(node, set_parameters_results); // Wait for the results.
printf("Got set_parameters result\n");

// Check to see if they were set.
for (auto & result : set_parameters_results.get()) {
ASSERT_EQ(result.successful, expected_result);
if (successful_up_to < 0 || successful_up_to > static_cast<int>(parameters.size())) {
successful_up_to = static_cast<int>(parameters.size());
}

const auto results = set_parameters_results.get();
ASSERT_EQ(results.size(), parameters.size());

// Check success values
for (int i = 0u; i < successful_up_to; ++i) {
ASSERT_TRUE(results[i].successful);
}
for (int i = successful_up_to; i < static_cast<int>(parameters.size()); ++i) {
ASSERT_FALSE(results[i].successful);
}
}

void verify_test_parameters(
void test_get_parameters_sync(
std::shared_ptr<rclcpp::SyncParametersClient> parameters_client)
{
printf("Listing parameters with recursive depth\n");
Expand Down Expand Up @@ -218,7 +230,7 @@ void verify_test_parameters(
}
}

void verify_get_parameters_async(
void test_get_parameters_async(
std::shared_ptr<rclcpp::Node> node,
std::shared_ptr<rclcpp::AsyncParametersClient> parameters_client)
{
Expand Down
12 changes: 6 additions & 6 deletions test_rclcpp/test/test_local_parameters.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ TEST(CLASSNAME(test_local_parameters, RMW_IMPLEMENTATION), local_synchronous) {
if (!parameters_client->wait_for_service(20s)) {
ASSERT_TRUE(false) << "service not available after waiting";
}
set_test_parameters(parameters_client);
verify_test_parameters(parameters_client);
test_set_parameters_sync(parameters_client);
test_get_parameters_sync(parameters_client);
}

TEST(CLASSNAME(test_local_parameters, RMW_IMPLEMENTATION), local_synchronous_repeated) {
Expand All @@ -85,10 +85,10 @@ TEST(CLASSNAME(test_local_parameters, RMW_IMPLEMENTATION), local_synchronous_rep
if (!parameters_client->wait_for_service(20s)) {
ASSERT_TRUE(false) << "service not available after waiting";
}
set_test_parameters(parameters_client);
test_set_parameters_sync(parameters_client);
for (int i = 0; i < 10; ++i) {
printf("iteration: %d\n", i);
verify_test_parameters(parameters_client);
test_get_parameters_sync(parameters_client);
}
}

Expand All @@ -100,8 +100,8 @@ TEST(CLASSNAME(test_local_parameters, RMW_IMPLEMENTATION), local_asynchronous) {
if (!parameters_client->wait_for_service(20s)) {
ASSERT_TRUE(false) << "service not available after waiting";
}
verify_set_parameters_async(node, parameters_client);
verify_get_parameters_async(node, parameters_client);
test_set_parameters_async(node, parameters_client);
test_get_parameters_async(node, parameters_client);
}

class ParametersAsyncNode : public rclcpp::Node
Expand Down
18 changes: 9 additions & 9 deletions test_rclcpp/test/test_remote_parameters.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ TEST(CLASSNAME(parameters, rmw_implementation), test_remote_parameters_async) {
ASSERT_TRUE(false) << "service not available after waiting";
}

verify_set_parameters_async(node, parameters_client);
test_set_parameters_async(node, parameters_client);

verify_get_parameters_async(node, parameters_client);
test_get_parameters_async(node, parameters_client);
}

TEST(CLASSNAME(parameters, rmw_implementation), test_remote_parameters_sync) {
Expand All @@ -64,9 +64,9 @@ TEST(CLASSNAME(parameters, rmw_implementation), test_remote_parameters_sync) {
ASSERT_TRUE(false) << "service not available after waiting";
}

set_test_parameters(parameters_client);
test_set_parameters_sync(parameters_client);

verify_test_parameters(parameters_client);
test_get_parameters_sync(parameters_client);
}

TEST(CLASSNAME(parameters, rmw_implementation), test_set_remote_parameters_atomically_sync) {
Expand All @@ -81,9 +81,9 @@ TEST(CLASSNAME(parameters, rmw_implementation), test_set_remote_parameters_atomi
ASSERT_TRUE(false) << "service not available after waiting";
}

set_test_parameters_atomically(parameters_client);
test_set_parameters_atomically_sync(parameters_client);

verify_test_parameters(parameters_client);
test_get_parameters_sync(parameters_client);
}

TEST(CLASSNAME(parameters_must_declare, rmw_implementation), test_remote_parameters_async) {
Expand All @@ -99,7 +99,7 @@ TEST(CLASSNAME(parameters_must_declare, rmw_implementation), test_remote_paramet
ASSERT_TRUE(false) << "service not available after waiting";
}

verify_set_parameters_async(node, parameters_client, false);
test_set_parameters_async(node, parameters_client, 0);
}

TEST(CLASSNAME(parameters_must_declare, rmw_implementation), test_remote_parameters_sync) {
Expand All @@ -115,7 +115,7 @@ TEST(CLASSNAME(parameters_must_declare, rmw_implementation), test_remote_paramet
ASSERT_TRUE(false) << "service not available after waiting";
}

set_test_parameters(parameters_client, false);
test_set_parameters_sync(parameters_client, 0);
}

TEST(
Expand All @@ -134,5 +134,5 @@ TEST(
ASSERT_TRUE(false) << "service not available after waiting";
}

set_test_parameters_atomically(parameters_client, false);
test_set_parameters_atomically_sync(parameters_client, false);
}

0 comments on commit 57b1da7

Please sign in to comment.