Skip to content

compiletest: Encapsulate all of the code that touches libtest #139317

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/tools/compiletest/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ use std::{fmt, iter};
use build_helper::git::GitConfig;
use semver::Version;
use serde::de::{Deserialize, Deserializer, Error as _};
use test::{ColorConfig, OutputFormat};

pub use self::Mode::*;
use crate::executor::{ColorConfig, OutputFormat};
use crate::util::{PathBufExt, add_dylib_path};

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

/// Stop as soon as possible after any test fails.
/// May run a few more tests before stopping, due to threading.
pub fail_fast: bool,

/// The library paths required for running the compiler.
pub compile_lib_path: PathBuf,

Expand Down
156 changes: 156 additions & 0 deletions src/tools/compiletest/src/executor.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
//! This module encapsulates all of the code that interacts directly with
//! libtest, to execute the collected tests.
//!
//! This will hopefully make it easier to migrate away from libtest someday.

use std::borrow::Cow;
use std::io;
use std::sync::Arc;

use crate::common::{Config, TestPaths};

/// Delegates to libtest to run the list of collected tests.
///
/// Returns `Ok(true)` if all tests passed, or `Ok(false)` if one or more tests failed.
pub(crate) fn execute_tests(config: &Config, tests: Vec<CollectedTest>) -> io::Result<bool> {
let opts = test_opts(config);
let tests = tests.into_iter().map(|t| t.into_libtest()).collect::<Vec<_>>();

test::run_tests_console(&opts, tests)
}

/// Information needed to create a `test::TestDescAndFn`.
pub(crate) struct CollectedTest {
pub(crate) desc: CollectedTestDesc,
pub(crate) config: Arc<Config>,
pub(crate) testpaths: TestPaths,
pub(crate) revision: Option<String>,
}

/// Information needed to create a `test::TestDesc`.
pub(crate) struct CollectedTestDesc {
pub(crate) name: String,
pub(crate) ignore: bool,
pub(crate) ignore_message: Option<Cow<'static, str>>,
pub(crate) should_panic: ShouldPanic,
}

impl CollectedTest {
fn into_libtest(self) -> test::TestDescAndFn {
let Self { desc, config, testpaths, revision } = self;
let CollectedTestDesc { name, ignore, ignore_message, should_panic } = desc;

// Libtest requires the ignore message to be a &'static str, so we might
// have to leak memory to create it. This is fine, as we only do so once
// per test, so the leak won't grow indefinitely.
let ignore_message = ignore_message.map(|msg| match msg {
Cow::Borrowed(s) => s,
Cow::Owned(s) => &*String::leak(s),
});

let desc = test::TestDesc {
name: test::DynTestName(name),
ignore,
ignore_message,
source_file: "",
start_line: 0,
start_col: 0,
end_line: 0,
end_col: 0,
should_panic: should_panic.to_libtest(),
compile_fail: false,
no_run: false,
test_type: test::TestType::Unknown,
};

// This closure is invoked when libtest returns control to compiletest
// to execute the test.
let testfn = test::DynTestFn(Box::new(move || {
crate::runtest::run(config, &testpaths, revision.as_deref());
Ok(())
}));

test::TestDescAndFn { desc, testfn }
}
}

/// Whether console output should be colored or not.
#[derive(Copy, Clone, Default, Debug)]
pub enum ColorConfig {
#[default]
AutoColor,
AlwaysColor,
NeverColor,
}

impl ColorConfig {
fn to_libtest(self) -> test::ColorConfig {
match self {
Self::AutoColor => test::ColorConfig::AutoColor,
Self::AlwaysColor => test::ColorConfig::AlwaysColor,
Self::NeverColor => test::ColorConfig::NeverColor,
}
}
}

/// Format of the test results output.
#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]
pub enum OutputFormat {
/// Verbose output
Pretty,
/// Quiet output
#[default]
Terse,
/// JSON output
Json,
}

impl OutputFormat {
fn to_libtest(self) -> test::OutputFormat {
match self {
Self::Pretty => test::OutputFormat::Pretty,
Self::Terse => test::OutputFormat::Terse,
Self::Json => test::OutputFormat::Json,
}
}
}

/// Whether test is expected to panic or not.
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
pub(crate) enum ShouldPanic {
No,
Yes,
}

impl ShouldPanic {
fn to_libtest(self) -> test::ShouldPanic {
match self {
Self::No => test::ShouldPanic::No,
Self::Yes => test::ShouldPanic::Yes,
}
}
}

fn test_opts(config: &Config) -> test::TestOpts {
test::TestOpts {
exclude_should_panic: false,
filters: config.filters.clone(),
filter_exact: config.filter_exact,
run_ignored: if config.run_ignored { test::RunIgnored::Yes } else { test::RunIgnored::No },
format: config.format.to_libtest(),
logfile: config.logfile.clone(),
run_tests: true,
bench_benchmarks: true,
nocapture: config.nocapture,
color: config.color.to_libtest(),
shuffle: false,
shuffle_seed: None,
test_threads: None,
skip: config.skip.clone(),
list: false,
options: test::Options::new(),
time_options: None,
force_run_in_process: false,
fail_fast: config.fail_fast,
}
}
33 changes: 9 additions & 24 deletions src/tools/compiletest/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use tracing::*;

use crate::common::{Config, Debugger, FailMode, Mode, PassMode};
use crate::debuggers::{extract_cdb_version, extract_gdb_version};
use crate::executor::{CollectedTestDesc, ShouldPanic};
use crate::header::auxiliary::{AuxProps, parse_and_update_aux};
use crate::header::needs::CachedNeedsConditions;
use crate::util::static_regex;
Expand Down Expand Up @@ -1355,15 +1356,15 @@ where
Some((min, max))
}

pub fn make_test_description<R: Read>(
pub(crate) fn make_test_description<R: Read>(
config: &Config,
cache: &HeadersCache,
name: test::TestName,
name: String,
path: &Path,
src: R,
test_revision: Option<&str>,
poisoned: &mut bool,
) -> test::TestDesc {
) -> CollectedTestDesc {
let mut ignore = false;
let mut ignore_message = None;
let mut should_fail = false;
Expand All @@ -1387,10 +1388,7 @@ pub fn make_test_description<R: Read>(
match $e {
IgnoreDecision::Ignore { reason } => {
ignore = true;
// The ignore reason must be a &'static str, so we have to leak memory to
// create it. This is fine, as the header is parsed only at the start of
// compiletest so it won't grow indefinitely.
ignore_message = Some(&*Box::leak(Box::<str>::from(reason)));
ignore_message = Some(reason.into());
}
IgnoreDecision::Error { message } => {
eprintln!("error: {}:{line_number}: {message}", path.display());
Expand Down Expand Up @@ -1431,25 +1429,12 @@ pub fn make_test_description<R: Read>(
// since we run the pretty printer across all tests by default.
// If desired, we could add a `should-fail-pretty` annotation.
let should_panic = match config.mode {
crate::common::Pretty => test::ShouldPanic::No,
_ if should_fail => test::ShouldPanic::Yes,
_ => test::ShouldPanic::No,
crate::common::Pretty => ShouldPanic::No,
_ if should_fail => ShouldPanic::Yes,
_ => ShouldPanic::No,
};

test::TestDesc {
name,
ignore,
ignore_message,
source_file: "",
start_line: 0,
start_col: 0,
end_line: 0,
end_col: 0,
should_panic,
compile_fail: false,
no_run: false,
test_type: test::TestType::Unknown,
}
CollectedTestDesc { name, ignore, ignore_message, should_panic }
}

fn ignore_cdb(config: &Config, line: &str) -> IgnoreDecision {
Expand Down
13 changes: 7 additions & 6 deletions src/tools/compiletest/src/header/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@ use super::{
parse_normalize_rule,
};
use crate::common::{Config, Debugger, Mode};
use crate::executor::{CollectedTestDesc, ShouldPanic};

fn make_test_description<R: Read>(
config: &Config,
name: test::TestName,
name: String,
path: &Path,
src: R,
revision: Option<&str>,
) -> test::TestDesc {
) -> CollectedTestDesc {
let cache = HeadersCache::load(config);
let mut poisoned = false;
let test = crate::header::make_test_description(
Expand Down Expand Up @@ -233,7 +234,7 @@ fn parse_rs(config: &Config, contents: &str) -> EarlyProps {
}

fn check_ignore(config: &Config, contents: &str) -> bool {
let tn = test::DynTestName(String::new());
let tn = String::new();
let p = Path::new("a.rs");
let d = make_test_description(&config, tn, p, std::io::Cursor::new(contents), None);
d.ignore
Expand All @@ -242,13 +243,13 @@ fn check_ignore(config: &Config, contents: &str) -> bool {
#[test]
fn should_fail() {
let config: Config = cfg().build();
let tn = test::DynTestName(String::new());
let tn = String::new();
let p = Path::new("a.rs");

let d = make_test_description(&config, tn.clone(), p, std::io::Cursor::new(""), None);
assert_eq!(d.should_panic, test::ShouldPanic::No);
assert_eq!(d.should_panic, ShouldPanic::No);
let d = make_test_description(&config, tn, p, std::io::Cursor::new("//@ should-fail"), None);
assert_eq!(d.should_panic, test::ShouldPanic::Yes);
assert_eq!(d.should_panic, ShouldPanic::Yes);
}

#[test]
Expand Down
Loading
Loading