fix: parallel access to IFileStream
#799
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When using parallel write to streams, in each stream only update the corresponding written bytes.
vbreuss
force-pushed
the
topic/fix-parallel-stream-access
branch
from
9a18f6a to
c27e6f4
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR enhances parallel write support for IFileStream by ensuring only the modified byte ranges are flushed per stream, and adds tests to validate this behavior.
- Added parallel write and flush behavior tests covering overlapping writes, differing lengths, and empty writes.
- Updated
FileStreamMockto track minimum and maximum write positions and flush only the modified segments. - Improved
TimerMockTestsby adding a null check before inspecting the exception message.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| Tests/Testably.Abstractions.Tests/FileSystem/FileStream/ParallelTests.cs | New tests verifying parallel write and flush behaviors under various scenarios. |
| Tests/Testably.Abstractions.Testing.Tests/TimeSystem/TimerMockTests.cs | Added an explicit null check for the exception message in the disposed timer test. |
| Source/Testably.Abstractions.Testing/FileSystem/FileStreamMock.cs | Introduced _minWrite/_maxWrite tracking, adjusted Write, WriteAsync, and flush logic to use these ranges. |
Comments suppressed due to low confidence (2)
Source/Testably.Abstractions.Testing/FileSystem/FileStreamMock.cs:180
- [nitpick] The fields
_minWriteand_maxWritecould be renamed to_minWritePositionand_maxWritePositionto more clearly convey that they represent stream positions.
private long _maxWrite;
Source/Testably.Abstractions.Testing/FileSystem/FileStreamMock.cs:607
- There are overrides for asynchronous writes that adjust
_minWrite/_maxWrite, but no tests appear to cover async write and flush behavior. Consider adding unit tests to ensure the range-flush logic works correctly forWriteAsync.
public override async Task WriteAsync(byte[] buffer, int offset, int count,
Source/Testably.Abstractions.Testing/FileSystem/FileStreamMock.cs
Outdated
Show resolved
Hide resolved
|
This PR should cover the cases mentioned in TestableIO/System.IO.Abstractions#1131. |
|
|
This is addressed in release v4.2.0. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR enhances parallel write support for
IFileStreamby ensuring only the modified byte ranges are flushed per stream, and adds tests to validate this behavior.FileStreamMockto track minimum and maximum write positions and flush only the modified segments.