Rollup merge of #135887 - onur-ozkan:testing-improvements, r=jieyouxu

improvements on `build_steps::test` implementation

Reviewing commits one-by-one should make it easier to understand.

Author: matthiaskrgr

src/bootstrap/src/core/build_steps/test.rs

@@ -87,7 +87,7 @@ impl Step for CrateBootstrap {
             &[],
         );
         let crate_name = path.rsplit_once('/').unwrap().1;
-        run_cargo_test(cargo, &[], &[], crate_name, crate_name, compiler, bootstrap_host, builder);
+        run_cargo_test(cargo, &[], &[], crate_name, crate_name, bootstrap_host, builder);
     }
 }
 
@@ -143,7 +143,6 @@ You can skip linkcheck with --skip src/tools/linkchecker"
             &[],
             "linkchecker",
             "linkchecker self tests",
-            compiler,
             bootstrap_host,
             builder,
         );
@@ -312,7 +311,7 @@ impl Step for Cargo {
         );
 
         // NOTE: can't use `run_cargo_test` because we need to overwrite `PATH`
-        let mut cargo = prepare_cargo_test(cargo, &[], &[], "cargo", compiler, self.host, builder);
+        let mut cargo = prepare_cargo_test(cargo, &[], &[], "cargo", self.host, builder);
 
         // Don't run cross-compile tests, we may not have cross-compiled libstd libs
         // available.
@@ -397,7 +396,7 @@ impl Step for RustAnalyzer {
         cargo.env("SKIP_SLOW_TESTS", "1");
 
         cargo.add_rustc_lib_path(builder);
-        run_cargo_test(cargo, &[], &[], "rust-analyzer", "rust-analyzer", compiler, host, builder);
+        run_cargo_test(cargo, &[], &[], "rust-analyzer", "rust-analyzer", host, builder);
     }
 }
 
@@ -445,7 +444,7 @@ impl Step for Rustfmt {
 
         cargo.add_rustc_lib_path(builder);
 
-        run_cargo_test(cargo, &[], &[], "rustfmt", "rustfmt", compiler, host, builder);
+        run_cargo_test(cargo, &[], &[], "rustfmt", "rustfmt", host, builder);
     }
 }
 
@@ -565,7 +564,7 @@ impl Step for Miri {
 
         // We can NOT use `run_cargo_test` since Miri's integration tests do not use the usual test
         // harness and therefore do not understand the flags added by `add_flags_and_try_run_test`.
-        let mut cargo = prepare_cargo_test(cargo, &[], &[], "miri", host_compiler, host, builder);
+        let mut cargo = prepare_cargo_test(cargo, &[], &[], "miri", host, builder);
 
         // miri tests need to know about the stage sysroot
         cargo.env("MIRI_SYSROOT", &miri_sysroot);
@@ -713,16 +712,7 @@ impl Step for CompiletestTest {
             &[],
         );
         cargo.allow_features("test");
-        run_cargo_test(
-            cargo,
-            &[],
-            &[],
-            "compiletest",
-            "compiletest self test",
-            compiler,
-            host,
-            builder,
-        );
+        run_cargo_test(cargo, &[], &[], "compiletest", "compiletest self test", host, builder);
     }
 }
 
@@ -769,7 +759,7 @@ impl Step for Clippy {
         cargo.env("HOST_LIBS", host_libs);
 
         cargo.add_rustc_lib_path(builder);
-        let cargo = prepare_cargo_test(cargo, &[], &[], "clippy", compiler, host, builder);
+        let cargo = prepare_cargo_test(cargo, &[], &[], "clippy", host, builder);
 
         let _guard = builder.msg_sysroot_tool(Kind::Test, compiler.stage, "clippy", host, host);
 
@@ -1294,7 +1284,6 @@ impl Step for CrateRunMakeSupport {
             &[],
             "run-make-support",
             "run-make-support self test",
-            compiler,
             host,
             builder,
         );
@@ -1334,16 +1323,7 @@ impl Step for CrateBuildHelper {
             &[],
         );
         cargo.allow_features("test");
-        run_cargo_test(
-            cargo,
-            &[],
-            &[],
-            "build_helper",
-            "build_helper self test",
-            compiler,
-            host,
-            builder,
-        );
+        run_cargo_test(cargo, &[], &[], "build_helper", "build_helper self test", host, builder);
     }
 }
 
@@ -2540,19 +2520,17 @@ impl Step for CrateLibrustc {
 /// Given a `cargo test` subcommand, add the appropriate flags and run it.
 ///
 /// Returns whether the test succeeded.
-#[allow(clippy::too_many_arguments)] // FIXME: reduce the number of args and remove this.
 fn run_cargo_test<'a>(
-    cargo: impl Into<BootstrapCommand>,
+    cargo: builder::Cargo,
     libtest_args: &[&str],
     crates: &[String],
     primary_crate: &str,
     description: impl Into<Option<&'a str>>,
-    compiler: Compiler,
     target: TargetSelection,
     builder: &Builder<'_>,
 ) -> bool {
-    let mut cargo =
-        prepare_cargo_test(cargo, libtest_args, crates, primary_crate, compiler, target, builder);
+    let compiler = cargo.compiler();
+    let mut cargo = prepare_cargo_test(cargo, libtest_args, crates, primary_crate, target, builder);
     let _time = helpers::timeit(builder);
     let _group = description.into().and_then(|what| {
         builder.msg_sysroot_tool(Kind::Test, compiler.stage, what, compiler.host, target)
@@ -2573,15 +2551,15 @@ fn run_cargo_test<'a>(
 
 /// Given a `cargo test` subcommand, pass it the appropriate test flags given a `builder`.
 fn prepare_cargo_test(
-    cargo: impl Into<BootstrapCommand>,
+    cargo: builder::Cargo,
     libtest_args: &[&str],
     crates: &[String],
     primary_crate: &str,
-    compiler: Compiler,
     target: TargetSelection,
     builder: &Builder<'_>,
 ) -> BootstrapCommand {
-    let mut cargo = cargo.into();
+    let compiler = cargo.compiler();
+    let mut cargo: BootstrapCommand = cargo.into();
 
     // Propagate `--bless` if it has not already been set/unset
     // Any tools that want to use this should bless if `RUSTC_BLESS` is set to
@@ -2793,7 +2771,6 @@ impl Step for Crate {
             &self.crates,
             &self.crates[0],
             &*crate_description(&self.crates),
-            compiler,
             target,
             builder,
         );
@@ -2895,7 +2872,6 @@ impl Step for CrateRustdoc {
             &["rustdoc:0.0.0".to_string()],
             "rustdoc",
             "rustdoc",
-            compiler,
             target,
             builder,
         );
@@ -2956,7 +2932,6 @@ impl Step for CrateRustdocJsonTypes {
             &["rustdoc-json-types".to_string()],
             "rustdoc-json-types",
             "rustdoc-json-types",
-            compiler,
             target,
             builder,
         );
@@ -3113,23 +3088,25 @@ impl Step for Bootstrap {
         // Use `python -m unittest` manually if you want to pass arguments.
         check_bootstrap.delay_failure().run(builder);
 
-        let mut cmd = command(&builder.initial_cargo);
-        cmd.arg("test")
-            .current_dir(builder.src.join("src/bootstrap"))
-            .env("RUSTFLAGS", "--cfg test -Cdebuginfo=2")
+        let mut cargo = tool::prepare_tool_cargo(
+            builder,
+            compiler,
+            Mode::ToolBootstrap,
+            host,
+            Kind::Test,
+            "src/bootstrap",
+            SourceType::InTree,
+            &[],
+        );
+
+        cargo
+            .rustflag("-Cdebuginfo=2")
             .env("CARGO_TARGET_DIR", builder.out.join("bootstrap"))
-            .env("RUSTC_BOOTSTRAP", "1")
-            .env("RUSTDOC", builder.rustdoc(compiler))
-            .env("RUSTC", &builder.initial_rustc);
-        if let Some(flags) = option_env!("RUSTFLAGS") {
-            // Use the same rustc flags for testing as for "normal" compilation,
-            // so that Cargo doesn’t recompile the entire dependency graph every time:
-            // https://github.com/rust-lang/rust/issues/49215
-            cmd.env("RUSTFLAGS", flags);
-        }
+            .env("RUSTC_BOOTSTRAP", "1");
+
         // bootstrap tests are racy on directory creation so just run them one at a time.
         // Since there's not many this shouldn't be a problem.
-        run_cargo_test(cmd, &["--test-threads=1"], &[], "bootstrap", None, compiler, host, builder);
+        run_cargo_test(cargo, &["--test-threads=1"], &[], "bootstrap", None, host, builder);
     }
 
     fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
@@ -3254,7 +3231,7 @@ impl Step for RustInstaller {
             bootstrap_host,
             bootstrap_host,
         );
-        run_cargo_test(cargo, &[], &[], "installer", None, compiler, bootstrap_host, builder);
+        run_cargo_test(cargo, &[], &[], "installer", None, bootstrap_host, builder);
 
         // We currently don't support running the test.sh script outside linux(?) environments.
         // Eventually this should likely migrate to #[test]s in rust-installer proper rather than a
@@ -3639,16 +3616,7 @@ impl Step for TestFloatParse {
             &[],
         );
 
-        run_cargo_test(
-            cargo_test,
-            &[],
-            &[],
-            crate_name,
-            crate_name,
-            compiler,
-            bootstrap_host,
-            builder,
-        );
+        run_cargo_test(cargo_test, &[], &[], crate_name, crate_name, bootstrap_host, builder);
 
         // Run the actual parse tests.
         let mut cargo_run = tool::prepare_tool_cargo(

src/bootstrap/src/core/builder/cargo.rs

@@ -121,6 +121,10 @@ impl Cargo {
         cargo
     }
 
+    pub fn compiler(&self) -> Compiler {
+        self.compiler
+    }
+
     pub fn into_cmd(self) -> BootstrapCommand {
         self.into()
     }

src/bootstrap/src/core/config/tests.rs

@@ -92,7 +92,7 @@ fn detect_src_and_out() {
             //     `{build-dir}/bootstrap/debug/deps/bootstrap-c7ee91d5661e2804`
             // `{build-dir}` can be anywhere, not just in the rust project directory.
             let dep = Path::new(args.first().unwrap());
-            let expected_out = dep.ancestors().nth(4).unwrap();
+            let expected_out = dep.ancestors().nth(5).unwrap();
 
             assert_eq!(&cfg.out, expected_out);
         }