From ecf9e204c942b8e9bd4f04c9e2873ba04036e7e6 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 3 Apr 2025 20:58:34 +1100 Subject: [PATCH] compiletest: Encapsulate all of the code that touches libtest --- src/tools/compiletest/src/common.rs | 2 +- src/tools/compiletest/src/executor.rs | 156 ++++++++++++++++++++++ src/tools/compiletest/src/header.rs | 33 ++--- src/tools/compiletest/src/header/tests.rs | 13 +- src/tools/compiletest/src/lib.rs | 88 +++--------- 5 files changed, 194 insertions(+), 98 deletions(-) create mode 100644 src/tools/compiletest/src/executor.rs diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index cee351cd8b3..9e35d2b4667 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -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 { diff --git a/src/tools/compiletest/src/executor.rs b/src/tools/compiletest/src/executor.rs new file mode 100644 index 00000000000..7588fee2b2b --- /dev/null +++ b/src/tools/compiletest/src/executor.rs @@ -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) -> io::Result { + let opts = test_opts(config); + let tests = tests.into_iter().map(|t| t.into_libtest()).collect::>(); + + 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, + pub(crate) testpaths: TestPaths, + pub(crate) revision: Option, +} + +/// Information needed to create a `test::TestDesc`. +pub(crate) struct CollectedTestDesc { + pub(crate) name: String, + pub(crate) ignore: bool, + pub(crate) ignore_message: Option>, + 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, + } +} diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index f654bd9c90b..a0178f4bcc5 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -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; @@ -1355,15 +1356,15 @@ where Some((min, max)) } -pub fn make_test_description( +pub(crate) fn make_test_description( 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; @@ -1387,10 +1388,7 @@ pub fn make_test_description( 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::::from(reason))); + ignore_message = Some(reason.into()); } IgnoreDecision::Error { message } => { eprintln!("error: {}:{line_number}: {message}", path.display()); @@ -1431,25 +1429,12 @@ pub fn make_test_description( // 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 { diff --git a/src/tools/compiletest/src/header/tests.rs b/src/tools/compiletest/src/header/tests.rs index 4d90f152ee2..ff6bc49b72a 100644 --- a/src/tools/compiletest/src/header/tests.rs +++ b/src/tools/compiletest/src/header/tests.rs @@ -8,14 +8,15 @@ use super::{ parse_normalize_rule, }; use crate::common::{Config, Debugger, Mode}; +use crate::executor::{CollectedTestDesc, ShouldPanic}; fn make_test_description( 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( @@ -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 @@ -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] diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index 01899c2c8a6..8145ae1c1bc 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -12,6 +12,7 @@ pub mod common; pub mod compute_diff; mod debuggers; pub mod errors; +mod executor; pub mod header; mod json; mod raise_fd_limit; @@ -32,7 +33,6 @@ use std::{env, fs, vec}; use build_helper::git::{get_git_modified_files, get_git_untracked_files}; use getopts::Options; -use test::ColorConfig; use tracing::*; use walkdir::WalkDir; @@ -41,6 +41,7 @@ use crate::common::{ CompareMode, Config, Debugger, Mode, PassMode, TestPaths, UI_EXTENSIONS, expected_output_path, output_base_dir, output_relative_path, }; +use crate::executor::{CollectedTest, ColorConfig, OutputFormat}; use crate::header::HeadersCache; use crate::util::logv; @@ -402,9 +403,9 @@ pub fn parse_config(args: Vec) -> Config { verbose: matches.opt_present("verbose"), format: match (matches.opt_present("quiet"), matches.opt_present("json")) { (true, true) => panic!("--quiet and --json are incompatible"), - (true, false) => test::OutputFormat::Terse, - (false, true) => test::OutputFormat::Json, - (false, false) => test::OutputFormat::Pretty, + (true, false) => OutputFormat::Terse, + (false, true) => OutputFormat::Json, + (false, false) => OutputFormat::Pretty, }, only_modified: matches.opt_present("only-modified"), color, @@ -535,8 +536,6 @@ pub fn run_tests(config: Arc) { // Let tests know which target they're running as env::set_var("TARGET", &config.target); - let opts = test_opts(&config); - let mut configs = Vec::new(); if let Mode::DebugInfo = config.mode { // Debugging emscripten code doesn't make sense today @@ -563,12 +562,12 @@ pub fn run_tests(config: Arc) { tests.extend(collect_and_make_tests(c)); } - tests.sort_by(|a, b| a.desc.name.as_slice().cmp(&b.desc.name.as_slice())); + tests.sort_by(|a, b| Ord::cmp(&a.desc.name, &b.desc.name)); // Delegate to libtest to filter and run the big list of structures created - // during test discovery. When libtest decides to run a test, it will invoke - // the corresponding closure created by `make_test_closure`. - let res = test::run_tests_console(&opts, tests); + // during test discovery. When libtest decides to run a test, it will + // return control to compiletest by invoking a closure. + let res = crate::executor::execute_tests(&config, tests); // Check the outcome reported by libtest. match res { @@ -612,30 +611,6 @@ pub fn run_tests(config: Arc) { } } -pub 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, - logfile: config.logfile.clone(), - run_tests: true, - bench_benchmarks: true, - nocapture: config.nocapture, - color: config.color, - 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, - } -} - /// Read-only context data used during test collection. struct TestCollectorCx { config: Arc, @@ -646,17 +621,17 @@ struct TestCollectorCx { /// Mutable state used during test collection. struct TestCollector { - tests: Vec, + tests: Vec, found_path_stems: HashSet, poisoned: bool, } -/// Creates libtest structures for every test/revision in the test suite directory. +/// Creates test structures for every test/revision in the test suite directory. /// /// This always inspects _all_ test files in the suite (e.g. all 17k+ ui tests), /// regardless of whether any filters/tests were specified on the command-line, /// because filtering is handled later by libtest. -pub fn collect_and_make_tests(config: Arc) -> Vec { +pub(crate) fn collect_and_make_tests(config: Arc) -> Vec { debug!("making tests from {}", config.src_test_suite_root.display()); let common_inputs_stamp = common_inputs_stamp(&config); let modified_tests = @@ -885,7 +860,7 @@ fn make_test(cx: &TestCollectorCx, collector: &mut TestCollector, testpaths: &Te }; // For each revision (or the sole dummy revision), create and append a - // `test::TestDescAndFn` that can be handed over to libtest. + // `CollectedTest` that can be handed over to the test executor. collector.tests.extend(revisions.into_iter().map(|revision| { // Create a test name and description to hand over to libtest. let src_file = fs::File::open(&test_path).expect("open test file to parse ignores"); @@ -908,13 +883,14 @@ fn make_test(cx: &TestCollectorCx, collector: &mut TestCollector, testpaths: &Te if !cx.config.force_rerun && is_up_to_date(cx, testpaths, &early_props, revision) { desc.ignore = true; // Keep this in sync with the "up-to-date" message detected by bootstrap. - desc.ignore_message = Some("up-to-date"); + desc.ignore_message = Some("up-to-date".into()); } - // Create the callback that will run this test/revision when libtest calls it. - let testfn = make_test_closure(Arc::clone(&cx.config), testpaths, revision); + let config = Arc::clone(&cx.config); + let testpaths = testpaths.clone(); + let revision = revision.map(str::to_owned); - test::TestDescAndFn { desc, testfn } + CollectedTest { desc, config, testpaths, revision } })); } @@ -1046,11 +1022,7 @@ impl Stamp { } /// Creates a name for this test/revision that can be handed over to libtest. -fn make_test_name( - config: &Config, - testpaths: &TestPaths, - revision: Option<&str>, -) -> test::TestName { +fn make_test_name(config: &Config, testpaths: &TestPaths, revision: Option<&str>) -> String { // Print the name of the file, relative to the sources root. let path = testpaths.file.strip_prefix(&config.src_root).unwrap(); let debugger = match config.debugger { @@ -1062,32 +1034,14 @@ fn make_test_name( None => String::new(), }; - test::DynTestName(format!( + format!( "[{}{}{}] {}{}", config.mode, debugger, mode_suffix, path.display(), revision.map_or("".to_string(), |rev| format!("#{}", rev)) - )) -} - -/// Creates a callback for this test/revision that libtest will call when it -/// decides to actually run the underlying test. -fn make_test_closure( - config: Arc, - testpaths: &TestPaths, - revision: Option<&str>, -) -> test::TestFn { - let testpaths = testpaths.clone(); - let revision = revision.map(str::to_owned); - - // This callback is the link between compiletest's test discovery code, - // and the parts of compiletest that know how to run an individual test. - test::DynTestFn(Box::new(move || { - runtest::run(config, &testpaths, revision.as_deref()); - Ok(()) - })) + ) } /// Checks that test discovery didn't find any tests whose name stem is a prefix