Add testing of FileStream not truncating files when opening fails#24163
Conversation
|
Is it overkill to have the test being run by all tests for ctors with FileShare? |
poizan42
changed the title
poizan42
force-pushed
the
no-truncate-on-error-test
branch
from
86dcfbf to
520a686
Compare
poizan42
force-pushed
the
no-truncate-on-error-test
branch
from
520a686 to
abcac7a
Compare
|
@poizan42 thanks for your contribution! I have added you as collaborator, so that we can assign the issue #24081 to you (I assume this is a fix for it in combo with dotnet/coreclr#14043). It is GH limitation for assignments :(. |
stephentoub
left a comment
stephentoub
left a comment
There was a problem hiding this comment.
Thanks for adding the test.
| using (FileStream reader = CreateFileStream(fileName, FileMode.Open, FileAccess.Read)) | ||
| { | ||
| byte[] buf = new byte[1]; | ||
| Assert.Equal(1, reader.Read(buf, 0, 1)); |
There was a problem hiding this comment.
Nit: let's also assert that buf[0] == 42
| fs.Flush(); | ||
| try | ||
| { | ||
| CreateFileStream(fileName, fileMode, FileAccess.Write, FileShare.None).Dispose(); |
There was a problem hiding this comment.
Looks like this should use Assert.Throws.
poizan42
changed the title
poizan42
force-pushed
the
no-truncate-on-error-test
branch
from
abcac7a to
6ee6fcc
Compare
|
@stephentoub I have fixed those now |
| { | ||
| fs.Write(new byte[] { 42 }, 0, 1); | ||
| fs.Flush(); | ||
| FSAssert.ThrowsSharingViolation(() => CreateFileStream(fileName, fileMode, FileAccess.Write, FileShare.None).Dispose()); |
There was a problem hiding this comment.
Nit: the Dispose shouldn't be necessary
There was a problem hiding this comment.
But if it doesn't throw then we leave the file open until a GC occurs, does that not matter?
There was a problem hiding this comment.
But if it doesn't throw
That's true, but if it doesn't throw, we have bigger problems :) I'm ok with it either way, but in general we haven't worried about such things elsewhere.
|
@dotnet-bot test this please |
1 similar comment
|
@dotnet-bot test this please |
…/corefx#14043) (dotnet/corefx#24163) Commit migrated from dotnet/corefx@819da8a
6 participants
Test of #24081 (succeeds after dotnet/coreclr#14043)