refactor executing tests to centralize actually invoking tests

Before this commit, tests were invoked in multiple places, especially
due to `-Z panic-abort-tests`, and adding a new test kind meant having
to chase down and update all these places.

This commit creates a new Runnable enum, and its children RunnableTest
and RunnableBench. The rest of the harness will now pass around the enum
rather than constructing and passing around boxed functions. The enum
has two children enums because invoking tests and invoking benchmarks
requires different parameters.
This commit is contained in:
Pietro Albini 2023-05-26 14:49:38 +02:00
parent 776f22292f
commit b1d60bc076
No known key found for this signature in database
GPG Key ID: CD76B35F7734769E
2 changed files with 91 additions and 42 deletions

View File

@ -178,11 +178,17 @@ pub fn test_main_static_abort(tests: &[&TestDescAndFn]) {
.next() .next()
.unwrap_or_else(|| panic!("couldn't find a test with the provided name '{name}'")); .unwrap_or_else(|| panic!("couldn't find a test with the provided name '{name}'"));
let TestDescAndFn { desc, testfn } = test; let TestDescAndFn { desc, testfn } = test;
let testfn = match testfn { match testfn.into_runnable() {
StaticTestFn(f) => f, Runnable::Test(runnable_test) => {
_ => panic!("only static tests are supported"), if runnable_test.is_dynamic() {
}; panic!("only static tests are supported");
run_test_in_spawned_subprocess(desc, Box::new(testfn)); }
run_test_in_spawned_subprocess(desc, runnable_test);
}
Runnable::Bench(_) => {
panic!("benchmarks should not be executed into child processes")
}
}
} }
let args = env::args().collect::<Vec<_>>(); let args = env::args().collect::<Vec<_>>();
@ -563,7 +569,7 @@ pub fn run_test(
id: TestId, id: TestId,
desc: TestDesc, desc: TestDesc,
monitor_ch: Sender<CompletedTest>, monitor_ch: Sender<CompletedTest>,
testfn: Box<dyn FnOnce() -> Result<(), String> + Send>, runnable_test: RunnableTest,
opts: TestRunOpts, opts: TestRunOpts,
) -> Option<thread::JoinHandle<()>> { ) -> Option<thread::JoinHandle<()>> {
let name = desc.name.clone(); let name = desc.name.clone();
@ -574,7 +580,7 @@ pub fn run_test(
desc, desc,
opts.nocapture, opts.nocapture,
opts.time.is_some(), opts.time.is_some(),
testfn, runnable_test,
monitor_ch, monitor_ch,
opts.time, opts.time,
), ),
@ -615,37 +621,21 @@ pub fn run_test(
let test_run_opts = let test_run_opts =
TestRunOpts { strategy, nocapture: opts.nocapture, time: opts.time_options }; TestRunOpts { strategy, nocapture: opts.nocapture, time: opts.time_options };
match testfn { match testfn.into_runnable() {
DynBenchFn(benchfn) => { Runnable::Test(runnable_test) => {
if runnable_test.is_dynamic() {
match strategy {
RunStrategy::InProcess => (),
_ => panic!("Cannot run dynamic test fn out-of-process"),
};
}
run_test_inner(id, desc, monitor_ch, runnable_test, test_run_opts)
}
Runnable::Bench(runnable_bench) => {
// Benchmarks aren't expected to panic, so we run them all in-process. // Benchmarks aren't expected to panic, so we run them all in-process.
crate::bench::benchmark(id, desc, monitor_ch, opts.nocapture, benchfn); runnable_bench.run(id, &desc, &monitor_ch, opts.nocapture);
None None
} }
StaticBenchFn(benchfn) => {
// Benchmarks aren't expected to panic, so we run them all in-process.
crate::bench::benchmark(id, desc, monitor_ch, opts.nocapture, benchfn);
None
}
DynTestFn(f) => {
match strategy {
RunStrategy::InProcess => (),
_ => panic!("Cannot run dynamic test fn out-of-process"),
};
run_test_inner(
id,
desc,
monitor_ch,
Box::new(move || __rust_begin_short_backtrace(f)),
test_run_opts,
)
}
StaticTestFn(f) => run_test_inner(
id,
desc,
monitor_ch,
Box::new(move || __rust_begin_short_backtrace(f)),
test_run_opts,
),
} }
} }
@ -663,7 +653,7 @@ fn run_test_in_process(
desc: TestDesc, desc: TestDesc,
nocapture: bool, nocapture: bool,
report_time: bool, report_time: bool,
testfn: Box<dyn FnOnce() -> Result<(), String> + Send>, runnable_test: RunnableTest,
monitor_ch: Sender<CompletedTest>, monitor_ch: Sender<CompletedTest>,
time_opts: Option<time::TestTimeOptions>, time_opts: Option<time::TestTimeOptions>,
) { ) {
@ -675,7 +665,7 @@ fn run_test_in_process(
} }
let start = report_time.then(Instant::now); let start = report_time.then(Instant::now);
let result = fold_err(catch_unwind(AssertUnwindSafe(testfn))); let result = fold_err(catch_unwind(AssertUnwindSafe(|| runnable_test.run())));
let exec_time = start.map(|start| { let exec_time = start.map(|start| {
let duration = start.elapsed(); let duration = start.elapsed();
TestExecTime(duration) TestExecTime(duration)
@ -760,10 +750,7 @@ fn spawn_test_subprocess(
monitor_ch.send(message).unwrap(); monitor_ch.send(message).unwrap();
} }
fn run_test_in_spawned_subprocess( fn run_test_in_spawned_subprocess(desc: TestDesc, runnable_test: RunnableTest) -> ! {
desc: TestDesc,
testfn: Box<dyn FnOnce() -> Result<(), String> + Send>,
) -> ! {
let builtin_panic_hook = panic::take_hook(); let builtin_panic_hook = panic::take_hook();
let record_result = Arc::new(move |panic_info: Option<&'_ PanicInfo<'_>>| { let record_result = Arc::new(move |panic_info: Option<&'_ PanicInfo<'_>>| {
let test_result = match panic_info { let test_result = match panic_info {
@ -789,7 +776,7 @@ fn run_test_in_spawned_subprocess(
}); });
let record_result2 = record_result.clone(); let record_result2 = record_result.clone();
panic::set_hook(Box::new(move |info| record_result2(Some(info)))); panic::set_hook(Box::new(move |info| record_result2(Some(info))));
if let Err(message) = testfn() { if let Err(message) = runnable_test.run() {
panic!("{}", message); panic!("{}", message);
} }
record_result(None); record_result(None);

View File

@ -2,8 +2,11 @@
use std::borrow::Cow; use std::borrow::Cow;
use std::fmt; use std::fmt;
use std::sync::mpsc::Sender;
use super::__rust_begin_short_backtrace;
use super::bench::Bencher; use super::bench::Bencher;
use super::event::CompletedTest;
use super::options; use super::options;
pub use NamePadding::*; pub use NamePadding::*;
@ -95,6 +98,15 @@ impl TestFn {
DynBenchFn(..) => PadOnRight, DynBenchFn(..) => PadOnRight,
} }
} }
pub(crate) fn into_runnable(self) -> Runnable {
match self {
StaticTestFn(f) => Runnable::Test(RunnableTest::Static(f)),
StaticBenchFn(f) => Runnable::Bench(RunnableBench::Static(f)),
DynTestFn(f) => Runnable::Test(RunnableTest::Dynamic(f)),
DynBenchFn(f) => Runnable::Bench(RunnableBench::Dynamic(f)),
}
}
} }
impl fmt::Debug for TestFn { impl fmt::Debug for TestFn {
@ -108,6 +120,56 @@ impl fmt::Debug for TestFn {
} }
} }
pub(crate) enum Runnable {
Test(RunnableTest),
Bench(RunnableBench),
}
pub(crate) enum RunnableTest {
Static(fn() -> Result<(), String>),
Dynamic(Box<dyn FnOnce() -> Result<(), String> + Send>),
}
impl RunnableTest {
pub(crate) fn run(self) -> Result<(), String> {
match self {
RunnableTest::Static(f) => __rust_begin_short_backtrace(f),
RunnableTest::Dynamic(f) => __rust_begin_short_backtrace(f),
}
}
pub(crate) fn is_dynamic(&self) -> bool {
match self {
RunnableTest::Static(_) => false,
RunnableTest::Dynamic(_) => true,
}
}
}
pub(crate) enum RunnableBench {
Static(fn(&mut Bencher) -> Result<(), String>),
Dynamic(Box<dyn Fn(&mut Bencher) -> Result<(), String> + Send>),
}
impl RunnableBench {
pub(crate) fn run(
self,
id: TestId,
desc: &TestDesc,
monitor_ch: &Sender<CompletedTest>,
nocapture: bool,
) {
match self {
RunnableBench::Static(f) => {
crate::bench::benchmark(id, desc.clone(), monitor_ch.clone(), nocapture, f)
}
RunnableBench::Dynamic(f) => {
crate::bench::benchmark(id, desc.clone(), monitor_ch.clone(), nocapture, f)
}
}
}
}
// A unique integer associated with each test. // A unique integer associated with each test.
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
pub struct TestId(pub usize); pub struct TestId(pub usize);