Skip to content

Commit 1c0cae3

Browse files
committed
Set up snapshot tests for bootstrap's path-to-step handling
1 parent c5dabe8 commit 1c0cae3

File tree

5 files changed

+179
-0
lines changed

5 files changed

+179
-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
@@ -3395,6 +3395,13 @@ impl Step for Bootstrap {
33953395
.env("INSTA_WORKSPACE_ROOT", &builder.src)
33963396
.env("RUSTC_BOOTSTRAP", "1");
33973397

3398+
if builder.config.cmd.bless() {
3399+
// Tell `insta` to automatically bless any failing `.snap` files.
3400+
// Unlike compiletest blessing, the tests might still report failure.
3401+
// Does not bless inline snapshots.
3402+
cargo.env("INSTA_UPDATE", "always");
3403+
}
3404+
33983405
run_cargo_test(cargo, &[], &[], None, host, builder);
33993406
}
34003407

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: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
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+
writeln!(buf, " - {pathset:?}").unwrap();
51+
}
52+
}));
53+
54+
builder.execute_cli();
55+
56+
String::clone(&buf.lock().unwrap())
57+
}
58+
59+
fn snapshot_test_inner(name: &str, args_str: &str) {
60+
let mut settings = insta::Settings::clone_current();
61+
// Use the test name as the snapshot filename, not its whole fully-qualified name.
62+
settings.set_prepend_module_to_snapshot(false);
63+
settings.bind(|| {
64+
insta::assert_snapshot!(name, render_steps_for_cli_args(args_str), args_str);
65+
});
66+
}
67+
68+
/// Keep the snapshots directory tidy by forbidding `.snap` files that don't
69+
/// correspond to a test name.
70+
fn no_unused_snapshots_inner(known_test_names: &[&str]) {
71+
let known_test_names = known_test_names.iter().copied().collect::<HashSet<&str>>();
72+
73+
let mut unexpected_file_names = BTreeSet::new();
74+
75+
// FIXME(Zalathar): Is there a better way to locate the snapshots dir?
76+
for entry in walkdir::WalkDir::new("src/core/builder/cli_paths/snapshots")
77+
.into_iter()
78+
.map(Result::unwrap)
79+
{
80+
let meta = entry.metadata().unwrap();
81+
if !meta.is_file() {
82+
continue;
83+
}
84+
85+
let name = entry.file_name().to_str().unwrap();
86+
if let Some(name_stub) = name.strip_suffix(".snap")
87+
&& !known_test_names.contains(name_stub)
88+
{
89+
unexpected_file_names.insert(name.to_owned());
90+
}
91+
}
92+
93+
assert!(
94+
unexpected_file_names.is_empty(),
95+
"Found snapshot files that don't correspond to a test name: {unexpected_file_names:#?}",
96+
);
97+
}
98+
99+
macro_rules! declare_tests {
100+
(
101+
$( ($name:ident, $args:literal) ),* $(,)?
102+
) => {
103+
$(
104+
#[test]
105+
fn $name() {
106+
snapshot_test_inner(stringify!($name), $args);
107+
}
108+
)*
109+
110+
#[test]
111+
fn no_unused_snapshots() {
112+
let known_test_names = &[ $( stringify!($name), )* ];
113+
no_unused_snapshots_inner(known_test_names);
114+
}
115+
};
116+
}
117+
118+
// Snapshot tests for bootstrap's command-line path-to-step handling.
119+
//
120+
// To bless these tests as necessary, choose one:
121+
// - Run `INSTA_UPDATE=always ./x test bootstrap`
122+
// - Run `./x test bootstrap --bless`
123+
// - Follow the instructions for `cargo-insta` in bootstrap's README.md
124+
//
125+
// These snapshot tests capture _current_ behavior, to prevent unintended
126+
// changes or regressions. If the current behavior is wrong or undersirable,
127+
// then any fix will necessarily have to re-bless the affected tests!
128+
declare_tests!(
129+
// tidy-alphabetical-start
130+
(x_build, "build"),
131+
// tidy-alphabetical-end
132+
);

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);
@@ -1078,6 +1091,7 @@ impl<'a> Builder<'a> {
10781091
time_spent_on_dependencies: Cell::new(Duration::new(0, 0)),
10791092
paths,
10801093
submodule_paths_cache: Default::default(),
1094+
log_cli_step_for_tests: None,
10811095
}
10821096
}
10831097

0 commit comments

Comments
 (0)