Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parametrize all rosbag2_tests for both supported storage plugins #1221

Merged
merged 7 commits into from Dec 21, 2022

Conversation

emersonknapp
Copy link
Collaborator

@emersonknapp emersonknapp commented Dec 20, 2022

Related to #1160

This way when both plugins are shipped by default, the full integration test suite is run on both.

Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
@emersonknapp emersonknapp marked this pull request as ready for review December 20, 2022 10:06
@emersonknapp emersonknapp requested a review from a team as a code owner December 20, 2022 10:06
@emersonknapp emersonknapp requested review from gbiggs, hidmic and james-rms and removed request for a team December 20, 2022 10:06
@@ -189,6 +189,7 @@ void Reindexer::aggregate_metadata(
rosbag2_cpp::ConverterOptions blank_converter_options {};
bag_reader->open(temp_so, blank_converter_options);
auto temp_metadata = bag_reader->get_metadata();
metadata_.storage_identifier = temp_metadata.storage_identifier;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note: before this was leaving the storage ID blank in a reindex operation, if storage id not specified by user.

@@ -42,7 +42,8 @@ if(BUILD_TESTING)

ament_add_gmock(test_rosbag2_record_end_to_end
test/rosbag2_tests/test_rosbag2_record_end_to_end.cpp
WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
TIMEOUT 120)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note: new test suite just takes longer than the default minute, given the doubled number of tests

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this timeout needs to be increased further for this test to pass.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was wrong - there was a broken test which caused this suite to time out. I've pushed a commit to fix it.

Copy link
Contributor

@james-rms james-rms left a comment

Choose a reason for hiding this comment

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

Great, thanks!

return test_name;
}

void create_test_bag(int messages_per_file, int num_files)
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

Signed-off-by: James Smith <james@foxglove.dev>
@james-rms james-rms force-pushed the emersonknapp/parametrize-rosbag2-tests branch from 23e0b39 to 770ac09 Compare December 21, 2022 04:19
@james-rms
Copy link
Contributor

Gist: https://gist.githubusercontent.com/james-rms/97f8ceec4535ad1bc1fa3f12784ad485/raw/f96b41102c6f6cbb1691ee95a871adcc247eed56/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_tests rosbag2_cpp
TEST args: --packages-above rosbag2_tests rosbag2_cpp
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/11304

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@james-rms james-rms merged commit 5d56fb9 into rolling Dec 21, 2022
@delete-merged-branch delete-merged-branch bot deleted the emersonknapp/parametrize-rosbag2-tests branch December 21, 2022 19:52
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.

None yet

2 participants