-
Notifications
You must be signed in to change notification settings - Fork 25
TestAssertionFailure
: retain full error chain in message
#666
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
base: main
Are you sure you want to change the base?
Conversation
so far the conversion from an `Error` to a `TestAssertionFailure` meant that all information about the source errors were lost. often, the `Display` implementation for `Error` implementations does not include the source information since that should instead be retrieved via `source`. while this has not yet been made into an official API guideline (see [rust-lang/api-guidelines#210]) this is nevertheless being followed. various crates like `anyhow` or `eyere` take care of pretty-printing the error chain on failure, however this does not work with `googletest` since `googletest::Result` has `TestAssertionFailure` as the error type which swallows any `Error` and only keeps the message. to resolve this a simple error chain implementation is added to the `From` implementation which pretty-prints the error chain. example: ``` Error: test3 Caused by: 1: test2 2: test1 ``` fixes google#657 [rust-lang/api-guidelines#210]: rust-lang/api-guidelines#210
let source2 = CustomError { message: "test2".to_string(), source: Some(source1.into()) }; | ||
let error = CustomError { message: "test3".to_string(), source: Some(source2.into()) }; | ||
let assertion_failure = TestAssertionFailure::from(error); | ||
assert_eq!(assertion_failure.description, "Error: test3\n\nCaused by:\n1: test2\n2: test1"); |
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.
assert_eq!(assertion_failure.description, "Error: test3\n\nCaused by:\n1: test2\n2: test1"); | |
verify_eq!(assertion_failure.description, "Error: test3\n\nCaused by:\n1: test2\n2: test1") |
... and change the return type to Result<()>
(see comment above)
use std::fmt::{Debug, Display, Formatter}; | ||
|
||
#[test] | ||
fn error_chain_from_error() { |
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.
fn error_chain_from_error() { | |
fn test_assertion_failure_chain_from_error() -> Result<()>{ |
@@ -246,3 +260,34 @@ impl From<TestAssertionFailure> for proptest::test_runner::TestCaseError { | |||
proptest::test_runner::TestCaseError::Fail(format!("{value}").into()) | |||
} | |||
} | |||
|
|||
#[cfg(test)] | |||
mod tests { |
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.
Could you do a bit more testing:
-
Check whether we can / should update the error message verification in
test_can_return_anyhow_generated_error()
in integration_tests/src/integration_tests.rs -
Add a new integration test that returns a Result with a custom error type like integration_tests/src/test_returning_anyhow_error.rs
Thank you for the fix! This PR generally looks good, only a few stylistic comments and a request to add an integration test. |
so far the conversion from an
Error
to aTestAssertionFailure
meant that all information about the source errors were lost. often, theDisplay
implementation forError
implementations does not include the source information since that should instead be retrieved viasource
. while this has not yet been made into an official API guideline (see rust-lang/api-guidelines#210) this is nevertheless being followed.various crates like
anyhow
oreyere
take care of pretty-printing the error chain on failure, however this does not work withgoogletest
sincegoogletest::Result
hasTestAssertionFailure
as the error type which swallows anyError
and only keeps the message.to resolve this a simple error chain implementation is added to the
From
implementation which pretty-prints the error chain.example:
fixes #657