Rollup merge of #140392 - Zalathar:goodbye-libtest, r=jieyouxu

compiletest: Remove the libtest-based executor and its dependency

Now that #140288 has landed and the new compiletest executor is used by default, we can now move forward with removing the libtest dependency from compiletest.

My hope is that after landing this, we can configure bootstrap to build compiletest with the pre-built stage0 library by default, instead of the in-tree stage0 library. That would give the stage0 redesign one less thing to worry about.

---

This PR has deliberately been kept small and simple, to make it easier to revert if necessary. Further cleanup can take palce after we're confident that it won't need to be reverted.

r? jieyouxu

Blocker for https://github.com/rust-lang/rust/pull/119899

src/bootstrap/src/core/build_steps/tool.rs

@@ -462,7 +462,7 @@ macro_rules! bootstrap_tool {
     }
 }
 
-pub(crate) const COMPILETEST_ALLOW_FEATURES: &str = "test,internal_output_capture";
+pub(crate) const COMPILETEST_ALLOW_FEATURES: &str = "internal_output_capture";
 
 bootstrap_tool!(
     // This is marked as an external tool because it includes dependencies

src/tools/compiletest/src/common.rs

@@ -413,14 +413,6 @@ pub struct Config {
     /// cross-compilation scenarios that do not otherwise want/need to `-Zbuild-std`. Used in e.g.
     /// ABI tests.
     pub minicore_path: Utf8PathBuf,
-
-    /// If true, disable the "new" executor, and use the older libtest-based
-    /// executor to run tests instead. This is a temporary fallback, to make
-    /// manual comparative testing easier if bugs are found in the new executor.
-    ///
-    /// FIXME(Zalathar): Eventually remove this flag and remove the libtest
-    /// dependency.
-    pub no_new_executor: bool,
 }
 
 impl Config {

src/tools/compiletest/src/executor.rs

@@ -12,7 +12,6 @@ use crate::common::{Config, TestPaths};
 
 mod deadline;
 mod json;
-pub(crate) mod libtest;
 
 pub(crate) fn run_tests(config: &Config, tests: Vec<CollectedTest>) -> bool {
     let tests_len = tests.len();

src/tools/compiletest/src/executor/libtest.rs

@@ -1,111 +0,0 @@
-//! This submodule encapsulates all of the code that actually interacts with
-//! libtest, so that it can be easily removed after the new executor becomes
-//! the default.
-
-use std::borrow::Cow;
-use std::io;
-
-use crate::common::Config;
-use crate::executor::{CollectedTest, CollectedTestDesc, ColorConfig, OutputFormat, ShouldPanic};
-
-/// 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)
-}
-
-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 }
-    }
-}
-
-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,
-        }
-    }
-}
-
-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,
-        }
-    }
-}
-
-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: None,
-        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,
-    }
-}

src/tools/compiletest/src/lib.rs

@@ -1,11 +1,10 @@
 #![crate_name = "compiletest"]
-// Needed by the libtest-based test executor.
-#![feature(test)]
 // Needed by the "new" test executor that does not depend on libtest.
+// FIXME(Zalathar): We should be able to get rid of `internal_output_capture`,
+// by having `runtest` manually capture all of its println-like output instead.
+// That would result in compiletest being written entirely in stable Rust!
 #![feature(internal_output_capture)]
 
-extern crate test;
-
 #[cfg(test)]
 mod tests;
 
@@ -448,8 +447,6 @@ pub fn parse_config(args: Vec<String>) -> Config {
         diff_command: matches.opt_str("compiletest-diff-tool"),
 
         minicore_path: opt_path(matches, "minicore-path"),
-
-        no_new_executor: matches.opt_present("no-new-executor"),
     }
 }
 
@@ -576,12 +573,10 @@ pub fn run_tests(config: Arc<Config>) {
     // Delegate to the executor to filter and run the big list of test structures
     // created during test discovery. When the executor decides to run a test,
     // it will return control to the rest of compiletest by calling `runtest::run`.
-    let res = if !config.no_new_executor {
-        Ok(executor::run_tests(&config, tests))
-    } else {
-        // FIXME(Zalathar): Eventually remove the libtest executor entirely.
-        crate::executor::libtest::execute_tests(&config, tests)
-    };
+    // FIXME(Zalathar): Once we're confident that we won't need to revert the
+    // removal of the libtest-based executor, remove this Result and other
+    // remnants of the old executor.
+    let res: io::Result<bool> = Ok(executor::run_tests(&config, tests));
 
     // Check the outcome reported by libtest.
     match res {