-
Notifications
You must be signed in to change notification settings - Fork 266
Description
In #588, code was added to MockFileSystem to ensure that C:\temp always exists in the list of directories. It does not appear that any kind of toggle was introduced to allow that default behavior to be disabled.
The Problem
The problem with this behavior is that it interferes with certain tests. Everyone's use case is different. To me, it's weird to have MockFileSystem assume certain directories should exist. Those assumptions should be left up to the end user, because how can we possibly know all the different ways the filesystem can be used or should be set up?
In my case, I have code that creates certain directories. I am testing to ensure that this code creates the appropriate directories. I do so by using an equivalency assertion:
var expectedDirs = new[]
{
Paths.CacheDirectory.FullName,
Paths.LogDirectory.FullName,
Paths.ConfigsDirectory.FullName
};
Fs.LeafDirectories().Should().BeEquivalentTo(expectedDirs);Where Fs is of type MockFileSystem and LeafDirectories() is a custom extension method I have to filter out preliminary/parent directories:
public static IEnumerable<string> LeafDirectories(this MockFileSystem fs)
{
return fs.AllDirectories.Where(x => !fs.AllDirectories.Any(y => y.StartsWith(x) && y != x));
}The assertion above fails because C:\temp exists amongst the other directories I'm interested in. In order to get the test to pass, I have to do this:
Fs.LeafDirectories().Should().IntersectWith(expectedDirs);This is a flawed assertion, however, because it doesn't catch the scenario of additional directories being created that I do not want to be created.
Proposed Solution
The ideal solution to me would be to completely remove this code from the constructor. Personally, I disagree with the mock filesystem adding any files or directories on its own. As I stated in the previous section, that should be left up to the tests.
However, I assume that's not an acceptable solution at this point. So there should be some way to toggle the behavior. Some thoughts:
-
Have the temp directory be lazily added when users call
MockPath.GetTempPath(). -
Create a factory class for
MockFileSystemthat allows this behavior to be customized. It's also probably a good idea to have any "setup" code outside of the constructor anyway. Example:var fs = MockFileSystemFactory.CreateWithDefaultPaths();
-
Add a new
MockFileSystemOptionsobject that you pass to a new, optional third parameter toMockFileSystem's constructor that allows configuration of its behavior/initialization. This seems a bit over-engineered, but I suspect this can be expanded on in the future and adding more and more constructor parameters is probably not ideal. Example:var fs = new MockFileSystem(new MockFileSystemOptions { CreateDefaultTempDir = true });
Honestly I'm not a fan of this solution, since it gets complicated by the fact that
MockFileSystemalready has some optional construction parameters and it gets ugly after we try to maintain backward compatibility. The factory is probably a better approach if we had to do something like this. But it's food for thought.
If I had to pick, I'd go with the first option (lazily add the directory) since it shouldn't interfere with my specific use case, since I never use the temp directory. The downside is that it still makes assumptions that might hurt other use cases later, even though I can't come up with any.
I'm happy to submit a PR for any changes, but I do want to discuss options first. Thanks for taking the time to read everything I had to write!