mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Rollup merge of #103681 - RalfJung:libtest-thread, r=thomcc

Browse Source 
libtest: run all tests in their own thread, if supported by the host

This reverts the threading changes of https://github.com/rust-lang/rust/pull/56243, which made it so that with `-j1`, the test harness does not spawn any threads. Those changes were done to enable Miri to run the test harness, but Miri supports threads nowadays, so this is no longer needed. Using a thread for each test is useful because the thread's name can be set to the test's name which makes panic messages consistent between `-j1` and `-j2` runs and also a bit more readable.

I did not revert the HashMap changes of https://github.com/rust-lang/rust/pull/56243; using a deterministic map seems fine for the test harness and the more deterministic testing is the better.

Fixes https://github.com/rust-lang/rust/issues/59122
Fixes https://github.com/rust-lang/rust/issues/70492
This commit is contained in:
Matthias Krüger 2022-11-04 18:52:26 +01:00 committed by GitHub
commit c38ee06b62
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 42 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

58
library/test/src/lib.rs
View File

@ -40,7 +40,7 @@ pub mod test {
cli::{parse_opts, TestOpts},
filter_tests,
helpers::metrics::{Metric, MetricMap},
options::{Concurrent, Options, RunIgnored, RunStrategy, ShouldPanic},
options::{Options, RunIgnored, RunStrategy, ShouldPanic},
run_test, test_main, test_main_static,
test_result::{TestResult, TrFailed, TrFailedMsg, TrIgnored, TrOk},
time::{TestExecTime, TestTimeOptions},
@ -85,7 +85,7 @@ pub mod test {
use helpers::concurrency::get_concurrency;
use helpers::exit_code::get_exit_code;
use helpers::shuffle::{get_shuffle_seed, shuffle_tests};
use options::{Concurrent, RunStrategy};
use options::RunStrategy;
use test_result::*;
use time::TestExecTime;
@ -267,6 +267,19 @@ struct RunningTest {
join_handle: Option<thread::JoinHandle<()>>,
}
impl RunningTest {
fn join(self, completed_test: &mut CompletedTest) {
if let Some(join_handle) = self.join_handle {
if let Err(_) = join_handle.join() {
if let TrOk = completed_test.result {
completed_test.result =
TrFailedMsg("panicked after reporting success".to_string());
}
}
}
}
}
// Use a deterministic hasher
type TestMap =
HashMap<TestId, RunningTest, BuildHasherDefault<collections::hash_map::DefaultHasher>>;
@ -366,10 +379,10 @@ fn calc_timeout(timeout_queue: &VecDeque<TimeoutEntry>) -> Option<Duration> {
let (id, test) = remaining.pop_front().unwrap();
let event = TestEvent::TeWait(test.desc.clone());
notify_about_test_event(event)?;
let join_handle =
run_test(opts, !opts.run_tests, id, test, run_strategy, tx.clone(), Concurrent::No);
assert!(join_handle.is_none());
let completed_test = rx.recv().unwrap();
let join_handle = run_test(opts, !opts.run_tests, id, test, run_strategy, tx.clone());
// Wait for the test to complete.
let mut completed_test = rx.recv().unwrap();
RunningTest { join_handle }.join(&mut completed_test);
let event = TestEvent::TeResult(completed_test);
notify_about_test_event(event)?;
@ -383,15 +396,8 @@ fn calc_timeout(timeout_queue: &VecDeque<TimeoutEntry>) -> Option<Duration> {
let event = TestEvent::TeWait(desc.clone());
notify_about_test_event(event)?; //here no pad
let join_handle = run_test(
opts,
!opts.run_tests,
id,
test,
run_strategy,
tx.clone(),
Concurrent::Yes,
);
let join_handle =
run_test(opts, !opts.run_tests, id, test, run_strategy, tx.clone());
running_tests.insert(id, RunningTest { join_handle });
timeout_queue.push_back(TimeoutEntry { id, desc, timeout });
pending += 1;
@ -423,14 +429,7 @@ fn calc_timeout(timeout_queue: &VecDeque<TimeoutEntry>) -> Option<Duration> {
let mut completed_test = res.unwrap();
let running_test = running_tests.remove(&completed_test.id).unwrap();
if let Some(join_handle) = running_test.join_handle {
if let Err(_) = join_handle.join() {
if let TrOk = completed_test.result {
completed_test.result =
TrFailedMsg("panicked after reporting success".to_string());
}
}
}
running_test.join(&mut completed_test);
let event = TestEvent::TeResult(completed_test);
notify_about_test_event(event)?;
@ -443,8 +442,10 @@ fn calc_timeout(timeout_queue: &VecDeque<TimeoutEntry>) -> Option<Duration> {
for (id, b) in filtered.benchs {
let event = TestEvent::TeWait(b.desc.clone());
notify_about_test_event(event)?;
run_test(opts, false, id, b, run_strategy, tx.clone(), Concurrent::No);
let completed_test = rx.recv().unwrap();
let join_handle = run_test(opts, false, id, b, run_strategy, tx.clone());
// Wait for the test to complete.
let mut completed_test = rx.recv().unwrap();
RunningTest { join_handle }.join(&mut completed_test);
let event = TestEvent::TeResult(completed_test);
notify_about_test_event(event)?;
@ -520,7 +521,6 @@ pub fn run_test(
test: TestDescAndFn,
strategy: RunStrategy,
monitor_ch: Sender<CompletedTest>,
concurrency: Concurrent,
) -> Option<thread::JoinHandle<()>> {
let TestDescAndFn { desc, testfn } = test;
@ -538,7 +538,6 @@ pub fn run_test(
struct TestRunOpts {
pub strategy: RunStrategy,
pub nocapture: bool,
pub concurrency: Concurrent,
pub time: Option<time::TestTimeOptions>,
}
@ -549,7 +548,6 @@ fn run_test_inner(
testfn: Box<dyn FnOnce() -> Result<(), String> + Send>,
opts: TestRunOpts,
) -> Option<thread::JoinHandle<()>> {
let concurrency = opts.concurrency;
let name = desc.name.clone();
let runtest = move || match opts.strategy {
@ -576,7 +574,7 @@ fn run_test_inner(
// the test synchronously, regardless of the concurrency
// level.
let supports_threads = !cfg!(target_os = "emscripten") && !cfg!(target_family = "wasm");
if concurrency == Concurrent::Yes && supports_threads {
if supports_threads {
let cfg = thread::Builder::new().name(name.as_slice().to_owned());
let mut runtest = Arc::new(Mutex::new(Some(runtest)));
let runtest2 = runtest.clone();
@ -597,7 +595,7 @@ fn run_test_inner(
}
let test_run_opts =
TestRunOpts { strategy, nocapture: opts.nocapture, concurrency, time: opts.time_options };
TestRunOpts { strategy, nocapture: opts.nocapture, time: opts.time_options };
match testfn {
DynBenchFn(benchfn) => {

7
library/test/src/options.rs
View File

@ -1,12 +1,5 @@
//! Enums denoting options for test execution.
/// Whether to execute tests concurrently or not
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub enum Concurrent {
Yes,
No,
}
/// Number of times to run a benchmarked function
#[derive(Clone, PartialEq, Eq)]
pub enum BenchMode {

26
library/test/src/tests.rs
View File

@ -102,7 +102,7 @@ fn f() -> Result<(), String> {
testfn: DynTestFn(Box::new(f)),
};
let (tx, rx) = channel();
run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx, Concurrent::No);
run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx);
let result = rx.recv().unwrap().result;
assert_ne!(result, TrOk);
}
@ -125,7 +125,7 @@ fn f() -> Result<(), String> {
testfn: DynTestFn(Box::new(f)),
};
let (tx, rx) = channel();
run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx, Concurrent::No);
run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx);
let result = rx.recv().unwrap().result;
assert_eq!(result, TrIgnored);
}
@ -150,7 +150,7 @@ fn f() -> Result<(), String> {
testfn: DynTestFn(Box::new(f)),
};
let (tx, rx) = channel();
run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx, Concurrent::No);
run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx);
let result = rx.recv().unwrap().result;
assert_eq!(result, TrOk);
}
@ -175,7 +175,7 @@ fn f() -> Result<(), String> {
testfn: DynTestFn(Box::new(f)),
};
let (tx, rx) = channel();
run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx, Concurrent::No);
run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx);
let result = rx.recv().unwrap().result;
assert_eq!(result, TrOk);
}
@ -205,7 +205,7 @@ fn f() -> Result<(), String> {
testfn: DynTestFn(Box::new(f)),
};
let (tx, rx) = channel();
run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx, Concurrent::No);
run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx);
let result = rx.recv().unwrap().result;
assert_eq!(result, TrFailedMsg(failed_msg.to_string()));
}
@ -239,7 +239,7 @@ fn f() -> Result<(), String> {
testfn: DynTestFn(Box::new(f)),
};
let (tx, rx) = channel();
run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx, Concurrent::No);
run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx);
let result = rx.recv().unwrap().result;
assert_eq!(result, TrFailedMsg(failed_msg));
}
@ -267,15 +267,7 @@ fn f() -> Result<(), String> {
testfn: DynTestFn(Box::new(f)),
};
let (tx, rx) = channel();
run_test(
&TestOpts::new(),
false,
TestId(0),
desc,
RunStrategy::InProcess,
tx,
Concurrent::No,
);
run_test(&TestOpts::new(), false, TestId(0), desc, RunStrategy::InProcess, tx);
let result = rx.recv().unwrap().result;
assert_eq!(
result,
@ -306,7 +298,7 @@ fn f() -> Result<(), String> {
let test_opts = TestOpts { time_options, ..TestOpts::new() };
let (tx, rx) = channel();
run_test(&test_opts, false, TestId(0), desc, RunStrategy::InProcess, tx, Concurrent::No);
run_test(&test_opts, false, TestId(0), desc, RunStrategy::InProcess, tx);
let exec_time = rx.recv().unwrap().exec_time;
exec_time
}
@ -345,7 +337,7 @@ fn f() -> Result<(), String> {
let test_opts = TestOpts { time_options: Some(time_options), ..TestOpts::new() };
let (tx, rx) = channel();
run_test(&test_opts, false, TestId(0), desc, RunStrategy::InProcess, tx, Concurrent::No);
run_test(&test_opts, false, TestId(0), desc, RunStrategy::InProcess, tx);
let result = rx.recv().unwrap().result;
result

2
src/test/run-make-fulldeps/libtest-json/output-default.json
View File

@ -2,7 +2,7 @@
{ "type": "test", "event": "started", "name": "a" }
{ "type": "test", "name": "a", "event": "ok" }
{ "type": "test", "event": "started", "name": "b" }
{ "type": "test", "name": "b", "event": "failed", "stdout": "thread 'main' panicked at 'assertion failed: false', f.rs:9:5\nnote: run with `RUST_BACKTRACE=1` environment variable to display a backtrace\n" }
{ "type": "test", "name": "b", "event": "failed", "stdout": "thread 'b' panicked at 'assertion failed: false', f.rs:9:5\nnote: run with `RUST_BACKTRACE=1` environment variable to display a backtrace\n" }
{ "type": "test", "event": "started", "name": "c" }
{ "type": "test", "name": "c", "event": "ok" }
{ "type": "test", "event": "started", "name": "d" }

4
src/test/run-make-fulldeps/libtest-json/output-stdout-success.json
View File

@ -2,9 +2,9 @@
{ "type": "test", "event": "started", "name": "a" }
{ "type": "test", "name": "a", "event": "ok", "stdout": "print from successful test\n" }
{ "type": "test", "event": "started", "name": "b" }
{ "type": "test", "name": "b", "event": "failed", "stdout": "thread 'main' panicked at 'assertion failed: false', f.rs:9:5\nnote: run with `RUST_BACKTRACE=1` environment variable to display a backtrace\n" }
{ "type": "test", "name": "b", "event": "failed", "stdout": "thread 'b' panicked at 'assertion failed: false', f.rs:9:5\nnote: run with `RUST_BACKTRACE=1` environment variable to display a backtrace\n" }
{ "type": "test", "event": "started", "name": "c" }
{ "type": "test", "name": "c", "event": "ok", "stdout": "thread 'main' panicked at 'assertion failed: false', f.rs:15:5\n" }
{ "type": "test", "name": "c", "event": "ok", "stdout": "thread 'c' panicked at 'assertion failed: false', f.rs:15:5\n" }
{ "type": "test", "event": "started", "name": "d" }
{ "type": "test", "name": "d", "event": "ignored", "message": "msg" }
{ "type": "suite", "event": "failed", "passed": 2, "failed": 1, "ignored": 1, "measured": 0, "filtered_out": 0, "exec_time": $TIME }

2
src/test/ui/test-attrs/test-thread-capture.run.stdout
View File

@ -10,7 +10,7 @@ fee
fie
foe
fum
thread 'main' panicked at 'explicit panic', $DIR/test-thread-capture.rs:32:5
thread 'thready_fail' panicked at 'explicit panic', $DIR/test-thread-capture.rs:32:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

2
src/test/ui/test-attrs/test-thread-nocapture.run.stderr
View File

@ -1,2 +1,2 @@
thread 'main' panicked at 'explicit panic', $DIR/test-thread-nocapture.rs:32:5
thread 'thready_fail' panicked at 'explicit panic', $DIR/test-thread-nocapture.rs:32:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace