search

ros-controls / ros_control

Generic and simple controls framework for ROS
http://wiki.ros.org/ros_control
BSD 3-Clause "New" or "Revised" License
470 stars 307 forks source link

[Noetic] Rework and re-enable spawning and switching CLI tests #448

Closed bmagyar closed 4 years ago

bmagyar commented 4 years ago

With the preparations for Noetic, the controller manager CLI tests became flaky. These tests were not compatible with Python3 in the first place and a very rudimentary porting took place.

Please 1) Rework the test in a more modern Python fashion. 2) Re-enable the commented out section checking for spawning and switching controllers.

Related PR where I disabled these tests: https://github.com/ros-controls/ros_control/pull/446

olivier-stasse commented 4 years ago

Dear @bmagyar, I gave it a try to this issue.

The test alone (all the other tests disables) works without python3 mistake. When cm_test AND cm_msgs_utils_rostest.test are enabled, my_controller3 is loaded before my_controller1 and then the resulting string is different from the expected output and makes the test failed.

Should we enforce the fact that my_controller1 has to be loaded before my_controller3 ? If this is not the case, then we should test both output possibilities I guess.

Wrt to new python3 do you suggest to use mock ?

olivier-stasse commented 4 years ago

PR #462 provides a way of testing both output. It does not implements a more modern Python test but is python3 compliant.

bmagyar commented 4 years ago

Thank you for this, I think we can live with the quality of that Python code.