From b1d60bc076f1ab84fa5b75e1bc34ec3fac3d169d Mon Sep 17 00:00:00 2001
From: Pietro Albini <pietro.albini@ferrous-systems.com>
Date: Fri, 26 May 2023 14:49:38 +0200
Subject: [PATCH] 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.
---
library/test/src/lib.rs | 71 ++++++++++++++++-----------------------
library/test/src/types.rs | 62 ++++++++++++++++++++++++++++++++++
2 files changed, 91 insertions(+), 42 deletions(-)
diff --git a/library/test/src/lib.rs b/library/test/src/lib.rs
index e76d6716b94..ca8000e9964 100644
--- a/library/test/src/lib.rs
+++ b/library/test/src/lib.rs
@@ -178,11 +178,17 @@ pub fn test_main_static_abort(tests: &[&TestDescAndFn]) {
.next()
.unwrap_or_else(|| panic!("couldn't find a test with the provided name '{name}'"));
let TestDescAndFn { desc, testfn } = test;
- let testfn = match testfn {
- StaticTestFn(f) => f,
- _ => panic!("only static tests are supported"),
- };
- run_test_in_spawned_subprocess(desc, Box::new(testfn));
+ match testfn.into_runnable() {
+ Runnable::Test(runnable_test) => {
+ if runnable_test.is_dynamic() {
+ panic!("only static tests are supported");
+ }
+ 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<_>>();
@@ -563,7 +569,7 @@ pub fn run_test(
id: TestId,
desc: TestDesc,
monitor_ch: Sender<CompletedTest>,
- testfn: Box<dyn FnOnce() -> Result<(), String> + Send>,
+ runnable_test: RunnableTest,
opts: TestRunOpts,
) -> Option<thread::JoinHandle<()>> {
let name = desc.name.clone();
@@ -574,7 +580,7 @@ pub fn run_test(
desc,
opts.nocapture,
opts.time.is_some(),
- testfn,
+ runnable_test,
monitor_ch,
opts.time,
),
@@ -615,37 +621,21 @@ pub fn run_test(
let test_run_opts =
TestRunOpts { strategy, nocapture: opts.nocapture, time: opts.time_options };
- match testfn {
- DynBenchFn(benchfn) => {
+ match testfn.into_runnable() {
+ 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.
- crate::bench::benchmark(id, desc, monitor_ch, opts.nocapture, benchfn);
+ runnable_bench.run(id, &desc, &monitor_ch, opts.nocapture);
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,
nocapture: bool,
report_time: bool,
- testfn: Box<dyn FnOnce() -> Result<(), String> + Send>,
+ runnable_test: RunnableTest,
monitor_ch: Sender<CompletedTest>,
time_opts: Option<time::TestTimeOptions>,
) {
@@ -675,7 +665,7 @@ fn run_test_in_process(
}
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 duration = start.elapsed();
TestExecTime(duration)
@@ -760,10 +750,7 @@ fn spawn_test_subprocess(
monitor_ch.send(message).unwrap();
}
-fn run_test_in_spawned_subprocess(
- desc: TestDesc,
- testfn: Box<dyn FnOnce() -> Result<(), String> + Send>,
-) -> ! {
+fn run_test_in_spawned_subprocess(desc: TestDesc, runnable_test: RunnableTest) -> ! {
let builtin_panic_hook = panic::take_hook();
let record_result = Arc::new(move |panic_info: Option<&'_ PanicInfo<'_>>| {
let test_result = match panic_info {
@@ -789,7 +776,7 @@ fn run_test_in_spawned_subprocess(
});
let record_result2 = record_result.clone();
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);
}
record_result(None);
diff --git a/library/test/src/types.rs b/library/test/src/types.rs
index e79914dbf4b..b67880a98ac 100644
--- a/library/test/src/types.rs
+++ b/library/test/src/types.rs
@@ -2,8 +2,11 @@
use std::borrow::Cow;
use std::fmt;
+use std::sync::mpsc::Sender;
+use super::__rust_begin_short_backtrace;
use super::bench::Bencher;
+use super::event::CompletedTest;
use super::options;
pub use NamePadding::*;
@@ -95,6 +98,15 @@ impl TestFn {
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 {
@@ -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.
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
pub struct TestId(pub usize);