MockFileSystem does not handle paths consistently across platforms

[rcdailey]

Describe the bug

Forward and backward slashes are not uniformly handled across operating systems; specifically Linux and Windows.

To Reproduce

Example unit test:

[Test, AutoMockData]
public void Return_app_data_dir_location_if_base_directory_location_not_present(
    [Frozen] IAppContext appContext,
    [Frozen(Matching.ImplementedInterfaces)] MockFileSystem fs,
    [Frozen] IAppPaths paths,
    ConfigurationFinder sut)
{
    paths.ConfigPath.Returns("app/data/recyclarr.yml");

    var path = sut.FindConfigPath();

    path.Should().EndWith(@"app/data/recyclarr.yml");
}

Expected behavior

The reproducible example above is not meant to be run directly. It's just to demonstrate how I like to handle paths.

What I expect is that, when hard-coded paths are provided, that any \\ are converted to / and / is used on all platforms. The one guarantee we have between all operating systems is that forward-slash works, even on Windows. When MockFileSystem is used, it should have logic to convert paths for me.

The workaround right now is to use _fs.Path.Combine(), but this is syntactically tedious. I deal with hard-coded paths rarely, if not never in production code. They're used extremely often and very useful in unit tests. So I think it makes sense to have some special handling for "path normalization" for purposes of testing.

If there is risk of this breaking some use cases, I'd be happy with a method on MockFileSystem to enable this globally or per-instance.

[fgreinacher]

Thanks for taking the time to describe your use-case nicely.

Does #754 help?

[rcdailey]

Thanks. That's definitely an improvement, but I would say I much prefer this be built into AddFile() and AddDirectory() and done automatically, if this does not create other problems. Example:

var fs = new MockFileSystem();
fs.AddFile(@"base\\path\\file.txt", new MockFileData(""));
// fs.AllFiles should have:
//   - Windows: "C:/base/path/file.txt" 
//   - Linux: "/base/path/file.txt"

Basically inside AddFile(), it should do path.Replace('\\', '/');. Then I know for sure in my unit tests all hard-coded strings can be treated the same, regardless of platform. Most importantly, at least as it concerns this issue, there's no additional "syntactic overhead" to get this behavior.

If we're concerned about backward compatibility, I can think of a couple of ways we can address it:

• Add something like MockFileSystem.EnablePathNormalization(bool) and have this be false by default. I'd make it static so that I can set it globally and all tests will inherit the behavior.
• A NormalizingMockFileSystem subclass that adds the normalization logic on top of the MockFileSystem.
• A new method, AddNormalizedPath(), that does the Replace() before passing it to AddFile(). In fact, having this as an extension method would be perfect. I can't say for sure this 100% addresses the problem as I'm sure you'll get something like this: "C:\base/path/file.txt". So the root path that gets added on needs to be converted as well.

I'm sure there's a number of ways to configure this that we can find that works. I'm sadly not that familiar with the implementation of MockFileSystem so I can't get much more specific than that.

[rcdailey]

To add to this: How is one supposed to created a rooted path for testing purposes in MockFileSystem? I have this utility method to help me create a MockFileSystem:

private static MockFileSystem NewMockFileSystem(IEnumerable<string> files, IEnumerable<string> dirs, string cwd)
{
    var dirData = dirs.Select(x => (x, (MockFileData) new MockDirectoryData()));
    var fileData = files.Select(x => (x, new MockFileData("")));

    return new MockFileSystem(
        fileData.Concat(dirData).ToDictionary(x => x.Item1, y => y.Item2),
        MockUnixSupport.Path(cwd));
}

I get an exception because 'cwd' is not rooted when I use it like this:

var files = new[]
{
    "path1/1/file1.txt",
    "path1/1/file2.txt",
    "path1/1/2/3/4/file3.txt",
    "path1/file4.txt"
};

var dirs = new[]
{
    "path1/empty1",
    "path1/empty2",
    "path1/1/2/empty3",
    "path1/1/2/3/4/empty4"
};

var fs = NewMockFileSystem(files, dirs, "root/path");

Note that I leave off the root path specification (e.g. / on linux or C:\ on windows) because that is not portable. Yet again, another aspect of the file system I feel like MockFileSystem should abstract away from me for testing purposes.

I don't see a workaround for this. Something like MockUnixSupport.MakeRootedPath("root/path").

[rcdailey]

I think I'd like to help fix this. I will clone the repo and play around with MockFileSystem and see if I can automate some of this stuff on Linux.

[fgreinacher]

To add to this: How is one supposed to created a rooted path for testing purposes in MockFileSystem? I have this utility method to help me create a MockFileSystem

Would you mind creating a new issue for this?

[fgreinacher]

I would say I much prefer this be built into AddFile() and AddDirectory() and done automatically, if this does not create other problems.

We generally try to stay with the behavior of the real file system and keep "special" concerns outside of the core. We have an extensions project maintained by @gigi81 though where I think this would fit very well.

[gigi81]

With the current extensions methods in TestableIO.System.IO.Abstractions.Extensions package, the initial example, instead of writing:

"app/data/recyclarr.yml"

you can write:

fs.CurrentDirectory().SubDirectory("app").SubDirectory("data").File("recyclarr.yml")

I understand it is not exactly compact, but it is portable, less verbose and more explicit than using Path.Combine

Happy to entertain other suggestions/idea. Please open a ticket here: https://github.com/TestableIO/System.IO.Abstractions.Extensions/issues

[rcdailey]

I have continued the discussion over on the extensions repo, as @gigi81 suggested. Thanks to both of you for your time and constructive feedback! I'll resolve this issue.