Skip to content

fix: correct ASSERT_EQ to ASSERT_NE in admittance controller load test (backport #2264)#2340

Open
mergify[bot] wants to merge 2 commits into
humblefrom
mergify/bp/humble/pr-2264
Open

fix: correct ASSERT_EQ to ASSERT_NE in admittance controller load test (backport #2264)#2340
mergify[bot] wants to merge 2 commits into
humblefrom
mergify/bp/humble/pr-2264

Conversation

@mergify
Copy link
Copy Markdown
Contributor

@mergify mergify Bot commented May 1, 2026

Fixes #1258

Problem

The test_load_controller test in admittance_controller was using ASSERT_EQ to check against nullptr, which means the test was asserting that the controller fails to load. Since the controller loads successfully with the params file provided, the test was always passing, even though it was checking the wrong condition.

Fix

Changed ASSERT_EQ to ASSERT_NE so the test correctly asserts that the controller loaded successfully (i.e. the result is not nullptr).

Note: range_sensor_broadcaster already has the correct ASSERT_NE in its load test and does not need changes.

Is this user-facing behavior change?

no

Did you use Generative AI?

no


This is an automatic backport of pull request #2264 done by Mergify.

#2264)

(cherry picked from commit b822e35)

# Conflicts:
#	admittance_controller/test/test_load_admittance_controller.cpp
@mergify mergify Bot added the conflicts label May 1, 2026
@mergify
Copy link
Copy Markdown
Contributor Author

mergify Bot commented May 1, 2026

Cherry-pick of b822e35 has failed:

On branch mergify/bp/humble/pr-2264
Your branch is up to date with 'origin/humble'.

You are currently cherry-picking commit b822e35.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   admittance_controller/test/test_params.yaml

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   admittance_controller/test/test_load_admittance_controller.cpp

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@SouriRishik
Copy link
Copy Markdown
Contributor

Hi @christophfroehlich The backport is currently failing on humble and it looks like there are differences in the test setup, such as missing test_file_path and a hardware plugin mismatch causing the controller load to fail.

Would you like me to adapt the test for humble compatibility, or will you handle it?

@christophfroehlich
Copy link
Copy Markdown
Member

@SouriRishik thank you for your help! I had a quick look in other controllers on humble, there are only a few using yaml files during the load-tests. I might have missed something

@christophfroehlich
Copy link
Copy Markdown
Member

You can just open a PR targeting the mergify/bp/humble/pr-2264 branch

@SouriRishik
Copy link
Copy Markdown
Contributor

okay! I'll do that soon

@SouriRishik
Copy link
Copy Markdown
Contributor

Hi @christophfroehlich, I found out something while trying to fix this issue
The backport test is currently failing on humble due to a mismatch in the test asset.

ros2_control_test_assets::minimal_robot_urdf uses hardware plugins like test_actuator, test_sensor, and test_system, which are not available in humble. The available plugins are from test_hardware_components (e.g., TestSingleJointActuator, TestForceTorqueSensor, etc.), leading to initialization failures before the controller is loaded.

Would you prefer adapting the test to use a compatible hardware setup available in humble (e.g., test_hardware_components systems), or is there a preferred test asset we should use for this backport?

@christophfroehlich
Copy link
Copy Markdown
Member

if possible choose one of the existing

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants