- 
                Notifications
    You must be signed in to change notification settings 
- Fork 5.2k
Re-enable Windows test that verifies DriveInfo.VolumeLabel setter fails on SUBST'd drive #59850
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
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? | 
| @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
        
              
          
                src/libraries/Common/tests/System/IO/VirtualDriveHelper.Windows.cs
              
                Outdated
          
            Show resolved
            Hide resolved
        
      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.
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.