Skip to content

Commit

Permalink
Simplify the test_secure_subscriber code. (#471)
Browse files Browse the repository at this point in the history
The main reason to do this is that we were sometimes running out
of memory on the buildfarm while building this code.  I suspect
that it is because of the templated attempt_subscriber() functions
that had to get expanded for quite a few different messages.
However, it turns out that we were only ever using 2 of those
messages in tests themselves.  Simplify this whole thing by
only creating templated functions for the messages we are
actually going to use.

Similarly, simplify the quite complicated CMake logic a bit to
only have a list of the messages that we are going to actually
use.  If we want to add more messages in the future, we can
always expand this list again.

Signed-off-by: Chris Lalancette <[email protected]>
  • Loading branch information
clalancette authored May 6, 2021
1 parent 8986434 commit 8fc14b4
Show file tree
Hide file tree
Showing 2 changed files with 119 additions and 195 deletions.
260 changes: 119 additions & 141 deletions test_security/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,24 +25,8 @@ if(BUILD_TESTING)
find_package(ament_cmake REQUIRED)
find_package(test_msgs REQUIRED)

ament_index_get_resource(interface_files "rosidl_interfaces" "test_msgs")
string(REPLACE "\n" ";" interface_files "${interface_files}")

set(message_files "")

# set(service_files "")
foreach(interface_file ${interface_files})
string_ends_with("${interface_file}" ".msg" is_message)
if(is_message)
list(APPEND message_files "${interface_file}")
continue()
endif()
# string_ends_with("${interface_file}" ".srv" is_service)
# if(is_service)
# list(APPEND service_files "${interface_file}")
# continue()
# endif()
endforeach()
# Test only a couple of message types to avoid taking too much test time
set(message_files "msg/Empty.msg;msg/UnboundedSequences.msg")

find_package(ament_lint_auto REQUIRED)
ament_lint_auto_find_test_dependencies()
Expand Down Expand Up @@ -122,133 +106,127 @@ if(BUILD_TESTING)
list(LENGTH not_connecting_PUBLISHER_ROS_SECURITY_ENABLE_LIST n_not_connecting_tests)

foreach(message_file ${message_files})
get_filename_component(TEST_MESSAGE_NS "${message_file}" DIRECTORY)
get_filename_component(TEST_MESSAGE_NS "${TEST_MESSAGE_NS}" NAME)
get_filename_component(TEST_MESSAGE_TYPE "${message_file}" NAME_WE)
# TODO(mikaelarguedas) test only few message types to avoid taking to much test time
if(TEST_MESSAGE_NS STREQUAL "msg" AND (
TEST_MESSAGE_TYPE STREQUAL "Empty" OR
TEST_MESSAGE_TYPE STREQUAL "UnboundedSequences"))
set(index 0)
# configure all non secure communication tests
set(SUBSCRIBER_SHOULD_TIMEOUT "false")
while(index LESS ${n_non_secure_tests})
# here we define all the variables needed for security template expansion
list(GET non_secure_comm_PUBLISHER_ROS_SECURITY_ENABLE_LIST ${index} PUBLISHER_ROS_SECURITY_ENABLE)
list(GET SUBSCRIBER_ROS_SECURITY_ENABLE_LIST ${index} SUBSCRIBER_ROS_SECURITY_ENABLE)
list(GET PUBLISHER_ROS_SECURITY_STRATEGY_LIST ${index} PUBLISHER_ROS_SECURITY_STRATEGY)
list(GET SUBSCRIBER_ROS_SECURITY_STRATEGY_LIST ${index} SUBSCRIBER_ROS_SECURITY_STRATEGY)
list(GET PUBLISHER_ROS_SECURITY_KEYSTORE_LIST ${index} PUBLISHER_ROS_SECURITY_KEYSTORE)
list(GET SUBSCRIBER_ROS_SECURITY_KEYSTORE_LIST ${index} SUBSCRIBER_ROS_SECURITY_KEYSTORE)

set(test_suffix "__${TEST_MESSAGE_TYPE}${suffix}__non_secure_comm_${index}")
configure_file(
test/test_secure_publisher_subscriber.py.in
test_secure_publisher_subscriber${test_suffix}.py.configured
@ONLY
)
file(GENERATE
OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/test_secure_publisher_subscriber${test_suffix}_$<CONFIG>.py"
INPUT "${CMAKE_CURRENT_BINARY_DIR}/test_secure_publisher_subscriber${test_suffix}.py.configured"
)
math(EXPR index "${index} + 1")

add_launch_test(
"${CMAKE_CURRENT_BINARY_DIR}/test_secure_publisher_subscriber${test_suffix}_$<CONFIG>.py"
TARGET test_secure_publisher_subscriber${test_suffix}
APPEND_LIBRARY_DIRS "${append_library_dirs}"
ENV
PATH="${TEST_PATH}"
TIMEOUT 20
)
if(TEST test_secure_publisher_subscriber${test_suffix})
set_tests_properties(
test_secure_publisher_subscriber${test_suffix}
PROPERTIES DEPENDS "test_secure_publisher_cpp__${rmw_implementation};test_secure_subscriber_cpp__${rmw_implementation}"
)
endif()
endwhile()

set(index 0)
set(SUBSCRIBER_SHOULD_TIMEOUT "false")
set(PUBLISHER_ROS_SECURITY_KEYSTORE "${KEYSTORE_DIRECTORY_NATIVE_PATH}")
set(SUBSCRIBER_ROS_SECURITY_KEYSTORE "${KEYSTORE_DIRECTORY_NATIVE_PATH}")
# configure all secure communication tests
while(index LESS ${n_secure_communication_tests})
# here we define all the variables needed for security template expansion
list(GET secure_comm_PUBLISHER_ROS_SECURITY_ENABLE_LIST ${index} PUBLISHER_ROS_SECURITY_ENABLE)
list(GET secure_comm_SUBSCRIBER_ROS_SECURITY_ENABLE_LIST ${index} SUBSCRIBER_ROS_SECURITY_ENABLE)
list(GET secure_comm_PUBLISHER_ROS_SECURITY_STRATEGY_LIST ${index} PUBLISHER_ROS_SECURITY_STRATEGY)
list(GET secure_comm_SUBSCRIBER_ROS_SECURITY_STRATEGY_LIST ${index} SUBSCRIBER_ROS_SECURITY_STRATEGY)

set(test_suffix "__${TEST_MESSAGE_TYPE}${suffix}__secure_comm_${index}")
configure_file(
test/test_secure_publisher_subscriber.py.in
test_secure_publisher_subscriber${test_suffix}.py.configured
@ONLY
)
file(GENERATE
OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/test_secure_publisher_subscriber${test_suffix}_$<CONFIG>.py"
INPUT "${CMAKE_CURRENT_BINARY_DIR}/test_secure_publisher_subscriber${test_suffix}.py.configured"
)
math(EXPR index "${index} + 1")

add_launch_test(
"${CMAKE_CURRENT_BINARY_DIR}/test_secure_publisher_subscriber${test_suffix}_$<CONFIG>.py"
TARGET test_secure_publisher_subscriber${test_suffix}
APPEND_LIBRARY_DIRS "${append_library_dirs}"
ENV
PATH="${TEST_PATH}"
TIMEOUT 20
)
if(TEST test_secure_publisher_subscriber${test_suffix})
set_tests_properties(
test_secure_publisher_subscriber${test_suffix}
PROPERTIES DEPENDS "test_secure_publisher_cpp__${rmw_implementation};test_secure_subscriber_cpp__${rmw_implementation}"
)
endif()
endwhile()

set(index 0)
set(PUBLISHER_ROS_SECURITY_KEYSTORE "${KEYSTORE_DIRECTORY_NATIVE_PATH}")
set(SUBSCRIBER_ROS_SECURITY_KEYSTORE "${KEYSTORE_DIRECTORY_NATIVE_PATH}")
set(SUBSCRIBER_SHOULD_TIMEOUT "true")
# configure all not connecting tests
while(index LESS ${n_not_connecting_tests})
# here we define all the variables needed for security template expansion
list(GET not_connecting_PUBLISHER_ROS_SECURITY_ENABLE_LIST ${index} PUBLISHER_ROS_SECURITY_ENABLE)
list(GET not_connecting_SUBSCRIBER_ROS_SECURITY_ENABLE_LIST ${index} SUBSCRIBER_ROS_SECURITY_ENABLE)
list(GET not_connecting_PUBLISHER_ROS_SECURITY_STRATEGY_LIST ${index} PUBLISHER_ROS_SECURITY_STRATEGY)
list(GET not_connecting_SUBSCRIBER_ROS_SECURITY_STRATEGY_LIST ${index} SUBSCRIBER_ROS_SECURITY_STRATEGY)

set(test_suffix "__${TEST_MESSAGE_TYPE}${suffix}__secure_not_connecting_${index}")
configure_file(
test/test_secure_publisher_subscriber.py.in
test_secure_publisher_subscriber${test_suffix}.py.configured
@ONLY

set(index 0)
# configure all non secure communication tests
set(SUBSCRIBER_SHOULD_TIMEOUT "false")
while(index LESS ${n_non_secure_tests})
# here we define all the variables needed for security template expansion
list(GET non_secure_comm_PUBLISHER_ROS_SECURITY_ENABLE_LIST ${index} PUBLISHER_ROS_SECURITY_ENABLE)
list(GET SUBSCRIBER_ROS_SECURITY_ENABLE_LIST ${index} SUBSCRIBER_ROS_SECURITY_ENABLE)
list(GET PUBLISHER_ROS_SECURITY_STRATEGY_LIST ${index} PUBLISHER_ROS_SECURITY_STRATEGY)
list(GET SUBSCRIBER_ROS_SECURITY_STRATEGY_LIST ${index} SUBSCRIBER_ROS_SECURITY_STRATEGY)
list(GET PUBLISHER_ROS_SECURITY_KEYSTORE_LIST ${index} PUBLISHER_ROS_SECURITY_KEYSTORE)
list(GET SUBSCRIBER_ROS_SECURITY_KEYSTORE_LIST ${index} SUBSCRIBER_ROS_SECURITY_KEYSTORE)

set(test_suffix "__${TEST_MESSAGE_TYPE}${suffix}__non_secure_comm_${index}")
configure_file(
test/test_secure_publisher_subscriber.py.in
test_secure_publisher_subscriber${test_suffix}.py.configured
@ONLY
)
file(GENERATE
OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/test_secure_publisher_subscriber${test_suffix}_$<CONFIG>.py"
INPUT "${CMAKE_CURRENT_BINARY_DIR}/test_secure_publisher_subscriber${test_suffix}.py.configured"
)
math(EXPR index "${index} + 1")

add_launch_test(
"${CMAKE_CURRENT_BINARY_DIR}/test_secure_publisher_subscriber${test_suffix}_$<CONFIG>.py"
TARGET test_secure_publisher_subscriber${test_suffix}
APPEND_LIBRARY_DIRS "${append_library_dirs}"
ENV
PATH="${TEST_PATH}"
TIMEOUT 20
)
if(TEST test_secure_publisher_subscriber${test_suffix})
set_tests_properties(
test_secure_publisher_subscriber${test_suffix}
PROPERTIES DEPENDS "test_secure_publisher_cpp__${rmw_implementation};test_secure_subscriber_cpp__${rmw_implementation}"
)
file(GENERATE
OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/test_secure_publisher_subscriber${test_suffix}_$<CONFIG>.py"
INPUT "${CMAKE_CURRENT_BINARY_DIR}/test_secure_publisher_subscriber${test_suffix}.py.configured"
endif()
endwhile()

set(index 0)
set(SUBSCRIBER_SHOULD_TIMEOUT "false")
set(PUBLISHER_ROS_SECURITY_KEYSTORE "${KEYSTORE_DIRECTORY_NATIVE_PATH}")
set(SUBSCRIBER_ROS_SECURITY_KEYSTORE "${KEYSTORE_DIRECTORY_NATIVE_PATH}")
# configure all secure communication tests
while(index LESS ${n_secure_communication_tests})
# here we define all the variables needed for security template expansion
list(GET secure_comm_PUBLISHER_ROS_SECURITY_ENABLE_LIST ${index} PUBLISHER_ROS_SECURITY_ENABLE)
list(GET secure_comm_SUBSCRIBER_ROS_SECURITY_ENABLE_LIST ${index} SUBSCRIBER_ROS_SECURITY_ENABLE)
list(GET secure_comm_PUBLISHER_ROS_SECURITY_STRATEGY_LIST ${index} PUBLISHER_ROS_SECURITY_STRATEGY)
list(GET secure_comm_SUBSCRIBER_ROS_SECURITY_STRATEGY_LIST ${index} SUBSCRIBER_ROS_SECURITY_STRATEGY)

set(test_suffix "__${TEST_MESSAGE_TYPE}${suffix}__secure_comm_${index}")
configure_file(
test/test_secure_publisher_subscriber.py.in
test_secure_publisher_subscriber${test_suffix}.py.configured
@ONLY
)
file(GENERATE
OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/test_secure_publisher_subscriber${test_suffix}_$<CONFIG>.py"
INPUT "${CMAKE_CURRENT_BINARY_DIR}/test_secure_publisher_subscriber${test_suffix}.py.configured"
)
math(EXPR index "${index} + 1")

add_launch_test(
"${CMAKE_CURRENT_BINARY_DIR}/test_secure_publisher_subscriber${test_suffix}_$<CONFIG>.py"
TARGET test_secure_publisher_subscriber${test_suffix}
APPEND_LIBRARY_DIRS "${append_library_dirs}"
ENV
PATH="${TEST_PATH}"
TIMEOUT 20
)
if(TEST test_secure_publisher_subscriber${test_suffix})
set_tests_properties(
test_secure_publisher_subscriber${test_suffix}
PROPERTIES DEPENDS "test_secure_publisher_cpp__${rmw_implementation};test_secure_subscriber_cpp__${rmw_implementation}"
)
math(EXPR index "${index} + 1")

add_launch_test(
"${CMAKE_CURRENT_BINARY_DIR}/test_secure_publisher_subscriber${test_suffix}_$<CONFIG>.py"
TARGET test_secure_publisher_subscriber${test_suffix}
APPEND_LIBRARY_DIRS "${append_library_dirs}"
ENV
PATH="${TEST_PATH}"
TIMEOUT 20
endif()
endwhile()

set(index 0)
set(PUBLISHER_ROS_SECURITY_KEYSTORE "${KEYSTORE_DIRECTORY_NATIVE_PATH}")
set(SUBSCRIBER_ROS_SECURITY_KEYSTORE "${KEYSTORE_DIRECTORY_NATIVE_PATH}")
set(SUBSCRIBER_SHOULD_TIMEOUT "true")
# configure all not connecting tests
while(index LESS ${n_not_connecting_tests})
# here we define all the variables needed for security template expansion
list(GET not_connecting_PUBLISHER_ROS_SECURITY_ENABLE_LIST ${index} PUBLISHER_ROS_SECURITY_ENABLE)
list(GET not_connecting_SUBSCRIBER_ROS_SECURITY_ENABLE_LIST ${index} SUBSCRIBER_ROS_SECURITY_ENABLE)
list(GET not_connecting_PUBLISHER_ROS_SECURITY_STRATEGY_LIST ${index} PUBLISHER_ROS_SECURITY_STRATEGY)
list(GET not_connecting_SUBSCRIBER_ROS_SECURITY_STRATEGY_LIST ${index} SUBSCRIBER_ROS_SECURITY_STRATEGY)

set(test_suffix "__${TEST_MESSAGE_TYPE}${suffix}__secure_not_connecting_${index}")
configure_file(
test/test_secure_publisher_subscriber.py.in
test_secure_publisher_subscriber${test_suffix}.py.configured
@ONLY
)
file(GENERATE
OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/test_secure_publisher_subscriber${test_suffix}_$<CONFIG>.py"
INPUT "${CMAKE_CURRENT_BINARY_DIR}/test_secure_publisher_subscriber${test_suffix}.py.configured"
)
math(EXPR index "${index} + 1")

add_launch_test(
"${CMAKE_CURRENT_BINARY_DIR}/test_secure_publisher_subscriber${test_suffix}_$<CONFIG>.py"
TARGET test_secure_publisher_subscriber${test_suffix}
APPEND_LIBRARY_DIRS "${append_library_dirs}"
ENV
PATH="${TEST_PATH}"
TIMEOUT 20
)
if(TEST test_secure_publisher_subscriber${test_suffix})
set_tests_properties(
test_secure_publisher_subscriber${test_suffix}
PROPERTIES DEPENDS "test_secure_publisher_cpp__${rmw_implementation};test_secure_subscriber_cpp__${rmw_implementation}"
)
if(TEST test_secure_publisher_subscriber${test_suffix})
set_tests_properties(
test_secure_publisher_subscriber${test_suffix}
PROPERTIES DEPENDS "test_secure_publisher_cpp__${rmw_implementation};test_secure_subscriber_cpp__${rmw_implementation}"
)
endif()
endwhile()
endif()
endif()
endwhile()
endforeach()
endmacro()

Expand Down
54 changes: 0 additions & 54 deletions test_security/test/test_secure_subscriber.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,36 +150,9 @@ int main(int argc, char ** argv)
if (message == "Empty") {
subscriber = attempt_subscribe<test_msgs::msg::Empty>(
node, topic_name, messages_empty, received_messages);
} else if (message == "BasicTypes") {
subscriber = attempt_subscribe<test_msgs::msg::BasicTypes>(
node, topic_name, messages_basic_types, received_messages);
} else if (message == "Arrays") {
subscriber = attempt_subscribe<test_msgs::msg::Arrays>(
node, topic_name, messages_arrays, received_messages);
} else if (message == "UnboundedSequences") {
subscriber = attempt_subscribe<test_msgs::msg::UnboundedSequences>(
node, topic_name, messages_unbounded_sequences, received_messages);
} else if (message == "BoundedSequences") {
subscriber = attempt_subscribe<test_msgs::msg::BoundedSequences>(
node, topic_name, messages_bounded_sequences, received_messages);
} else if (message == "Nested") {
subscriber = attempt_subscribe<test_msgs::msg::Nested>(
node, topic_name, messages_nested, received_messages);
} else if (message == "MultiNested") {
subscriber = attempt_subscribe<test_msgs::msg::MultiNested>(
node, topic_name, messages_multi_nested, received_messages);
} else if (message == "Strings") {
subscriber = attempt_subscribe<test_msgs::msg::Strings>(
node, topic_name, messages_strings, received_messages);
} else if (message == "Constants") {
subscriber = attempt_subscribe<test_msgs::msg::Constants>(
node, topic_name, messages_constants, received_messages);
} else if (message == "Defaults") {
subscriber = attempt_subscribe<test_msgs::msg::Defaults>(
node, topic_name, messages_defaults, received_messages);
} else if (message == "Builtins") {
subscriber = attempt_subscribe<test_msgs::msg::Builtins>(
node, topic_name, messages_builtins, received_messages);
} else {
fprintf(stderr, "Unknown message argument '%s'\n", message.c_str());
rclcpp::shutdown();
Expand All @@ -195,36 +168,9 @@ int main(int argc, char ** argv)
if (message == "Empty") {
subscriber = attempt_subscribe<test_msgs::msg::Empty>(
node, topic_name, sub_callback_called, executor);
} else if (message == "BasicTypes") {
subscriber = attempt_subscribe<test_msgs::msg::BasicTypes>(
node, topic_name, sub_callback_called, executor);
} else if (message == "Arrays") {
subscriber = attempt_subscribe<test_msgs::msg::Arrays>(
node, topic_name, sub_callback_called, executor);
} else if (message == "UnboundedSequences") {
subscriber = attempt_subscribe<test_msgs::msg::UnboundedSequences>(
node, topic_name, sub_callback_called, executor);
} else if (message == "BoundedSequences") {
subscriber = attempt_subscribe<test_msgs::msg::BoundedSequences>(
node, topic_name, sub_callback_called, executor);
} else if (message == "Nested") {
subscriber = attempt_subscribe<test_msgs::msg::Nested>(
node, topic_name, sub_callback_called, executor);
} else if (message == "MultiNested") {
subscriber = attempt_subscribe<test_msgs::msg::MultiNested>(
node, topic_name, sub_callback_called, executor);
} else if (message == "Strings") {
subscriber = attempt_subscribe<test_msgs::msg::Strings>(
node, topic_name, sub_callback_called, executor);
} else if (message == "Constants") {
subscriber = attempt_subscribe<test_msgs::msg::Constants>(
node, topic_name, sub_callback_called, executor);
} else if (message == "Defaults") {
subscriber = attempt_subscribe<test_msgs::msg::Defaults>(
node, topic_name, sub_callback_called, executor);
} else if (message == "Builtins") {
subscriber = attempt_subscribe<test_msgs::msg::Builtins>(
node, topic_name, sub_callback_called, executor);
} else {
fprintf(stderr, "Unknown message argument '%s'\n", message.c_str());
rclcpp::shutdown();
Expand Down

0 comments on commit 8fc14b4

Please sign in to comment.