Skip to content
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

Augment tests for FileMode.Append #55513

Merged
merged 1 commit into from
Jul 13, 2021

Conversation

stephentoub
Copy link
Member

To validate that seeking and writing are valid in the region of the file since its initial length. Codifying my comments from:
#55465 (comment)
#55465 (comment)
#55465 (comment)

@ghost
Copy link

ghost commented Jul 12, 2021

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

To validate that seeking and writing are valid in the region of the file since its initial length. Codifying my comments from:
#55465 (comment)
#55465 (comment)
#55465 (comment)

Author: stephentoub
Assignees: -
Labels:

area-System.IO

Milestone: -

To validate that seeking and writing are valid in the region of the file since its initial length.
fs.Write(Encoding.ASCII.GetBytes("abcde"));
Assert.Equal(5, fs.Length);
Assert.Equal(5, fs.Position);
Assert.Equal(1, fs.Seek(1, SeekOrigin.Begin));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect that FileMode.Append means "I want to append data to EOF" rather than "I want to write data at requested offset". The official doc for FileMode.Append says:

Trying to seek to a position before the end of the file throws an IOException exception

I believe that current implementation of Append is buggy and we should fix it instead of adding a test that ensures that it's buggy. I am going to reopen #55465 for that

Copy link
Member Author

@stephentoub stephentoub Aug 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I interpret that to mean before the end of the file at the time it was opened, and the current implementation is consistent with that and has been for 20 years. If you believe it's worth taking a breaking change, that's a fine discussion to have, but it's absolutely a breaking change.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding a test that ensures that it's buggy

The tests validate the expected behavior.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants