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 6 commits into
base: main
Choose a base branch
from
+21 −0
Open

🧪 Add unit tests for path_to_string_lossy utility #55

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
119cf59
test(utilities): add unit tests for path_to_string_lossy
google-labs-jules[bot] Apr 20, 2026
3f4df53
Update utilities.rs
bashandbone Apr 26, 2026
39bc3a4
Update utilities.rs
bashandbone Apr 26, 2026
5c486d4
test(utilities): add tests and fix unconditional warning in path_to_s…
google-labs-jules[bot] Apr 26, 2026
8a2bf73
fix: only warn in path_to_string_lossy when conversion is actually lossy
Copilot Apr 26, 2026
4250cd4
fix: remove redundant Cow::Owned guard and rename os_str to os_string
Copilot Apr 26, 2026
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
21 changes: 21 additions & 0 deletions src/utilities.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,9 @@ pub fn path_to_string(path: &std::path::Path) -> Result<String, anyhow::Error> {

/// Convert a `Path` to a `String`, using lossy conversion for non-UTF-8 characters
pub fn path_to_string_lossy(path: &std::path::Path) -> String {
if let Some(s) = path.to_str() {
return s.to_string();
}
let lossy = path.to_string_lossy();
eprintln!(
"Warning: Path contains non-UTF-8 characters, using lossy conversion: {lossy}"
Expand Down Expand Up @@ -443,4 +446,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 +451 to +454
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.
Copy link
Copy Markdown
Owner Author

@bashandbone bashandbone Apr 26, 2026

Choose a reason for hiding this comment

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

@copilot please address and correct this issue you identified

Copy link
Copy Markdown
Contributor

@google-labs-jules google-labs-jules Bot Apr 26, 2026

Choose a reason for hiding this comment

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

I've addressed the issue by modifying path_to_string_lossy to only print a warning when an actual lossy conversion occurs. I've also added unit tests for both valid and invalid UTF-8 paths to ensure the behavior is correct and doesn't regress.

Copy link
Copy Markdown
Contributor

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

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

Fixed in commit 4250cd4. path_to_string_lossy now checks path.to_str() first and returns early for valid UTF-8 paths without any warning. The warning is only emitted when to_str() returns None (i.e., the path truly contains non-UTF-8 bytes and the conversion is lossy).


#[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_string = std::ffi::OsString::from_vec(bytes);
let path = std::path::Path::new(&os_string);

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