Skip to content

FileStreamWrapper breaks patternmatching type testing #963

Closed
Closed
FileStreamWrapper breaks patternmatching type testing#963
Labels
area: coreIssues that address the core abstractions & the wrappersstate: won't doIssues that won't be addressed. Reasons differ and will be provided as comments.type: bugIssues that describe misbehaving functionality

Description

@cryolithic
cryolithic
opened on Mar 19, 2023

Describe the bug
I was updating a library and moved from 17.11 to latest. This broke a section of code that was testing for the type of stream.
if(Stream is not FileStream fileStream) { throw new ..... }
This was introduced in v 18 with the changes to FileInfoBase/FileInfoWrapper to return FileStreamWrapper over the previous Stream.
v17.2.3...v18.0.1#diff-a71278a0c7619aba1cbf96a25bf62ef95a0441ff6e8c6f8a9ce28ccf7dbca603

To Reproduce
With a version at 18 or higher try the following code:

var fs = new System.IO.Abstractions.FileSystem();
var fi = fs.FileInfo.New("path to a real file");
var st = fi.Open(FileMode.Open);
if (st is not FileStream)
{
	throw new Exception("stream is not filestream");
}

Observe exception is thrown.

Expected behavior
A clear and concise description of what you expected to happen.
With similar code prior to v18

var fs = new System.IO.Abstractions.FileSystem();
var fi = fs.FileInfo.FromFileName("path to a real file");
var st = fi.Open(FileMode.Open);
if (st is not FileStream)
{
	throw new Exception("stream is not filestream");
}

The exception is not thrown.

Activity

  1. cryolithic
    added
    state: needs discussionIssues that need further discussion
    type: bugIssues that describe misbehaving functionality
    on Mar 19, 2023

  2. vbreuss commented on Apr 5, 2023

    @vbreuss
    vbreuss
    on Apr 5, 2023
    Member

    @cryolithic :
    I am afraid, that we don't have any options to resolve your issue.

    • We intentionally changed the return type to a custom Stream class due to IFile.Create has different return type than File.Create #779.
    • Directly using FileStream is not possible, as the class is sealed and directly affects the file system, so we cannot overwrite/replace it's functionality in the test class.
      So in order to work with Streams we had to replace them with a custom wrapper
    • Using an interface as in other factory classes (e.g. IFileStream) instead would also not help in your case, as FileStream does not implement it and we would no longer return a Stream at all...

    What do you want to achieve with this test of yours?

  3. cryolithic commented on Apr 9, 2023

    @cryolithic
    cryolithic
    on Apr 9, 2023
    Author

    The method in question acts as a factory (implementation is a bit more complex, but the details shouldn't matter). It accepts a Stream, and creates a StorageProvider to match the specifics of the stream type as we have different behaviour for a FileStream vs say a MemoryStream.

    I worked around it in my code, but any libraries that make a similar check will break when using Abstractions.

  4. vbreuss commented on Apr 10, 2023

    @vbreuss
    vbreuss
    on Apr 10, 2023
    Member

    @cryolithic:
    Unfortunately I underestimated this impact in #906 and didn't mark it explicitely as breaking change in itself, so the major version got bumped due to the refactoring in general, but this change is not explicitely mentioned in the changelog.

    I don't have any idea for a workaround that doesn't involve this breaking change. Do you?

    @fgreinacher:
    Can we improve the visibility of this breaking change in the changelog for version 18?

  5. fgreinacher commented on Apr 13, 2023

    @fgreinacher
    fgreinacher
    on Apr 13, 2023
    Contributor

    Can we improve the visibility of this breaking change in the changelog for version 18?

    @vbreuss I added this to the release notes: https://github.com/TestableIO/System.IO.Abstractions/releases/tag/v18.0.1

  6. fgreinacher commented on Jul 14, 2023

    @fgreinacher
    fgreinacher
    on Jul 14, 2023
    Contributor

    I'll go ahead close this. Feel free to reopen/comment if anything else can be improved.

  7. fgreinacher
    added
    area: coreIssues that address the core abstractions & the wrappers
    state: won't doIssues that won't be addressed. Reasons differ and will be provided as comments.
    and removed
    state: needs discussionIssues that need further discussion
    on Jul 14, 2023
  8. vbreuss
    closed this as completedon Mar 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area: coreIssues that address the core abstractions & the wrappersstate: won't doIssues that won't be addressed. Reasons differ and will be provided as comments.type: bugIssues that describe misbehaving functionality

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions