src/error: Add From<ConnectionError> for io::Error implementation#136
src/error: Add From<ConnectionError> for io::Error implementation#136thomaseizinger wants to merge 2 commits intolibp2p:masterfrom
From<ConnectionError> for io::Error implementation#136Conversation
This allows the use of `?` in functions that return `io::Error`.
mxinden
left a comment
mxinden
left a comment
There was a problem hiding this comment.
I am fine with this.
Just needs a changelog entry and patch version bump.
src/error.rs
Outdated
| #[test] | ||
| fn connection_error_can_be_question_marked_to_io_error() { | ||
| returns_io_error().expect_err("to fail"); | ||
| } | ||
|
|
||
| fn returns_io_error() -> std::io::Result<()> { | ||
| always_err()?; | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| fn always_err() -> Result<(), ConnectionError> { | ||
| Err(ConnectionError::Closed) | ||
| } | ||
| } |
There was a problem hiding this comment.
| #[test] | |
| fn connection_error_can_be_question_marked_to_io_error() { | |
| returns_io_error().expect_err("to fail"); | |
| } | |
| fn returns_io_error() -> std::io::Result<()> { | |
| always_err()?; | |
| Ok(()) | |
| } | |
| fn always_err() -> Result<(), ConnectionError> { | |
| Err(ConnectionError::Closed) | |
| } | |
| } | |
| #[test] | |
| fn connection_error_can_be_question_marked_to_io_error() { | |
| fn returns_io_error() -> std::io::Result<()> { | |
| always_err()?; | |
| Ok(()) | |
| } | |
| fn always_err() -> Result<(), ConnectionError> { | |
| Err(ConnectionError::Closed) | |
| } | |
| returns_io_error().expect_err("to fail"); | |
| } | |
| } |
How about nesting them?
src/error.rs
Outdated
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
|
|
||
| #[test] | ||
| fn connection_error_can_be_question_marked_to_io_error() { | ||
| returns_io_error().expect_err("to fail"); | ||
| } | ||
|
|
||
| fn returns_io_error() -> std::io::Result<()> { | ||
| always_err()?; | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| fn always_err() -> Result<(), ConnectionError> { | ||
| Err(ConnectionError::Closed) | ||
| } | ||
| } |
There was a problem hiding this comment.
connection_error_can_be_question_marked_to_io_error
I have to admit that I don't really see the point of this test. It's general rust logic that we can question mark one error into another if From<> is implemented. Since we implemented impl From<ConnectionError> for std::io::Error we know for sure that this will not fail.
From my understanding, tests should check for implementation errors, edge cases, side effect, ... etc. This is not the case here. What is the gain from this test?
There was a problem hiding this comment.
I wanted to test that what I wanted to achieve in rust-libp2p actually works. Happy to delete the test again if you think it is bloat (I think I agree with you).
We now know that this test works, so we don't need to keep it around.
|
Closing in favor of libp2p/rust-libp2p#2710. |
3 participants
This allows the use of
?in functions that returnio::Error.