improve diagnostics for tests with custom return values #52453
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @TimNN (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
|
Ping from triage @TimNN will you have time to review this PR? |
src/libtest/lib.rs
Outdated
| @@ -326,7 +326,9 @@ pub fn test_main_static(tests: &[TestDescAndFn]) { | |||
| /// test is considered a failure. By default, invokes `report()` | |||
| /// and checks for a `0` result. | |||
| pub fn assert_test_result<T: Termination>(result: T) { | |||
| assert_eq!(result.report(), 0); | |||
| if result.report() != 0 { | |||
There was a problem hiding this comment.
nit: This can be written as assert_eq!(result.report(), 0, "some custom message")
I would also mention the actual and expected status codes in the message.
TimNN
added
S-waiting-on-author
|
Ping from triage @srijs! It's been a while since we heard from you, will you have time to work on this again? |
|
Apologies for the delay here, have addressed @TimNN's feedback now! |
src/libtest/lib.rs
Outdated
| assert_eq!(result.report(), 0); | ||
| let code = result.report(); | ||
| assert_eq!(code, 0, | ||
| "the test returned a termination value with a non-zero status code ({})\ |
There was a problem hiding this comment.
nit: missing space before the \
src/libtest/lib.rs
Outdated
| @@ -326,7 +326,10 @@ pub fn test_main_static(tests: &[TestDescAndFn]) { | |||
| /// test is considered a failure. By default, invokes `report()` | |||
| /// and checks for a `0` result. | |||
| pub fn assert_test_result<T: Termination>(result: T) { | |||
| assert_eq!(result.report(), 0); | |||
| let code = result.report(); | |||
| assert_eq!(code, 0, | |||
There was a problem hiding this comment.
nit (formatting): I believe the official style recommendation would be this:
assert_eq!(
code, 0,
"msg \
....",
code
);
TimNN
added
A-allocators
|
Feedback addressed! |
|
@bors r+ rollup |
|
📌 Commit 6411aef has been approved by |
bors
added
S-waiting-on-bors
improve diagnostics for tests with custom return values This is an attempt at getting the ball rolling to improve the diagnostics for test functions that return custom `impl Termination` values (see rust-lang#52436). An alternative could be to use `eprintln!`, but including this in the panic message felt nicely consistent with how failing test assertions would be reported. Let me know what you think!
Rollup of 8 pull requests Successful merges: - #52453 (improve diagnostics for tests with custom return values) - #53271 (use ? to simplify `TransitiveRelation.maybe_map`) - #53279 (Extend documentation of `rustc_on_unimplemented`) - #53342 (fix error for unsized packed struct field) - #53344 (Add doc examples for std::alloc::{alloc,alloc_zeroed}.) - #53368 (Ignore test that fails on stage1) - #53388 (Fix links' color) - #53396 (Fix since of Iterator::flatten to be a proper semver) Failed merges: r? @ghost
This is an attempt at getting the ball rolling to improve the diagnostics for test functions that return custom
impl Terminationvalues (see #52436).An alternative could be to use
eprintln!, but including this in the panic message felt nicely consistent with how failing test assertions would be reported.Let me know what you think!