Skip to content

Conversation

peter-sereda
Copy link
Contributor

Attempt to fix the subject bug.

{
var fileSystem = new MockFileSystem();
var path = XFS.Path("C:\\test");
var path = XFS.Path("C:\\some_folder\\test");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this change necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of potential bug. Consider this code:

            var fileSystem = new MockFileSystem();
            var path = XFS.Path("C:\\test.txt");
            DirectoryInfoBase dir = fileSystem.Directory.GetParent(path);

It works ok on Windows but throws NullReferenceException on Linux. And after my changes GetParent() method is called inside File.Delete(), so this test started to fail. Adding a new intermediate folder "some_folder" is the fastest way to make the test work again.

I can create a new issue with this if you think it is a real defect.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah OK, thanks. Could you please create an issue for this problem and add a comment to the changed test case that refers to the issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.
New issue is #395

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link
Contributor

@fgreinacher fgreinacher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments inline, otherwise this looks good!

@fgreinacher
Copy link
Contributor

Thanks again!

@fgreinacher fgreinacher merged commit 127d89f into TestableIO:master Nov 5, 2018
@peter-sereda
Copy link
Contributor Author

Welcome!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants