-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
[Deps] Upgrade System.IO.Abstractions #35656
base: main
Are you sure you want to change the base?
[Deps] Upgrade System.IO.Abstractions #35656
Conversation
Love this. |
620b2d2
to
896ea12
Compare
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Code LGTM. Thanks for the contribution.
Will wait for and test a Dart build before approving.
Added a commit to fix signing of the new dependencies as well.
[DataRow(@"c:\Test>", 3, 3, true, DisplayName = "2 Folders recursive")] | ||
[DataRow(@"c:\not-exist>", 3, 3, true, DisplayName = "Folder not exist, return root recursive")] | ||
[DataRow(@"c:\Test>", 3, 0, true, DisplayName = "2 Folders recursive")] | ||
[DataRow(@"c:\not-exist>", 3, 0, true, DisplayName = "Folder not exist, return root recursive")] |
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.
Interesting, folders stopped being reported as files as well?
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.
Not clear but there is something weird on mock file system side (previous returned items were wrong too). I think we can keep the test anyway. Plugin is working as expected.
EDIT: my bad, ignore the previous statement. Thanks for adding them to signing! |
Summary of the Pull Request
PowerToys is still using an old version of System.IO.Abstraction, dated 2022: https://www.nuget.org/packages/System.IO.Abstractions
This PR upgrade it to the latest stable version and address changes.
PR Checklist
Detailed Description of the Pull Request / Additional comments
Reduce dependency/complexity a little bit: Common Logger and Language Helper aren't tested and the file system abstraction isn't really needed.
Asserts were wrong even using previous package version. It seems that the mocked file system is acting differently while enumerating files and folders. Think we shouldn't remove these tests in case it will be fixed in future releases.
Validation Steps Performed