Skip to content

Commit 9d84676

Browse files
authored
Rollup merge of rust-lang#139317 - Zalathar:hide-libtest, r=jieyouxu
compiletest: Encapsulate all of the code that touches libtest Compiletest currently relies on unstable libtest APIs in order to actually execute tests. That's unfortunate, but removing the dependency isn't trivial. However, we can make a small step towards removing the libtest dependency by encapsulating the libtest interactions into a single dedicated module. That makes it easier to see what parts of libtest are actually used. --- As a side-effect of moving the `test_opts` function into that dedicated module, this PR also ends up allowing `--fail-fast` to be passed on the command line, instead of requiring an environment variable. --- There is still (at least) one other aspect of the libtest dependency that this PR does not address, namely the fact that we rely on libtest's output capture (via unstable std APIs) to capture the output that we print during individual tests. I hope to do something about that at some point. r? jieyouxu
2 parents 96ab10c + ecf9e20 commit 9d84676

File tree

5 files changed

+208
-105
lines changed

5 files changed

+208
-105
lines changed

src/tools/compiletest/src/common.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ use std::{fmt, iter};
99
use build_helper::git::GitConfig;
1010
use semver::Version;
1111
use serde::de::{Deserialize, Deserializer, Error as _};
12-
use test::{ColorConfig, OutputFormat};
1312

1413
pub use self::Mode::*;
14+
use crate::executor::{ColorConfig, OutputFormat};
1515
use crate::util::{PathBufExt, add_dylib_path};
1616

1717
macro_rules! string_enum {
@@ -178,6 +178,10 @@ pub struct Config {
178178
/// `true` to overwrite stderr/stdout files instead of complaining about changes in output.
179179
pub bless: bool,
180180

181+
/// Stop as soon as possible after any test fails.
182+
/// May run a few more tests before stopping, due to threading.
183+
pub fail_fast: bool,
184+
181185
/// The library paths required for running the compiler.
182186
pub compile_lib_path: PathBuf,
183187

src/tools/compiletest/src/executor.rs

+156
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
//! This module encapsulates all of the code that interacts directly with
2+
//! libtest, to execute the collected tests.
3+
//!
4+
//! This will hopefully make it easier to migrate away from libtest someday.
5+
6+
use std::borrow::Cow;
7+
use std::io;
8+
use std::sync::Arc;
9+
10+
use crate::common::{Config, TestPaths};
11+
12+
/// Delegates to libtest to run the list of collected tests.
13+
///
14+
/// Returns `Ok(true)` if all tests passed, or `Ok(false)` if one or more tests failed.
15+
pub(crate) fn execute_tests(config: &Config, tests: Vec<CollectedTest>) -> io::Result<bool> {
16+
let opts = test_opts(config);
17+
let tests = tests.into_iter().map(|t| t.into_libtest()).collect::<Vec<_>>();
18+
19+
test::run_tests_console(&opts, tests)
20+
}
21+
22+
/// Information needed to create a `test::TestDescAndFn`.
23+
pub(crate) struct CollectedTest {
24+
pub(crate) desc: CollectedTestDesc,
25+
pub(crate) config: Arc<Config>,
26+
pub(crate) testpaths: TestPaths,
27+
pub(crate) revision: Option<String>,
28+
}
29+
30+
/// Information needed to create a `test::TestDesc`.
31+
pub(crate) struct CollectedTestDesc {
32+
pub(crate) name: String,
33+
pub(crate) ignore: bool,
34+
pub(crate) ignore_message: Option<Cow<'static, str>>,
35+
pub(crate) should_panic: ShouldPanic,
36+
}
37+
38+
impl CollectedTest {
39+
fn into_libtest(self) -> test::TestDescAndFn {
40+
let Self { desc, config, testpaths, revision } = self;
41+
let CollectedTestDesc { name, ignore, ignore_message, should_panic } = desc;
42+
43+
// Libtest requires the ignore message to be a &'static str, so we might
44+
// have to leak memory to create it. This is fine, as we only do so once
45+
// per test, so the leak won't grow indefinitely.
46+
let ignore_message = ignore_message.map(|msg| match msg {
47+
Cow::Borrowed(s) => s,
48+
Cow::Owned(s) => &*String::leak(s),
49+
});
50+
51+
let desc = test::TestDesc {
52+
name: test::DynTestName(name),
53+
ignore,
54+
ignore_message,
55+
source_file: "",
56+
start_line: 0,
57+
start_col: 0,
58+
end_line: 0,
59+
end_col: 0,
60+
should_panic: should_panic.to_libtest(),
61+
compile_fail: false,
62+
no_run: false,
63+
test_type: test::TestType::Unknown,
64+
};
65+
66+
// This closure is invoked when libtest returns control to compiletest
67+
// to execute the test.
68+
let testfn = test::DynTestFn(Box::new(move || {
69+
crate::runtest::run(config, &testpaths, revision.as_deref());
70+
Ok(())
71+
}));
72+
73+
test::TestDescAndFn { desc, testfn }
74+
}
75+
}
76+
77+
/// Whether console output should be colored or not.
78+
#[derive(Copy, Clone, Default, Debug)]
79+
pub enum ColorConfig {
80+
#[default]
81+
AutoColor,
82+
AlwaysColor,
83+
NeverColor,
84+
}
85+
86+
impl ColorConfig {
87+
fn to_libtest(self) -> test::ColorConfig {
88+
match self {
89+
Self::AutoColor => test::ColorConfig::AutoColor,
90+
Self::AlwaysColor => test::ColorConfig::AlwaysColor,
91+
Self::NeverColor => test::ColorConfig::NeverColor,
92+
}
93+
}
94+
}
95+
96+
/// Format of the test results output.
97+
#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]
98+
pub enum OutputFormat {
99+
/// Verbose output
100+
Pretty,
101+
/// Quiet output
102+
#[default]
103+
Terse,
104+
/// JSON output
105+
Json,
106+
}
107+
108+
impl OutputFormat {
109+
fn to_libtest(self) -> test::OutputFormat {
110+
match self {
111+
Self::Pretty => test::OutputFormat::Pretty,
112+
Self::Terse => test::OutputFormat::Terse,
113+
Self::Json => test::OutputFormat::Json,
114+
}
115+
}
116+
}
117+
118+
/// Whether test is expected to panic or not.
119+
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
120+
pub(crate) enum ShouldPanic {
121+
No,
122+
Yes,
123+
}
124+
125+
impl ShouldPanic {
126+
fn to_libtest(self) -> test::ShouldPanic {
127+
match self {
128+
Self::No => test::ShouldPanic::No,
129+
Self::Yes => test::ShouldPanic::Yes,
130+
}
131+
}
132+
}
133+
134+
fn test_opts(config: &Config) -> test::TestOpts {
135+
test::TestOpts {
136+
exclude_should_panic: false,
137+
filters: config.filters.clone(),
138+
filter_exact: config.filter_exact,
139+
run_ignored: if config.run_ignored { test::RunIgnored::Yes } else { test::RunIgnored::No },
140+
format: config.format.to_libtest(),
141+
logfile: config.logfile.clone(),
142+
run_tests: true,
143+
bench_benchmarks: true,
144+
nocapture: config.nocapture,
145+
color: config.color.to_libtest(),
146+
shuffle: false,
147+
shuffle_seed: None,
148+
test_threads: None,
149+
skip: config.skip.clone(),
150+
list: false,
151+
options: test::Options::new(),
152+
time_options: None,
153+
force_run_in_process: false,
154+
fail_fast: config.fail_fast,
155+
}
156+
}

src/tools/compiletest/src/header.rs

+9-24
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use tracing::*;
1111

1212
use crate::common::{Config, Debugger, FailMode, Mode, PassMode};
1313
use crate::debuggers::{extract_cdb_version, extract_gdb_version};
14+
use crate::executor::{CollectedTestDesc, ShouldPanic};
1415
use crate::header::auxiliary::{AuxProps, parse_and_update_aux};
1516
use crate::header::needs::CachedNeedsConditions;
1617
use crate::util::static_regex;
@@ -1355,15 +1356,15 @@ where
13551356
Some((min, max))
13561357
}
13571358

1358-
pub fn make_test_description<R: Read>(
1359+
pub(crate) fn make_test_description<R: Read>(
13591360
config: &Config,
13601361
cache: &HeadersCache,
1361-
name: test::TestName,
1362+
name: String,
13621363
path: &Path,
13631364
src: R,
13641365
test_revision: Option<&str>,
13651366
poisoned: &mut bool,
1366-
) -> test::TestDesc {
1367+
) -> CollectedTestDesc {
13671368
let mut ignore = false;
13681369
let mut ignore_message = None;
13691370
let mut should_fail = false;
@@ -1387,10 +1388,7 @@ pub fn make_test_description<R: Read>(
13871388
match $e {
13881389
IgnoreDecision::Ignore { reason } => {
13891390
ignore = true;
1390-
// The ignore reason must be a &'static str, so we have to leak memory to
1391-
// create it. This is fine, as the header is parsed only at the start of
1392-
// compiletest so it won't grow indefinitely.
1393-
ignore_message = Some(&*Box::leak(Box::<str>::from(reason)));
1391+
ignore_message = Some(reason.into());
13941392
}
13951393
IgnoreDecision::Error { message } => {
13961394
eprintln!("error: {}:{line_number}: {message}", path.display());
@@ -1431,25 +1429,12 @@ pub fn make_test_description<R: Read>(
14311429
// since we run the pretty printer across all tests by default.
14321430
// If desired, we could add a `should-fail-pretty` annotation.
14331431
let should_panic = match config.mode {
1434-
crate::common::Pretty => test::ShouldPanic::No,
1435-
_ if should_fail => test::ShouldPanic::Yes,
1436-
_ => test::ShouldPanic::No,
1432+
crate::common::Pretty => ShouldPanic::No,
1433+
_ if should_fail => ShouldPanic::Yes,
1434+
_ => ShouldPanic::No,
14371435
};
14381436

1439-
test::TestDesc {
1440-
name,
1441-
ignore,
1442-
ignore_message,
1443-
source_file: "",
1444-
start_line: 0,
1445-
start_col: 0,
1446-
end_line: 0,
1447-
end_col: 0,
1448-
should_panic,
1449-
compile_fail: false,
1450-
no_run: false,
1451-
test_type: test::TestType::Unknown,
1452-
}
1437+
CollectedTestDesc { name, ignore, ignore_message, should_panic }
14531438
}
14541439

14551440
fn ignore_cdb(config: &Config, line: &str) -> IgnoreDecision {

src/tools/compiletest/src/header/tests.rs

+7-6
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,15 @@ use super::{
88
parse_normalize_rule,
99
};
1010
use crate::common::{Config, Debugger, Mode};
11+
use crate::executor::{CollectedTestDesc, ShouldPanic};
1112

1213
fn make_test_description<R: Read>(
1314
config: &Config,
14-
name: test::TestName,
15+
name: String,
1516
path: &Path,
1617
src: R,
1718
revision: Option<&str>,
18-
) -> test::TestDesc {
19+
) -> CollectedTestDesc {
1920
let cache = HeadersCache::load(config);
2021
let mut poisoned = false;
2122
let test = crate::header::make_test_description(
@@ -233,7 +234,7 @@ fn parse_rs(config: &Config, contents: &str) -> EarlyProps {
233234
}
234235

235236
fn check_ignore(config: &Config, contents: &str) -> bool {
236-
let tn = test::DynTestName(String::new());
237+
let tn = String::new();
237238
let p = Path::new("a.rs");
238239
let d = make_test_description(&config, tn, p, std::io::Cursor::new(contents), None);
239240
d.ignore
@@ -242,13 +243,13 @@ fn check_ignore(config: &Config, contents: &str) -> bool {
242243
#[test]
243244
fn should_fail() {
244245
let config: Config = cfg().build();
245-
let tn = test::DynTestName(String::new());
246+
let tn = String::new();
246247
let p = Path::new("a.rs");
247248

248249
let d = make_test_description(&config, tn.clone(), p, std::io::Cursor::new(""), None);
249-
assert_eq!(d.should_panic, test::ShouldPanic::No);
250+
assert_eq!(d.should_panic, ShouldPanic::No);
250251
let d = make_test_description(&config, tn, p, std::io::Cursor::new("//@ should-fail"), None);
251-
assert_eq!(d.should_panic, test::ShouldPanic::Yes);
252+
assert_eq!(d.should_panic, ShouldPanic::Yes);
252253
}
253254

254255
#[test]

0 commit comments

Comments
 (0)