From 885cd48ecab3edf4058513f0d7060e30cac81ab3 Mon Sep 17 00:00:00 2001 From: "Addisu Z. Taddese" Date: Tue, 19 Dec 2023 16:41:12 -0600 Subject: [PATCH 1/4] Fix bug in `create` where command line arguments were truncated Signed-off-by: Addisu Z. Taddese --- ros_gz_sim/src/create.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ros_gz_sim/src/create.cpp b/ros_gz_sim/src/create.cpp index 09bbfec0..c58e9c4f 100644 --- a/ros_gz_sim/src/create.cpp +++ b/ros_gz_sim/src/create.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include @@ -59,7 +60,7 @@ int main(int _argc, char ** _argv) char ** filtered_argv = new char *[(filtered_argc + 1)]; for (int ii = 0; ii < filtered_argc; ++ii) { filtered_argv[ii] = new char[filtered_arguments[ii].size() + 1]; - snprintf(filtered_argv[ii], sizeof(filtered_argv), "%s", filtered_arguments[ii].c_str()); + std::strcpy(filtered_argv[ii], filtered_arguments[ii].c_str()); } filtered_argv[filtered_argc] = nullptr; From 139d1941bf10e74bf6b94e22a7a8490b35ab9b64 Mon Sep 17 00:00:00 2001 From: "Addisu Z. Taddese" Date: Wed, 20 Dec 2023 15:14:07 -0600 Subject: [PATCH 2/4] Add test Signed-off-by: Addisu Z. Taddese --- ros_gz_sim/CMakeLists.txt | 10 +++- ros_gz_sim/package.xml | 3 ++ ros_gz_sim/test/test_create.cpp | 56 ++++++++++++++++++++++ ros_gz_sim/test/test_create_node.launch.py | 48 +++++++++++++++++++ 4 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 ros_gz_sim/test/test_create.cpp create mode 100644 ros_gz_sim/test/test_create_node.launch.py diff --git a/ros_gz_sim/CMakeLists.txt b/ros_gz_sim/CMakeLists.txt index 514587de..7f420e09 100644 --- a/ros_gz_sim/CMakeLists.txt +++ b/ros_gz_sim/CMakeLists.txt @@ -123,11 +123,15 @@ if(BUILD_TESTING) # GTest find_package(ament_cmake_gtest REQUIRED) + find_package(launch_testing_ament_cmake REQUIRED) ament_find_gtest() ament_add_gtest_executable(test_stopwatch test/test_stopwatch.cpp ) + ament_add_gtest_executable(test_create + test/test_create.cpp + ) ament_target_dependencies(test_stopwatch rclcpp) @@ -139,12 +143,16 @@ if(BUILD_TESTING) target_link_libraries(test_stopwatch ${PROJECT_NAME} ) + target_link_libraries(test_create + ${GZ_TARGET_PREFIX}-transport${GZ_TRANSPORT_VER}::core + ) install( - TARGETS test_stopwatch + TARGETS test_stopwatch test_create DESTINATION lib/${PROJECT_NAME} ) ament_add_gtest_test(test_stopwatch) + add_launch_test(test/test_create_node.launch.py TIMEOUT 200) endif() ament_package() diff --git a/ros_gz_sim/package.xml b/ros_gz_sim/package.xml index bec29ff7..a4cad906 100644 --- a/ros_gz_sim/package.xml +++ b/ros_gz_sim/package.xml @@ -33,6 +33,9 @@ ament_lint_auto ament_lint_common + launch_testing_ament_cmake + launch_ros + launch_testing ament_cmake diff --git a/ros_gz_sim/test/test_create.cpp b/ros_gz_sim/test/test_create.cpp new file mode 100644 index 00000000..ac537d44 --- /dev/null +++ b/ros_gz_sim/test/test_create.cpp @@ -0,0 +1,56 @@ +// Copyright 2023 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + + +#include +#include +#include +#include +#include +#include +#include +#include + +// Simple application that provides a `/create` service and prints out the +// sdf_filename of the request. This works in conjection with +// test_create_node.launch.py +int main() +{ + std::mutex m; + std::condition_variable cv; + bool test_complete = false; + + gz::transport::Node node; + auto cb = std::function([&]( + const gz::msgs::EntityFactory &_req, + gz::msgs::Boolean &_res) -> bool { + + std::cout << _req.sdf_filename() << std::endl; + _res.set_data(true); + + { + std::lock_guard lk(m); + test_complete = true; + } + cv.notify_one(); + return true; + }); + + node.Advertise("/world/default/create", cb); + // wait until we receive a message. + std::unique_lock lk(m); + cv.wait(lk, [&]{return test_complete;}); + // Sleep so that the service response can be sent before exiting. + std::this_thread::sleep_for(std::chrono::seconds(1)); +} diff --git a/ros_gz_sim/test/test_create_node.launch.py b/ros_gz_sim/test/test_create_node.launch.py new file mode 100644 index 00000000..ebffcfe9 --- /dev/null +++ b/ros_gz_sim/test/test_create_node.launch.py @@ -0,0 +1,48 @@ +# Copyright 2023 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os +import sys +import unittest + +from launch import LaunchDescription + +from launch_ros.actions import Node + +import launch_testing + + +def generate_test_description(): + expected_file_name = 'nonexistent/long/file_name' + create = Node(package='ros_gz_sim', executable='create', arguments=['-world', + 'default', '-file', expected_file_name], output='screen') + test_create = Node(package='ros_gz_sim', executable='test_create', output='screen') + return LaunchDescription([ + create, + test_create, + launch_testing.util.KeepAliveProc(), + launch_testing.actions.ReadyToTest(), + ]), locals() + +class WaitForTests(unittest.TestCase): + + def test_termination(self, test_create, proc_info): + proc_info.assertWaitForShutdown(process=test_create, timeout=200) + +@launch_testing.post_shutdown_test() +class CreateTest(unittest.TestCase): + + def test_output(self, expected_file_name, test_create, proc_output): + launch_testing.asserts.assertInStdout(proc_output, expected_file_name, test_create) + From 4f47fccbbec978f4446077d395076b1805eedb83 Mon Sep 17 00:00:00 2001 From: "Addisu Z. Taddese" Date: Wed, 20 Dec 2023 15:14:17 -0600 Subject: [PATCH 3/4] Use sprintf instead of strcpy Signed-off-by: Addisu Z. Taddese --- ros_gz_sim/src/create.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ros_gz_sim/src/create.cpp b/ros_gz_sim/src/create.cpp index c58e9c4f..ab504f20 100644 --- a/ros_gz_sim/src/create.cpp +++ b/ros_gz_sim/src/create.cpp @@ -19,7 +19,6 @@ #include #include -#include #include #include @@ -60,7 +59,7 @@ int main(int _argc, char ** _argv) char ** filtered_argv = new char *[(filtered_argc + 1)]; for (int ii = 0; ii < filtered_argc; ++ii) { filtered_argv[ii] = new char[filtered_arguments[ii].size() + 1]; - std::strcpy(filtered_argv[ii], filtered_arguments[ii].c_str()); + snprintf(filtered_argv[ii], filtered_arguments[ii].size() + 1, "%s", filtered_arguments[ii].c_str()); } filtered_argv[filtered_argc] = nullptr; From 597b32ffc002af58466d09b2e4ea033574dbb929 Mon Sep 17 00:00:00 2001 From: "Addisu Z. Taddese" Date: Wed, 20 Dec 2023 16:39:39 -0600 Subject: [PATCH 4/4] Fix linters Signed-off-by: Addisu Z. Taddese --- ros_gz_sim/src/create.cpp | 4 ++- ros_gz_sim/test/test_create.cpp | 32 +++++++++++----------- ros_gz_sim/test/test_create_node.launch.py | 11 ++++---- 3 files changed, 24 insertions(+), 23 deletions(-) diff --git a/ros_gz_sim/src/create.cpp b/ros_gz_sim/src/create.cpp index ab504f20..3fa5bbcc 100644 --- a/ros_gz_sim/src/create.cpp +++ b/ros_gz_sim/src/create.cpp @@ -59,7 +59,9 @@ int main(int _argc, char ** _argv) char ** filtered_argv = new char *[(filtered_argc + 1)]; for (int ii = 0; ii < filtered_argc; ++ii) { filtered_argv[ii] = new char[filtered_arguments[ii].size() + 1]; - snprintf(filtered_argv[ii], filtered_arguments[ii].size() + 1, "%s", filtered_arguments[ii].c_str()); + snprintf( + filtered_argv[ii], + filtered_arguments[ii].size() + 1, "%s", filtered_arguments[ii].c_str()); } filtered_argv[filtered_argc] = nullptr; diff --git a/ros_gz_sim/test/test_create.cpp b/ros_gz_sim/test/test_create.cpp index ac537d44..ee92bf39 100644 --- a/ros_gz_sim/test/test_create.cpp +++ b/ros_gz_sim/test/test_create.cpp @@ -13,13 +13,13 @@ // limitations under the License. +#include +#include #include #include #include #include #include -#include -#include #include // Simple application that provides a `/create` service and prints out the @@ -32,25 +32,25 @@ int main() bool test_complete = false; gz::transport::Node node; - auto cb = std::function([&]( - const gz::msgs::EntityFactory &_req, - gz::msgs::Boolean &_res) -> bool { - - std::cout << _req.sdf_filename() << std::endl; - _res.set_data(true); + auto cb = std::function( + [&]( + const gz::msgs::EntityFactory & _req, + gz::msgs::Boolean & _res) -> bool { + std::cout << _req.sdf_filename() << std::endl; + _res.set_data(true); - { - std::lock_guard lk(m); - test_complete = true; - } - cv.notify_one(); - return true; - }); + { + std::lock_guard lk(m); + test_complete = true; + } + cv.notify_one(); + return true; + }); node.Advertise("/world/default/create", cb); // wait until we receive a message. std::unique_lock lk(m); - cv.wait(lk, [&]{return test_complete;}); + cv.wait(lk, [&] {return test_complete;}); // Sleep so that the service response can be sent before exiting. std::this_thread::sleep_for(std::chrono::seconds(1)); } diff --git a/ros_gz_sim/test/test_create_node.launch.py b/ros_gz_sim/test/test_create_node.launch.py index ebffcfe9..14710f58 100644 --- a/ros_gz_sim/test/test_create_node.launch.py +++ b/ros_gz_sim/test/test_create_node.launch.py @@ -12,8 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import os -import sys import unittest from launch import LaunchDescription @@ -24,9 +22,9 @@ def generate_test_description(): - expected_file_name = 'nonexistent/long/file_name' - create = Node(package='ros_gz_sim', executable='create', arguments=['-world', - 'default', '-file', expected_file_name], output='screen') + expected_file_name = 'nonexistent/long/file_name' + create = Node(package='ros_gz_sim', executable='create', + arguments=['-world', 'default', '-file', expected_file_name], output='screen') test_create = Node(package='ros_gz_sim', executable='test_create', output='screen') return LaunchDescription([ create, @@ -35,14 +33,15 @@ def generate_test_description(): launch_testing.actions.ReadyToTest(), ]), locals() + class WaitForTests(unittest.TestCase): def test_termination(self, test_create, proc_info): proc_info.assertWaitForShutdown(process=test_create, timeout=200) + @launch_testing.post_shutdown_test() class CreateTest(unittest.TestCase): def test_output(self, expected_file_name, test_create, proc_output): launch_testing.asserts.assertInStdout(proc_output, expected_file_name, test_create) -