Skip to content

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

Open
RalfJung wants to merge 1 commit intorust-lang:mainfrom
RalfJung:fs-error-str
Open

std fs tests: avoid matching on OS-provided error string#156387
RalfJung wants to merge 1 commit intorust-lang:mainfrom
RalfJung:fs-error-str

Conversation

@RalfJung
Copy link
Copy Markdown
Member

@RalfJung RalfJung commented May 10, 2026

These tests are ancient. No idea why there written in this style back then, but today we'd clearly check the ErrorKind, not the string.

r? @ChrisDenton

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 10, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 10, 2026

ChrisDenton is not on the review rotation at the moment.
They may take a while to respond.

RalfJung
RalfJung commented May 10, 2026
View reviewed changes
Comment thread library/std/src/fs/tests.rs
}

#[test]
#[cfg(windows)]
Copy link
Copy Markdown
Member Author

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

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

Copy link
Copy Markdown
Member

@ChrisDenton ChrisDenton May 10, 2026

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.

Copy link
Copy Markdown
Member Author

@RalfJung RalfJung May 10, 2026

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. :)

@RalfJung
Copy link
Copy Markdown
Member Author

RalfJung commented May 10, 2026

@bors try jobs=x86_64-msvc-*,test-various,aarch64-apple

@rust-bors

This comment has been minimized.

Sign in to view
rust-bors Bot pushed a commit that referenced this pull request May 10, 2026
@bors
Auto merge of #156387 - RalfJung:fs-error-str, r=<try>
e5cad87 
std fs tests: avoid matching on OS-provided error string


try-job: x86_64-msvc-*
try-job: test-various
try-job: aarch64-apple
@RalfJung
Copy link
Copy Markdown
Member Author

RalfJung commented May 10, 2026

@bors try jobs=x86_64-msvc-*,test-various,aarch64-apple

@rust-bors

This comment has been minimized.

Sign in to view
rust-bors Bot pushed a commit that referenced this pull request May 10, 2026
@bors
Auto merge of #156387 - RalfJung:fs-error-str, r=<try>
8498627 
std fs tests: avoid matching on OS-provided error string


try-job: x86_64-msvc-*
try-job: test-various
try-job: aarch64-apple
@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented May 10, 2026

☀️ Try build successful (CI)
Build commit: 8498627 (849862729d217499f1cd22f747c7ba0495841c7c, parent: d1961bebe091816d8ce9771f29ad471dda398f5d)

@ChrisDenton
Copy link
Copy Markdown
Member

ChrisDenton commented May 10, 2026

Thanks!

@bors r+

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented May 10, 2026

📌 Commit a0298aa has been approved by ChrisDenton

It is now in the queue for this repository.

@rust-bors rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 10, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request May 10, 2026
@JonathanBrouwer
Rollup merge of rust-lang#156387 - RalfJung:fs-error-str, r=ChrisDenton
dd38e55 
std fs tests: avoid matching on OS-provided error string

These tests are [ancient](rust-lang@6bfbad9). No idea why there written in this style back then, but today we'd clearly check the `ErrorKind`, not the string.

r? @ChrisDenton
@JonathanBrouwer JonathanBrouwer mentioned this pull request May 10, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ChrisDenton ChrisDenton ChrisDenton left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@ChrisDenton ChrisDenton

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@RalfJung @rustbot @ChrisDenton