Less confusing message for failed test returning Result

[printercu]

Failure message contained a part that is related to assert_eq! matcher.
This is confusing in the cases when test fails because of Err result.

Test:

#[test]
fn test() -> Result<(), &'static str> {
    Err("some failure")?;
    assert_eq!(1, 2);
    Ok(())
}

Before:

---- tests::test stdout ----
Error: "some failure"
thread 'tests::test' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `0`: the test returned a termination value with a non-zero status code (1) which indicates a failure', /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/test/src/lib.rs:187:5

After:

 ---- tests::test stdout ----
Error: "some failure"
thread 'tests::test' panicked at 'the test returned a termination value with a non-zero status code (1) which indicates a failure', /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c/library/test/src/lib.rs:187:5

Fixes #69517

[rust-highfive]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs to request review from a libs-api team reviewer. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @m-ou-se (or someone else) soon.

Please see the contribution instructions for more information.

[ehuss]

This may close #69517.

[Dylan-DPC]

assert_eq also accepts an error message

[printercu]

It was assert_eq with error message. It results in confusing output, please see examples in initial comment or diff's comment

[ssomers]

The assert_eq here doesn't help, because the message already reports the code value, if it needs to be reported at all. Instead, it harms as shown in the description, in #69517 and in #59263.

In fact the fix was already approved as #59263 but eventually landed in an outdated repository.

[Mark-Simulacrum]

@bors r+

[bors]

📌 Commit b4cc991 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

[klensy]

And without test and/or comment, this can be accidentally changed back.

[Mark-Simulacrum]

@bors r-

Good call! We can add a test pretty easily along the lines of src/test/ui/test-attrs/test-panic-abort-nocapture.rs, which runs some tests and captures the stdout in src/test/ui/test-attrs/test-panic-abort-nocapture.run.stdout.

(Obviously, we don't need the panic=abort-ness for this test).

[bors]

☔ The latest upstream changes (presumably #102586) made this pull request unmergeable. Please resolve the merge conflicts.

[bobhy]

It seems that this PR was unmergable (by the time it was accepted) and the issue was simply closed. Would a new PR allow the issue to be reopened?

The original problem is that test functions cannot report meaningful errors if they return a Err(E) from a ? operator (but can if same line uses .unwrap() instead), agreed?
That does force developers to use non-idiomatic syntax, but only in tests. That's kind of hard on our poor little brains. So, I would be glad to take a crack at a PR to fix this if it could be reviewed soonish.

[metasim]

@printercu @Mark-Simulacrum Would you consider re-submitting this?

[printercu]

@metasim #100451 made this pr obsolete.

[printercu]

@bobhy As I understand tests will try to print error into stderr if it is fmt::Debug (https://doc.rust-lang.org/src/std/process.rs.html#2211). I'm not sure if it's possible to display any arbitrary type in the same way.

[cole-abbeduto-particle]

@bobhy As I understand tests will try to print error into stderr if it is fmt::Debug (https://doc.rust-lang.org/src/std/process.rs.html#2211). I'm not sure if it's possible to display any arbitrary type in the same way.

That is true and a slight improvement. But we've also lost the line number in the process.

unwrap:

---- test::will_this_fail stdout ----
thread 'test::will_this_fail' panicked at 'called `Result::unwrap()` on an `Err` value: Is this a failure', /src/bin/file.rs:179:65
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

return Err or ?:

---- test::will_this_fail stdout ----
Error: Is this a failure?