TestAssertionFailure: retain full error chain in message#666
Open
rursprung wants to merge 1 commit intogoogle:mainfrom
Open
TestAssertionFailure: retain full error chain in message#666rursprung wants to merge 1 commit intogoogle:mainfrom
TestAssertionFailure: retain full error chain in message#666rursprung wants to merge 1 commit intogoogle:mainfrom
Conversation
gribozavr
requested changes
Aug 20, 2025
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { |
Collaborator
There was a problem hiding this comment.
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
Author
There was a problem hiding this comment.
i've added an integration test for it.
i'm not sure what should be updated for anyhow?
Collaborator
|
Thank you for the fix! This PR generally looks good, only a few stylistic comments and a request to add an integration test. |
rursprung
commented
Mar 29, 2026
| TestAssertionFailure::create(format!("{value}")) | ||
| // print the full error chain, not just the first error. | ||
| let mut description = String::new(); | ||
| description.push_str(&format!("Error: {value}\n")); |
Author
There was a problem hiding this comment.
i've removed the explicit Error: here since it's being added elsewhere, here's the output from the integration test when i had Error: present here:
running 1 test
test tests::should_fail_due_to_error_in_subroutine ... FAILED
failures:
---- tests::should_fail_due_to_error_in_subroutine stdout ----
Error: Error: test3
Caused by:
1: test2
2: test1
at integration_tests\src\test_returning_custom_error.rs:44:9
failures:
tests::should_fail_due_to_error_in_subroutine
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
i think that's one Error: too many
rursprung
force-pushed
the
print-error-chain
branch
from
4f5ed44 to
706cee7
Compare
Author
|
sorry that this took so long to update, i've now updated the PR with your feedback. i should be responding (way) faster now |
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. note that if you use `or_fail` from the `OrFail` trait then you have to manually convert your `Error` to a `TestAssertionFailure` first as otherwise you will not profit from this new rendering. this is because the `OrFail` `impl` is a blanket-impl for all `T: std::fmt::Debug` and since `std::error::Error` requires `std::fmt::Debug` and specialization has not yet been stabilised we cannot add a second `impl` for `T: std::error::Error` where we'd do this conversion. example: ``` Error: test3 Caused by: 1: test2 2: test1 ``` fixes google#657 [rust-lang/api-guidelines#210]: rust-lang/api-guidelines#210
rursprung
force-pushed
the
print-error-chain
branch
from
706cee7 to
f18963c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
so far the conversion from an
Errorto aTestAssertionFailuremeant that all information about the source errors were lost. often, theDisplayimplementation forErrorimplementations 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
anyhoworeyeretake care of pretty-printing the error chain on failure, however this does not work withgoogletestsincegoogletest::ResulthasTestAssertionFailureas the error type which swallows anyErrorand only keeps the message.to resolve this a simple error chain implementation is added to the
Fromimplementation which pretty-prints the error chain.example:
fixes #657