Make split size error clearer #428
Conversation
Signed-off-by: Anas Abou Allaban <aabouallaban@pm.me>
|
|
There's unrelated windows warnings https://ci.ros2.org/job/ci_windows/10918/msbuild/new/ I'll merge this if there's no objections (this is part of Foxy QA) @Karsten1987 @zmichaels11 |
| throw std::invalid_argument{ | ||
| "Invalid bag splitting size given. Please provide a different value."}; | ||
| std::stringstream error; | ||
| error << "Invalid bag splitting size given. Please provide a value greater than " << |
There was a problem hiding this comment.
Nit: do you want to add the size specified to the thrown message?
There was a problem hiding this comment.
I don't think that makes sense since it's already known by the user.
The goal of this PR is to inform the user of a suitable alternative to what they provided beforehand.
| throw std::runtime_error( | ||
| "Invalid bag splitting size given. Please provide a different value."); | ||
| std::stringstream error; | ||
| error << "Invalid bag splitting size given. Please provide a value greater than " << |
There was a problem hiding this comment.
Nit: do you want to add the size specified to the thrown message?
| @@ -154,7 +154,7 @@ TEST_F(SequentialWriterTest, open_throws_error_on_invalid_splitting_size) { | |||
| ON_CALL(*storage_, get_minimum_split_file_size()).WillByDefault(Return(min_split_file_size)); | |||
| storage_options_.max_bagfile_size = max_bagfile_size; | |||
|
|
|||
| EXPECT_CALL(*storage_, get_minimum_split_file_size).Times(1); | |||
| EXPECT_CALL(*storage_, get_minimum_split_file_size).Times(2); | |||
There was a problem hiding this comment.
Don't you need a test with the EXPECT_THROW macro? Edit, nevermind expanding the code shows that case handled. Nit to expect the runtime_error thrown.
Make the error for setting the split bag file size state the minimum bag file size.
Current error is:
Signed-off-by: Anas Abou Allaban aabouallaban@pm.me