From 8ed9b749204a1d8a84f1c032ab29eb3ad9c05a8f Mon Sep 17 00:00:00 2001 From: Andrea Sorbini Date: Fri, 5 Mar 2021 12:52:05 -0800 Subject: [PATCH 1/5] Replace rmw_connext_cpp with rmw_connextdds Signed-off-by: Andrea Sorbini --- test_communication/CMakeLists.txt | 24 +------------------ .../test/test_deadline.cpp | 2 +- .../test/test_lifespan.cpp | 2 +- test_security/CMakeLists.txt | 5 ++-- 4 files changed, 5 insertions(+), 28 deletions(-) diff --git a/test_communication/CMakeLists.txt b/test_communication/CMakeLists.txt index edfd4c7b..cd6047ab 100644 --- a/test_communication/CMakeLists.txt +++ b/test_communication/CMakeLists.txt @@ -249,16 +249,7 @@ if(BUILD_TESTING) # TODO(mikaelarguedas) Simpler way to blacklist specific tests (e.g. regex matching) # TODO different vendors can't talk to each other right now - if(NOT rmw_implementation1 STREQUAL rmw_implementation2 AND - (NOT(rmw_implementation1 MATCHES "(.*)connext(.*)" AND rmw_implementation2 MATCHES "(.*)connext(.*)")) - ) - set(SKIP_TEST "SKIP_TEST") - endif() - - # TODO(mikaelarguedas) connext_dynamic doesn't support C services for now - if((client_library1 STREQUAL "rclpy" AND rmw_implementation1 MATCHES "rmw_connext_dynamic_cpp") OR - (client_library2 STREQUAL "rclpy" AND rmw_implementation2 MATCHES "rmw_connext_dynamic_cpp") - ) + if(NOT rmw_implementation1 STREQUAL rmw_implementation2) set(SKIP_TEST "SKIP_TEST") endif() @@ -449,12 +440,6 @@ if(BUILD_TESTING) endif() macro(serialize) - set(SKIP_TEST "") - if(${rmw_implementation} STREQUAL "rmw_connext_dynamic_cpp") - message(STATUS "skipping serialize tests for ${rmw_implementation}") - set(SKIP_TEST "SKIP_TEST") - endif() - set(serialize_target_name "test_serialize${target}${target_suffix}") ament_add_gtest( ${serialize_target_name} test/test_message_serialization.cpp @@ -462,7 +447,6 @@ if(BUILD_TESTING) ENV RCL_ASSERT_RMW_ID_MATCHES=${rmw_implementation} RMW_IMPLEMENTATION=${rmw_implementation} - ${SKIP_TEST} ) if(TARGET ${serialize_target_name}) add_dependencies(${serialize_target_name} ${PROJECT_NAME}) @@ -481,11 +465,6 @@ if(BUILD_TESTING) endmacro() macro(pub_sub_serialized) - set(SKIP_TEST "") - if(${rmw_implementation} STREQUAL "rmw_connext_dynamic_cpp") - message(STATUS "skipping serialize tests for ${rmw_implementation}") - set(SKIP_TEST "SKIP_TEST") - endif() set(target_name "test_publisher_subscriber_serialized${target}${target_suffix}") ament_add_gtest( ${target_name} test/test_publisher_subscriber_serialized.cpp @@ -493,7 +472,6 @@ if(BUILD_TESTING) ENV RCL_ASSERT_RMW_ID_MATCHES=${rmw_implementation} RMW_IMPLEMENTATION=${rmw_implementation} - ${SKIP_TEST} ) if(TARGET ${target_name}) add_dependencies(${target_name} ${PROJECT_NAME}) diff --git a/test_quality_of_service/test/test_deadline.cpp b/test_quality_of_service/test/test_deadline.cpp index 393c5aa4..89ebd765 100644 --- a/test_quality_of_service/test/test_deadline.cpp +++ b/test_quality_of_service/test/test_deadline.cpp @@ -33,7 +33,7 @@ using namespace std::chrono_literals; /// Test Deadline with a single publishing node and single subscriber node TEST_F(QosRclcppTestFixture, test_deadline) { int expected_number_of_events = 4; - // Bump deadline duration when testing against rmw_connext_cpp to + // Bump deadline duration when testing against rmw_connextdds to // cope with the longer discovery times it entails. const std::chrono::milliseconds deadline_duration = this_rmw_implementation.find("connext") != std::string::npos ? 2s : 1s; diff --git a/test_quality_of_service/test/test_lifespan.cpp b/test_quality_of_service/test/test_lifespan.cpp index f7a1642d..f6e69742 100644 --- a/test_quality_of_service/test/test_lifespan.cpp +++ b/test_quality_of_service/test/test_lifespan.cpp @@ -32,7 +32,7 @@ using namespace std::chrono_literals; TEST_F(QosRclcppTestFixture, test_lifespan) { const int history = 2; - // Bump lifespan duration when testing against rmw_connext_cpp to + // Bump lifespan duration when testing against rmw_connextdds to // cope with the longer discovery times it entails. const std::chrono::milliseconds lifespan_duration = this_rmw_implementation.find("connext") != std::string::npos ? 2s : 1s; diff --git a/test_security/CMakeLists.txt b/test_security/CMakeLists.txt index b667d802..dd6aee47 100644 --- a/test_security/CMakeLists.txt +++ b/test_security/CMakeLists.txt @@ -256,7 +256,7 @@ if(BUILD_TESTING) set(ENV_PATH "$ENV{PATH}") file(TO_CMAKE_PATH "${ENV_PATH}" ENV_PATH) set(TEST_PATH "${ENV_PATH}") - if(rmw_implementation STREQUAL "rmw_connext_cpp") + if(rmw_implementation STREQUAL "rmw_connextdds") # Connext 5.3.1 needs RTI's OpenSSL binaries (based on EOL 1.0.2) to be # on the PATH at runtime as the system version of OpenSSL is not supported set(RTI_BIN_PATH "$ENV{RTI_OPENSSL_BIN}") @@ -270,8 +270,7 @@ if(BUILD_TESTING) # TODO(jacobperron) Disable Connext on Windows until we fix issue with CI if( - (rmw_implementation STREQUAL "rmw_connext_cpp" AND NOT WIN32) OR - (rmw_implementation STREQUAL "rmw_connext_dynamic_cpp" AND NOT WIN32) OR + (rmw_implementation STREQUAL "rmw_connextdds" AND NOT WIN32) OR rmw_implementation STREQUAL "rmw_fastrtps_cpp" OR rmw_implementation STREQUAL "rmw_fastrtps_dynamic_cpp" OR rmw_implementation STREQUAL "rmw_cyclonedds_cpp" From c69e3df3099572f02dfe3f8a8c727538a188be2d Mon Sep 17 00:00:00 2001 From: Andrea Sorbini Date: Mon, 8 Mar 2021 16:17:38 -0800 Subject: [PATCH 2/5] Restore exceptions for ros2/rmw_connext to ease transition to rticommunity/rmw_connextdds Signed-off-by: Andrea Sorbini --- test_communication/CMakeLists.txt | 22 +++++++++++++++++++++- test_security/CMakeLists.txt | 3 +++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/test_communication/CMakeLists.txt b/test_communication/CMakeLists.txt index cd6047ab..03194c75 100644 --- a/test_communication/CMakeLists.txt +++ b/test_communication/CMakeLists.txt @@ -249,7 +249,11 @@ if(BUILD_TESTING) # TODO(mikaelarguedas) Simpler way to blacklist specific tests (e.g. regex matching) # TODO different vendors can't talk to each other right now - if(NOT rmw_implementation1 STREQUAL rmw_implementation2) + # TODO(asorbini): Remove Connext exceptions once ros2/rmw_connext is deprecated. + if(NOT rmw_implementation1 STREQUAL rmw_implementation2 OR + (client_library1 STREQUAL "rclpy" AND rmw_implementation1 MATCHES "rmw_connext_dynamic_cpp") OR + (client_library2 STREQUAL "rclpy" AND rmw_implementation2 MATCHES "rmw_connext_dynamic_cpp") + ) set(SKIP_TEST "SKIP_TEST") endif() @@ -440,6 +444,13 @@ if(BUILD_TESTING) endif() macro(serialize) + # TODO(asorbini): Remove this exception once ros2/rmw_connext is deprecated. + set(SKIP_TEST "") + if(${rmw_implementation} STREQUAL "rmw_connext_dynamic_cpp") + message(STATUS "skipping serialize tests for ${rmw_implementation}") + set(SKIP_TEST "SKIP_TEST") + endif() + set(serialize_target_name "test_serialize${target}${target_suffix}") ament_add_gtest( ${serialize_target_name} test/test_message_serialization.cpp @@ -447,6 +458,7 @@ if(BUILD_TESTING) ENV RCL_ASSERT_RMW_ID_MATCHES=${rmw_implementation} RMW_IMPLEMENTATION=${rmw_implementation} + ${SKIP_TEST} ) if(TARGET ${serialize_target_name}) add_dependencies(${serialize_target_name} ${PROJECT_NAME}) @@ -465,6 +477,13 @@ if(BUILD_TESTING) endmacro() macro(pub_sub_serialized) + # TODO(asorbini): Remove this exception once ros2/rmw_connext is deprecated. + set(SKIP_TEST "") + if(${rmw_implementation} STREQUAL "rmw_connext_dynamic_cpp") + message(STATUS "skipping serialize tests for ${rmw_implementation}") + set(SKIP_TEST "SKIP_TEST") + endif() + set(target_name "test_publisher_subscriber_serialized${target}${target_suffix}") ament_add_gtest( ${target_name} test/test_publisher_subscriber_serialized.cpp @@ -472,6 +491,7 @@ if(BUILD_TESTING) ENV RCL_ASSERT_RMW_ID_MATCHES=${rmw_implementation} RMW_IMPLEMENTATION=${rmw_implementation} + ${SKIP_TEST} ) if(TARGET ${target_name}) add_dependencies(${target_name} ${PROJECT_NAME}) diff --git a/test_security/CMakeLists.txt b/test_security/CMakeLists.txt index dd6aee47..ce67c623 100644 --- a/test_security/CMakeLists.txt +++ b/test_security/CMakeLists.txt @@ -269,7 +269,10 @@ if(BUILD_TESTING) endif() # TODO(jacobperron) Disable Connext on Windows until we fix issue with CI + # TODO(asorbini): Remove exceptions once ros2/rmw_connext is deprecated. if( + (rmw_implementation STREQUAL "rmw_connext_cpp" AND NOT WIN32) OR + (rmw_implementation STREQUAL "rmw_connext_dynamic_cpp" AND NOT WIN32) OR (rmw_implementation STREQUAL "rmw_connextdds" AND NOT WIN32) OR rmw_implementation STREQUAL "rmw_fastrtps_cpp" OR rmw_implementation STREQUAL "rmw_fastrtps_dynamic_cpp" OR From 5b0a8c3d2331688e5d0b5e73c417154c36cd55de Mon Sep 17 00:00:00 2001 From: Andrea Sorbini Date: Wed, 10 Mar 2021 13:09:12 -0800 Subject: [PATCH 3/5] Disable tests between rmw_connextdds and rmw_connext_cpp Signed-off-by: Andrea Sorbini --- test_communication/CMakeLists.txt | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test_communication/CMakeLists.txt b/test_communication/CMakeLists.txt index 03194c75..16977686 100644 --- a/test_communication/CMakeLists.txt +++ b/test_communication/CMakeLists.txt @@ -182,6 +182,32 @@ if(BUILD_TESTING) set(rmw_implementation2_is_connext TRUE) endif() + # TODO(asorbini) Skip tests between rmw_connext_cpp and rmw_connextdds + # since they are not meant to coexist (and used incompatible typecodes by default). + set(rmw_implementation1_is_connextdds FALSE) + if(rmw_implementation1 MATCHES "(.*)connextdds") + set(rmw_implementation1_is_connextdds TRUE) + endif() + set(rmw_implementation2_is_connextdds FALSE) + if(rmw_implementation2 MATCHES "(.*)connextdds") + set(rmw_implementation2_is_connextdds TRUE) + endif() + + set(rmw_implementation1_is_connext_cpp FALSE) + if(rmw_implementation1 MATCHES "(.*)connext(.*)_cpp") + set(rmw_implementation1_is_connext_cpp TRUE) + endif() + set(rmw_implementation2_is_connext_cpp FALSE) + if(rmw_implementation2 MATCHES "(.*)connext(.*)_cpp") + set(rmw_implementation2_is_connext_cpp TRUE) + endif() + + if((rmw_implementation1_is_connextdds AND rmw_implementation2_is_connext_cpp) OR + (rmw_implementation1_is_connext_cpp AND rmw_implementation2_is_connextdds) + ) + set(SKIP_TEST "SKIP_TEST") + endif() + set(rmw_implementation1_is_cyclonedds FALSE) set(rmw_implementation2_is_cyclonedds FALSE) if(rmw_implementation1 MATCHES "(.*)cyclonedds(.*)") From 7069da00fed8ae1fd00ba97d108943df82f7319d Mon Sep 17 00:00:00 2001 From: Andrea Sorbini Date: Thu, 11 Mar 2021 08:19:40 -0800 Subject: [PATCH 4/5] Apply review feedback Signed-off-by: Andrea Sorbini --- test_security/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_security/CMakeLists.txt b/test_security/CMakeLists.txt index ce67c623..732cbc7f 100644 --- a/test_security/CMakeLists.txt +++ b/test_security/CMakeLists.txt @@ -256,7 +256,7 @@ if(BUILD_TESTING) set(ENV_PATH "$ENV{PATH}") file(TO_CMAKE_PATH "${ENV_PATH}" ENV_PATH) set(TEST_PATH "${ENV_PATH}") - if(rmw_implementation STREQUAL "rmw_connextdds") + if(rmw_implementation STREQUAL "rmw_connext(.*)") # Connext 5.3.1 needs RTI's OpenSSL binaries (based on EOL 1.0.2) to be # on the PATH at runtime as the system version of OpenSSL is not supported set(RTI_BIN_PATH "$ENV{RTI_OPENSSL_BIN}") From 12dc4fed9b2d09dd857e2f80bcdd041cdb50018a Mon Sep 17 00:00:00 2001 From: Andrea Sorbini Date: Thu, 11 Mar 2021 11:50:45 -0800 Subject: [PATCH 5/5] Use MATCHES instead of STREQUAL Signed-off-by: Andrea Sorbini --- test_security/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_security/CMakeLists.txt b/test_security/CMakeLists.txt index 732cbc7f..a1f6d888 100644 --- a/test_security/CMakeLists.txt +++ b/test_security/CMakeLists.txt @@ -256,7 +256,7 @@ if(BUILD_TESTING) set(ENV_PATH "$ENV{PATH}") file(TO_CMAKE_PATH "${ENV_PATH}" ENV_PATH) set(TEST_PATH "${ENV_PATH}") - if(rmw_implementation STREQUAL "rmw_connext(.*)") + if(rmw_implementation MATCHES "rmw_connext(.*)") # Connext 5.3.1 needs RTI's OpenSSL binaries (based on EOL 1.0.2) to be # on the PATH at runtime as the system version of OpenSSL is not supported set(RTI_BIN_PATH "$ENV{RTI_OPENSSL_BIN}")