New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MockFileSystem sets current directory to actual FS #350

Open
dzarda opened this Issue Aug 24, 2018 · 6 comments

Comments

Projects
None yet
5 participants
@dzarda

dzarda commented Aug 24, 2018

It appears that the MockFileSystem constructor calls real filesystem API when the currentDirectory parameter is left empty.

The current directory is then set as C:\Users\...\AppData\Local\Temp

I suspect the following code

    public MockFileSystem(IDictionary<string, MockFileData> files, string currentDirectory = "")
    {
        if (string.IsNullOrEmpty(currentDirectory))
        {
            currentDirectory = IO.Path.GetTempPath();
        }
        ...

in

@fgreinacher

This comment has been minimized.

Show comment
Hide comment
@fgreinacher

fgreinacher Sep 16, 2018

Collaborator

Thanks for reporting this! I see a few options here:

  • Not setting the current directory unless explicitly specified. Unsure what would be the effect of this though.
  • Setting it to some hardcoded path, e.g. c:\temp. Should be safe and is predictable.
  • Leaving it as is. I don't see too big a problem with the current approach as getting the temp path is non-invasive.

Thoughts?

Collaborator

fgreinacher commented Sep 16, 2018

Thanks for reporting this! I see a few options here:

  • Not setting the current directory unless explicitly specified. Unsure what would be the effect of this though.
  • Setting it to some hardcoded path, e.g. c:\temp. Should be safe and is predictable.
  • Leaving it as is. I don't see too big a problem with the current approach as getting the temp path is non-invasive.

Thoughts?

@josteink

This comment has been minimized.

Show comment
Hide comment
@josteink

josteink Oct 2, 2018

Setting it to some hardcoded path, e.g. c:\temp. Should be safe and is predictable.

That's a very Windows-centric point of view :)

josteink commented Oct 2, 2018

Setting it to some hardcoded path, e.g. c:\temp. Should be safe and is predictable.

That's a very Windows-centric point of view :)

@ewlloyd

This comment has been minimized.

Show comment
Hide comment
@ewlloyd

ewlloyd Oct 2, 2018

After a quick glance, I think that not setting it at all would cause issues in the MockDirectory.EnsureAbsolutePath() method (used by MockDirectory.GetFiles() and MockDirectory.EnumerateDirectories()).

The only question I'd have with leaving it as is, is whether there'd ever be a case where you wouldn't have the real IO.Path available.

ewlloyd commented Oct 2, 2018

After a quick glance, I think that not setting it at all would cause issues in the MockDirectory.EnsureAbsolutePath() method (used by MockDirectory.GetFiles() and MockDirectory.EnumerateDirectories()).

The only question I'd have with leaving it as is, is whether there'd ever be a case where you wouldn't have the real IO.Path available.

@dzarda

This comment has been minimized.

Show comment
Hide comment
@dzarda

dzarda Oct 2, 2018

I feel having a safe default is desired. Why can't we use the "root" concept for this; "C:" and "/" respectively?

I think the current behavior is unwanted. Reason why this issue exists is because I spent an hour figuring out the unexpected behavior.

dzarda commented Oct 2, 2018

I feel having a safe default is desired. Why can't we use the "root" concept for this; "C:" and "/" respectively?

I think the current behavior is unwanted. Reason why this issue exists is because I spent an hour figuring out the unexpected behavior.

@ewlloyd

This comment has been minimized.

Show comment
Hide comment
@ewlloyd

ewlloyd Oct 3, 2018

Fair enough. What was expected behavior, in your case?

ewlloyd commented Oct 3, 2018

Fair enough. What was expected behavior, in your case?

@dzarda

This comment has been minimized.

Show comment
Hide comment
@dzarda

dzarda Oct 3, 2018

The struggle boiled down to something along the lines...

var fs = new MockFileSystem();
var dirInfo = fs.DirectoryInfo.FromDirectoryName("."); // System.IO.DirectoryNotFoundException : Could not find a part of the path 'C:\Users\....\AppData\Local\Temp

In such case I'd like fs to have an empty filesystem present, with only the root directory. Having CD pointing here as well makes perfect sense to me.

dzarda commented Oct 3, 2018

The struggle boiled down to something along the lines...

var fs = new MockFileSystem();
var dirInfo = fs.DirectoryInfo.FromDirectoryName("."); // System.IO.DirectoryNotFoundException : Could not find a part of the path 'C:\Users\....\AppData\Local\Temp

In such case I'd like fs to have an empty filesystem present, with only the root directory. Having CD pointing here as well makes perfect sense to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment