Set up snapshot tests for bootstrap's path-to-step handling

Author: Zalathar

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

@@ -3395,6 +3395,13 @@ impl Step for Bootstrap {
             .env("INSTA_WORKSPACE_ROOT", &builder.src)
             .env("RUSTC_BOOTSTRAP", "1");
 
+        if builder.config.cmd.bless() {
+            // Tell `insta` to automatically bless any failing `.snap` files.
+            // Unlike compiletest blessing, the tests might still report failure.
+            // Does not bless inline snapshots.
+            cargo.env("INSTA_UPDATE", "always");
+        }
+
         run_cargo_test(cargo, &[], &[], None, host, builder);
     }
 

src/bootstrap/src/core/builder/cli_paths.rs

@@ -7,6 +7,9 @@ use std::path::PathBuf;
 
 use crate::core::builder::{Builder, Kind, PathSet, ShouldRun, StepDescription};
 
+#[cfg(test)]
+mod tests;
+
 pub(crate) const PATH_REMAP: &[(&str, &[&str])] = &[
     // bootstrap.toml uses `rust-analyzer-proc-macro-srv`, but the
     // actual path is `proc-macro-srv-cli`

src/bootstrap/src/core/builder/cli_paths/snapshots/x_build.snap

@@ -0,0 +1,23 @@
+---
+source: src/bootstrap/src/core/builder/cli_paths/tests.rs
+expression: build
+---
+[Build] compile::Std
+    targets: [aarch64-unknown-linux-gnu]
+    - Set({build::library})
+    - Set({build::library/alloc})
+    - Set({build::library/compiler-builtins/compiler-builtins})
+    - Set({build::library/core})
+    - Set({build::library/panic_abort})
+    - Set({build::library/panic_unwind})
+    - Set({build::library/proc_macro})
+    - Set({build::library/rustc-std-workspace-core})
+    - Set({build::library/std})
+    - Set({build::library/std_detect})
+    - Set({build::library/sysroot})
+    - Set({build::library/test})
+    - Set({build::library/unwind})
+[Build] tool::Rustdoc
+    targets: [x86_64-unknown-linux-gnu]
+    - Set({build::src/librustdoc})
+    - Set({build::src/tools/rustdoc})

src/bootstrap/src/core/builder/cli_paths/tests.rs

@@ -0,0 +1,123 @@
+use std::collections::{BTreeSet, HashSet};
+use std::fs;
+use std::path::Path;
+use std::sync::{Arc, Mutex};
+
+use crate::Build;
+use crate::core::builder::cli_paths::match_paths_to_steps_and_run;
+use crate::core::builder::{Builder, StepDescription};
+use crate::utils::tests::TestCtx;
+
+fn render_steps_for_cli_args(args_str: &str) -> String {
+    // Split a single string into a step kind and subsequent arguments.
+    // E.g. "test ui" => ("test", &["ui"])
+    let args = args_str.split_ascii_whitespace().collect::<Vec<_>>();
+    let (kind, args) = args.split_first().unwrap();
+
+    // Arbitrary tuple to represent the host system.
+    let hosts = &["x86_64-unknown-linux-gnu"];
+    // Arbitrary tuple to represent the target system, which might not be the host.
+    let targets = &["aarch64-unknown-linux-gnu"];
+
+    let config =
+        TestCtx::new().config(kind).args(args).hosts(hosts).targets(targets).create_config();
+    let build = Build::new(config);
+    let mut builder = Builder::new(&build);
+
+    // Tell the builder to log steps that it would run, instead of running them.
+    let mut buf = Arc::new(Mutex::new(String::new()));
+    let buf2 = Arc::clone(&buf);
+    builder.log_cli_step_for_tests = Some(Box::new(move |step_desc, pathsets, targets| {
+        use std::fmt::Write;
+        let mut buf = buf2.lock().unwrap();
+
+        let StepDescription { name, kind, .. } = step_desc;
+        // Strip boilerplate to make step names easier to read.
+        let name = name.strip_prefix("bootstrap::core::build_steps::").unwrap_or(name);
+
+        writeln!(buf, "[{kind:?}] {name}").unwrap();
+        writeln!(buf, "    targets: {targets:?}").unwrap();
+        for pathset in pathsets {
+            writeln!(buf, "    - {pathset:?}").unwrap();
+        }
+    }));
+
+    builder.execute_cli();
+
+    String::clone(&buf.lock().unwrap())
+}
+
+fn snapshot_test_inner(name: &str, args_str: &str) {
+    let mut settings = insta::Settings::clone_current();
+    // Use the test name as the snapshot filename, not its whole fully-qualified name.
+    settings.set_prepend_module_to_snapshot(false);
+    settings.bind(|| {
+        insta::assert_snapshot!(name, render_steps_for_cli_args(args_str), args_str);
+    });
+}
+
+/// Keep the snapshots directory tidy by forbidding `.snap` files that don't
+/// correspond to a test name.
+fn no_unused_snapshots_inner(known_test_names: &[&str]) {
+    let known_test_names = known_test_names.iter().copied().collect::<HashSet<&str>>();
+
+    let mut unexpected_file_names = BTreeSet::new();
+
+    // FIXME(Zalathar): Is there a better way to locate the snapshots dir?
+    for entry in walkdir::WalkDir::new("src/core/builder/cli_paths/snapshots")
+        .into_iter()
+        .map(Result::unwrap)
+    {
+        let meta = entry.metadata().unwrap();
+        if !meta.is_file() {
+            continue;
+        }
+
+        let name = entry.file_name().to_str().unwrap();
+        if let Some(name_stub) = name.strip_suffix(".snap")
+            && !known_test_names.contains(name_stub)
+        {
+            unexpected_file_names.insert(name.to_owned());
+        }
+    }
+
+    assert!(
+        unexpected_file_names.is_empty(),
+        "Found snapshot files that don't correspond to a test name: {unexpected_file_names:#?}",
+    );
+}
+
+macro_rules! declare_tests {
+    (
+        $( ($name:ident, $args:literal) ),* $(,)?
+    ) => {
+        $(
+            #[test]
+            fn $name() {
+                snapshot_test_inner(stringify!($name), $args);
+            }
+        )*
+
+        #[test]
+        fn no_unused_snapshots() {
+            let known_test_names = &[ $( stringify!($name), )* ];
+            no_unused_snapshots_inner(known_test_names);
+        }
+    };
+}
+
+// Snapshot tests for bootstrap's command-line path-to-step handling.
+//
+// To bless these tests as necessary, choose one:
+// - Run `INSTA_UPDATE=always ./x test bootstrap`
+// - Run `./x test bootstrap --bless`
+// - Follow the instructions for `cargo-insta` in bootstrap's README.md
+//
+// These snapshot tests capture _current_ behavior, to prevent unintended
+// changes or regressions. If the current behavior is wrong or undersirable,
+// then any fix will necessarily have to re-bless the affected tests!
+declare_tests!(
+    // tidy-alphabetical-start
+    (x_build, "build"),
+    // tidy-alphabetical-end
+);

src/bootstrap/src/core/builder/mod.rs

@@ -66,6 +66,11 @@ pub struct Builder<'a> {
 
     /// Cached list of submodules from self.build.src.
     submodule_paths_cache: OnceLock<Vec<String>>,
+
+    /// When enabled by tests, this causes the top-level steps that _would_ be
+    /// executed to be logged instead. Used by snapshot tests of command-line
+    /// paths-to-steps handling.
+    log_cli_step_for_tests: Option<Box<dyn Fn(&StepDescription, &[PathSet], &[TargetSelection])>>,
 }
 
 impl Deref for Builder<'_> {
@@ -447,6 +452,13 @@ impl StepDescription {
         // Determine the targets participating in this rule.
         let targets = if self.is_host { &builder.hosts } else { &builder.targets };
 
+        // Log the step that's about to run, for snapshot tests.
+        if let Some(ref log_cli_step) = builder.log_cli_step_for_tests {
+            log_cli_step(self, &pathsets, &targets);
+            // Return so that the step won't actually run in snapshot tests.
+            return;
+        }
+
         for target in targets {
             let run = RunConfig { builder, paths: pathsets.clone(), target: *target };
             (self.make_run)(run);
@@ -1078,6 +1090,7 @@ impl<'a> Builder<'a> {
             time_spent_on_dependencies: Cell::new(Duration::new(0, 0)),
             paths,
             submodule_paths_cache: Default::default(),
+            log_cli_step_for_tests: None,
         }
     }