-
Couldn't load subscription status.
- Fork 266
feat: Add FileVersionInfo support #1177
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
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.
Thanks for suggesting this change, @t4m45!
While I do like the idea of adding support for FileVersionInfo, I don't like adding it to the IFileInfo interface, as this is different to the public API.
Instead I would suggest adding a IFileInfoFactory to the IFileSystem with a static method GetVersionInfo(string) which returns an instance of IFileInfo. This way you could use it as follows:
// Instead of
FileVersionInfo version = FileVersionInfo.GetVersionInfo("my-library.dll");
// you can use
MockFileSystem fileSystem = new();
IFileVersionInfo version = fileSystem.FileVersionInfo.GetVersionInfo("my-library.dll");This works the same way as the existing API. You could then adapt the parity tests, to also verify, that the interface IFileVersionInfo matches FileVersionInfo.
Would you be willing to adapt your pull request as suggested?
|
Hello @vbreuss! Just for clarity, in the sentence "I would suggest adding a Did you mean 'IFileVersionInfoFactory' and 'IFileVersionInfo' instead of 'IFileInfoFactory' and 'IFileInfo'? |
|
Also, since I'm not really familiar with Github, can you please help me how can I change my pull request? Thanks. |
…uery from IFileInfo to IFileSystem - Added a missing API test for the parity test of IFileVersionInfo and FileVersionInfo
|
I implemented the factory you asked for, and removed the |
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.
Even though I have some comments, I am impressed by the overall approach; great work!
src/TestableIO.System.IO.Abstractions.TestingHelpers/MockFileVersionInfo.cs
Outdated
Show resolved
Hide resolved
tests/TestableIO.System.IO.Abstractions.TestingHelpers.Tests/MockFileSystemTests.cs
Outdated
Show resolved
Hide resolved
tests/TestableIO.System.IO.Abstractions.TestingHelpers.Tests/MockFileVersionInfoFactoryTests.cs
Outdated
Show resolved
Hide resolved
tests/TestableIO.System.IO.Abstractions.TestingHelpers.Tests/MockFileVersionInfoFactoryTests.cs
Outdated
Show resolved
Hide resolved
tests/TestableIO.System.IO.Abstractions.TestingHelpers.Tests/MockFileVersionInfoTests.cs
Outdated
Show resolved
Hide resolved
src/TestableIO.System.IO.Abstractions.TestingHelpers/MockFileVersionInfo.cs
Outdated
Show resolved
Hide resolved
…ockFileSystemTests.cs Co-authored-by: Valentin Breuß <[email protected]>
Co-authored-by: Valentin Breuß <[email protected]>
…ockFileVersionInfoFactoryTests.cs
Major,Minor,Build and Private versions will be calculated from FileVersion and ProductVersion Added tests for the version parsing Converted the expected string to verbatim string
…ional parameters." Reordered the parameters and the properties for convenience
|
When it comes to the If you're okay with it, I can remove the constructor, create a static method called |
Sorry, that was misleading by me. I meant, that you should mimick the static method in the System.Diagnostics namespace the same way we mimick the static |
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.
I have only found some minor comments.
Please also update the version in version.json to "21.2".
src/TestableIO.System.IO.Abstractions.TestingHelpers/MockFileVersionInfo.cs
Outdated
Show resolved
Hide resolved
src/TestableIO.System.IO.Abstractions.TestingHelpers/MockFileVersionInfo.cs
Show resolved
Hide resolved
tests/TestableIO.System.IO.Abstractions.Wrappers.Tests/ApiParityTests.cs
Show resolved
Hide resolved
…lyInformationalVersion attribute. Added Version.TryParse to the FileVersion parsing for a similar reason.
…default value for the optional parameters
…emove the need for this 'out' modifier."
This reverts commit eb509ac.
src/TestableIO.System.IO.Abstractions.TestingHelpers/ProductVersionParser.cs
Outdated
Show resolved
Hide resolved
src/TestableIO.System.IO.Abstractions.TestingHelpers/MockFileVersionInfo.cs
Show resolved
Hide resolved
src/TestableIO.System.IO.Abstractions.TestingHelpers/MockFileVersionInfo.cs
Outdated
Show resolved
Hide resolved
tests/TestableIO.System.IO.Abstractions.TestingHelpers.Tests/MockFileVersionInfoFactoryTests.cs
Outdated
Show resolved
Hide resolved
tests/TestableIO.System.IO.Abstractions.TestingHelpers.Tests/MockFileVersionInfoTests.cs
Outdated
Show resolved
Hide resolved
…uery from IFileInfo to IFileSystem - Added a missing API test for the parity test of IFileVersionInfo and FileVersionInfo
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.
Looks good to me, thanks a lot!
|
Awesome! Thank you for the help! |
|
This is addressed in release v21.2.1. |
Support for `FileVersionInfo` was added in TestableIO/System.IO.Abstractions#1177. Add basic support for the updated interfaces. *It is not yet possible to influence the mocked `IFileVersionInfo` in any way...*


Added a wrapper for the immutable
FileVersionInfo.For testing, the
IFileVersionInfocan be stored in theMockFileDataas a property.Since testing it is rarely neccessary, I didn't add a constructor for it but it's a mutable property, so it can be set anytime after initialization.
A
MockFileVersionInfohas also been added which ismutable.In a normal scenario, the
FileVersionInfocan be accessed through theFileVersionInfoproperty ofIFileInfo.The
MockFileSystem'sAddFilemethod will initialize this version if it's null, and the values will be the default (Only it's FileName will be set).Since the
System.IO.FileInfodoes not contain aFileVersionInfoproperty, I excluded it from theApiParityTestsPS.: Sorry, seems like were some auto formats on my side which removed some extra spaces and the Visual Studio's Diff viewer did not show these before commit. I'm not really familiar with github, thats why I didn't try to revert them after commit.