Skip to content

Commit ad14280

Browse files
authored
Added better filesystem layout testing harness (#15874)
### What does this PR try to resolve? The goal is to make filesystem layout tests easier to understand and review. The `build-dir` test's have a [helper fns](https://github.com/rust-lang/cargo/blob/master/tests/testsuite/build_dir.rs#L720) that have imperative checks. While this works, its not easy to see what its validating at a glance. My thought is to reuse the snapbox infrastructure we already have for the cargo ui tests. The prototype in this PR generates a file tree (like the unix tree command) and uses snapbox to compare against a snapshot. This makes the layout tests much easier to read and failures are much more obvious what what went wrong. ### Implementation Details * The `LayoutTree` struct handles comparison logic (as opposed to using snapbox directly) * This avoids the issues initial implementation (ordering and platform specific files, see full details below) * If the comparison fails, 2 human readable tree's are generated and snapbox is used to display the diffs. * Note: These diffs are in-memory thus, do not support `SNAPSHOTS=overwrite`. This is arguably good as often the overwrite would be incorrect as the snapshots are platform specific. * Snapshots would support conditional files based on the target os / platform env * Files/directories may be suffixed with `[target_platform=<target-platform>]` * Multiple platforms may be specified separated by comma <details><summary>initial implementation issues</summary> However there are some problems with the current implementation that limit the usefulness of it. 1. Different platforms have different files which cause some tests to fail * Examples * lib prefix/suffixes based on platform * `dSYM` on macos * One idea I have would be to have a cfg suffix after file name in the tree like so * `└── foo-[HASH].dSYM [cfg(target_os = "macos")]` * This would also require rethinking the "tree lines" (`└`, `├`, `│`) to handle optional files. 2. When dealing with build scripts there are multiple `target/<profile>/build/pkg-[HASH]` directories. The hash changes the order of the directories when generating the tree making snapshots inconsistent. * We have redactions to handle replacing the hash with a placeholder `[HASH]`, but this does not help with the order. </details> --- Fun fact: These changes were written and tested at [Rust Forge 2025](https://rustforgeconf.com/) in New Zealand. 🇳🇿 🥳
2 parents c49e38c + 1e93a2d commit ad14280

File tree

7 files changed

+429
-149
lines changed

7 files changed

+429
-149
lines changed

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ cargo-credential-macos-keychain = { version = "0.4.18", path = "credential/cargo
3232
cargo-credential-wincred = { version = "0.4.18", path = "credential/cargo-credential-wincred" }
3333
cargo-platform = { path = "crates/cargo-platform", version = "0.3.0" }
3434
cargo-test-macro = { version = "0.4.7", path = "crates/cargo-test-macro" }
35-
cargo-test-support = { version = "0.8.2", path = "crates/cargo-test-support" }
35+
cargo-test-support = { version = "0.9.0", path = "crates/cargo-test-support" }
3636
cargo-util = { version = "0.2.25", path = "crates/cargo-util" }
3737
cargo-util-schemas = { version = "0.10.1", path = "crates/cargo-util-schemas" }
3838
cargo_metadata = "0.21.0"

crates/cargo-test-support/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "cargo-test-support"
3-
version = "0.8.2"
3+
version = "0.9.0"
44
edition.workspace = true
55
rust-version = "1.90" # MSRV:1
66
license.workspace = true

crates/cargo-test-support/src/compare.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,12 @@ fn add_regex_redactions(subs: &mut snapbox::Redactions) {
229229
.unwrap();
230230
subs.insert("[HASH]", regex!(r"/[a-z0-9\-_]+-(?<redacted>[0-9a-f]{16})"))
231231
.unwrap();
232+
// Match multi-part hashes like `06/b451d0d6f88b1d` used in directory paths
233+
subs.insert("[HASH]", regex!(r"/(?<redacted>[a-f0-9]{2}\/[0-9a-f]{14})"))
234+
.unwrap();
235+
// Match file name hashes like `foo-06b451d0d6f88b1d`
236+
subs.insert("[HASH]", regex!(r"[a-z0-9]+-(?<redacted>[a-f0-9]{16})"))
237+
.unwrap();
232238
subs.insert(
233239
"[AVG_ELAPSED]",
234240
regex!(r"(?<redacted>[0-9]+(\.[0-9]+)?) ns/iter"),

crates/cargo-test-support/src/paths.rs

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
//! Access common paths and manipulate the filesystem
22
33
use filetime::FileTime;
4+
use itertools::Itertools;
5+
use walkdir::WalkDir;
46

57
use std::cell::RefCell;
68
use std::env;
@@ -12,6 +14,9 @@ use std::sync::Mutex;
1214
use std::sync::OnceLock;
1315
use std::sync::atomic::{AtomicUsize, Ordering};
1416

17+
use crate::compare::assert_e2e;
18+
use crate::compare::match_contains;
19+
1520
static CARGO_INTEGRATION_TEST_DIR: &str = "cit";
1621

1722
static GLOBAL_ROOT: OnceLock<Mutex<Option<PathBuf>>> = OnceLock::new();
@@ -152,6 +157,10 @@ pub trait CargoPathExt {
152157
fn move_in_time<F>(&self, travel_amount: F)
153158
where
154159
F: Fn(i64, u32) -> (i64, u32);
160+
161+
fn assert_build_dir_layout(&self, expected: impl snapbox::IntoData);
162+
163+
fn assert_dir_layout(&self, expected: impl snapbox::IntoData, ignored_path_patterns: &[String]);
155164
}
156165

157166
impl CargoPathExt for Path {
@@ -236,6 +245,43 @@ impl CargoPathExt for Path {
236245
});
237246
}
238247
}
248+
249+
#[track_caller]
250+
fn assert_build_dir_layout(&self, expected: impl snapbox::IntoData) {
251+
// We call `unordered()` here to because the build-dir has some scenarios that make
252+
// consistent ordering not possible.
253+
// Notably:
254+
// 1. Binaries with `.exe` on Windows causing the ordering to change with the dep-info `.d`
255+
// file.
256+
// 2. Directories with hashes are often reordered differently by platform.
257+
self.assert_dir_layout(expected.unordered(), &build_dir_ignored_path_patterns());
258+
}
259+
260+
#[track_caller]
261+
fn assert_dir_layout(
262+
&self,
263+
expected: impl snapbox::IntoData,
264+
ignored_path_patterns: &[String],
265+
) {
266+
let assert = assert_e2e();
267+
let actual = WalkDir::new(self)
268+
.sort_by_file_name()
269+
.into_iter()
270+
.filter_map(|e| e.ok())
271+
.filter(|e| e.file_type().is_file())
272+
.map(|e| e.path().to_string_lossy().into_owned())
273+
.filter(|file| {
274+
for ignored in ignored_path_patterns {
275+
if match_contains(&ignored, file, &assert.redactions()).is_ok() {
276+
return false;
277+
}
278+
}
279+
return true;
280+
})
281+
.join("\n");
282+
283+
assert.eq(format!("{actual}\n"), expected);
284+
}
239285
}
240286

241287
impl CargoPathExt for PathBuf {
@@ -260,6 +306,21 @@ impl CargoPathExt for PathBuf {
260306
{
261307
self.as_path().move_in_time(travel_amount)
262308
}
309+
310+
#[track_caller]
311+
fn assert_build_dir_layout(&self, expected: impl snapbox::IntoData) {
312+
self.as_path().assert_build_dir_layout(expected);
313+
}
314+
315+
#[track_caller]
316+
fn assert_dir_layout(
317+
&self,
318+
expected: impl snapbox::IntoData,
319+
ignored_path_patterns: &[String],
320+
) {
321+
self.as_path()
322+
.assert_dir_layout(expected, ignored_path_patterns);
323+
}
263324
}
264325

265326
fn do_op<F>(path: &Path, desc: &str, mut f: F)
@@ -290,6 +351,20 @@ where
290351
}
291352
}
292353

354+
/// The paths to ignore when [`CargoPathExt::assert_build_dir_layout`] is called
355+
fn build_dir_ignored_path_patterns() -> Vec<String> {
356+
vec![
357+
// Ignore MacOS debug symbols as there are many files/directories that would clutter up
358+
// tests few not a lot of benefit.
359+
"[..].dSYM/[..]",
360+
// Ignore Windows debub symbols files (.pdb)
361+
"[..].pdb",
362+
]
363+
.into_iter()
364+
.map(ToString::to_string)
365+
.collect()
366+
}
367+
293368
/// Get the filename for a library.
294369
///
295370
/// `kind` should be one of:

src/doc/contrib/src/tests/writing.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,16 @@ test.
6464
- See [`support::compare`] for an explanation of the string pattern matching.
6565
Patterns are used to make it easier to match against the expected output.
6666

67+
#### Filesystem layout testing
68+
69+
Tests often to need to verify Cargo created/removed files.
70+
The `CargoPathExt` trait (implemented by `Path` and `PathBuf`) provides a `assert_dir_layout()` to verify the files in a directory (including nested directories).
71+
This takes a snapshot of file paths for the given directory and asserts that all files are present and no new files have been created.
72+
This function also takes a list of patterns to ignore from the snapshot to make working with platform specific files easier.
73+
74+
Note: You will commonly need to call `unordered()` before passing your snapshot to deal with platform differences like binaries having `.exe` on Windows.
75+
`assert_build_dir_layout` is a more specialized version of `assert_dir_layout()` that is automatically unordered and ignores common platform specific files designed for the Cargo build cache.
76+
6777
#### Testing Nightly Features
6878

6979
If you are testing a Cargo feature that only works on "nightly" Cargo, then

0 commit comments

Comments
 (0)