fix(gpio_controllers): resolve build race condition in tests

[Ishan1923]

This PR fixes the build failure reported in #2117.

Changes:
Added ${PROJECT_NAME} to target_link_libraries for the test_load_gpio_command_controller target. This ensures the parameter headers are generated before the test is compiled.

Fixes #2117

[saikishor]

Suggested change

	${PROJECT_NAME}
	gpio_controllers

Let's change it to the name of the library for now, to have consistency with other controllers.

I agree it is better with ${PROJECT_NAME} though

[Ishan1923]

Ok i will just do this instead

change: ${PROJECT_NAME} -> gpio_controllers

[bmagyar]

Thank you!

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.76%. Comparing base (9817475) to head (f0b8c7b).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2128   +/-   ##
=======================================
  Coverage   84.76%   84.76%           
=======================================
  Files         151      151           
  Lines       14619    14619           
  Branches     1269     1269           
=======================================
  Hits        12392    12392           
  Misses       1767     1767           
  Partials      460      460           

Flag	Coverage Δ
unittests	84.76% <ø> (ø)

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[christophfroehlich]

We never link against the library itself (aka the controller plugins) in test_load_* tests of other controllers. Why should this be necessary here?

[christophfroehlich]

Maybe target_include_directories(test_load_gpio_command_controller PRIVATE include) is the culprit, do we need the include folder here? We have this in some of the test_load_* tests, but not in every package here.

[saikishor]

We never link against the library itself (aka the controller plugins) in test_load_* tests of other controllers. Why should this be necessary here?

I agree. I also thought the same at first and then I verified other controllers and there we are linking too. Not sure why

[christophfroehlich]

We never link against the library itself (aka the controller plugins) in test_load_* tests of other controllers. Why should this be necessary here?

I agree. I also thought the same at first and then I verified other controllers and there we are linking too. Not sure why

In which ones are we linking against? I only had a look in a few..

[saikishor]

In which ones are we linking against? I only had a look in a few..

ros2_controllers/joint_state_broadcaster/CMakeLists.txt

Lines 58 to 66 in 9817475

	ament_add_gmock(test_load_joint_state_broadcaster
	test/test_load_joint_state_broadcaster.cpp
	)
	target_link_libraries(test_load_joint_state_broadcaster
	joint_state_broadcaster
	controller_manager::controller_manager
	hardware_interface::hardware_interface
	rclcpp::rclcpp
	ros2_control_test_assets::ros2_control_test_assets)

ros2_controllers/joint_trajectory_controller/CMakeLists.txt

Lines 96 to 103 in 9817475

	ament_add_gmock(test_load_joint_trajectory_controller
	test/test_load_joint_trajectory_controller.cpp
	)
	target_link_libraries(test_load_joint_trajectory_controller
	joint_trajectory_controller
	controller_manager::controller_manager
	ros2_control_test_assets::ros2_control_test_assets
	)

ros2_controllers/position_controllers/CMakeLists.txt

Lines 42 to 49 in 9817475

	ament_add_gmock(test_load_joint_group_position_controller
	test/test_load_joint_group_position_controller.cpp
	)
	target_link_libraries(test_load_joint_group_position_controller
	position_controllers
	controller_manager::controller_manager
	ros2_control_test_assets::ros2_control_test_assets
	)

There might be few more with the linking, I didn't go through all of them. I saw few without too, but I agree that it shouldn't be linked.

[Ishan1923]

@christophfroehlich Thanks for the clarification!

The reason I added the link is to resolve a build race condition: this test includes headers generated by generate_parameter_library within gpio_controllers. Without an explicit dependency, the test tries to compile before those headers are generated.

Since we prefer not to link the plugin library directly, I can replace target_link_libraries with add_dependencies:

add_dependencies(test_load_gpio_command_controller gpio_controllers)

what should I do?

[christophfroehlich]

@christophfroehlich Thanks for the clarification!

The reason I added the link is to resolve a build race condition: this test includes headers generated by generate_parameter_library within gpio_controllers. Without an explicit dependency, the test tries to compile before those headers are generated.

No this is not correct, it does not use any package-related headers (or library) at compile time. It only loads the plugin at test-time.

what should I do?

Have you tried to remove the target_include_directories?

[Ishan1923]

This was overlooked by me. The headers included via this:

target_include_directories(test_load_gpio_command_controller PRIVATE include)

is not required at the compile time, but in the runtime; for this pluginlib will handle it. It was extra-safety line which was not needed, as it was creating unnecessary compile time dependency.

[christophfroehlich]

this is not needed now, right?

Suggested change

gpio_controllers

[Ishan1923]

I have removed gpio_controllers.
Apologies for the oversight.