Skip to content

🧪 Add unit tests for path_to_string_lossy utility #55

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bashandbone wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🧪 Add unit tests for path_to_string_lossy utility #55

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/utilities.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,4 +443,22 @@ mod tests {
let result = get_sparse_paths(Some(vec![])).unwrap();
assert_eq!(result, Some(vec![]));
}

#[test]
fn test_path_to_string_lossy_valid() {
let path = std::path::Path::new("valid_utf8");
assert_eq!(path_to_string_lossy(path), "valid_utf8");
}
Comment on lines +448 to +451
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

path_to_string_lossy() currently always prints the "non-UTF-8" warning via eprintln! even when the input path is valid UTF-8. This new test will therefore emit a misleading warning (and adds noise when running tests with --nocapture). Consider changing path_to_string_lossy to only warn when the conversion is actually lossy (e.g., when to_string_lossy() returns an owned Cow, or when path.to_str() is None).

Copilot uses AI. Check for mistakes.

#[test]
#[cfg(unix)]
fn test_path_to_string_lossy_invalid() {
use std::os::unix::ffi::OsStringExt;
let bytes = vec![0x61, 0xFF, 0x62]; // 'a', invalid, 'b'
let os_str = std::ffi::OsString::from_vec(bytes);
let path = std::path::Path::new(&os_str);
Comment on lines +458 to +459
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os_str is an OsString here, not an OsStr. Renaming this local to something like os_string would better reflect the type and avoid confusion with OsStr.

Suggested change
let os_str = std::ffi::OsString::from_vec(bytes);
let path = std::path::Path::new(&os_str);
let os_string = std::ffi::OsString::from_vec(bytes);
let path = std::path::Path::new(&os_string);

Copilot uses AI. Check for mistakes.

let result = path_to_string_lossy(path);
assert_eq!(result, format!("a{}b", std::char::REPLACEMENT_CHARACTER));
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assertion builds the expected string with format!, which allocates unnecessarily in a tight unit test. Prefer comparing against a literal like "a\u{FFFD}b" (or otherwise constructing the expected String without formatting) to keep the test simpler and allocation-free.

Suggested change
assert_eq!(result, format!("a{}b", std::char::REPLACEMENT_CHARACTER));
assert_eq!(result, "a\u{FFFD}b");

Copilot uses AI. Check for mistakes.
}
}
Loading