-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Allow for passing zero to disable buffering for methods that accept bufferSize and use FileStream #53774
Conversation
Tagging subscribers to this area: @carlossanlop Issue DetailsIssue #53497 Please review
|
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.
@Marusyk overall it looks good to me, but some tests are failing. To run the tests locally:
.\build.cmd -rc Release -subset clr+libs+libs.tests
.\.dotnet\dotnet.exe build -c Debug -f net6.0-windows .\src\libraries\System.IO.FileSystem\tests\System.IO.FileSystem.Tests.csproj /t:Test
If you want to run specific test you need to pass the following property:
/p:XunitMethodName=Namespace.TypeName.MehtodName
src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/IO/StreamReader.cs
Outdated
Show resolved
Hide resolved
@adamsitnik, if 0 is now to be considered valid, why do we still have tests like this? runtime/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_sfh_fa_buffer.cs Lines 21 to 30 in 8709d59
Thanks. |
I've changed that only for |
Ok, but that issue mentions nothing about this public FileStream surface area. |
Because I have a fix for that in #53669 |
Ok, then this PR is blocked on that one. The test failures here are because of that. |
…ufferSize and use FileStream
In #53669 we have fixed it for @Marusyk thank you for your help and apologies for inconvenience from our side |
@adamsitnik could you please check if I on the right way?
Issue #53497