-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
Add initial unit tests #71
Conversation
Using big brother implementation: https://github.com/dotnet/runtime/tree/main/src/libraries/System.IO.FileSystem.Primitives/tests
Add fixes to enums.
Note that FileAttributes are (actually (or can be up to)) 3 bytes in length, so probably should be handling Reasoning is that we should support it (even if FatFS does not) for future support with FileX etc. |
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.
Good start, few comments
System.IO.FileSystem/tests/System.IO.FileSystem.Tests/Properties/AssemblyInfo.cs
Outdated
Show resolved
Hide resolved
</RunConfiguration> | ||
<nanoFrameworkAdapter> | ||
<Logging>None</Logging> | ||
<IsRealHardware>False</IsRealHardware> |
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.
are the tests running in the virtual device? Is the System.IO.File having a native implementation supported there?
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.
Currently the tests are just checking that the enumerations are correct (in theory) aligned to the full fat .NET implementation. We need to make sure these can align with the native etc..
Update some nuget packages.
Something seems broken with the latest test framework (nuget) so CI build will fail.
This reverts commit 5526eb6.
OK and kudos for adding unit tests! Always welcome. OK to for making the enum type match the full .NET ones. |
As nF does not (currently) use them.
I see your point, but would rather see a not supported exception for them. We can always remove them on the next pass as more unit tests are added. |
run tests
Can we throw an exception for a non existing enum item? 😯 I don't think I've seen that... 🤔
|
Pesudo code:
And I still think that the And, there is nothing stopping someone just specifiying a byte instead of the enum anyway, so should possibily be handled in native. Also noting that we have never supported file share attributes, yet still include them?! But this is why we need to have some unit tests! |
@networkfusion I get that you can add those checks on the library code. My concern is with the code calling our library. We can't enforce those to be checked. Any user code using those enums that have no implementation it will build succesfully but those can not be honoured (whathever they are doing). |
Something seems broken with the latest test framework (nuget) so CI build will fail.
This reverts commit 5526eb6.
As nF does not (currently) use them.
run tests
Seems broken locally, but does build!
Allows quicker intergration.
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Closing this as #98 is adding them. |
Description
This library did not contain unit tests.
Adds Initial tests using big brother implementation from: https://github.com/dotnet/runtime/tree/main/src/libraries/System.IO.FileSystem.Primitives/tests
Further tests "could" be obtained from https://github.com/dotnet/runtime/tree/main/src/libraries/System.IO.FileSystem/tests
Motivation and Context
.Net
as far as possible.How Has This Been Tested?
All changes should be non intrusive.
Screenshots
Types of changes
Checklist: