Skip to content

Commit 9d4620d

Browse files
committed
Set up snapshot tests for bootstrap's path-to-step handling
1 parent 11339a0 commit 9d4620d

File tree

5 files changed

+183
-0
lines changed

5 files changed

+183
-0
lines changed

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3413,6 +3413,13 @@ impl Step for Bootstrap {
34133413
.env("INSTA_WORKSPACE_ROOT", &builder.src)
34143414
.env("RUSTC_BOOTSTRAP", "1");
34153415

3416+
if builder.config.cmd.bless() {
3417+
// Tell `insta` to automatically bless any failing `.snap` files.
3418+
// Unlike compiletest blessing, the tests might still report failure.
3419+
// Does not bless inline snapshots.
3420+
cargo.env("INSTA_UPDATE", "always");
3421+
}
3422+
34163423
run_cargo_test(cargo, &[], &[], None, host, builder);
34173424
}
34183425

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ use std::path::PathBuf;
77

88
use crate::core::builder::{Builder, Kind, PathSet, ShouldRun, StepDescription};
99

10+
#[cfg(test)]
11+
mod tests;
12+
1013
pub(crate) const PATH_REMAP: &[(&str, &[&str])] = &[
1114
// bootstrap.toml uses `rust-analyzer-proc-macro-srv`, but the
1215
// actual path is `proc-macro-srv-cli`
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
source: src/bootstrap/src/core/builder/cli_paths/tests.rs
3+
expression: build
4+
---
5+
[Build] compile::Std
6+
targets: [aarch64-unknown-linux-gnu]
7+
- Set({build::library})
8+
- Set({build::library/alloc})
9+
- Set({build::library/compiler-builtins/compiler-builtins})
10+
- Set({build::library/core})
11+
- Set({build::library/panic_abort})
12+
- Set({build::library/panic_unwind})
13+
- Set({build::library/proc_macro})
14+
- Set({build::library/rustc-std-workspace-core})
15+
- Set({build::library/std})
16+
- Set({build::library/std_detect})
17+
- Set({build::library/sysroot})
18+
- Set({build::library/test})
19+
- Set({build::library/unwind})
20+
[Build] tool::Rustdoc
21+
targets: [x86_64-unknown-linux-gnu]
22+
- Set({build::src/librustdoc})
23+
- Set({build::src/tools/rustdoc})
Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
use std::collections::{BTreeSet, HashSet};
2+
use std::fs;
3+
use std::path::{Path, PathBuf};
4+
use std::sync::{Arc, Mutex};
5+
6+
use crate::Build;
7+
use crate::core::builder::cli_paths::match_paths_to_steps_and_run;
8+
use crate::core::builder::{Builder, StepDescription};
9+
use crate::utils::tests::TestCtx;
10+
11+
fn render_steps_for_cli_args(args_str: &str) -> String {
12+
// Split a single string into a step kind and subsequent arguments.
13+
// E.g. "test ui" => ("test", &["ui"])
14+
let args = args_str.split_ascii_whitespace().collect::<Vec<_>>();
15+
let (kind, args) = args.split_first().unwrap();
16+
17+
// Arbitrary tuple to represent the host system.
18+
let hosts = &["x86_64-unknown-linux-gnu"];
19+
// Arbitrary tuple to represent the target system, which might not be the host.
20+
let targets = &["aarch64-unknown-linux-gnu"];
21+
22+
let config = TestCtx::new()
23+
.config(kind)
24+
// `test::Bootstrap` is only run by default in CI, causing inconsistency.
25+
.arg("--ci=false")
26+
.args(args)
27+
.hosts(hosts)
28+
.targets(targets)
29+
.create_config();
30+
let mut build = Build::new(config);
31+
// Some rustdoc test steps are only run by default if nodejs is
32+
// configured/discovered, causing inconsistency.
33+
build.config.nodejs = Some(PathBuf::from("node"));
34+
let mut builder = Builder::new(&build);
35+
36+
// Tell the builder to log steps that it would run, instead of running them.
37+
let mut buf = Arc::new(Mutex::new(String::new()));
38+
let buf2 = Arc::clone(&buf);
39+
builder.log_cli_step_for_tests = Some(Box::new(move |step_desc, pathsets, targets| {
40+
use std::fmt::Write;
41+
let mut buf = buf2.lock().unwrap();
42+
43+
let StepDescription { name, kind, .. } = step_desc;
44+
// Strip boilerplate to make step names easier to read.
45+
let name = name.strip_prefix("bootstrap::core::build_steps::").unwrap_or(name);
46+
47+
writeln!(buf, "[{kind:?}] {name}").unwrap();
48+
writeln!(buf, " targets: {targets:?}").unwrap();
49+
for pathset in pathsets {
50+
// Normalize backslashes in paths, to avoid snapshot differences on Windows.
51+
// FIXME(Zalathar): Doing a string-replace on <PathSet as Debug>
52+
// is a bit unprincipled, but it's good enough for now.
53+
let pathset_str = format!("{pathset:?}").replace('\\', "/");
54+
writeln!(buf, " - {pathset_str}").unwrap();
55+
}
56+
}));
57+
58+
builder.execute_cli();
59+
60+
String::clone(&buf.lock().unwrap())
61+
}
62+
63+
fn snapshot_test_inner(name: &str, args_str: &str) {
64+
let mut settings = insta::Settings::clone_current();
65+
// Use the test name as the snapshot filename, not its whole fully-qualified name.
66+
settings.set_prepend_module_to_snapshot(false);
67+
settings.bind(|| {
68+
insta::assert_snapshot!(name, render_steps_for_cli_args(args_str), args_str);
69+
});
70+
}
71+
72+
/// Keep the snapshots directory tidy by forbidding `.snap` files that don't
73+
/// correspond to a test name.
74+
fn no_unused_snapshots_inner(known_test_names: &[&str]) {
75+
let known_test_names = known_test_names.iter().copied().collect::<HashSet<&str>>();
76+
77+
let mut unexpected_file_names = BTreeSet::new();
78+
79+
// FIXME(Zalathar): Is there a better way to locate the snapshots dir?
80+
for entry in walkdir::WalkDir::new("src/core/builder/cli_paths/snapshots")
81+
.into_iter()
82+
.map(Result::unwrap)
83+
{
84+
let meta = entry.metadata().unwrap();
85+
if !meta.is_file() {
86+
continue;
87+
}
88+
89+
let name = entry.file_name().to_str().unwrap();
90+
if let Some(name_stub) = name.strip_suffix(".snap")
91+
&& !known_test_names.contains(name_stub)
92+
{
93+
unexpected_file_names.insert(name.to_owned());
94+
}
95+
}
96+
97+
assert!(
98+
unexpected_file_names.is_empty(),
99+
"Found snapshot files that don't correspond to a test name: {unexpected_file_names:#?}",
100+
);
101+
}
102+
103+
macro_rules! declare_tests {
104+
(
105+
$( ($name:ident, $args:literal) ),* $(,)?
106+
) => {
107+
$(
108+
#[test]
109+
fn $name() {
110+
snapshot_test_inner(stringify!($name), $args);
111+
}
112+
)*
113+
114+
#[test]
115+
fn no_unused_snapshots() {
116+
let known_test_names = &[ $( stringify!($name), )* ];
117+
no_unused_snapshots_inner(known_test_names);
118+
}
119+
};
120+
}
121+
122+
// Snapshot tests for bootstrap's command-line path-to-step handling.
123+
//
124+
// To bless these tests as necessary, choose one:
125+
// - Run `INSTA_UPDATE=always ./x test bootstrap`
126+
// - Run `./x test bootstrap --bless`
127+
// - Follow the instructions for `cargo-insta` in bootstrap's README.md
128+
//
129+
// These snapshot tests capture _current_ behavior, to prevent unintended
130+
// changes or regressions. If the current behavior is wrong or undersirable,
131+
// then any fix will necessarily have to re-bless the affected tests!
132+
declare_tests!(
133+
// tidy-alphabetical-start
134+
(x_build, "build"),
135+
// tidy-alphabetical-end
136+
);

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,12 @@ pub struct Builder<'a> {
6666

6767
/// Cached list of submodules from self.build.src.
6868
submodule_paths_cache: OnceLock<Vec<String>>,
69+
70+
/// When enabled by tests, this causes the top-level steps that _would_ be
71+
/// executed to be logged instead. Used by snapshot tests of command-line
72+
/// paths-to-steps handling.
73+
#[expect(clippy::type_complexity)]
74+
log_cli_step_for_tests: Option<Box<dyn Fn(&StepDescription, &[PathSet], &[TargetSelection])>>,
6975
}
7076

7177
impl Deref for Builder<'_> {
@@ -447,6 +453,13 @@ impl StepDescription {
447453
// Determine the targets participating in this rule.
448454
let targets = if self.is_host { &builder.hosts } else { &builder.targets };
449455

456+
// Log the step that's about to run, for snapshot tests.
457+
if let Some(ref log_cli_step) = builder.log_cli_step_for_tests {
458+
log_cli_step(self, &pathsets, targets);
459+
// Return so that the step won't actually run in snapshot tests.
460+
return;
461+
}
462+
450463
for target in targets {
451464
let run = RunConfig { builder, paths: pathsets.clone(), target: *target };
452465
(self.make_run)(run);
@@ -1079,6 +1092,7 @@ impl<'a> Builder<'a> {
10791092
time_spent_on_dependencies: Cell::new(Duration::new(0, 0)),
10801093
paths,
10811094
submodule_paths_cache: Default::default(),
1095+
log_cli_step_for_tests: None,
10821096
}
10831097
}
10841098

0 commit comments

Comments
 (0)