[humble] rosbag2_cpp: test more than one storage plugin (backport #1196…

…) (#1721) * rosbag2_cpp: test more than one storage plugin (#1196) * rosbag2_storage_mcap: fix rosbag2_cpp tests Signed-off-by: James Smith <[email protected]> * rosbag2_cpp: test several storage plugins Signed-off-by: James Smith <[email protected]> * rosbag2_transport: specify test dependency on plugins Signed-off-by: James Smith <[email protected]> * ros2bag: add default plugins for testing Signed-off-by: James Smith <[email protected]> Signed-off-by: James Smith <[email protected]> (cherry picked from commit 9d7d7c3) # Conflicts: # mcap_vendor/CMakeLists.txt # rosbag2_cpp/test/rosbag2_cpp/test_info.cpp # rosbag2_cpp/test/rosbag2_cpp/test_sequential_reader.cpp # rosbag2_cpp/test/rosbag2_cpp/test_sequential_writer.cpp # rosbag2_storage_mcap/CMakeLists.txt # rosbag2_storage_mcap/src/mcap_storage.cpp * Rebase and address merge conflicts Signed-off-by: Michael Orlov <[email protected]> --------- Signed-off-by: Michael Orlov <[email protected]> Co-authored-by: james-rms <[email protected]> Co-authored-by: Michael Orlov <[email protected]>
ros2 · Jun 23, 2024 · 9659874 · 9659874
1 parent ca07426
commit 9659874
Show file tree

Hide file tree

Showing 8 changed files with 110 additions and 19 deletions.
diff --git a/ros2bag/package.xml b/ros2bag/package.xml
@@ -24,6 +24,7 @@
   <test_depend>launch_testing</test_depend>
   <test_depend>launch_testing_ros</test_depend>
   <test_depend>python3-pytest</test_depend>
+  <test_depend>rosbag2_storage_default_plugins</test_depend>
 
   <export>
     <build_type>ament_python</build_type>

diff --git a/rosbag2_cpp/package.xml b/rosbag2_cpp/package.xml
@@ -26,7 +26,9 @@
   <depend>rosidl_typesupport_introspection_cpp</depend>
   <depend>shared_queues_vendor</depend>
 
-  <exec_depend>rosbag2_storage_default_plugins</exec_depend>
+  <test_depend>rosbag2_storage_default_plugins</test_depend>
+  <!-- TODO: remove dependency when rosbag2_storage_default_plugins depends on MCAP -->
+  <test_depend>rosbag2_storage_mcap</test_depend>
 
   <test_depend>ament_cmake_gmock</test_depend>
   <test_depend>ament_lint_auto</test_depend>

diff --git a/rosbag2_cpp/test/rosbag2_cpp/test_info.cpp b/rosbag2_cpp/test/rosbag2_cpp/test_info.cpp
@@ -29,14 +29,15 @@
 #include "rosbag2_storage/metadata_io.hpp"
 
 #include "rosbag2_test_common/temporary_directory_fixture.hpp"
+#include "rosbag2_test_common/tested_storage_ids.hpp"
 
 #include "test_msgs/msg/basic_types.hpp"
 
 using namespace ::testing;  // NOLINT
-using rosbag2_test_common::TemporaryDirectoryFixture;
+using rosbag2_test_common::ParametrizedTemporaryDirectoryFixture;
 
-TEST_F(TemporaryDirectoryFixture, read_metadata_supports_version_2) {
-  const auto expected_storage_id = rosbag2_storage::get_default_storage_id();
+TEST_P(ParametrizedTemporaryDirectoryFixture, read_metadata_supports_version_2) {
+  const auto expected_storage_id = GetParam();
   const std::string bagfile = "rosbag2_bagfile_information:\n"
     "  version: 2\n"
     "  storage_identifier: " + expected_storage_id + "\n"
@@ -102,8 +103,10 @@ TEST_F(TemporaryDirectoryFixture, read_metadata_supports_version_2) {
   }
 }
 
-TEST_F(TemporaryDirectoryFixture, read_metadata_makes_appropriate_call_to_metadata_io_method) {
-  const auto expected_storage_id = rosbag2_storage::get_default_storage_id();
+TEST_P(
+  ParametrizedTemporaryDirectoryFixture,
+  read_metadata_makes_appropriate_call_to_metadata_io_method) {
+  const auto expected_storage_id = GetParam();
   std::string bagfile(
     "rosbag2_bagfile_information:\n"
     "  version: 3\n"
@@ -181,21 +184,36 @@ TEST_F(TemporaryDirectoryFixture, read_metadata_makes_appropriate_call_to_metada
   EXPECT_EQ(read_metadata.compression_mode, "FILE");
 }
 
-TEST_F(TemporaryDirectoryFixture, info_for_standalone_bagfile) {
+TEST_P(ParametrizedTemporaryDirectoryFixture, info_for_standalone_bagfile) {
+  const auto storage_id = GetParam();
   const auto bag_path = rcpputils::fs::path(temporary_dir_path_) / "bag";
-  const auto expected_bagfile_path = bag_path / "bag_0.db3";
   {
     // Create an empty bag with default storage
     rosbag2_cpp::Writer writer;
-    writer.open(bag_path.string());
+    rosbag2_storage::StorageOptions storage_options;
+    storage_options.storage_id = storage_id;
+    storage_options.uri = bag_path.string();
+    writer.open(storage_options);
     test_msgs::msg::BasicTypes msg;
     writer.write(msg, "testtopic", rclcpp::Time{});
   }
 
+  std::string first_storage_file_path;
+  {
+    rosbag2_storage::MetadataIo metadata_io;
+    auto metadata = metadata_io.read_metadata(bag_path.string());
+    first_storage_file_path = (bag_path / metadata.relative_file_paths[0]).string();
+  }
   rosbag2_cpp::Info info;
   rosbag2_storage::BagMetadata metadata;
   EXPECT_NO_THROW(
-    metadata = info.read_metadata(expected_bagfile_path.string())
+    metadata = info.read_metadata(first_storage_file_path)
   );
   EXPECT_THAT(metadata.topics_with_message_count, SizeIs(1));
 }
+
+INSTANTIATE_TEST_SUITE_P(
+  RosbagInfoTests,
+  ParametrizedTemporaryDirectoryFixture,
+  ValuesIn(rosbag2_test_common::kTestedStorageIDs)
+);
diff --git a/rosbag2_cpp/test/rosbag2_cpp/test_sequential_reader.cpp b/rosbag2_cpp/test/rosbag2_cpp/test_sequential_reader.cpp
@@ -30,6 +30,7 @@
 #include "rosbag2_storage/metadata_io.hpp"
 #include "rosbag2_storage/topic_metadata.hpp"
 
+#include "rosbag2_test_common/tested_storage_ids.hpp"
 #include "rosbag2_test_common/temporary_directory_fixture.hpp"
 
 #include "test_msgs/msg/basic_types.hpp"
@@ -41,7 +42,7 @@
 #include "mock_storage_factory.hpp"
 
 using namespace testing;  // NOLINT
-using rosbag2_test_common::TemporaryDirectoryFixture;
+using rosbag2_test_common::ParametrizedTemporaryDirectoryFixture;
 
 class SequentialReaderTest : public Test
 {
@@ -218,20 +219,32 @@ TEST_F(SequentialReaderTest, next_file_calls_callback) {
   EXPECT_EQ(opened_file, bag_file_2_path_.string());
 }
 
-TEST_F(TemporaryDirectoryFixture, reader_accepts_bare_file) {
+TEST_P(ParametrizedTemporaryDirectoryFixture, reader_accepts_bare_file) {
   const auto bag_path = rcpputils::fs::path(temporary_dir_path_) / "bag";
-  const auto expected_bagfile_path = bag_path / "bag_0.db3";
+  const auto storage_id = GetParam();
 
   {
     // Create an empty bag with default storage
     rosbag2_cpp::Writer writer;
-    writer.open(bag_path.string());
+    rosbag2_storage::StorageOptions options;
+    options.uri = bag_path.string();
+    options.storage_id = storage_id;
+    writer.open(options);
     test_msgs::msg::BasicTypes msg;
     writer.write(msg, "testtopic", rclcpp::Time{});
   }
 
+  rosbag2_storage::MetadataIo metadata_io;
+  auto metadata = metadata_io.read_metadata(bag_path.string());
+  auto first_storage = bag_path / metadata.relative_file_paths[0];
   rosbag2_cpp::Reader reader;
-  EXPECT_NO_THROW(reader.open(expected_bagfile_path.string()));
+  EXPECT_NO_THROW(reader.open(first_storage.string()));
   EXPECT_TRUE(reader.has_next());
   EXPECT_THAT(reader.get_metadata().topics_with_message_count, SizeIs(1));
 }
+
+INSTANTIATE_TEST_SUITE_P(
+  BareFileTests,
+  ParametrizedTemporaryDirectoryFixture,
+  ValuesIn(rosbag2_test_common::kTestedStorageIDs)
+);
diff --git a/rosbag2_cpp/test/rosbag2_cpp/test_sequential_writer.cpp b/rosbag2_cpp/test/rosbag2_cpp/test_sequential_writer.cpp
@@ -32,6 +32,7 @@
 #include "rosbag2_storage/topic_metadata.hpp"
 
 #include "rosbag2_test_common/temporary_directory_fixture.hpp"
+#include "rosbag2_test_common/tested_storage_ids.hpp"
 
 #include "mock_converter.hpp"
 #include "mock_converter_factory.hpp"
@@ -40,7 +41,7 @@
 #include "mock_storage_factory.hpp"
 
 using namespace testing;  // NOLINT
-using rosbag2_test_common::TemporaryDirectoryFixture;
+using rosbag2_test_common::ParametrizedTemporaryDirectoryFixture;
 
 class SequentialWriterTest : public Test
 {
@@ -671,15 +672,14 @@ void write_sample_split_bag(
   writer.close();
 }
 
-
-TEST_F(TemporaryDirectoryFixture, split_bag_metadata_has_full_duration) {
+TEST_P(ParametrizedTemporaryDirectoryFixture, split_bag_metadata_has_full_duration) {
   const std::vector<std::vector<rcutils_time_point_value_t>> message_timestamps_by_file {
     {100, 300, 200},
     {500, 400, 600}
   };
   rosbag2_storage::StorageOptions storage_options;
   storage_options.uri = (rcpputils::fs::path(temporary_dir_path_) / "split_duration_bag").string();
-  storage_options.storage_id = rosbag2_storage::get_default_storage_id();
+  storage_options.storage_id = GetParam();
   write_sample_split_bag(storage_options, message_timestamps_by_file);
 
   rosbag2_storage::MetadataIo metadata_io;
@@ -689,3 +689,9 @@ TEST_F(TemporaryDirectoryFixture, split_bag_metadata_has_full_duration) {
     std::chrono::high_resolution_clock::time_point(std::chrono::nanoseconds(100)));
   ASSERT_EQ(metadata.duration, std::chrono::nanoseconds(500));
 }
+
+INSTANTIATE_TEST_SUITE_P(
+  SplitMetadataTest,
+  ParametrizedTemporaryDirectoryFixture,
+  ValuesIn(rosbag2_test_common::kTestedStorageIDs)
+);
diff --git a/rosbag2_test_common/include/rosbag2_test_common/temporary_directory_fixture.hpp b/rosbag2_test_common/include/rosbag2_test_common/temporary_directory_fixture.hpp
@@ -42,6 +42,14 @@ class TemporaryDirectoryFixture : public Test
   std::string temporary_dir_path_;
 };
 
+/**
+ * @brief parametrizes the temporary directory fixture above with a string parameter,
+ * which can be used for testing across several storage plugins.
+ */
+class ParametrizedTemporaryDirectoryFixture
+  : public TemporaryDirectoryFixture,
+  public WithParamInterface<std::string> {};
+
 }  // namespace rosbag2_test_common
 
 #endif  // ROSBAG2_TEST_COMMON__TEMPORARY_DIRECTORY_FIXTURE_HPP_
diff --git a/rosbag2_test_common/include/rosbag2_test_common/tested_storage_ids.hpp b/rosbag2_test_common/include/rosbag2_test_common/tested_storage_ids.hpp
@@ -0,0 +1,42 @@
+// Copyright 2022, Foxglove Technologies Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef ROSBAG2_TEST_COMMON__TESTED_STORAGE_IDS_HPP_
+#define ROSBAG2_TEST_COMMON__TESTED_STORAGE_IDS_HPP_
+#include <array>
+#include <string>
+
+
+namespace rosbag2_test_common
+{
+static const std::array<std::string, 2> kTestedStorageIDs = {
+  "sqlite3",
+  "mcap",
+};
+
+std::string bag_filename_for_storage_id(const std::string & name, const std::string & storage_id)
+{
+  const std::array<std::string, 2> extensions = {".db3", ".mcap"};
+  static_assert(kTestedStorageIDs.size() == extensions.size());
+  for (size_t i = 0; i < extensions.size(); ++i) {
+    if (kTestedStorageIDs[i] == storage_id) {
+      return name + extensions[i];
+    }
+  }
+  throw std::runtime_error("unknown storage id: " + storage_id);
+}
+
+}  // namespace rosbag2_test_common
+
+#endif  // ROSBAG2_TEST_COMMON__TESTED_STORAGE_IDS_HPP_
diff --git a/rosbag2_transport/package.xml b/rosbag2_transport/package.xml
@@ -30,6 +30,7 @@
   <test_depend>rmw_implementation_cmake</test_depend>
   <test_depend>rosbag2_compression_zstd</test_depend>
   <test_depend>rosbag2_test_common</test_depend>
+  <test_depend>rosbag2_storage_default_plugins</test_depend>
   <test_depend>test_msgs</test_depend>
 
   <export>