Skip to content

Commit

Permalink
Use a local temporary directory, rather than the global one, for more…
Browse files Browse the repository at this point in the history
… portability.

Signed-off-by: Emerson Knapp <[email protected]>
  • Loading branch information
Emerson Knapp committed Jan 27, 2021
1 parent bf9bd04 commit 028f859
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 14 deletions.
2 changes: 1 addition & 1 deletion rosbag2_compression/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ if(BUILD_TESTING)
test/rosbag2_compression/test_sequential_compression_writer.cpp)
target_include_directories(test_sequential_compression_writer PUBLIC include)
target_link_libraries(test_sequential_compression_writer ${PROJECT_NAME})
ament_target_dependencies(test_sequential_compression_writer rosbag2_cpp)
ament_target_dependencies(test_sequential_compression_writer rosbag2_cpp rosbag2_test_common)
endif()

ament_package()
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,8 @@ void SequentialCompressionWriter::compress_file(
{
using rcpputils::fs::path;

auto file_relative_to_pwd = path(base_folder_) / file_relative_to_bag;
ROSBAG2_COMPRESSION_LOG_DEBUG_STREAM("Compressing file: " << file_relative_to_pwd.string());
const auto file_relative_to_pwd = path(base_folder_) / file_relative_to_bag;
ROSBAG2_COMPRESSION_LOG_INFO_STREAM("Compressing file: " << file_relative_to_pwd.string());

if (file_relative_to_pwd.exists() && file_relative_to_pwd.file_size() > 0u) {
const auto compressed_uri = compressor.compress_uri(file_relative_to_pwd.string());
Expand All @@ -251,7 +251,7 @@ void SequentialCompressionWriter::compress_file(
// Must search for the entry because other threads may have changed the order of the vector
// and invalidated any index or iterator we held to it.
std::lock_guard<std::recursive_mutex> lock(storage_mutex_);
auto iter = std::find(
const auto iter = std::find(
metadata_.relative_file_paths.begin(),
metadata_.relative_file_paths.end(),
file_relative_to_bag);
Expand All @@ -271,7 +271,7 @@ void SequentialCompressionWriter::compress_file(
"will not be halted because the compressed output was successfully created.");
}
} else {
ROSBAG2_COMPRESSION_LOG_DEBUG_STREAM(
ROSBAG2_COMPRESSION_LOG_INFO_STREAM(
"Removing last file: \"" << file_relative_to_pwd.string() <<
"\" because it either is empty or does not exist.");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "rosbag2_cpp/writer.hpp"

#include "rosbag2_storage/storage_options.hpp"
#include "rosbag2_test_common/temporary_directory_fixture.hpp"

#include "mock_converter_factory.hpp"
#include "mock_metadata_io.hpp"
Expand All @@ -38,20 +39,19 @@

using namespace testing; // NOLINT

class SequentialCompressionWriterTest : public Test
class SequentialCompressionWriterTest : public rosbag2_test_common::TemporaryDirectoryFixture
{
public:
SequentialCompressionWriterTest()
: storage_factory_{std::make_unique<StrictMock<MockStorageFactory>>()},
storage_{std::make_shared<NiceMock<MockStorage>>()},
converter_factory_{std::make_shared<StrictMock<MockConverterFactory>>()},
metadata_io_{std::make_unique<NiceMock<MockMetadataIo>>()},
tmp_dir_{rcpputils::fs::temp_directory_path() / bag_name_},
tmp_dir_{rcpputils::fs::path(temporary_dir_path_) / bag_name_},
tmp_dir_storage_options_{},
serialization_format_{"rmw_format"}
{
tmp_dir_storage_options_.uri = tmp_dir_.string();
rcpputils::fs::remove_all(tmp_dir_);
ON_CALL(*storage_factory_, open_read_write(_)).WillByDefault(Return(storage_));
EXPECT_CALL(*storage_factory_, open_read_write(_)).Times(AtLeast(0));
// intercept the metadata write so we can analyze it.
Expand All @@ -73,8 +73,10 @@ class SequentialCompressionWriterTest : public Test
fake_storage_uri_ = storage_options.uri;
// Touch the file
std::ofstream output(storage_options.uri);
ASSERT_TRUE(output.is_open());
// Put some arbitrary bytes in the file so it isn't interpreted as being empty
output << "Fake storage data" << std::endl;
output.close();
}),
Return(storage_)));
ON_CALL(
Expand Down Expand Up @@ -151,8 +153,6 @@ TEST_F(SequentialCompressionWriterTest, open_throws_on_bad_compression_format)
EXPECT_THROW(
writer_->open(tmp_dir_storage_options_, {serialization_format_, serialization_format_}),
std::invalid_argument);

EXPECT_TRUE(rcpputils::fs::remove(tmp_dir_));
}

TEST_F(SequentialCompressionWriterTest, open_throws_on_invalid_splitting_size)
Expand Down Expand Up @@ -188,8 +188,6 @@ TEST_F(SequentialCompressionWriterTest, open_succeeds_on_supported_compression_f

EXPECT_NO_THROW(
writer_->open(tmp_dir_storage_options_, {serialization_format_, serialization_format_}));

EXPECT_TRUE(rcpputils::fs::remove(tmp_dir_));
}

TEST_F(SequentialCompressionWriterTest, writer_calls_create_compressor)
Expand All @@ -207,8 +205,6 @@ TEST_F(SequentialCompressionWriterTest, writer_calls_create_compressor)
EXPECT_THROW(
writer_->open(tmp_dir_storage_options_, {serialization_format_, serialization_format_}),
std::runtime_error);

EXPECT_TRUE(rcpputils::fs::remove(tmp_dir_));
}

TEST_F(SequentialCompressionWriterTest, writer_creates_correct_metadata_relative_filepaths)
Expand Down

0 comments on commit 028f859

Please sign in to comment.