use verbatim on test target to fix testing with ninja

[mikaelarguedas]

TL;DR VERBATIM is needed for the run_tests* target to work with ninja

More details:

https://answers.ros.org/question/288290/catkin_make_isolated-run_tests/ unveiled that the run_tests target is not able to run_tests when using ninja as the build system.

The issue comes from the backslashes appended to the commands in the build files.

How to reproduce the issue:

source /opt/ros/<ROSDISTRO>/setup.bash
mkdir -p /tmp/run_tests_ws/src && /tmp/run_tests_ws/src
git clone https://github.com/gocarlos/ros_unit_tests_example
cd /tmp/run_tests_ws

Build and run with ninja:

rm -r build_isolated/ devel_isolated/ install_isolated/; \
  CTEST_OUTPUT_ON_FAILURE=1  catkin_make_isolated --use-ninja --install && \
  CTEST_OUTPUT_ON_FAILURE=1  catkin_make_isolated --use-ninja --install --catkin-make-args run_tests

error message: [/tmp/run_tests_ws/src/ros_unit_tests_example/launch/node_launcher.launch\] is not a launch file name

Build and run with make:

rm -r build_isolated/ devel_isolated/ install_isolated/; \
  CTEST_OUTPUT_ON_FAILURE=1  catkin_make_isolated --install && \
  CTEST_OUTPUT_ON_FAILURE=1  catkin_make_isolated --install --catkin-make-args run_tests

Test is ran successfully.

With this patch

Apply this patch and retry build and run with ninja and Make. The test is ran successfully in both cases.

This comes form inside of one test case


... logging to /root/.ros/log/rostest-f6506ac887d3-14588.log
[ROSUNIT] Outputting test results to /root/test_ws/build_isolated/ros_unit_tests_example/test_results/ros_unit_tests_example/rostest-launch_node_launcher.xml
[Testcase: testmy_unit_test] ... ok

[ROSTEST]-----------------------------------------------------------------------

[ros_unit_tests_example.rosunit-my_unit_test/test_case_1][passed]

SUMMARY
 * RESULT: SUCCESS
 * TESTS: 1
 * ERRORS: 0
 * FAILURES: 0

[gaschler]

Thanks for preparing this fix.
As noted in cartographer-project/cartographer_ros#813, I additionally needed to add a third VERBATIM

# hidden test target which depends on building all tests and cleaning test results
  add_custom_target(_run_tests_${PROJECT_NAME}_${type}_${name}
    COMMAND ${cmd}
    VERBATIM)

(I use Ros Lunar with Catkin 0.7.11 with Ninja 1.8.2, using catkin_make_isolated --use-ninja --install.)

[mikaelarguedas]

Thanks @gaschler for testing this. I added the additional verbatim.

@sloretz @dirk-thomas How do you feel about adding VERBATIM to the custom targets ? Is there a downside to doing so? Should I add it to all custom test targets or only the ones that proved to be breaking testing with ninja in cartographer-project/cartographer_ros#813?

Thanks!

[dirk-thomas]

The patch looks fine to me. But CI is currently failing. Can you please look into it. Happy to merge it once it is green.

[mikaelarguedas]

CI fails on trying to get the branch from this repository.
My guess is that I may have pushed the branch on this repo and not my fork when experimenting and travis picked it up. I think that the failure can be safely ignored and travis on the pr and all jobs on the ROS buildfarm passed.
Otherwise I can push the same branch on ros/catkin if you prefer

[dirk-thomas]

Ok, after the merge the target branch will trigger a CI build anyway to catch potential problems.