-
Notifications
You must be signed in to change notification settings - Fork 240
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
Do not expect empty StorageOptions URI to work in *CompressionWriterTest #526
Do not expect empty StorageOptions URI to work in *CompressionWriterTest #526
Conversation
|
Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
7b129ec
to
4dd6af9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
std::move(metadata_io_)); | ||
writer_ = std::make_unique<rosbag2_cpp::Writer>(std::move(sequential_writer)); | ||
|
||
EXPECT_THROW( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
http://build.ros2.org/job/Rpr__rosbag2__ubuntu_focal_amd64/209/testReport/rosbag2_compression/SequentialCompressionWriterTest/open_throws_on_empty_storage_options_uri/, currently this is expected to fail unless ros2/rcpputils#95 is merged.
@emersonknapp / @jaisontj could you take a look at this? It's needed for other PRs. |
CI is at ros2/rcpputils#98 (comment) |
CI passed (see ros2/rcpputils#98 (comment)), merging together with ros2/rcpputils#98. (I will release the rcpputils repo to make the PR checker pass again) |
…est (#526) Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
…est (#526) Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
…est (ros2#526) Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
For ros2/rcpputils#94, I proposed ros2/rcpputils#95, which made it so that
rcpputils::fs::create_directories()
returns false if the directory path is invalid, e.g. if the path is empty, invalid, not a directory, etc.This was the case for
SequentialCompressionWriterTest
, which assumed that using an emptyrosbag2_cpp::StorageOptions
URI would work. That test then started failing and ros2/rcpputils#95 was reverted. This PR changesSequentialCompressionWriterTest
so that ros2/rcpputils#95 can be merged back in.See ros2/rcpputils#97 (review)