Resolving Windows environment test failures#156562
Conversation
|
rustbot has assigned @Mark-Simulacrum. Use Why was this reviewer chosen?The reviewer was selected based on:
|
| use crate::os::windows::fs::FileTimesExt; | ||
|
|
||
| let tmp = tmpdir(); | ||
| #[cfg(windows)] |
There was a problem hiding this comment.
The cfgs are redundant, none of the other uses of got_symlink_permission use them.
There was a problem hiding this comment.
Then, do I just need to make sure that the test passes locally in a Windows environment as well?
There was a problem hiding this comment.
I would recommend testing it in a Windows env/VM, and run the same command as AngelicosPhosphorus: python x.py test library/std --stage 1 (confirm that the OS doesn't have the required privileges for the test as mentioned by Angelicos). It should report no errors if it works.
Also, repeating what petro is saying, the #[cfg(windows)] line is redundant because internally got_symlink_permission returns true if the OS this test is running on is not windows or env var is "CI", so you don't need to worry about the return early exit here.
There was a problem hiding this comment.
I confirmed that when running python x.py test library/std --stage 1 on a Windows 10 environment, all tests passed successfully regarding the PR I submitted.
It is correct that the part I modified skips two tests due to permission issues in the Windows environment, so it has been confirmed to be working correctly.
There was a problem hiding this comment.
I'm glad to hear that the tests are passing. Are you able to amend your commit to remove the #[cfg(windows)] lines?
There was a problem hiding this comment.
I have modified and completed the commit.
This resolves an issue where the
fs::tests::test_fs_set_times follows symlinkandfs::tests::test_fs_set_times_nofollowtests failed locally due to permission issues in a Windows environment.The code has been modified so that these tests do not proceed if permissions are not granted.
Since these tests can be passed with the necessary permissions via CI before merging, I believe it is appropriate for them to pass locally due to permission issues rather than fail.
Close #156558