Rollup merge of #89082 - smoelius:master, r=kennytm

Implement #85440 (Random test ordering)

This PR adds `--shuffle` and `--shuffle-seed` options to `libtest`. The options are similar to the [`-shuffle` option](c894b442d1/src/testing/testing.go (L1482-L1499)) that was recently added to Go.

Here are the relevant parts of the help message:
```
        --shuffle       Run tests in random order
        --shuffle-seed SEED
                        Run tests in random order; seed the random number
                        generator with SEED
...
By default, the tests are run in alphabetical order. Use --shuffle or set
RUST_TEST_SHUFFLE to run the tests in random order. Pass the generated
"shuffle seed" to --shuffle-seed (or set RUST_TEST_SHUFFLE_SEED) to run the
tests in the same order again. Note that --shuffle and --shuffle-seed do not
affect whether the tests are run in parallel.
```
Is an RFC needed for this?
This commit is contained in:
Jubilee 2021-10-07 20:26:12 -07:00 committed by GitHub
commit 37f17bca7c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 313 additions and 40 deletions

View File

@ -21,6 +21,8 @@ pub struct TestOpts {
pub nocapture: bool, pub nocapture: bool,
pub color: ColorConfig, pub color: ColorConfig,
pub format: OutputFormat, pub format: OutputFormat,
pub shuffle: bool,
pub shuffle_seed: Option<u64>,
pub test_threads: Option<usize>, pub test_threads: Option<usize>,
pub skip: Vec<String>, pub skip: Vec<String>,
pub time_options: Option<TestTimeOptions>, pub time_options: Option<TestTimeOptions>,
@ -138,6 +140,13 @@ fn optgroups() -> getopts::Options {
`CRITICAL_TIME` here means the limit that should not be exceeded by test. `CRITICAL_TIME` here means the limit that should not be exceeded by test.
", ",
)
.optflag("", "shuffle", "Run tests in random order")
.optopt(
"",
"shuffle-seed",
"Run tests in random order; seed the random number generator with SEED",
"SEED",
); );
opts opts
} }
@ -155,6 +164,12 @@ By default, all tests are run in parallel. This can be altered with the
--test-threads flag or the RUST_TEST_THREADS environment variable when running --test-threads flag or the RUST_TEST_THREADS environment variable when running
tests (set it to 1). tests (set it to 1).
By default, the tests are run in alphabetical order. Use --shuffle or set
RUST_TEST_SHUFFLE to run the tests in random order. Pass the generated
"shuffle seed" to --shuffle-seed (or set RUST_TEST_SHUFFLE_SEED) to run the
tests in the same order again. Note that --shuffle and --shuffle-seed do not
affect whether the tests are run in parallel.
All tests have their standard output and standard error captured by default. All tests have their standard output and standard error captured by default.
This can be overridden with the --nocapture flag or setting RUST_TEST_NOCAPTURE This can be overridden with the --nocapture flag or setting RUST_TEST_NOCAPTURE
environment variable to a value other than "0". Logging is not captured by default. environment variable to a value other than "0". Logging is not captured by default.
@ -218,6 +233,21 @@ macro_rules! unstable_optflag {
}}; }};
} }
// Gets the option value and checks if unstable features are enabled.
macro_rules! unstable_optopt {
($matches:ident, $allow_unstable:ident, $option_name:literal) => {{
let opt = $matches.opt_str($option_name);
if !$allow_unstable && opt.is_some() {
return Err(format!(
"The \"{}\" option is only accepted on the nightly compiler with -Z unstable-options",
$option_name
));
}
opt
}};
}
// Implementation of `parse_opts` that doesn't care about help message // Implementation of `parse_opts` that doesn't care about help message
// and returns a `Result`. // and returns a `Result`.
fn parse_opts_impl(matches: getopts::Matches) -> OptRes { fn parse_opts_impl(matches: getopts::Matches) -> OptRes {
@ -227,6 +257,8 @@ fn parse_opts_impl(matches: getopts::Matches) -> OptRes {
let force_run_in_process = unstable_optflag!(matches, allow_unstable, "force-run-in-process"); let force_run_in_process = unstable_optflag!(matches, allow_unstable, "force-run-in-process");
let exclude_should_panic = unstable_optflag!(matches, allow_unstable, "exclude-should-panic"); let exclude_should_panic = unstable_optflag!(matches, allow_unstable, "exclude-should-panic");
let time_options = get_time_options(&matches, allow_unstable)?; let time_options = get_time_options(&matches, allow_unstable)?;
let shuffle = get_shuffle(&matches, allow_unstable)?;
let shuffle_seed = get_shuffle_seed(&matches, allow_unstable)?;
let include_ignored = matches.opt_present("include-ignored"); let include_ignored = matches.opt_present("include-ignored");
let quiet = matches.opt_present("quiet"); let quiet = matches.opt_present("quiet");
@ -260,6 +292,8 @@ fn parse_opts_impl(matches: getopts::Matches) -> OptRes {
nocapture, nocapture,
color, color,
format, format,
shuffle,
shuffle_seed,
test_threads, test_threads,
skip, skip,
time_options, time_options,
@ -303,6 +337,46 @@ fn get_time_options(
Ok(options) Ok(options)
} }
fn get_shuffle(matches: &getopts::Matches, allow_unstable: bool) -> OptPartRes<bool> {
let mut shuffle = unstable_optflag!(matches, allow_unstable, "shuffle");
if !shuffle && allow_unstable {
shuffle = match env::var("RUST_TEST_SHUFFLE") {
Ok(val) => &val != "0",
Err(_) => false,
};
}
Ok(shuffle)
}
fn get_shuffle_seed(matches: &getopts::Matches, allow_unstable: bool) -> OptPartRes<Option<u64>> {
let mut shuffle_seed = match unstable_optopt!(matches, allow_unstable, "shuffle-seed") {
Some(n_str) => match n_str.parse::<u64>() {
Ok(n) => Some(n),
Err(e) => {
return Err(format!(
"argument for --shuffle-seed must be a number \
(error: {})",
e
));
}
},
None => None,
};
if shuffle_seed.is_none() && allow_unstable {
shuffle_seed = match env::var("RUST_TEST_SHUFFLE_SEED") {
Ok(val) => match val.parse::<u64>() {
Ok(n) => Some(n),
Err(_) => panic!("RUST_TEST_SHUFFLE_SEED is `{}`, should be a number.", val),
},
Err(_) => None,
};
}
Ok(shuffle_seed)
}
fn get_test_threads(matches: &getopts::Matches) -> OptPartRes<Option<usize>> { fn get_test_threads(matches: &getopts::Matches) -> OptPartRes<Option<usize>> {
let test_threads = match matches.opt_str("test-threads") { let test_threads = match matches.opt_str("test-threads") {
Some(n_str) => match n_str.parse::<usize>() { Some(n_str) => match n_str.parse::<usize>() {

View File

@ -225,9 +225,9 @@ fn on_test_event(
out: &mut dyn OutputFormatter, out: &mut dyn OutputFormatter,
) -> io::Result<()> { ) -> io::Result<()> {
match (*event).clone() { match (*event).clone() {
TestEvent::TeFiltered(ref filtered_tests) => { TestEvent::TeFiltered(ref filtered_tests, shuffle_seed) => {
st.total = filtered_tests.len(); st.total = filtered_tests.len();
out.write_run_start(filtered_tests.len())?; out.write_run_start(filtered_tests.len(), shuffle_seed)?;
} }
TestEvent::TeFilteredOut(filtered_out) => { TestEvent::TeFilteredOut(filtered_out) => {
st.filtered_out = filtered_out; st.filtered_out = filtered_out;

View File

@ -28,7 +28,7 @@ impl CompletedTest {
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
pub enum TestEvent { pub enum TestEvent {
TeFiltered(Vec<TestDesc>), TeFiltered(Vec<TestDesc>, Option<u64>),
TeWait(TestDesc), TeWait(TestDesc),
TeResult(CompletedTest), TeResult(CompletedTest),
TeTimeout(TestDesc), TeTimeout(TestDesc),

View File

@ -60,10 +60,15 @@ impl<T: Write> JsonFormatter<T> {
} }
impl<T: Write> OutputFormatter for JsonFormatter<T> { impl<T: Write> OutputFormatter for JsonFormatter<T> {
fn write_run_start(&mut self, test_count: usize) -> io::Result<()> { fn write_run_start(&mut self, test_count: usize, shuffle_seed: Option<u64>) -> io::Result<()> {
let shuffle_seed_json = if let Some(shuffle_seed) = shuffle_seed {
format!(r#", "shuffle_seed": {}"#, shuffle_seed)
} else {
String::new()
};
self.writeln_message(&*format!( self.writeln_message(&*format!(
r#"{{ "type": "suite", "event": "started", "test_count": {} }}"#, r#"{{ "type": "suite", "event": "started", "test_count": {}{} }}"#,
test_count test_count, shuffle_seed_json
)) ))
} }

View File

@ -27,7 +27,11 @@ impl<T: Write> JunitFormatter<T> {
} }
impl<T: Write> OutputFormatter for JunitFormatter<T> { impl<T: Write> OutputFormatter for JunitFormatter<T> {
fn write_run_start(&mut self, _test_count: usize) -> io::Result<()> { fn write_run_start(
&mut self,
_test_count: usize,
_shuffle_seed: Option<u64>,
) -> io::Result<()> {
// We write xml header on run start // We write xml header on run start
self.out.write_all(b"\n")?; self.out.write_all(b"\n")?;
self.write_message("<?xml version=\"1.0\" encoding=\"UTF-8\"?>") self.write_message("<?xml version=\"1.0\" encoding=\"UTF-8\"?>")

View File

@ -18,7 +18,7 @@ pub(crate) use self::pretty::PrettyFormatter;
pub(crate) use self::terse::TerseFormatter; pub(crate) use self::terse::TerseFormatter;
pub(crate) trait OutputFormatter { pub(crate) trait OutputFormatter {
fn write_run_start(&mut self, test_count: usize) -> io::Result<()>; fn write_run_start(&mut self, test_count: usize, shuffle_seed: Option<u64>) -> io::Result<()>;
fn write_test_start(&mut self, desc: &TestDesc) -> io::Result<()>; fn write_test_start(&mut self, desc: &TestDesc) -> io::Result<()>;
fn write_timeout(&mut self, desc: &TestDesc) -> io::Result<()>; fn write_timeout(&mut self, desc: &TestDesc) -> io::Result<()>;
fn write_result( fn write_result(

View File

@ -181,9 +181,14 @@ impl<T: Write> PrettyFormatter<T> {
} }
impl<T: Write> OutputFormatter for PrettyFormatter<T> { impl<T: Write> OutputFormatter for PrettyFormatter<T> {
fn write_run_start(&mut self, test_count: usize) -> io::Result<()> { fn write_run_start(&mut self, test_count: usize, shuffle_seed: Option<u64>) -> io::Result<()> {
let noun = if test_count != 1 { "tests" } else { "test" }; let noun = if test_count != 1 { "tests" } else { "test" };
self.write_plain(&format!("\nrunning {} {}\n", test_count, noun)) let shuffle_seed_msg = if let Some(shuffle_seed) = shuffle_seed {
format!(" (shuffle seed: {})", shuffle_seed)
} else {
String::new()
};
self.write_plain(&format!("\nrunning {} {}{}\n", test_count, noun, shuffle_seed_msg))
} }
fn write_test_start(&mut self, desc: &TestDesc) -> io::Result<()> { fn write_test_start(&mut self, desc: &TestDesc) -> io::Result<()> {

View File

@ -170,10 +170,15 @@ impl<T: Write> TerseFormatter<T> {
} }
impl<T: Write> OutputFormatter for TerseFormatter<T> { impl<T: Write> OutputFormatter for TerseFormatter<T> {
fn write_run_start(&mut self, test_count: usize) -> io::Result<()> { fn write_run_start(&mut self, test_count: usize, shuffle_seed: Option<u64>) -> io::Result<()> {
self.total_test_count = test_count; self.total_test_count = test_count;
let noun = if test_count != 1 { "tests" } else { "test" }; let noun = if test_count != 1 { "tests" } else { "test" };
self.write_plain(&format!("\nrunning {} {}\n", test_count, noun)) let shuffle_seed_msg = if let Some(shuffle_seed) = shuffle_seed {
format!(" (shuffle seed: {})", shuffle_seed)
} else {
String::new()
};
self.write_plain(&format!("\nrunning {} {}{}\n", test_count, noun, shuffle_seed_msg))
} }
fn write_test_start(&mut self, desc: &TestDesc) -> io::Result<()> { fn write_test_start(&mut self, desc: &TestDesc) -> io::Result<()> {

View File

@ -5,3 +5,4 @@ pub mod concurrency;
pub mod exit_code; pub mod exit_code;
pub mod isatty; pub mod isatty;
pub mod metrics; pub mod metrics;
pub mod shuffle;

View File

@ -0,0 +1,67 @@
use crate::cli::TestOpts;
use crate::types::{TestDescAndFn, TestId, TestName};
use std::collections::hash_map::DefaultHasher;
use std::hash::Hasher;
use std::time::{SystemTime, UNIX_EPOCH};
pub fn get_shuffle_seed(opts: &TestOpts) -> Option<u64> {
opts.shuffle_seed.or_else(|| {
if opts.shuffle {
Some(
SystemTime::now()
.duration_since(UNIX_EPOCH)
.expect("Failed to get system time")
.as_nanos() as u64,
)
} else {
None
}
})
}
pub fn shuffle_tests(shuffle_seed: u64, tests: &mut [(TestId, TestDescAndFn)]) {
let test_names: Vec<&TestName> = tests.iter().map(|test| &test.1.desc.name).collect();
let test_names_hash = calculate_hash(&test_names);
let mut rng = Rng::new(shuffle_seed, test_names_hash);
shuffle(&mut rng, tests);
}
// `shuffle` is from `rust-analyzer/src/cli/analysis_stats.rs`.
fn shuffle<T>(rng: &mut Rng, slice: &mut [T]) {
for i in 0..slice.len() {
randomize_first(rng, &mut slice[i..]);
}
fn randomize_first<T>(rng: &mut Rng, slice: &mut [T]) {
assert!(!slice.is_empty());
let idx = rng.rand_range(0..slice.len() as u64) as usize;
slice.swap(0, idx);
}
}
struct Rng {
state: u64,
extra: u64,
}
impl Rng {
fn new(seed: u64, extra: u64) -> Self {
Self { state: seed, extra }
}
fn rand_range(&mut self, range: core::ops::Range<u64>) -> u64 {
self.rand_u64() % (range.end - range.start) + range.start
}
fn rand_u64(&mut self) -> u64 {
self.state = calculate_hash(&(self.state, self.extra));
self.state
}
}
// `calculate_hash` is from `core/src/hash/mod.rs`.
fn calculate_hash<T: core::hash::Hash>(t: &T) -> u64 {
let mut s = DefaultHasher::new();
t.hash(&mut s);
s.finish()
}

View File

@ -91,6 +91,7 @@ mod tests;
use event::{CompletedTest, TestEvent}; use event::{CompletedTest, TestEvent};
use helpers::concurrency::get_concurrency; use helpers::concurrency::get_concurrency;
use helpers::exit_code::get_exit_code; use helpers::exit_code::get_exit_code;
use helpers::shuffle::{get_shuffle_seed, shuffle_tests};
use options::{Concurrent, RunStrategy}; use options::{Concurrent, RunStrategy};
use test_result::*; use test_result::*;
use time::TestExecTime; use time::TestExecTime;
@ -247,7 +248,9 @@ where
let filtered_descs = filtered_tests.iter().map(|t| t.desc.clone()).collect(); let filtered_descs = filtered_tests.iter().map(|t| t.desc.clone()).collect();
let event = TestEvent::TeFiltered(filtered_descs); let shuffle_seed = get_shuffle_seed(opts);
let event = TestEvent::TeFiltered(filtered_descs, shuffle_seed);
notify_about_test_event(event)?; notify_about_test_event(event)?;
let (filtered_tests, filtered_benchs): (Vec<_>, _) = filtered_tests let (filtered_tests, filtered_benchs): (Vec<_>, _) = filtered_tests
@ -259,7 +262,11 @@ where
let concurrency = opts.test_threads.unwrap_or_else(get_concurrency); let concurrency = opts.test_threads.unwrap_or_else(get_concurrency);
let mut remaining = filtered_tests; let mut remaining = filtered_tests;
remaining.reverse(); if let Some(shuffle_seed) = shuffle_seed {
shuffle_tests(shuffle_seed, &mut remaining);
} else {
remaining.reverse();
}
let mut pending = 0; let mut pending = 0;
let (tx, rx) = channel::<CompletedTest>(); let (tx, rx) = channel::<CompletedTest>();

View File

@ -45,6 +45,8 @@ impl TestOpts {
nocapture: false, nocapture: false,
color: AutoColor, color: AutoColor,
format: OutputFormat::Pretty, format: OutputFormat::Pretty,
shuffle: false,
shuffle_seed: None,
test_threads: None, test_threads: None,
skip: vec![], skip: vec![],
time_options: None, time_options: None,
@ -565,11 +567,7 @@ pub fn exact_filter_match() {
assert_eq!(exact.len(), 2); assert_eq!(exact.len(), 2);
} }
#[test] fn sample_tests() -> Vec<TestDescAndFn> {
pub fn sort_tests() {
let mut opts = TestOpts::new();
opts.run_tests = true;
let names = vec![ let names = vec![
"sha1::test".to_string(), "sha1::test".to_string(),
"isize::test_to_str".to_string(), "isize::test_to_str".to_string(),
@ -583,26 +581,32 @@ pub fn sort_tests() {
"test::run_include_ignored_option".to_string(), "test::run_include_ignored_option".to_string(),
"test::sort_tests".to_string(), "test::sort_tests".to_string(),
]; ];
let tests = { fn testfn() {}
fn testfn() {} let mut tests = Vec::new();
let mut tests = Vec::new(); for name in &names {
for name in &names { let test = TestDescAndFn {
let test = TestDescAndFn { desc: TestDesc {
desc: TestDesc { name: DynTestName((*name).clone()),
name: DynTestName((*name).clone()), ignore: false,
ignore: false, should_panic: ShouldPanic::No,
should_panic: ShouldPanic::No, allow_fail: false,
allow_fail: false, compile_fail: false,
compile_fail: false, no_run: false,
no_run: false, test_type: TestType::Unknown,
test_type: TestType::Unknown, },
}, testfn: DynTestFn(Box::new(testfn)),
testfn: DynTestFn(Box::new(testfn)), };
}; tests.push(test);
tests.push(test); }
} tests
tests }
};
#[test]
pub fn sort_tests() {
let mut opts = TestOpts::new();
opts.run_tests = true;
let tests = sample_tests();
let filtered = filter_tests(&opts, tests); let filtered = filter_tests(&opts, tests);
let expected = vec![ let expected = vec![
@ -624,6 +628,71 @@ pub fn sort_tests() {
} }
} }
#[test]
pub fn shuffle_tests() {
let mut opts = TestOpts::new();
opts.shuffle = true;
let shuffle_seed = get_shuffle_seed(&opts).unwrap();
let left =
sample_tests().into_iter().enumerate().map(|(i, e)| (TestId(i), e)).collect::<Vec<_>>();
let mut right =
sample_tests().into_iter().enumerate().map(|(i, e)| (TestId(i), e)).collect::<Vec<_>>();
assert!(left.iter().zip(&right).all(|(a, b)| a.1.desc.name == b.1.desc.name));
helpers::shuffle::shuffle_tests(shuffle_seed, right.as_mut_slice());
assert!(left.iter().zip(right).any(|(a, b)| a.1.desc.name != b.1.desc.name));
}
#[test]
pub fn shuffle_tests_with_seed() {
let mut opts = TestOpts::new();
opts.shuffle = true;
let shuffle_seed = get_shuffle_seed(&opts).unwrap();
let mut left =
sample_tests().into_iter().enumerate().map(|(i, e)| (TestId(i), e)).collect::<Vec<_>>();
let mut right =
sample_tests().into_iter().enumerate().map(|(i, e)| (TestId(i), e)).collect::<Vec<_>>();
helpers::shuffle::shuffle_tests(shuffle_seed, left.as_mut_slice());
helpers::shuffle::shuffle_tests(shuffle_seed, right.as_mut_slice());
assert!(left.iter().zip(right).all(|(a, b)| a.1.desc.name == b.1.desc.name));
}
#[test]
pub fn order_depends_on_more_than_seed() {
let mut opts = TestOpts::new();
opts.shuffle = true;
let shuffle_seed = get_shuffle_seed(&opts).unwrap();
let mut left_tests = sample_tests();
let mut right_tests = sample_tests();
left_tests.pop();
right_tests.remove(0);
let mut left =
left_tests.into_iter().enumerate().map(|(i, e)| (TestId(i), e)).collect::<Vec<_>>();
let mut right =
right_tests.into_iter().enumerate().map(|(i, e)| (TestId(i), e)).collect::<Vec<_>>();
assert_eq!(left.len(), right.len());
assert!(left.iter().zip(&right).all(|(a, b)| a.0 == b.0));
helpers::shuffle::shuffle_tests(shuffle_seed, left.as_mut_slice());
helpers::shuffle::shuffle_tests(shuffle_seed, right.as_mut_slice());
assert!(left.iter().zip(right).any(|(a, b)| a.0 != b.0));
}
#[test] #[test]
pub fn test_metricmap_compare() { pub fn test_metricmap_compare() {
let mut m1 = MetricMap::new(); let mut m1 = MetricMap::new();

View File

@ -181,6 +181,40 @@ unstable-options` flag. See [tracking issue
#64888](https://github.com/rust-lang/rust/issues/64888) and the [unstable #64888](https://github.com/rust-lang/rust/issues/64888) and the [unstable
docs](../../unstable-book/compiler-flags/report-time.html) for more information. docs](../../unstable-book/compiler-flags/report-time.html) for more information.
#### `--shuffle`
Runs the tests in random order, as opposed to the default alphabetical order.
This may also be specified by setting the `RUST_TEST_SHUFFLE` environment
variable to anything but `0`.
The random number generator seed that is output can be passed to
[`--shuffle-seed`](#--shuffle-seed-seed) to run the tests in the same order
again.
Note that `--shuffle` does not affect whether the tests are run in parallel. To
run the tests in random order sequentially, use `--shuffle --test-threads 1`.
⚠️ 🚧 This option is [unstable](#unstable-options), and requires the `-Z
unstable-options` flag. See [tracking issue
#89583](https://github.com/rust-lang/rust/issues/89583) for more information.
#### `--shuffle-seed` _SEED_
Like [`--shuffle`](#--shuffle), but seeds the random number generator with
_SEED_. Thus, calling the test harness with `--shuffle-seed` _SEED_ twice runs
the tests in the same order both times.
_SEED_ is any 64-bit unsigned integer, for example, one produced by
[`--shuffle`](#--shuffle).
This can also be specified with the `RUST_TEST_SHUFFLE_SEED` environment
variable.
⚠️ 🚧 This option is [unstable](#unstable-options), and requires the `-Z
unstable-options` flag. See [tracking issue
#89583](https://github.com/rust-lang/rust/issues/89583) for more information.
### Output options ### Output options
The following options affect the output behavior. The following options affect the output behavior.
@ -197,7 +231,7 @@ to the console. Usually the output is captured, and only displayed if the test
fails. fails.
This may also be specified by setting the `RUST_TEST_NOCAPTURE` environment This may also be specified by setting the `RUST_TEST_NOCAPTURE` environment
variable set to anything but `0`. variable to anything but `0`.
#### `--show-output` #### `--show-output`

View File

@ -506,6 +506,8 @@ pub fn test_opts(config: &Config) -> test::TestOpts {
Err(_) => false, Err(_) => false,
}, },
color: config.color, color: config.color,
shuffle: false,
shuffle_seed: None,
test_threads: None, test_threads: None,
skip: vec![], skip: vec![],
list: false, list: false,