Skip to content

Commit 02d93a5

Browse files
committed
refactor(test): return probe results from fuse_probe instead of discarding them
Rename `fuse_available` to `fuse_probe` and have it return `FuseTools` containing the discovered fusermount command. The caller now uses this instead of re-probing at unmount time. Also fix misleading error messages: on Ubuntu, `fuse2fs` is a separate package from `e2fsprogs`, so the hints now recommend the correct package names. https://claude.ai/code/session_01LfpnUZrgq93MVZgA3KVqE6
1 parent 6506aa3 commit 02d93a5

1 file changed

Lines changed: 39 additions & 26 deletions

File tree

tests/one_file_system.rs

Lines changed: 39 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -63,24 +63,37 @@ fn same_device_on_sample_workspace() {
6363
);
6464
}
6565

66-
/// Checks that `fuse2fs` and FUSE infrastructure are available.
66+
/// Information about the available FUSE tools, discovered by [`fuse_probe`].
67+
#[cfg(target_os = "linux")]
68+
#[cfg(not(pdu_test_skip_cross_device))]
69+
struct FuseTools {
70+
/// The fusermount command to use for unmounting (`"fusermount"` or `"fusermount3"`).
71+
fusermount: &'static str,
72+
}
73+
74+
/// Probes for `fuse2fs` and FUSE infrastructure.
6775
///
6876
/// Verifies:
6977
/// 1. `fuse2fs` binary exists
7078
/// 2. `/dev/fuse` is accessible
7179
/// 3. `fusermount` (or `fusermount3`) binary exists
7280
///
73-
/// Returns `Ok(())` on success, or `Err` with a diagnostic message on failure.
81+
/// Returns `Ok(FuseTools)` with the discovered tool paths, or `Err` with a diagnostic message.
7482
#[cfg(target_os = "linux")]
7583
#[cfg(not(pdu_test_skip_cross_device))]
76-
fn fuse_available() -> Result<(), String> {
84+
fn fuse_probe() -> Result<FuseTools, String> {
7785
use std::{path::Path, process::Command};
7886

7987
// Check that fuse2fs is installed
8088
Command::new("fuse2fs")
8189
.arg("--help")
8290
.output()
83-
.map_err(|error| format!("`fuse2fs` not found: {error}. Install e2fsprogs or fuse2fs."))?;
91+
.map_err(|error| {
92+
format!(
93+
"`fuse2fs` not found: {error}. \
94+
Install the `fuse2fs` package (or `e2fsprogs` on distros that bundle it)."
95+
)
96+
})?;
8497

8598
// Check that /dev/fuse is accessible
8699
if !Path::new("/dev/fuse").exists() {
@@ -94,13 +107,17 @@ fn fuse_available() -> Result<(), String> {
94107
// Check that fusermount is available (needed for unmounting)
95108
let has_fusermount = Command::new("fusermount").arg("-V").output().is_ok();
96109
let has_fusermount3 = Command::new("fusermount3").arg("-V").output().is_ok();
97-
if !has_fusermount && !has_fusermount3 {
98-
return Err(
99-
"Neither `fusermount` nor `fusermount3` found. Install fuse or fuse3.".to_string(),
100-
);
101-
}
110+
let fusermount = match (has_fusermount, has_fusermount3) {
111+
(true, _) => "fusermount",
112+
(_, true) => "fusermount3",
113+
_ => {
114+
return Err(
115+
"Neither `fusermount` nor `fusermount3` found. Install fuse or fuse3.".to_string(),
116+
);
117+
}
118+
};
102119

103-
Ok(())
120+
Ok(FuseTools { fusermount })
104121
}
105122

106123
/// When a subdirectory is a mount point for a different filesystem, `-x` should exclude it.
@@ -120,13 +137,15 @@ fn cross_device_excludes_mount() {
120137
time::Duration,
121138
};
122139

123-
if let Err(reason) = fuse_available() {
124-
panic!(
140+
let fuse_tools = match fuse_probe() {
141+
Ok(tools) => tools,
142+
Err(reason) => panic!(
125143
"error: This test requires FUSE (`fuse2fs`, `/dev/fuse`, `fusermount`) but the probe failed.\n\
126144
reason: {reason}\n\
127-
hint: Install e2fsprogs and fuse, or set `RUSTFLAGS='--cfg pdu_test_skip_cross_device'` to skip this test.",
128-
);
129-
}
145+
hint: Install `fuse2fs` and `fuse3` packages, or set \
146+
`RUSTFLAGS='--cfg pdu_test_skip_cross_device'` to skip this test.",
147+
),
148+
};
130149

131150
let pdu = env!("CARGO_BIN_EXE_pdu");
132151
let temp = Temp::new_dir().expect("create temp dir for cross-device test");
@@ -236,21 +255,15 @@ fn cross_device_excludes_mount() {
236255
);
237256
}));
238257

239-
// Always unmount — try fusermount first, fall back to fusermount3
240-
let unmount_status = Command::new("fusermount")
258+
// Always unmount using the fusermount variant discovered by fuse_probe
259+
let unmount_status = Command::new(fuse_tools.fusermount)
241260
.with_arg("-u")
242261
.with_arg(&mount_point)
243-
.status()
244-
.or_else(|_| {
245-
Command::new("fusermount3")
246-
.with_arg("-u")
247-
.with_arg(&mount_point)
248-
.status()
249-
});
262+
.status();
250263
match unmount_status {
251264
Ok(status) if status.success() => {}
252-
Ok(status) => eprintln!("warning: fusermount exited with {status}"),
253-
Err(error) => eprintln!("warning: failed to run fusermount: {error}"),
265+
Ok(status) => eprintln!("warning: {} exited with {status}", fuse_tools.fusermount),
266+
Err(error) => eprintln!("warning: failed to run {}: {error}", fuse_tools.fusermount),
254267
}
255268

256269
if let Err(payload) = test_result {

0 commit comments

Comments
 (0)