Skip to content
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

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

Merged
merged 3 commits into from
Dec 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 20 additions & 12 deletions src/libtest/test_result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())) => {
Expand Down
47 changes: 39 additions & 8 deletions src/libtest/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use crate::{
// TestType, TrFailedMsg, TrIgnored, TrOk,
},
};
use std::any::TypeId;
use std::sync::mpsc::channel;
use std::time::Duration;

Expand Down Expand Up @@ -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]
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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"),
Expand All @@ -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)
Expand All @@ -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> {
Expand Down Expand Up @@ -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());
}
}

Expand Down