Skip to content

Do not panic when a test function returns Result::Err. #100451

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

Merged
merged 1 commit into from
Oct 2, 2022
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
21 changes: 11 additions & 10 deletions library/test/src/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ impl Bencher {
self.summary = Some(iter(&mut inner));
}

pub fn bench<F>(&mut self, mut f: F) -> Option<stats::Summary>
pub fn bench<F>(&mut self, mut f: F) -> Result<Option<stats::Summary>, String>
where
F: FnMut(&mut Bencher),
F: FnMut(&mut Bencher) -> Result<(), String>,
{
f(self);
self.summary
let result = f(self);
result.map(|_| self.summary)
}
}

Expand Down Expand Up @@ -195,7 +195,7 @@ pub fn benchmark<F>(
nocapture: bool,
f: F,
) where
F: FnMut(&mut Bencher),
F: FnMut(&mut Bencher) -> Result<(), String>,
{
let mut bs = Bencher { mode: BenchMode::Auto, summary: None, bytes: 0 };

Expand All @@ -211,32 +211,33 @@ pub fn benchmark<F>(

let test_result = match result {
//bs.bench(f) {
Ok(Some(ns_iter_summ)) => {
Ok(Ok(Some(ns_iter_summ))) => {
let ns_iter = cmp::max(ns_iter_summ.median as u64, 1);
let mb_s = bs.bytes * 1000 / ns_iter;

let bs = BenchSamples { ns_iter_summ, mb_s: mb_s as usize };
TestResult::TrBench(bs)
}
Ok(None) => {
Ok(Ok(None)) => {
// iter not called, so no data.
// FIXME: error in this case?
let samples: &mut [f64] = &mut [0.0_f64; 1];
let bs = BenchSamples { ns_iter_summ: stats::Summary::new(samples), mb_s: 0 };
TestResult::TrBench(bs)
}
Err(_) => TestResult::TrFailed,
Ok(Err(_)) => TestResult::TrFailed,
};

let stdout = data.lock().unwrap().to_vec();
let message = CompletedTest::new(id, desc, test_result, None, stdout);
monitor_ch.send(message).unwrap();
}

pub fn run_once<F>(f: F)
pub fn run_once<F>(f: F) -> Result<(), String>
where
F: FnMut(&mut Bencher),
F: FnMut(&mut Bencher) -> Result<(), String>,
{
let mut bs = Bencher { mode: BenchMode::Single, summary: None, bytes: 0 };
bs.bench(f);
bs.bench(f).map(|_| ())
}
61 changes: 42 additions & 19 deletions library/test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
//! benchmarks themselves) should be done via the `#[test]` and
//! `#[bench]` attributes.
//!
//! See the [Testing Chapter](../book/ch11-00-testing.html) of the book for more details.
//! See the [Testing Chapter](../book/ch11-00-testing.html) of the book for more
//! details.

// Currently, not much of this is meant for users. It is intended to
// support the simplest interface possible for representing and
Expand Down Expand Up @@ -77,6 +78,7 @@ mod types;
#[cfg(test)]
mod tests;

use core::any::Any;
use event::{CompletedTest, TestEvent};
use helpers::concurrency::get_concurrency;
use helpers::exit_code::get_exit_code;
Expand Down Expand Up @@ -176,17 +178,20 @@ fn make_owned_test(test: &&TestDescAndFn) -> TestDescAndFn {
}
}

/// Invoked when unit tests terminate. Should panic if the unit
/// Tests is considered a failure. By default, invokes `report()`
/// and checks for a `0` result.
pub fn assert_test_result<T: Termination>(result: T) {
/// Invoked when unit tests terminate. Returns `Result::Err` if the test is
/// considered a failure. By default, invokes `report() and checks for a `0`
/// result.
pub fn assert_test_result<T: Termination>(result: T) -> Result<(), String> {
let code = result.report().to_i32();
assert_eq!(
code, 0,
"the test returned a termination value with a non-zero status code ({}) \
which indicates a failure",
code
);
if code == 0 {
Ok(())
} else {
Err(format!(
"the test returned a termination value with a non-zero status code \
({}) which indicates a failure",
code
))
}
}

pub fn run_tests<F>(
Expand Down Expand Up @@ -479,7 +484,7 @@ pub fn run_test(
id: TestId,
desc: TestDesc,
monitor_ch: Sender<CompletedTest>,
testfn: Box<dyn FnOnce() + Send>,
testfn: Box<dyn FnOnce() -> Result<(), String> + Send>,
opts: TestRunOpts,
) -> Option<thread::JoinHandle<()>> {
let concurrency = opts.concurrency;
Expand Down Expand Up @@ -568,19 +573,19 @@ pub fn run_test(

/// Fixed frame used to clean the backtrace with `RUST_BACKTRACE=1`.
#[inline(never)]
fn __rust_begin_short_backtrace<F: FnOnce()>(f: F) {
f();
fn __rust_begin_short_backtrace<T, F: FnOnce() -> T>(f: F) -> T {
let result = f();

// prevent this frame from being tail-call optimised away
black_box(());
black_box(result)
}

fn run_test_in_process(
id: TestId,
desc: TestDesc,
nocapture: bool,
report_time: bool,
testfn: Box<dyn FnOnce() + Send>,
testfn: Box<dyn FnOnce() -> Result<(), String> + Send>,
monitor_ch: Sender<CompletedTest>,
time_opts: Option<time::TestTimeOptions>,
) {
Expand All @@ -592,7 +597,7 @@ fn run_test_in_process(
}

let start = report_time.then(Instant::now);
let result = catch_unwind(AssertUnwindSafe(testfn));
let result = fold_err(catch_unwind(AssertUnwindSafe(testfn)));
let exec_time = start.map(|start| {
let duration = start.elapsed();
TestExecTime(duration)
Expand All @@ -609,6 +614,19 @@ fn run_test_in_process(
monitor_ch.send(message).unwrap();
}

fn fold_err<T, E>(
result: Result<Result<T, E>, Box<dyn Any + Send>>,
) -> Result<T, Box<dyn Any + Send>>
where
E: Send + 'static,
{
match result {
Ok(Err(e)) => Err(Box::new(e)),
Ok(Ok(v)) => Ok(v),
Err(e) => Err(e),
}
}

fn spawn_test_subprocess(
id: TestId,
desc: TestDesc,
Expand Down Expand Up @@ -664,7 +682,10 @@ fn spawn_test_subprocess(
monitor_ch.send(message).unwrap();
}

fn run_test_in_spawned_subprocess(desc: TestDesc, testfn: Box<dyn FnOnce() + Send>) -> ! {
fn run_test_in_spawned_subprocess(
desc: TestDesc,
testfn: Box<dyn FnOnce() -> Result<(), String> + Send>,
) -> ! {
let builtin_panic_hook = panic::take_hook();
let record_result = Arc::new(move |panic_info: Option<&'_ PanicInfo<'_>>| {
let test_result = match panic_info {
Expand All @@ -690,7 +711,9 @@ fn run_test_in_spawned_subprocess(desc: TestDesc, testfn: Box<dyn FnOnce() + Sen
});
let record_result2 = record_result.clone();
panic::set_hook(Box::new(move |info| record_result2(Some(&info))));
testfn();
if let Err(message) = testfn() {
panic!("{}", message);
}
record_result(None);
unreachable!("panic=abort callback should have exited the process")
}
Loading