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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions rosbag2_cpp/src/rosbag2_cpp/reindexer.cpp
Expand Up @@ -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.


if (temp_metadata.starting_time < metadata_.starting_time) {
metadata_.starting_time = temp_metadata.starting_time;
Expand Down
6 changes: 4 additions & 2 deletions rosbag2_tests/CMakeLists.txt
Expand Up @@ -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.

if(TARGET test_rosbag2_record_end_to_end)
ament_target_dependencies(test_rosbag2_record_end_to_end
rclcpp
Expand Down Expand Up @@ -96,7 +97,8 @@ if(BUILD_TESTING)
rosbag2_cpp
rosbag2_storage
rosbag2_storage_default_plugins
rosbag2_test_common)
rosbag2_test_common
std_msgs)
endif()


Expand Down
2 changes: 2 additions & 0 deletions rosbag2_tests/package.xml
Expand Up @@ -24,6 +24,8 @@
<test_depend>rosbag2_compression_zstd</test_depend>
<test_depend>rosbag2_cpp</test_depend>
<test_depend>rosbag2_storage_default_plugins</test_depend>
<!-- TODO(mcap): remove when mcap is added to default plugins -->
<test_depend>rosbag2_storage_mcap</test_depend>
james-rms marked this conversation as resolved.
Show resolved Hide resolved
<test_depend>rosbag2_storage</test_depend>
<test_depend>rosbag2_test_common</test_depend>
<test_depend>std_msgs</test_depend>
Expand Down
Binary file not shown.
Binary file not shown.
33 changes: 33 additions & 0 deletions rosbag2_tests/resources/mcap/cdr_test/metadata.yaml
@@ -0,0 +1,33 @@
rosbag2_bagfile_information:
version: 6
storage_identifier: mcap
duration:
nanoseconds: 151137181
starting_time:
nanoseconds_since_epoch: 1586406456763032325
message_count: 7
topics_with_message_count:
- topic_metadata:
name: /array_topic
type: test_msgs/msg/Arrays
serialization_format: cdr
offered_qos_profiles: "- history: 3\n depth: 0\n reliability: 1\n durability: 2\n deadline:\n sec: 0\n nsec: 0\n lifespan:\n sec: 0\n nsec: 0\n liveliness: 1\n liveliness_lease_duration:\n sec: 0\n nsec: 0\n avoid_ros_namespace_conventions: false"
message_count: 4
- topic_metadata:
name: /test_topic
type: test_msgs/msg/BasicTypes
serialization_format: cdr
offered_qos_profiles: "- history: 3\n depth: 0\n reliability: 1\n durability: 2\n deadline:\n sec: 0\n nsec: 0\n lifespan:\n sec: 0\n nsec: 0\n liveliness: 1\n liveliness_lease_duration:\n sec: 0\n nsec: 0\n avoid_ros_namespace_conventions: false"
message_count: 3
compression_format: ""
compression_mode: ""
relative_file_paths:
- cdr_test_0.mcap
files:
- path: cdr_test_0.mcap
starting_time:
nanoseconds_since_epoch: 1586406456763032325
duration:
nanoseconds: 151137181
message_count: 7
custom_data: ~
33 changes: 33 additions & 0 deletions rosbag2_tests/resources/mcap/wrong_rmw_test/metadata.yaml
@@ -0,0 +1,33 @@
rosbag2_bagfile_information:
version: 6
storage_identifier: mcap
duration:
nanoseconds: 151137181
starting_time:
nanoseconds_since_epoch: 1586406456763032325
message_count: 7
topics_with_message_count:
- topic_metadata:
name: /array_topic
type: test_msgs/msg/Arrays
serialization_format: wrong_format
offered_qos_profiles: "- history: 3\n depth: 0\n reliability: 1\n durability: 2\n deadline:\n sec: 0\n nsec: 0\n lifespan:\n sec: 0\n nsec: 0\n liveliness: 1\n liveliness_lease_duration:\n sec: 0\n nsec: 0\n avoid_ros_namespace_conventions: false"
message_count: 4
- topic_metadata:
name: /test_topic
type: test_msgs/msg/BasicTypes
serialization_format: wrong_format
offered_qos_profiles: "- history: 3\n depth: 0\n reliability: 1\n durability: 2\n deadline:\n sec: 0\n nsec: 0\n lifespan:\n sec: 0\n nsec: 0\n liveliness: 1\n liveliness_lease_duration:\n sec: 0\n nsec: 0\n avoid_ros_namespace_conventions: false"
message_count: 3
compression_format: ""
compression_mode: ""
relative_file_paths:
- wrong_rmw_test_0.mcap
files:
- path: wrong_rmw_test_0.mcap
starting_time:
nanoseconds_since_epoch: 1586406456763032325
duration:
nanoseconds: 151137181
message_count: 7
custom_data: ~
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.

This file was deleted.

This file was deleted.

This file was deleted.

Binary file not shown.
Empty file.
33 changes: 14 additions & 19 deletions rosbag2_tests/test/rosbag2_tests/record_fixture.hpp
Expand Up @@ -32,6 +32,8 @@
#include "rosbag2_storage/default_storage_id.hpp"
#include "rosbag2_storage/storage_filter.hpp"
#include "rosbag2_test_common/memory_management.hpp"
#include "rosbag2_test_common/temporary_directory_fixture.hpp"
#include "rosbag2_test_common/tested_storage_ids.hpp"

#include "test_msgs/msg/arrays.hpp"
#include "test_msgs/msg/basic_types.hpp"
Expand All @@ -41,26 +43,14 @@ using namespace ::testing; // NOLINT
using namespace std::chrono_literals; // NOLINT
using namespace rosbag2_test_common; // NOLINT

const std::unordered_map<std::string, std::string> storage_plugins_to_extension {
{"sqlite3", ".db3"}
};

class RecordFixture : public TestWithParam<std::string>
class RecordFixture : public ParametrizedTemporaryDirectoryFixture
{
public:
RecordFixture()
{
temporary_dir_path_ = rcpputils::fs::create_temp_directory("tmp_test_dir_").string();
}

~RecordFixture() override
{
rcpputils::fs::remove_all(rcpputils::fs::path(temporary_dir_path_));
}

void SetUp() override
{
root_bag_path_ = rcpputils::fs::path(temporary_dir_path_) / get_test_name();
auto bag_name = get_test_name() + "_" + GetParam();
root_bag_path_ = rcpputils::fs::path(temporary_dir_path_) / bag_name;

// Clean up potentially leftover bag files.
// There may be leftovers if the system reallocates a temp directory
Expand All @@ -83,6 +73,11 @@ class RecordFixture : public TestWithParam<std::string>
rclcpp::shutdown();
}

std::string get_base_record_command() const
{
return "ros2 bag record --storage " + GetParam() + " --output " + root_bag_path_.string();
}

std::string get_test_name() const
{
const auto * test_info = UnitTest::GetInstance()->current_test_info();
Expand All @@ -95,7 +90,7 @@ class RecordFixture : public TestWithParam<std::string>
std::string get_bag_file_name(int split_index) const
{
std::stringstream bag_file_name;
bag_file_name << get_test_name() << "_" << split_index;
bag_file_name << get_test_name() << "_" << GetParam() << "_" << split_index;

return bag_file_name.str();
}
Expand All @@ -113,8 +108,9 @@ class RecordFixture : public TestWithParam<std::string>
rcpputils::fs::path get_relative_bag_file_path(int split_index)
{
const auto storage_id = GetParam();
const auto extension = storage_plugins_to_extension.at(storage_id);
return rcpputils::fs::path(get_bag_file_name(split_index) + extension);
return rcpputils::fs::path(
rosbag2_test_common::bag_filename_for_storage_id(
get_bag_file_name(split_index), storage_id));
}

void wait_for_metadata(std::chrono::duration<float> timeout = std::chrono::seconds(5)) const
Expand Down Expand Up @@ -223,7 +219,6 @@ class RecordFixture : public TestWithParam<std::string>
#endif
}

std::string temporary_dir_path_;
// relative path to the root of the bag file.
rcpputils::fs::path root_bag_path_;

Expand Down