Rollup merge of #66503 - thomasetter:panic-error-msg, r=joshtriplett

More useful test error messages on should_panic(expected=...) mismatch

Fixes  #66304
r? @gilescope

Shows both the actual as well as the expected panic value when a test with `should_panic(expected=...)` fails.
This makes `should_panic` more consistent with `assert_eq`.

I am not sure whether printing the `Any::type_id()` is useful, is there something better that we could print for non-string panic values?
This commit is contained in:
Mazdak Farrokhzad 2019-12-01 04:49:19 +01:00 committed by GitHub
commit 6110d3ebc8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 59 additions and 20 deletions

View file

@ -37,22 +37,30 @@ pub fn calc_result<'a>(
let result = match (&desc.should_panic, task_result) {
(&ShouldPanic::No, Ok(())) | (&ShouldPanic::Yes, Err(_)) => TestResult::TrOk,
(&ShouldPanic::YesWithMessage(msg), Err(ref err)) => {
if err
let maybe_panic_str = err
.downcast_ref::<String>()
.map(|e| &**e)
.or_else(|| err.downcast_ref::<&'static str>().map(|e| *e))
.map(|e| e.contains(msg))
.unwrap_or(false)
{
.or_else(|| err.downcast_ref::<&'static str>().map(|e| *e));
if maybe_panic_str.map(|e| e.contains(msg)).unwrap_or(false) {
TestResult::TrOk
} else if desc.allow_fail {
TestResult::TrAllowedFail
} else if let Some(panic_str) = maybe_panic_str {
TestResult::TrFailedMsg(format!(
r#"panic did not contain expected string
panic message: `{:?}`,
expected substring: `{:?}`"#,
panic_str, msg
))
} else {
if desc.allow_fail {
TestResult::TrAllowedFail
} else {
TestResult::TrFailedMsg(
format!("panic did not include expected string '{}'", msg)
)
}
TestResult::TrFailedMsg(format!(
r#"expected panic with string value,
found non-string value: `{:?}`
expected substring: `{:?}`"#,
(**err).type_id(),
msg
))
}
}
(&ShouldPanic::Yes, Ok(())) => {

View file

@ -15,6 +15,7 @@ use crate::{
// TestType, TrFailedMsg, TrIgnored, TrOk,
},
};
use std::any::TypeId;
use std::sync::mpsc::channel;
use std::time::Duration;
@ -84,7 +85,7 @@ pub fn do_not_run_ignored_tests() {
let (tx, rx) = channel();
run_test(&TestOpts::new(), false, desc, RunStrategy::InProcess, tx, Concurrent::No);
let result = rx.recv().unwrap().result;
assert!(result != TrOk);
assert_ne!(result, TrOk);
}
#[test]
@ -103,7 +104,7 @@ pub fn ignored_tests_result_in_ignored() {
let (tx, rx) = channel();
run_test(&TestOpts::new(), false, desc, RunStrategy::InProcess, tx, Concurrent::No);
let result = rx.recv().unwrap().result;
assert!(result == TrIgnored);
assert_eq!(result, TrIgnored);
}
// FIXME: Re-enable emscripten once it can catch panics again (introduced by #65251)
@ -126,7 +127,7 @@ fn test_should_panic() {
let (tx, rx) = channel();
run_test(&TestOpts::new(), false, desc, RunStrategy::InProcess, tx, Concurrent::No);
let result = rx.recv().unwrap().result;
assert!(result == TrOk);
assert_eq!(result, TrOk);
}
// FIXME: Re-enable emscripten once it can catch panics again (introduced by #65251)
@ -149,7 +150,7 @@ fn test_should_panic_good_message() {
let (tx, rx) = channel();
run_test(&TestOpts::new(), false, desc, RunStrategy::InProcess, tx, Concurrent::No);
let result = rx.recv().unwrap().result;
assert!(result == TrOk);
assert_eq!(result, TrOk);
}
// FIXME: Re-enable emscripten once it can catch panics again (introduced by #65251)
@ -161,7 +162,9 @@ fn test_should_panic_bad_message() {
panic!("an error message");
}
let expected = "foobar";
let failed_msg = "panic did not include expected string";
let failed_msg = r#"panic did not contain expected string
panic message: `"an error message"`,
expected substring: `"foobar"`"#;
let desc = TestDescAndFn {
desc: TestDesc {
name: StaticTestName("whatever"),
@ -175,7 +178,35 @@ fn test_should_panic_bad_message() {
let (tx, rx) = channel();
run_test(&TestOpts::new(), false, desc, RunStrategy::InProcess, tx, Concurrent::No);
let result = rx.recv().unwrap().result;
assert!(result == TrFailedMsg(format!("{} '{}'", failed_msg, expected)));
assert_eq!(result, TrFailedMsg(failed_msg.to_string()));
}
// FIXME: Re-enable emscripten once it can catch panics again (introduced by #65251)
#[test]
#[cfg(not(target_os = "emscripten"))]
fn test_should_panic_non_string_message_type() {
use crate::tests::TrFailedMsg;
fn f() {
panic!(1i32);
}
let expected = "foobar";
let failed_msg = format!(r#"expected panic with string value,
found non-string value: `{:?}`
expected substring: `"foobar"`"#, TypeId::of::<i32>());
let desc = TestDescAndFn {
desc: TestDesc {
name: StaticTestName("whatever"),
ignore: false,
should_panic: ShouldPanic::YesWithMessage(expected),
allow_fail: false,
test_type: TestType::Unknown,
},
testfn: DynTestFn(Box::new(f)),
};
let (tx, rx) = channel();
run_test(&TestOpts::new(), false, desc, RunStrategy::InProcess, tx, Concurrent::No);
let result = rx.recv().unwrap().result;
assert_eq!(result, TrFailedMsg(failed_msg));
}
// FIXME: Re-enable emscripten once it can catch panics again (introduced by #65251)
@ -196,7 +227,7 @@ fn test_should_panic_but_succeeds() {
let (tx, rx) = channel();
run_test(&TestOpts::new(), false, desc, RunStrategy::InProcess, tx, Concurrent::No);
let result = rx.recv().unwrap().result;
assert!(result == TrFailedMsg("test did not panic as expected".to_string()));
assert_eq!(result, TrFailedMsg("test did not panic as expected".to_string()));
}
fn report_time_test_template(report_time: bool) -> Option<TestExecTime> {
@ -570,7 +601,7 @@ pub fn sort_tests() {
];
for (a, b) in expected.iter().zip(filtered) {
assert!(*a == b.desc.name.to_string());
assert_eq!(*a, b.desc.name.to_string());
}
}