Pull Request #352 introduces breaking change while NuGet package only increases patch version

[ThomasBarnekow]

Today, I tried to update to the latest NuGet package, which is v2.1.0.256 at the time of writing. I was using v2.1.0.178 and did not expect any breaking changes, since only the patch number was updated. However, the projects using the previously parameterless PathWrapper constructor no longer compiled.

Further, even after providing a FileSystem object to the constructor, some of my unit tests then failed. They have been passing with all the previous versions, including v2.1.0.178. This might or might not be related to the breaking API change.

Are you using semantic versioning for your NuGet packages?

[fgreinacher]

We're trying to follow semver, but obviously missed out on this one, sorry about that :/

Can you share some details on the failures you are seeing after the upgrade?

[ThomasBarnekow]

@fgreinacher, as I said, this change causes compile-time errors, because a previously parameter-less constructor now takes a parameter.

After fixing the compile-time errors, some unit tests fail, which is likely due to an issue with the MockFileSystem or MockFile classes. For example, the following code throws a System.IO.DirectoryNotFoundException with a message "Could not find a part of the path 'g1g4sv5d.l4e'.", where 'g1g4sv5d.l4e' is the value of _cacheFilePath.

private void SerializeToCacheFile()
{
    lock (FileLock)
    {
        try
        {
            using (Stream stream = _fileSystem.File.OpenWrite(_cacheFilePath))
            {
                // Do stuff
            }
        }
        catch (Exception ex)
        {
            // Log exception
        }
    }
}

The _fileSystem and _cacheFilePath fields are initialized in the constructor:

public FileServiceInformationCache(string cacheFileName, IFileSystem fileSystem)
{
    if (cacheFileName == null) throw new ArgumentNullException(nameof(cacheFileName));

    _fileSystem = fileSystem ?? throw new ArgumentNullException(nameof(fileSystem));
    _cacheFilePath = _fileSystem.FileInfo.FromFileName(cacheFileName).FullName;

    // Do other stuff
}

This constructor is called from the unit test class' constructor:

public FileServiceInformationCacheTests(ITestOutputHelper output) : base(output)
{
    _fileSystem = new MockFileSystem();
    _cacheFileName = _fileSystem.Path.GetRandomFileName();
}

_cacheFileName is also equal to 'g1g4sv5d.l4e', i.e., the following line of code does not change anything:

_cacheFilePath = _fileSystem.FileInfo.FromFileName(cacheFileName).FullName

Did you change the behavior of those testing helpers as well?

[ThomasBarnekow]

@fgreinacher, I have produced another unit test, which uses the same features and also fails to open the file for writing (and creating) in v2.1.0.256.

[Fact]
public void CanUseMockFileSystem()
{
    var fileSystem = new MockFileSystem();
    string fileName = _fileSystem.Path.GetRandomFileName();

    using (Stream stream = fileSystem.File.OpenWrite(fileName))
    using (var writer = new StreamWriter(stream))
    {
        writer.WriteLine("Hello World!");
    }

    using (Stream stream = fileSystem.File.OpenRead(fileName))
    using (var reader = new StreamReader(stream))
    {
        string line = reader.ReadLine();

        Assert.Equal("Hello World!", line);
    }
}

Here's the relevant portion of the exception:

System.IO.DirectoryNotFoundException
Could not find a part of the path 'cuylzl0q.non'.
   at System.IO.Abstractions.TestingHelpers.MockFile.CreateInternal(String path, Int32 bufferSize, FileOptions options, FileSecurity fileSecurity)
   at System.IO.Abstractions.TestingHelpers.MockFile.Create(String path, Int32 bufferSize, FileOptions options)
   at System.IO.Abstractions.TestingHelpers.MockFile.OpenWriteInternal(String path, FileOptions options)

[ThomasBarnekow]

Having looked at #401, I made some changes to my unit test above. With those changes, it now works. However, as pointed out in #401, the current behavior is not expected and different from the previous behavior.

[Fact]
public void CanUseMockFileSystem2()
{
    var fileSystem = new MockFileSystem();

    // Added to explicitly create and set working directory.
    const string workingDirectory = "C:\\Temp";
    fileSystem.Directory.CreateDirectory(workingDirectory);
    fileSystem.Directory.SetCurrentDirectory(workingDirectory);

    // Changed to create absolute instead of relative path.
    string fileName = fileSystem.Path.Combine(workingDirectory, _fileSystem.Path.GetRandomFileName());

    using (Stream stream = fileSystem.File.OpenWrite(fileName))
    using (var writer = new StreamWriter(stream))
    {
        writer.WriteLine("Hello World!");
    }

    using (Stream stream = fileSystem.File.OpenRead(fileName))
    using (var reader = new StreamReader(stream))
    {
        string line = reader.ReadLine();

        Assert.Equal("Hello World!", line);
    }
}

[fgreinacher]

Thanks for investigating deeper - I’ll try to find some time within the next couple of days!

[fgreinacher]

@ThomasBarnekow The problem is fixed with ec1b673. Could you verify your use case with the prerelease version https://www.nuget.org/packages/System.IO.Abstractions/2.2.10-beta?

[ThomasBarnekow]

@fgreinacher Just installed the above prerelease NuGet package in my solution and ran all unit tests. I can confirm the bug is fixed. All my unit tests pass again.