use rc 0 when CTest has failing tests

[dirk-thomas]

While I couldn't find any documentation about it CTest returns 8 when tests are failing. Since tests failures should not be considered an "error" in terms of invoking the test verb that case is handled differently. This unifies the return value semantic across other types.

Before the failing bridge test made the build fail: https://ci.ros2.org/view/colcon/job/colcon_ci_packaging_linux-aarch64/5/
After the failing bridge test make the build only unstable: https://ci.ros2.org/view/colcon/job/colcon_ci_packaging_linux-aarch64/7/

[mikaelarguedas]

For future reference: there is no documentation about ctest return codes anywhere. This is ticketed at https://gitlab.kitware.com/cmake/cmake/issues/16151

[mikaelarguedas]

I remember (couldnt find the discussion so far) user requests in the past to have the option of making builds fail on test failure. Can this override of the return code be made optional so that this can be toggled when invoking colcon test?

[dirk-thomas]

I remember (couldnt find the discussion so far) user requests in the past to have the option of making builds fail on test failure. Can this override of the return code be made optional so that this can be toggled when invoking colcon test?

I guess you are remember this discussion: ros-infrastructure/ros_buildfarm#367 (comment)

At the time we settled on using the test result verb to get an error code for failing tests.

[mikaelarguedas]

Thanks for the pointer, yeah I think that;s the discussion I was thinking of.

In this case "colcon test" isn't able to perform the build or install steps, is that right ?
So having an opt-in option on the "test" verb to return an error code on test failure may make more sense here as there is less ambiguity about what failed

[dirk-thomas]

In this case "colcon test" isn't able to perform the build or install steps, is that right ?

Correct, build builds and installs, test only runs the tests.

So having an opt-in option on the "test" verb to return an error code on test failure may make more sense here as there is less ambiguity about what failed

Sure, such an option can be added as a new feature.

I would keep that separate from this patch though. This patch addresses the problem which currently fails our CI builds and is therefore blocking the deployment of colcon on CI.

[mikaelarguedas]

Feature addition ticketed at colcon/colcon-core#21

[mikaelarguedas]

lgtm