search

ament / ament_lint

Apache License 2.0
37 stars 107 forks source link

[ament_cmake_clang_tidy] failed test is detected as passed #350

Open wmmc88 opened 2 years ago

wmmc88 commented 2 years ago

This is using the latest master of ament_lint and latest rolling release.

From https://github.com/uwrobotics/uwrt_mars_rover/runs/4834656922?check_suite_focus=true#step:4:1520 :

  test 2
      Start 2: clang_tidy

  2: Test command: /usr/bin/python3 "-u" "/opt/ros/galactic/share/ament_cmake_test/cmake/run_test.py" "/__w/uwrt_mars_rover/uwrt_mars_rover/ros_ws/build/uwrt_mars_rover_drivetrain_hw/test_results/uwrt_mars_rover_drivetrain_hw/clang_tidy.xunit.xml" "--package-name" "uwrt_mars_rover_drivetrain_hw" "--output-file" "/__w/uwrt_mars_rover/uwrt_mars_rover/ros_ws/build/uwrt_mars_rover_drivetrain_hw/ament_clang_tidy/clang_tidy.txt" "--command" "/__w/uwrt_mars_rover/uwrt_mars_rover/ros_ws/install/ament_clang_tidy/bin/ament_clang_tidy" "--xunit-file" "/__w/uwrt_mars_rover/uwrt_mars_rover/ros_ws/build/uwrt_mars_rover_drivetrain_hw/test_results/uwrt_mars_rover_drivetrain_hw/clang_tidy.xunit.xml" "/__w/uwrt_mars_rover/uwrt_mars_rover/ros_ws/build/uwrt_mars_rover_drivetrain_hw" "--config" "/__w/uwrt_mars_rover/uwrt_mars_rover/ros_ws/src/tsv4w6qdap/uwrt_mars_rover/uwrt_mars_rover_drivetrain/uwrt_mars_rover_drivetrain_hw/../../.clang-tidy"
  2: Test timeout computed to be: 300
  2: -- run_test.py: invoking following command in '/__w/uwrt_mars_rover/uwrt_mars_rover/ros_ws/src/tsv4w6qdap/uwrt_mars_rover/uwrt_mars_rover_drivetrain/uwrt_mars_rover_drivetrain_hw':
  2:  - /__w/uwrt_mars_rover/uwrt_mars_rover/ros_ws/install/ament_clang_tidy/bin/ament_clang_tidy --xunit-file /__w/uwrt_mars_rover/uwrt_mars_rover/ros_ws/build/uwrt_mars_rover_drivetrain_hw/test_results/uwrt_mars_rover_drivetrain_hw/clang_tidy.xunit.xml /__w/uwrt_mars_rover/uwrt_mars_rover/ros_ws/build/uwrt_mars_rover_drivetrain_hw --config /__w/uwrt_mars_rover/uwrt_mars_rover/ros_ws/src/tsv4w6qdap/uwrt_mars_rover/uwrt_mars_rover_drivetrain/uwrt_mars_rover_drivetrain_hw/../../.clang-tidy
  2: The invocation of "clang-tidy" failed with error code 4: Command '['/usr/bin/clang-tidy', '-p', '/__w/uwrt_mars_rover/uwrt_mars_rover/ros_ws/build/uwrt_mars_rover_drivetrain_hw', "--config={CheckOptions: [{key: readability-identifier-naming.ClassCase, value: CamelCase}, {key: readability-identifier-naming.EnumCase, value: CamelCase}, {key: readability-identifier-naming.UnionCase, value: CamelCase}, {key: readability-identifier-naming.FunctionCase, value: camelBack}, {key: readability-identifier-naming.VariableCase, value: lower_case}, {key: readability-identifier-naming.ClassMemberSuffix, value: _}, {key: readability-identifier-naming.GlobalVariableCase, value: UPPER_CASE}, {key: readability-identifier-naming.StaticConstantCase, value: UPPER_CASE}, {key: readability-identifier-naming.ConstexprVariableCase, value: UPPER_CASE}, {key: readability-identifier-naming.EnumConstantCase, value: UPPER_CASE}, {key: readability-inconsistent-declaration-parameter-name.Strict, value: 1}], Checks: '-*, boost-*, bugprone-*, cppcoreguidelines-*, misc-*, modernize-*, performance-*, readability-*, -cppcoreguidelines-pro-bounds-array-to-pointer-decay, -cppcoreguidelines-pro-bounds-constant-array-index, -cppcoreguidelines-pro-type-vararg, -cppcoreguidelines-pro-type-union-access, -cppcoreguidelines-non-private-member-variables-in-classes, -cppcoreguidelines-owning-memory, -misc-non-private-member-variables-in-classes, -modernize-use-trailing-return-type,', HeaderFilterRegex: .*/src/uwrt_mars_rover/.*, WarningsAsErrors: '*'}\n", '--header-filter', 'include/uwrt_mars_rover_drivetrain_hw/.*', '/__w/uwrt_mars_rover/uwrt_mars_rover/ros_ws/src/tsv4w6qdap/uwrt_mars_rover/uwrt_mars_rover_drivetrain/uwrt_mars_rover_drivetrain_hw/src/uwrt_mars_rover_drivetrain_hw_actuator_interface.cpp']' returned non-zero exit status 4.
  2: found compilation database for package "uwrt_mars_rover_drivetrain_hw"...
  2: 
  2: -- run_test.py: return code 0
  2: -- run_test.py: verify result file '/__w/uwrt_mars_rover/uwrt_mars_rover/ros_ws/build/uwrt_mars_rover_drivetrain_hw/test_results/uwrt_mars_rover_drivetrain_hw/clang_tidy.xunit.xml'
  2/5 Test #2: clang_tidy .......................   Passed   17.71 sec

Exit status 4 indicating 4 clang-tidy violation errors, yet its still reported as a passed test

When running locally using latest rolling binary via colcon test:

  1/5 Test #1: clang_format .....................   Passed    0.23 sec
test 2
    Start 2: clang_tidy

2: Test command: /usr/bin/python3.8 "-u" "/opt/ros/rolling/share/ament_cmake_test/cmake/run_test.py" "/home/wmmc88/git-repos/ros2_uwrt_ws/build/uwrt_mars_rover_drivetrain_hw/test_results/uwrt_mars_rover_drivetrain_hw/clang_tidy.xunit.xml" "--package-name" "uwrt_mars_rover_drivetrain_hw" "--output-file" "/home/wmmc88/git-repos/ros2_uwrt_ws/build/uwrt_mars_rover_drivetrain_hw/ament_clang_tidy/clang_tidy.txt" "--command" "/opt/ros/rolling/bin/ament_clang_tidy" "--xunit-file" "/home/wmmc88/git-repos/ros2_uwrt_ws/build/uwrt_mars_rover_drivetrain_hw/test_results/uwrt_mars_rover_drivetrain_hw/clang_tidy.xunit.xml" "/home/wmmc88/git-repos/ros2_uwrt_ws/build/uwrt_mars_rover_drivetrain_hw" "--config" "/home/wmmc88/git-repos/ros2_uwrt_ws/src/uwrt_mars_rover/uwrt_mars_rover_drivetrain/uwrt_mars_rover_drivetrain_hw/../../.clang-tidy"
2: Test timeout computed to be: 300
2: -- run_test.py: invoking following command in '/home/wmmc88/git-repos/ros2_uwrt_ws/src/uwrt_mars_rover/uwrt_mars_rover_drivetrain/uwrt_mars_rover_drivetrain_hw':
2:  - /opt/ros/rolling/bin/ament_clang_tidy --xunit-file /home/wmmc88/git-repos/ros2_uwrt_ws/build/uwrt_mars_rover_drivetrain_hw/test_results/uwrt_mars_rover_drivetrain_hw/clang_tidy.xunit.xml /home/wmmc88/git-repos/ros2_uwrt_ws/build/uwrt_mars_rover_drivetrain_hw --config /home/wmmc88/git-repos/ros2_uwrt_ws/src/uwrt_mars_rover/uwrt_mars_rover_drivetrain/uwrt_mars_rover_drivetrain_hw/../../.clang-tidy
2: The invocation of "clang-tidy" failed with error code 4: Command '['/usr/bin/clang-tidy', '-p', '/home/wmmc88/git-repos/ros2_uwrt_ws/build/uwrt_mars_rover_drivetrain_hw', "--config={CheckOptions: [{key: readability-identifier-naming.ClassCase, value: CamelCase}, {key: readability-identifier-naming.EnumCase, value: CamelCase}, {key: readability-identifier-naming.UnionCase, value: CamelCase}, {key: readability-identifier-naming.FunctionCase, value: camelBack}, {key: readability-identifier-naming.VariableCase, value: lower_case}, {key: readability-identifier-naming.ClassMemberSuffix, value: _}, {key: readability-identifier-naming.GlobalVariableCase, value: UPPER_CASE}, {key: readability-identifier-naming.StaticConstantCase, value: UPPER_CASE}, {key: readability-identifier-naming.ConstexprVariableCase, value: UPPER_CASE}, {key: readability-identifier-naming.EnumConstantCase, value: UPPER_CASE}, {key: readability-inconsistent-declaration-parameter-name.Strict, value: 1}], Checks: '-*, boost-*, bugprone-*, cppcoreguidelines-*, misc-*, modernize-*, performance-*, readability-*, -cppcoreguidelines-pro-bounds-array-to-pointer-decay, -cppcoreguidelines-pro-bounds-constant-array-index, -cppcoreguidelines-pro-type-vararg, -cppcoreguidelines-pro-type-union-access, -cppcoreguidelines-non-private-member-variables-in-classes, -cppcoreguidelines-owning-memory, -misc-non-private-member-variables-in-classes, -modernize-use-trailing-return-type,', HeaderFilterRegex: .*/src/uwrt_mars_rover/.*, WarningsAsErrors: '*'}\n", '--header-filter', 'include/uwrt_mars_rover_drivetrain_hw/.*', '/home/wmmc88/git-repos/ros2_uwrt_ws/src/uwrt_mars_rover/uwrt_mars_rover_drivetrain/uwrt_mars_rover_drivetrain_hw/src/uwrt_mars_rover_drivetrain_hw_actuator_interface.cpp']' returned non-zero exit status 4.
2: found compilation database for package "uwrt_mars_rover_drivetrain_hw"...
2: 
2: -- run_test.py: return code 0
2: -- run_test.py: verify result file '/home/wmmc88/git-repos/ros2_uwrt_ws/build/uwrt_mars_rover_drivetrain_hw/test_results/uwrt_mars_rover_drivetrain_hw/clang_tidy.xunit.xml'
2/5 Test #2: clang_tidy .......................   Passed   11.15 sec

^ same result as CI running it using latest ament_lint master from source

When running locally using latest rolling binary via ament_clang_tidy:

❯ ament_clang_tidy
found compilation database for package "build"...
The invocation of "clang-tidy" failed with error code 1: Command '['/usr/bin/clang-tidy', '-p', 'build', '--header-filter', 'include/build/.*', '/home/wmmc88/git-repos/ros2_uwrt_ws/src/uwrt_mars_rover/uwrt_mars_rover_drivetrain/uwrt_mars_rover_drivetrain_hw/src/uwrt_mars_rover_drivetrain_hw_actuator_interface.cpp']' returned non-zero exit status 1.
found compilation database for package "uwrt_mars_rover_drivetrain_hw"...
The invocation of "clang-tidy" failed with error code 4: Command '['/usr/bin/clang-tidy', '-p', 'build/uwrt_mars_rover_drivetrain_hw', '--header-filter', 'include/uwrt_mars_rover_drivetrain_hw/.*', '/home/wmmc88/git-repos/ros2_uwrt_ws/src/uwrt_mars_rover/uwrt_mars_rover_drivetrain/uwrt_mars_rover_drivetrain_hw/src/uwrt_mars_rover_drivetrain_hw_actuator_interface.cpp']' returned non-zero exit status 4.

Prints that theres an error, but echo $? still reports 0. Also doesnt print what the errors actually are (and there doesnt seem to be any verbosity option in ament_clang_tidy)

Manually invoking the printed clang-tidy commands:

❯ /usr/bin/clang-tidy -p build --header-filter include/build/.* /home/wmmc88/git-repos/ros2_uwrt_ws/src/uwrt_mars_rover/uwrt_mars_rover_drivetrain/uwrt_mars_rover_drivetrain_hw/src/uwrt_mars_rover_drivetrain_hw_actuator_interface.cpp
31643 warnings generated.
/home/wmmc88/git-repos/ros2_uwrt_ws/src/uwrt_mars_rover/uwrt_mars_rover_drivetrain/uwrt_mars_rover_drivetrain_hw/src/uwrt_mars_rover_drivetrain_hw_actuator_interface.cpp:4:1: error: constructor does not initialize these fields: actuator_state_position_, actuator_state_velocity_, actuator_state_iq_current_, joint_velocity_command_ [cppcoreguidelines-pro-type-member-init,-warnings-as-errors]
UWRTMarsRoverDrivetrainHWActuatorInterface::UWRTMarsRoverDrivetrainHWActuatorInterface()
^
Suppressed 31650 warnings (31642 in non-user code, 8 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
1 warning treated as error
❯ /usr/bin/clang-tidy -p build/uwrt_mars_rover_drivetrain_hw --header-filter include/uwrt_mars_rover_drivetrain_hw/.* /home/wmmc88/git-repos/ros2_uwrt_ws/src/uwrt_mars_rover/uwrt_mars_rover_drivetrain/uwrt_mars_rover_drivetrain_hw/src/uwrt_mars_rover_drivetrain_hw_actuator_interface.cpp
31643 warnings generated.
/home/wmmc88/git-repos/ros2_uwrt_ws/src/uwrt_mars_rover/uwrt_mars_rover_drivetrain/uwrt_mars_rover_drivetrain_hw/include/uwrt_mars_rover_drivetrain_hw/visibility_control.hpp:23:9: error: macro 'UWRT_MARS_ROVER_DRIVETRAIN_HW_EXPORT' used to declare a constant; consider using a 'constexpr' constant [cppcoreguidelines-macro-usage,-warnings-as-errors]
#define UWRT_MARS_ROVER_DRIVETRAIN_HW_EXPORT __attribute__((visibility("default")))
        ^
/home/wmmc88/git-repos/ros2_uwrt_ws/src/uwrt_mars_rover/uwrt_mars_rover_drivetrain/uwrt_mars_rover_drivetrain_hw/include/uwrt_mars_rover_drivetrain_hw/visibility_control.hpp:26:9: error: macro 'UWRT_MARS_ROVER_DRIVETRAIN_HW_PUBLIC' used to declare a constant; consider using a 'constexpr' constant [cppcoreguidelines-macro-usage,-warnings-as-errors]
#define UWRT_MARS_ROVER_DRIVETRAIN_HW_PUBLIC __attribute__((visibility("default")))
        ^
/home/wmmc88/git-repos/ros2_uwrt_ws/src/uwrt_mars_rover/uwrt_mars_rover_drivetrain/uwrt_mars_rover_drivetrain_hw/include/uwrt_mars_rover_drivetrain_hw/visibility_control.hpp:27:9: error: macro 'UWRT_MARS_ROVER_DRIVETRAIN_HW_LOCAL' used to declare a constant; consider using a 'constexpr' constant [cppcoreguidelines-macro-usage,-warnings-as-errors]
#define UWRT_MARS_ROVER_DRIVETRAIN_HW_LOCAL __attribute__((visibility("hidden")))
        ^
/home/wmmc88/git-repos/ros2_uwrt_ws/src/uwrt_mars_rover/uwrt_mars_rover_drivetrain/uwrt_mars_rover_drivetrain_hw/src/uwrt_mars_rover_drivetrain_hw_actuator_interface.cpp:4:1: error: constructor does not initialize these fields: actuator_state_position_, actuator_state_velocity_, actuator_state_iq_current_, joint_velocity_command_ [cppcoreguidelines-pro-type-member-init,-warnings-as-errors]
UWRTMarsRoverDrivetrainHWActuatorInterface::UWRTMarsRoverDrivetrainHWActuatorInterface()
^
Suppressed 31647 warnings (31639 in non-user code, 8 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
4 warnings treated as errors

I finally can see the actual errors!

So the two issues I'm looking to get addressed here are failed tests being detected as passed tests, and failed tests not printing out the actual errors.

hidmic commented 2 years ago

Thanks for the report @wmmc88. I can see why ament_clang_tidy behaves like this.

  1. All stderr coming from clang-tidy processes is flat out ignored
  2. All stdout coming from any clang-tidy process is ignored if the process exits with a nonzero return code
  3. ament_clang_tidy return codes are decoupled from those of the clang-tidy processes it invokes

(1.) may be adequate, not sure. (2.) and (3.) are bugs, I agree. Bugfixes are most welcome.