naturerobots / mesh_tools

Tools and Messages for Annotated 3D Triangle Meshes in ROS
https://wiki.ros.org/mesh_tools
BSD 3-Clause "New" or "Revised" License
229 stars 42 forks source link

remove linter tests for generated msg code #31

Closed Cakem1x closed 7 months ago

Cakem1x commented 7 months ago

Hi, I just ran tests in my workspace that included mesh_tools. The line I remove with this PR adds a bunch of tests for linting. In principle, this sounds nice. However, one of these test fail at the moment, because the code generated by the rosidl generator does not abide the code style:

test 15
      Start 15: uncrustify_rosidl_generated_cpp

15: Test command: /usr/bin/python3.10 "-u" "/opt/ros/humble/share/ament_cmake_test/cmake/run_test.py" "/home/docker/ros2_ws/build/mesh_msgs/test_results/mesh_msgs/uncrustify_rosidl_generated_cpp.xunit.xml" "--package-name" "mesh_msgs" "--output-file" "/home/docker/ros2_ws/build/mesh_msgs/ament_uncrustify/uncrustify_rosidl_generated_cpp.txt" "--command" "/opt/ros/humble/bin/ament_uncrustify" "--xunit-file" "/home/docker/ros2_ws/build/mesh_msgs/test_results/mesh_msgs/uncrustify_rosidl_generated_cpp.xunit.xml" "--linelength" "0" "/home/docker/ros2_ws/build/mesh_msgs/rosidl_generator_cpp/mesh_msgs"
15: Test timeout computed to be: 60
15: -- run_test.py: invoking following command in '/home/docker/ros2_ws/src/mesh_tools/mesh_msgs':
15:  - /opt/ros/humble/bin/ament_uncrustify --xunit-file /home/docker/ros2_ws/build/mesh_msgs/test_results/mesh_msgs/uncrustify_rosidl_generated_cpp.xunit.xml --linelength 0 /home/docker/ros2_ws/build/mesh_msgs/rosidl_generator_cpp/mesh_msgs
15: Code style divergence in file '/home/docker/ros2_ws/build/mesh_msgs/rosidl_generator_cpp/mesh_msgs/msg/detail/mesh_face_cluster_stamped__builder.hpp':
15:
15: --- /home/docker/ros2_ws/build/mesh_msgs/rosidl_generator_cpp/mesh_msgs/msg/detail/mesh_face_cluster_stamped__builder.hpp
15: +++ /home/docker/ros2_ws/build/mesh_msgs/rosidl_generator_cpp/mesh_msgs/msg/detail/mesh_face_cluster_stamped__builder.hpp.uncrustify
15: @@ -30 +30 @@
15: -  ::mesh_msgs::msg::MeshFaceClusterStamped override(::mesh_msgs::msg::MeshFaceClusterStamped::_override_type arg)
15: +  ::mesh_msgs::msg::MeshFaceClusterStamped override (::mesh_msgs::msg::MeshFaceClusterStamped::_override_type arg)

@aock, did you add the ADD_LINTER_TESTSduring the ROS2 migration intentionally? I couldn't find out what exactly this line is supposed to be used for. Should we file a bug report? I don't think that the failing test is about something in the mesh_msgs pkg. Or is it?

Cakem1x commented 7 months ago

If we find a sensible way to make these tests work again, that would also be great. Probably better than removing them!

amock commented 7 months ago

@Cakem1x No I don't think I was adding them intentionally. I think it was one line I just copy-pasted from somewhere. But I agree we should keep them if possible.