Skip to content

std fs tests: avoid matching on OS-provided error string #156387

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

Merged
rust-bors merged 1 commit into from
May 10, 2026
Merged

std fs tests: avoid matching on OS-provided error string #156387

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
55 changes: 10 additions & 45 deletions library/std/src/fs/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,25 +31,6 @@ macro_rules! check {
};
}

#[cfg(windows)]
macro_rules! error {
($e:expr, $s:expr) => {
match $e {
Ok(_) => panic!("Unexpected success. Should've been: {:?}", $s),
Err(ref err) => {
assert!(err.raw_os_error() == Some($s), "`{}` did not have a code of `{}`", err, $s)
}
}
};
}

#[cfg(unix)]
macro_rules! error {
($e:expr, $s:expr) => {
error_contains!($e, $s)
};
}

macro_rules! error_contains {
($e:expr, $s:expr) => {
match $e {
Expand Down Expand Up @@ -105,29 +86,17 @@ fn file_test_io_smoke_test() {
fn invalid_path_raises() {
let tmpdir = tmpdir();
let filename = &tmpdir.join("file_that_does_not_exist.txt");
let result = File::open(filename);

#[cfg(all(unix, not(target_os = "vxworks")))]
error!(result, "No such file or directory");
#[cfg(target_os = "vxworks")]
error!(result, "no such file or directory");
#[cfg(windows)]
error!(result, 2); // ERROR_FILE_NOT_FOUND
let err = File::open(filename).unwrap_err();
assert_eq!(err.kind(), ErrorKind::NotFound);
}

#[test]
fn file_test_iounlinking_invalid_path_should_raise_condition() {
let tmpdir = tmpdir();
let filename = &tmpdir.join("file_another_file_that_does_not_exist.txt");

let result = fs::remove_file(filename);

#[cfg(all(unix, not(target_os = "vxworks")))]
error!(result, "No such file or directory");
#[cfg(target_os = "vxworks")]
error!(result, "no such file or directory");
#[cfg(windows)]
error!(result, 2); // ERROR_FILE_NOT_FOUND
let err = fs::remove_file(filename).unwrap_err();
assert_eq!(err.kind(), ErrorKind::NotFound);
}

#[test]
Expand Down Expand Up @@ -939,12 +908,8 @@ fn recursive_rmdir_of_file_fails() {
let tmpdir = tmpdir();
let canary = tmpdir.join("do_not_delete");
check!(check!(File::create(&canary)).write(b"foo"));
let result = fs::remove_dir_all(&canary);
#[cfg(unix)]
error!(result, "Not a directory");
#[cfg(windows)]
error!(result, 267); // ERROR_DIRECTORY - The directory name is invalid.
assert!(result.is_err());
let err = fs::remove_dir_all(&canary).unwrap_err();
assert_eq!(err.kind(), ErrorKind::NotADirectory);
assert!(canary.exists());
}

Expand Down Expand Up @@ -1404,6 +1369,7 @@ fn open_flavors() {
let mut ra = OO::new();
ra.read(true).append(true);

// This error string is set by std itself so we are not at the whim of the OS here.
let invalid_options = "creating or truncating a file requires write or append access";

// Test various combinations of creation modes and access modes.
Expand Down Expand Up @@ -2148,7 +2114,6 @@ fn test_hidden_file_truncation() {

// See https://github.com/rust-lang/rust/pull/131072 for more details about why
// these two tests are disabled under Windows 7 here.
#[cfg(windows)]
#[test]
#[cfg_attr(target_vendor = "win7", ignore = "Unsupported under Windows 7.")]
fn test_rename_file_over_open_file() {
Expand All @@ -2174,10 +2139,9 @@ fn test_rename_file_over_open_file() {
}

#[test]
#[cfg(windows)]

@RalfJung RalfJung May 10, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't know why this test was made Windows-only, but if we check the ErrorKind it also works on my Linux machine.

View changes since the review

@ChrisDenton ChrisDenton May 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The test was added to ensure the new Windows fs::rename implementation worked as intended. Still, there should be no issue making it a cross-platform test. The only risk would be if there's a platform with odd behaviour here.

@RalfJung RalfJung May 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I found one more test which also seems to work fine on Linux, so I enabled that one too. Always nice when things are consistent across targets. :)

#[cfg_attr(target_vendor = "win7", ignore = "Unsupported under Windows 7.")]
fn test_rename_directory_to_non_empty_directory() {
// Renaming a directory over a non-empty existing directory should fail on Windows.
// Renaming a directory over a non-empty existing directory should fail.
let tmpdir: TempDir = tmpdir();

let source_path = tmpdir.join("source_directory");
Expand All @@ -2188,7 +2152,8 @@ fn test_rename_directory_to_non_empty_directory() {

fs::write(target_path.join("target_file.txt"), b"target hello world").unwrap();

error!(fs::rename(source_path, target_path), 145); // ERROR_DIR_NOT_EMPTY
let err = fs::rename(source_path, target_path).unwrap_err();
assert_eq!(err.kind(), ErrorKind::DirectoryNotEmpty);
}

#[test]
Expand Down
Loading