-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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.
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
Re-enable Windows test that verifies DriveInfo.VolumeLabel setter fails on SUBST'd drive #59850
Conversation
Tagging subscribers to this area: @dotnet/area-system-io Issue DetailsFixes #14027 I'm reusing code I recently added to verify SUBST with symbolic links. I moved it to its own class for virtual drive manipulation on Windows.
|
Investigating this failure. I don't know why it is happening in x64 Windows Debug, since it's the one I tested locally:
Edit: Ah, I think it's the Net5Compat test project. |
@carlossanlop could you solve the conflict? |
396a5dd
to
a3e5080
Compare
@jozkee @adamsitnik sorry for the delay. I resolved the conflict: Net5Compat has been recently removed, and that project was also the source of the CI failure. Can you please take a look again? |
Edit: I opened issue #62028 The CI leg
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I left some comments but they're all related to refactoring ideas. Thank you @carlossanlop !
src/libraries/Common/tests/System/IO/VirtualDriveHelper.Windows.cs
Outdated
Show resolved
Hide resolved
string system32 = Path.Join(systemRoot, "System32"); | ||
return Path.Join(system32, "subst.exe"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: you can provide multiple parameters to Combine
and avoid the need of having an additional local variable
string system32 = Path.Join(systemRoot, "System32"); | |
return Path.Join(system32, "subst.exe"); | |
return Path.Join(systemRoot, "System32", "subst.exe"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You know, this could be an interesting candidate for a Roslyn Analyzer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/libraries/System.IO.FileSystem.DriveInfo/tests/DriveInfo.Windows.Tests.cs
Outdated
Show resolved
Hide resolved
src/libraries/Common/tests/System/IO/VirtualDriveHelper.Windows.cs
Outdated
Show resolved
Hide resolved
src/libraries/Common/tests/System/IO/VirtualDriveHelper.Windows.cs
Outdated
Show resolved
Hide resolved
src/libraries/Common/tests/System/IO/VirtualDriveHelper.Windows.cs
Outdated
Show resolved
Hide resolved
src/libraries/Common/tests/System/IO/VirtualDriveHelper.Windows.cs
Outdated
Show resolved
Hide resolved
a3e5080
to
c5676d7
Compare
Fixes #14027
I'm reusing code I recently added to verify SUBST with symbolic links. I moved it to its own class for virtual drive manipulation on Windows.