Skip to content

Commit

Permalink
gate rostest dependency hack
Browse files Browse the repository at this point in the history 
with debian sbuild builds this does not exist atm.
  • Loading branch information
@v4hn
v4hn committed May 28, 2023
1 parent 20f2017 commit d33ca46
Showing 1 changed file with 5 additions and 0 deletions.
5 changes: 5 additions & 0 deletions test/test_rosbag/bag_migration_tests/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,20 +84,25 @@ configure_file(test/migrate_test.py.in
catkin_add_nosetests(${PROJECT_BINARY_DIR}/test/migrate_test.py)
configure_file(test/random_record.xml.in
${PROJECT_BINARY_DIR}/test/random_record.xml)

add_rostest(${PROJECT_BINARY_DIR}/test/random_record.xml)
# Make sure that the random_record test runs before either of the random_play
# tests, both of which require a .bag file that the random_record test will
# write into /tmp. We're making an assumption about the target names generated
# by add_rostest(), but that naming scheme seems to be pretty stable.
configure_file(test/random_play.xml.in
${PROJECT_BINARY_DIR}/test/random_play.xml)
if(TARGET run_tests_test_rosbag_rostest_test_random_record.xml)

This comment has been minimized.

Sign in to view

Copy link
@rhaschke

rhaschke Nov 21, 2023

Contributor

@v4hn, can you explain why this "hack" was required for you? I don't have any issues without it.
Isn't this dependency declared in the line below?

ros_comm/test/test_rosbag/bag_migration_tests/CMakeLists.txt

Lines 94 to 95 in 845f746

add_rostest(${PROJECT_BINARY_DIR}/test/random_play.xml
DEPENDENCIES run_tests_test_rosbag_rostest_test_random_record.xml)

This comment has been minimized.

Sign in to view

Copy link
@v4hn

v4hn Nov 22, 2023

Author Contributor

Looking at the sources multiple times I wanted to agree, but the ros-o-builder CI still failed without the patch stating the targets don't exist.

A few hours later, here is the real patch for the underlying issue: #2361
I will rebase this branch and add the proper patch instead of this one.

This comment has been minimized.

Sign in to view

Copy link
@rhaschke

rhaschke Nov 22, 2023

Contributor

So, the problem for you was caused by building inside the source folder? I'm curious, because I never faced an issue with my ros-o-builder.

This comment has been minimized.

Sign in to view

Copy link
@v4hn

v4hn via email Nov 23, 2023

Author Contributor

This comment has been minimized.

Sign in to view

Copy link
@jspricke

jspricke Nov 23, 2023

Member

I'm curious, because I never faced an issue with my ros-o-builder.

I am surprised by that because that's how debhelper does it. Can you share an sbuild log?

This comment has been minimized.

Sign in to view

Copy link
@rhaschke

rhaschke Nov 23, 2023

Contributor

Sure. I triggered a new build of ros_comm for you:
https://github.com/ubi-agni/ros-builder-action/actions/runs/6973913392/job/18978742275#step:5:48114

This comment has been minimized.

Sign in to view

Copy link
@jspricke

jspricke Nov 24, 2023 •
edited
Loading

Member

Thanks, the log has -DBUILD_TESTING=OFF set which disables the problematic code:
https://github.com/ubi-agni/ros-builder-action/actions/runs/6973913392/job/18978742275#step:5:40431

This comment has been minimized.

Sign in to view

Copy link
@rhaschke

rhaschke Nov 24, 2023

Contributor

Right. I copied this from your version. How did the ROS buildfarm handle building debians?
Probably, because testing is handled by other means, they disable testing for the debian builds?

This comment has been minimized.

Sign in to view

Copy link
@jspricke

jspricke Nov 24, 2023

Member

test_rosbag is not released via rosdistro and last time I checked the ROS buildfarm they ran tests but did not check the result.
add_rostest(${PROJECT_BINARY_DIR}/test/random_play.xml
DEPENDENCIES run_tests_test_rosbag_rostest_test_random_record.xml)
configure_file(test/random_play_sim.xml.in
${PROJECT_BINARY_DIR}/test/random_play_sim.xml)
add_rostest(${PROJECT_BINARY_DIR}/test/random_play_sim.xml
DEPENDENCIES run_tests_test_rosbag_rostest_test_random_record.xml)
endif()

# Make sure that rostest random_record.xml is not executed twice, running in
# parallel (https://github.com/ros/ros_comm/pull/1651#issuecomment-482148146):
if(TARGET run_tests_test_rosbag_rostest_test_random_play.xml)
add_dependencies(run_tests_test_rosbag_rostest_test_random_play.xml run_tests_test_rosbag_rostest_test_random_play_sim.xml)
endif()

0 comments on commit d33ca46

Please sign in to comment.