Skip to content

Conversation

@christophfroehlich
Copy link
Contributor

For #199 I wrote new tests, which are also applicable for KDL.

For easier review:
In a first step, I propose to add the new testcases to the old file. As an upcoming step, I'd use a templated test file to be used by all implementations.

@christophfroehlich christophfroehlich added the backport-jazzy Triggers PR backport to ROS 2 jazzy. label Oct 20, 2025
@codecov
Copy link

codecov bot commented Oct 20, 2025

Codecov Report

❌ Patch coverage is 98.68421% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 88.19%. Comparing base (4a831a8) to head (1f9c887).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...ics_interface_kdl/src/kinematics_interface_kdl.cpp 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #210      +/-   ##
==========================================
+ Coverage   86.64%   88.19%   +1.55%     
==========================================
  Files           4        4              
  Lines         277      305      +28     
  Branches       60       53       -7     
==========================================
+ Hits          240      269      +29     
  Misses         21       21              
+ Partials       16       15       -1     
Flag Coverage Δ
unittests 88.19% <98.68%> (+1.55%) ⬆️

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

Files with missing lines Coverage Δ
...lude/kinematics_interface/kinematics_interface.hpp 100.00% <ø> (ø)
...terface_kdl/test/test_kinematics_interface_kdl.cpp 100.00% <100.00%> (ø)
...ics_interface_kdl/src/kinematics_interface_kdl.cpp 81.95% <66.66%> (+1.18%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@christophfroehlich christophfroehlich changed the title Refactor tests Refactor and extend tests Oct 20, 2025
christophfroehlich and others added 6 commits October 20, 2025 21:20
Co-authored-by: Sai Kishor Kothakota <sai.kishor@pal-robotics.com>
Co-authored-by: Sai Kishor Kothakota <sai.kishor@pal-robotics.com>
Co-authored-by: Sai Kishor Kothakota <sai.kishor@pal-robotics.com>
Co-authored-by: Sai Kishor Kothakota <sai.kishor@pal-robotics.com>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors and extends the test suite for the KDL kinematics interface by adding new test cases, improving test readability with custom matchers, and enhancing error messages. The changes prepare for future test templating across implementations.

Key changes:

  • Added custom GoogleTest matchers (MatrixNear, DoubleNear) to replace manual element-by-element comparisons
  • Added new test cases for reduced kinematic models (testing both reduced tip and base configurations)
  • Improved error logging by adding RCLCPP_ERROR messages for verification failures and enhancing existing error message formatting

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
kinematics_interface_kdl/test/test_kinematics_interface_kdl.cpp Refactored test helper methods to accept parameters, added new test cases for reduced models and frame difference calculations, replaced manual assertions with custom matchers
kinematics_interface_kdl/src/kinematics_interface_kdl.cpp Added detailed error logging for input verification failures, improved error message formatting with quotes, fixed logic for empty base parameter handling
kinematics_interface/include/kinematics_interface/kinematics_interface.hpp Added cross-platform FUNCTION_SIGNATURE macro and Vector6d type alias for improved code reusability

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

set explicit size for reference output methods
  error: cannot bind non-const lvalue reference of type
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-jazzy Triggers PR backport to ROS 2 jazzy.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants