Add test_utils file for transition tests

[christophfroehlich]

ros-controls/ros2_controllers#1682

[codecov]

Codecov Report

❌ Patch coverage is 93.10345% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.33%. Comparing base (64b0963) to head (dd6c9c1).

Files with missing lines	Patch %	Lines
controller_interface/test/test_test_utils.cpp	89.69%	7 Missing and 3 partials ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##           master    #3048    +/-   ##
========================================
  Coverage   89.32%   89.33%            
========================================
  Files         158      160     +2     
  Lines       19497    19642   +145     
  Branches     1584     1587     +3     
========================================
+ Hits        17416    17547   +131     
- Misses       1428     1440    +12     
- Partials      653      655     +2     

Flag	Coverage Δ
unittests	89.33% <93.10%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...erface/include/controller_interface/test_utils.hpp	100.00% <100.00%> (ø)
controller_interface/test/test_test_utils.cpp	89.69% <89.69%> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jsantoso91]

Looks good to me, I looked at the design document for the lifecycle found here
https://design.ros2.org/articles/node_lifecycle.html

There are some discrepancies (mentioned in the line comments) for deactivate and cleanup transitions, but since test cases written for these are passing I assume it is just the issue of outdated documentation.

[jsantoso91]

Is this comment accurate? Based on the diagram included in https://design.ros2.org/articles/node_lifecycle.html
Suppose error is raised during onShutdown() then state will go to ErrorProcessing and further to Unconfigured or Finalized depending on the success/failure of onError.

[christophfroehlich]

this caused some headache, but this was the result of playing around with the tests.

The diagram also shows no [SUCCESS] condition for the shutdown.

[jsantoso91]

I think you meant to say "The diagram also shows no [FAILURE] condition for the shutdown" ?

I think a more accurate comment for the default branch (at least based on the lifecycle diagram) would be
"if transition returns error, the state will end up in the Unconfigured or Finalized depending on the success/failure of onError"

[christophfroehlich]

In principle you are right, but the behavior is different as the design document. The tests in 7084fa1 fail with:

1: [ RUN      ] ControllerInterfaceTestUtils.shutdown_succeeds_returns_true_for_finalized_state
1: Controller shutdown_return in on_shutdown: 97
1: Controller state in shutdown_succeeds: 4
1: [       OK ] ControllerInterfaceTestUtils.shutdown_succeeds_returns_true_for_finalized_state (3 ms)
1: [ RUN      ] ControllerInterfaceTestUtils.shutdown_succeeds_returns_false_for_unconfigured_state
1: Controller shutdown_return in on_shutdown: 98
1: Controller state in shutdown_succeeds: 4
1: /workspaces/ros2_rolling_ws/src/ros2_control/controller_interface/test/test_test_utils.cpp:274: Failure
1: Value of: controller_interface::shutdown_succeeds(controller)
1: Expected: is equal to false
1:   Actual: true (of type bool)
1: 
1: [  FAILED  ] ControllerInterfaceTestUtils.shutdown_succeeds_returns_false_for_unconfigured_state (3 ms)
1: [ RUN      ] ControllerInterfaceTestUtils.shutdown_succeeds_throws_for_real_controller_on_error
1: [WARN] [1774721202.159787007] [pal_statistics]: Unable to register entity test_utils_controller_11.total_triggers in ros2_control, as the registry is not found. Try initializing it!
1: [WARN] [1774721202.159796485] [pal_statistics]: Unable to register entity test_utils_controller_11.failed_triggers in ros2_control, as the registry is not found. Try initializing it!
1: Controller shutdown_return in on_shutdown: 99
1: [WARN] [1774721202.159808337] [test_utils_controller_11]: Callback returned ERROR during the transition: shutdown
1: Controller state in shutdown_succeeds: 1
1: /workspaces/ros2_rolling_ws/src/ros2_control/controller_interface/test/test_test_utils.cpp:283: Failure
1: Expected: controller_interface::shutdown_succeeds(controller) throws an exception of type std::runtime_error.
1:   Actual: it throws nothing.
1: 
1: [  FAILED  ] ControllerInterfaceTestUtils.shutdown_succeeds_throws_for_real_controller_on_error (3 ms)

[christophfroehlich]

I updated the docstring according to the current behavior.
See also ros2/rclcpp#1763

[jsantoso91]

Hi @christophfroehlich, thank you for digging into this. After reading the comments for ros2/rclcpp#1763, I think we have come to the same agreement that shutdown will lead to the Finalized state for both SUCCESS and FAILURE.

However my previous comment:

I think a more accurate comment for the default branch (at least based on the lifecycle diagram) would be
"if transition returns error, the state will end up in the Unconfigured or Finalized depending on the success/failure of onError"

is meant for the default branch of the switch case (in this case an error is raised during shutdown).
The test case "shutdown_succeeds_throws_for_real_controller_on_error" you referenced above for 7084fa1
failed because no exception was thrown. My understanding this is because the state transitions to ErrorProcessing and then Unconfigured instead. Transition goes from ErrorProcessing to Unconfigured because on_error is not defined/overridden in this test (default returns true). Lastly, no exception is thrown for the test case because you added Unconfigured in the switch case as part of 7084fa1.

[ RUN ] ControllerInterfaceTestUtils.shutdown_succeeds_throws_for_real_controller_on_error
1: [WARN] [1774721202.159787007] [pal_statistics]: Unable to register entity test_utils_controller_11.total_triggers in ros2_control, as the registry is not found. Try initializing it!
1: [WARN] [1774721202.159796485] [pal_statistics]: Unable to register entity test_utils_controller_11.failed_triggers in ros2_control, as the registry is not found. Try initializing it!
1: Controller shutdown_return in on_shutdown: 99
1: [WARN] [1774721202.159808337] [test_utils_controller_11]: Callback returned ERROR during the transition: shutdown
1: Controller state in shutdown_succeeds: 1
1: /workspaces/ros2_rolling_ws/src/ros2_control/controller_interface/test/test_test_utils.cpp:283: Failure
1: Expected: controller_interface::shutdown_succeeds(controller) throws an exception of type std::runtime_error.
1: Actual: it throws nothing.
1:
1: [ FAILED ] ControllerInterfaceTestUtils.shutdown_succeeds_throws_for_real_controller_on_error (3 ms)

Apologize if this is not clear, but my suggestion is to modify the comment on line 160 of controller_interface/include/controller_interface/test_utils.hpp to be "if transition returns error, the state will end up in the Unconfigured or Finalized depending on the success/failure of onError".
dd6c9c1#diff-5f1b385cd1c0a79d7df4f0559ed4f548fc3f164cdd640911831b20de142f8e3fR160

Unless we have a different understanding?

[jsantoso91]

Suggested comment change for L157:

"if transition returns error, the state will end up in the Unconfigured or Finalized depending on the success/failure of onError"

Looks good otherwise.