FileStreamWrapper breaks patternmatching type testing

[cryolithic]

trafficstars

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. https://github.com/TestableIO/System.IO.Abstractions/compare/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.

[vbreuss]

@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 #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?

[cryolithic]

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.

[vbreuss]

@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?

[fgreinacher]

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

[fgreinacher]

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