Add tests for user-defined signal handler (#215)

* Add tests for user-defined signal handler

* Skip signal handler tests on Windows

launch_testing will terminate the process instead of sending SIGINT, so the tests can't check the response to interrupt

* Fixup

* Remove argument parsing

test_rclcpp/CMakeLists.txt

@@ -73,7 +73,7 @@ if(BUILD_TESTING)
       "rclcpp")
   endfunction()
 
-  macro(custom_launch_test test_name executable1 executable2)
+  macro(custom_launch_test_two_executables test_name executable1 executable2)
     set(TEST_NAME "${test_name}")
     set(TEST_EXECUTABLE1 "$<TARGET_FILE:${executable1}>")
     set(TEST_EXECUTABLE1_NAME "${executable1}")
@@ -101,6 +101,39 @@ if(BUILD_TESTING)
     )
   endmacro()
 
+  # Macro for tests that trigger the shutdown of an executable based on particular console output,
+  # then check the output of the executable against different console output.
+  macro(custom_launch_test_executable_output test_name executable)
+
+    set(TEST_NAME "${test_name}")
+    set(TEST_EXECUTABLE "$<TARGET_FILE:${executable}>")
+    set(TEST_EXECUTABLE_NAME "${executable}")
+    set(TEST_EXECUTABLE_TRIGGER_SHUTDOWN_OUTPUT
+      "${CMAKE_CURRENT_SOURCE_DIR}/test/${test_name}__trigger_shutdown")
+    set(TEST_EXECUTABLE_EXPECTED_OUTPUT
+      "${CMAKE_CURRENT_SOURCE_DIR}/test/${test_name}__expected_output")
+    configure_file(
+      test/test_executable_output.py.in
+      ${test_name}${target_suffix}.py.configure
+      @ONLY
+    )
+    file(GENERATE
+      OUTPUT "${test_name}${target_suffix}_$<CONFIG>.py"
+      INPUT "${CMAKE_CURRENT_BINARY_DIR}/${test_name}${target_suffix}.py.configure"
+    )
+    ament_add_nose_test(${test_name}${target_suffix}
+      "${CMAKE_CURRENT_BINARY_DIR}/${test_name}${target_suffix}_$<CONFIG>.py"
+      ${ARGN}
+      APPEND_LIBRARY_DIRS "${append_library_dirs}"
+      ENV
+      RCL_ASSERT_RMW_ID_MATCHES=${rmw_implementation}
+      RMW_IMPLEMENTATION=${rmw_implementation}
+    )
+    set_tests_properties(${test_name}${target_suffix}
+      PROPERTIES DEPENDS "${executable}${target_suffix}"
+    )
+  endmacro()
+
   macro(targets)
     custom_gtest(gtest_publisher
       "test/test_publisher.cpp"
@@ -146,22 +179,42 @@ if(BUILD_TESTING)
       TIMEOUT 30)
 
     # Parameter tests single implementation
-    custom_launch_test(test_parameter_server_cpp
+    custom_launch_test_two_executables(test_parameter_server_cpp
       test_parameters_server_cpp test_remote_parameters_cpp
       TIMEOUT 15)
 
     # Service tests single implementation
-    custom_launch_test(test_services_cpp
+    custom_launch_test_two_executables(test_services_cpp
       test_services_server_cpp test_services_client_cpp
       TIMEOUT 60)
 
-    custom_launch_test(test_client_scope_cpp
+    custom_launch_test_two_executables(test_client_scope_cpp
       test_client_scope_server_cpp test_client_scope_client_cpp
       TIMEOUT 30)
 
-    custom_launch_test(test_client_scope_consistency_cpp
+    custom_launch_test_two_executables(test_client_scope_consistency_cpp
       test_client_scope_consistency_server_cpp test_client_scope_consistency_client_cpp
       TIMEOUT 30)
+
+    # Note (dhood): signal handler tests will be skipped on Windows because there is no opportunity
+    # for signal handling once shutdown is triggered by launch_testing.
+    set(SKIP_TEST "")
+    if(WIN32)
+      set(SKIP_TEST "SKIP_TEST")
+    endif()
+    # Test that a user-defined signal handler is called on interrupt:
+    # after rclcpp::init has been called, but before rclcpp::shutdown has been called.
+    custom_launch_test_executable_output(test_signal_handler_before_shutdown
+      test_signal_handler
+      TIMEOUT 30
+      ${SKIP_TEST})
+
+    # Test that a user-defined signal handler is restored after rclcpp::init and rclcpp::shutdown
+    # have been called.
+    custom_launch_test_executable_output(test_signal_handler_after_shutdown
+      test_signal_handler
+      TIMEOUT 30
+      ${SKIP_TEST})
   endmacro()
 
   set(append_library_dirs "${CMAKE_CURRENT_BINARY_DIR}")
@@ -171,6 +224,8 @@ if(BUILD_TESTING)
 
   call_for_each_rmw_implementation(targets)
 
+  custom_executable(test_signal_handler "test/test_signal_handler.cpp")
+
   # Parameter tests single implementation
   custom_executable(test_parameters_server_cpp "test/test_parameters_server.cpp")
   custom_executable(test_remote_parameters_cpp "test/test_remote_parameters.cpp")

test_rclcpp/package.xml

@@ -17,6 +17,7 @@
   <test_depend>ament_lint_auto</test_depend>
   <test_depend>ament_lint_common</test_depend>
   <test_depend>launch</test_depend>
+  <test_depend>launch_testing</test_depend>
   <test_depend>rclcpp</test_depend>
   <test_depend>rmw_implementation</test_depend>
   <test_depend>rmw_implementation_cmake</test_depend>

test_rclcpp/test/test_executable_output.py.in

@@ -0,0 +1,52 @@
+# generated from test_rclcpp/test/test_executable_output.py.in
+# generated code does not contain a copyright notice
+
+from launch import LaunchDescriptor
+from launch.exit_handler import primary_exit_handler
+from launch.launcher import DefaultLauncher
+from launch.output_handler import ConsoleOutput
+
+from launch_testing import create_handler
+
+
+def @TEST_NAME@():
+    ld = LaunchDescriptor()
+
+    output_handlers = [ConsoleOutput()]
+
+    shutdown_trigger_handler = create_handler(
+        '@TEST_EXECUTABLE_NAME@',
+        ld,
+        '@TEST_EXECUTABLE_TRIGGER_SHUTDOWN_OUTPUT@',
+        exit_on_match=True,
+        filtered_rmw_implementation='@rmw_implementation@',
+    )
+    output_handlers.append(shutdown_trigger_handler)
+
+    output_check_handler = create_handler(
+        '@TEST_EXECUTABLE_NAME@',
+        ld,
+        '@TEST_EXECUTABLE_EXPECTED_OUTPUT@',
+        exit_on_match=False,
+        filtered_rmw_implementation='@rmw_implementation@',
+    )
+    output_handlers.append(output_check_handler)
+
+    ld.add_process(
+        cmd=['@TEST_EXECUTABLE@'],
+        name='@TEST_EXECUTABLE_NAME@',
+        exit_handler=primary_exit_handler,
+        output_handlers=output_handlers,
+    )
+
+    launcher = DefaultLauncher()
+    launcher.add_launch_descriptor(ld)
+    rc = launcher.launch()
+
+    assert rc == 0, "The launch file failed with exit code '" + str(rc) + "'. "
+
+    output_check_handler.check()
+
+
+if __name__ == '__main__':
+    @TEST_NAME@()

test_rclcpp/test/test_signal_handler.cpp

@@ -0,0 +1,54 @@
+// Copyright 2017 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 <iostream>
+#include <memory>
+
+#include "rclcpp/rclcpp.hpp"
+
+using namespace std::chrono_literals;
+
+void sigintHandler(int sig)
+{
+  (void)sig;
+  printf("Custom sigint handler called.\n");
+}
+
+int main(int argc, char ** argv)
+{
+  // This exectuable is used in combination with a script that interrupts it at different times.
+  // Its purpose is to test that a user-defined signal handler gets called even when the rclcpp
+  // signal handler is/has been in use.
+  setvbuf(stdout, NULL, _IONBF, BUFSIZ);
+
+  // Override default sigint handler
+  signal(SIGINT, sigintHandler);
+  printf("Registered custom signal handler.\n");
+
+  rclcpp::init(argc, argv);
+  printf("Called rclcpp::init.\n");
+
+  printf("Waiting to give an opportunity for interrupt...\n");
+  std::this_thread::sleep_for(5s);
+
+  printf("Calling rclcpp::shutdown...\n");
+  rclcpp::shutdown();
+  printf("Called rclcpp::shutdown.\n");
+
+  printf("Waiting to give an opportunity for interrupt...\n");
+  std::this_thread::sleep_for(5s);
+
+  printf("Exiting.\n");
+  return 0;
+}

test_rclcpp/test/test_signal_handler_after_shutdown__expected_output.txt

@@ -0,0 +1,8 @@
+Registered custom signal handler.
+Called rclcpp::init.
+Waiting to give an opportunity for interrupt...
+Calling rclcpp::shutdown...
+Called rclcpp::shutdown.
+Waiting to give an opportunity for interrupt...
+Custom sigint handler called.
+Exiting.

test_rclcpp/test/test_signal_handler_after_shutdown__trigger_shutdown.txt

@@ -0,0 +1,6 @@
+Registered custom signal handler.
+Called rclcpp::init.
+Waiting to give an opportunity for interrupt...
+Calling rclcpp::shutdown...
+Called rclcpp::shutdown.
+Waiting to give an opportunity for interrupt...

test_rclcpp/test/test_signal_handler_before_shutdown__expected_output.txt

@@ -0,0 +1,9 @@
+Registered custom signal handler.
+Called rclcpp::init.
+Waiting to give an opportunity for interrupt...
+signal_handler(2)
+Custom sigint handler called.
+Calling rclcpp::shutdown...
+Called rclcpp::shutdown.
+Waiting to give an opportunity for interrupt...
+Exiting.

test_rclcpp/test/test_signal_handler_before_shutdown__trigger_shutdown.txt

@@ -0,0 +1,3 @@
+Registered custom signal handler.
+Called rclcpp::init.
+Waiting to give an opportunity for interrupt...