From fb9d192e0165bea560f79acd58bea8e32e5a65c8 Mon Sep 17 00:00:00 2001 From: Alexander Pruss Date: Fri, 18 Nov 2016 13:54:00 +0100 Subject: [PATCH 1/7] Opening a MockFileStream to read from a nonexistent file will throw an exception. Implementation note - instead of using System.IO.FileAccess/FileMode, I ended up creating an internal StreamType enum instead to keep the interface simple. That's potentially something to change upon on review, but I think it's good the way it is. --- TestHelpers.Tests/MockFileStreamTests.cs | 16 +++++++++++++++- TestingHelpers/MockFile.cs | 2 +- TestingHelpers/MockFileInfo.cs | 7 ++++--- TestingHelpers/MockFileStream.cs | 15 +++++++++++++-- 4 files changed, 33 insertions(+), 7 deletions(-) diff --git a/TestHelpers.Tests/MockFileStreamTests.cs b/TestHelpers.Tests/MockFileStreamTests.cs index 66b6412b9..a07f7e743 100644 --- a/TestHelpers.Tests/MockFileStreamTests.cs +++ b/TestHelpers.Tests/MockFileStreamTests.cs @@ -15,7 +15,7 @@ public void MockFileStream_Flush_WritesByteToFile() // Arrange var filepath = XFS.Path(@"c:\something\foo.txt"); var filesystem = new MockFileSystem(new Dictionary()); - var cut = new MockFileStream(filesystem, filepath); + var cut = new MockFileStream(filesystem, filepath, MockFileStream.StreamType.WRITE); // Act cut.WriteByte(255); @@ -44,5 +44,19 @@ public void MockFileStream_Dispose_ShouldNotResurrectFile() Assert.AreEqual(0, fileCount2, "File should have been deleted"); Assert.AreEqual(0, fileCount3, "Disposing stream should not have resurrected the file"); } + + [Test] + [ExpectedException(typeof(FileNotFoundException))] + public void MockFileStream_Constructor_Reading_Nonexistent_File_Throws_Exception() + { + // Arrange + var nonexistentFilePath = XFS.Path(@"c:\something\foo.txt"); + var filesystem = new MockFileSystem(new Dictionary()); + + // Act + var illegalFileStream = new MockFileStream(filesystem, nonexistentFilePath, MockFileStream.StreamType.READ); + + // Assert - expect an exception + } } } diff --git a/TestingHelpers/MockFile.cs b/TestingHelpers/MockFile.cs index 0d2506032..affc56da4 100644 --- a/TestingHelpers/MockFile.cs +++ b/TestingHelpers/MockFile.cs @@ -399,7 +399,7 @@ public override Stream OpenWrite(string path) { mockFileDataAccessor.PathVerifier.IsLegalAbsoluteOrRelative(path, "path"); - return new MockFileStream(mockFileDataAccessor, path); + return new MockFileStream(mockFileDataAccessor, path, MockFileStream.StreamType.WRITE); } public override byte[] ReadAllBytes(string path) diff --git a/TestingHelpers/MockFileInfo.cs b/TestingHelpers/MockFileInfo.cs index d881af155..a4ffedf9f 100644 --- a/TestingHelpers/MockFileInfo.cs +++ b/TestingHelpers/MockFileInfo.cs @@ -155,7 +155,7 @@ public override string Name { public override StreamWriter AppendText() { if (MockFileData == null) throw new FileNotFoundException("File not found", path); - return new StreamWriter(new MockFileStream(mockFileSystem, FullName, true)); + return new StreamWriter(new MockFileStream(mockFileSystem, FullName, MockFileStream.StreamType.APPEND)); //return ((MockFileDataModifier) MockFileData).AppendText(); } @@ -231,7 +231,8 @@ public override Stream Open(FileMode mode, FileAccess access, FileShare share) public override Stream OpenRead() { - return new MockFileStream(mockFileSystem, path); + if (MockFileData == null) throw new FileNotFoundException("File not found", path); + return new MockFileStream(mockFileSystem, path, MockFileStream.StreamType.READ); } public override StreamReader OpenText() @@ -241,7 +242,7 @@ public override StreamReader OpenText() public override Stream OpenWrite() { - return new MockFileStream(mockFileSystem, path); + return new MockFileStream(mockFileSystem, path, MockFileStream.StreamType.WRITE); } public override FileInfoBase Replace(string destinationFileName, string destinationBackupFileName) diff --git a/TestingHelpers/MockFileStream.cs b/TestingHelpers/MockFileStream.cs index 0af1ac962..88312b3c3 100644 --- a/TestingHelpers/MockFileStream.cs +++ b/TestingHelpers/MockFileStream.cs @@ -6,7 +6,14 @@ public class MockFileStream : MemoryStream private readonly IMockFileDataAccessor mockFileDataAccessor; private readonly string path; - public MockFileStream(IMockFileDataAccessor mockFileDataAccessor, string path, bool forAppend = false) + public enum StreamType + { + READ, + WRITE, + APPEND + } + + public MockFileStream(IMockFileDataAccessor mockFileDataAccessor, string path, StreamType streamType) { if (mockFileDataAccessor == null) { @@ -23,13 +30,17 @@ public MockFileStream(IMockFileDataAccessor mockFileDataAccessor, string path, b if (data != null && data.Length > 0) { Write(data, 0, data.Length); - Seek(0, forAppend + Seek(0, StreamType.APPEND.Equals(streamType) ? SeekOrigin.End : SeekOrigin.Begin); } } else { + if (StreamType.READ.Equals(streamType)) + { + throw new FileNotFoundException("File not found.", path); + } mockFileDataAccessor.AddFile(path, new MockFileData(new byte[] { })); } } From 445b0c07352d9246420cd3f3c6904f61021b6060 Mon Sep 17 00:00:00 2001 From: Alexander Pruss Date: Mon, 21 Nov 2016 15:40:59 +0100 Subject: [PATCH 2/7] #181: MockFileInfo and MockFile no longer throw exceptions if moving a file to itself. See http://stackoverflow.com/questions/40722205/should-mscorlibs-fileinfo-moveto-throw-an-exception-if-trying-to-move-a-file --- TestHelpers.Tests/MockFileInfoTests.cs | 15 +++++++++++++++ TestHelpers.Tests/MockFileMoveTests.cs | 19 +++++++++++++++++++ TestingHelpers/MockFile.cs | 11 ++++++++++- TestingHelpers/MockFileInfo.cs | 2 +- 4 files changed, 45 insertions(+), 2 deletions(-) diff --git a/TestHelpers.Tests/MockFileInfoTests.cs b/TestHelpers.Tests/MockFileInfoTests.cs index 659820d56..e9752e3ab 100644 --- a/TestHelpers.Tests/MockFileInfoTests.cs +++ b/TestHelpers.Tests/MockFileInfoTests.cs @@ -476,5 +476,20 @@ public void MockFileInfo_MoveTo_ShouldUpdateFileInfoDirectoryAndFullName() Assert.AreEqual(fileInfo.DirectoryName, destinationFolder); Assert.AreEqual(fileInfo.FullName, destination); } + + [Test] + public void MockFileInfo_MoveTo_SameSourceAndTargetIsANoOp() + { + // Arrange + var fileSystem = new MockFileSystem(); + fileSystem.AddFile(XFS.Path(@"c:\temp\file.txt"), new MockFileData(@"line 1\r\nline 2")); + var fileInfo = fileSystem.FileInfo.FromFileName(XFS.Path(@"c:\temp\file.txt")); + + // Act + string destination = XFS.Path(XFS.Path(@"c:\temp\file.txt")); + fileInfo.MoveTo(destination); + + Assert.AreEqual(fileInfo.FullName, destination); + } } } \ No newline at end of file diff --git a/TestHelpers.Tests/MockFileMoveTests.cs b/TestHelpers.Tests/MockFileMoveTests.cs index 090a6b23f..c86283849 100644 --- a/TestHelpers.Tests/MockFileMoveTests.cs +++ b/TestHelpers.Tests/MockFileMoveTests.cs @@ -29,6 +29,25 @@ public void MockFile_Move_ShouldMoveFileWithinMemoryFileSystem() Assert.That(fileSystem.FileExists(sourceFilePath), Is.False); } + [Test] + public void MockFile_Move_SameSourceAndTargetIsANoOp() + { + string sourceFilePath = XFS.Path(@"c:\something\demo.txt"); + string sourceFileContent = "this is some content"; + var fileSystem = new MockFileSystem(new Dictionary + { + {sourceFilePath, new MockFileData(sourceFileContent)}, + {XFS.Path(@"c:\somethingelse\dummy.txt"), new MockFileData(new byte[] {0})} + }); + + string destFilePath = XFS.Path(@"c:\somethingelse\demo.txt"); + + fileSystem.File.Move(sourceFilePath, destFilePath); + + Assert.That(fileSystem.FileExists(destFilePath), Is.True); + Assert.That(fileSystem.GetFile(destFilePath).TextContents, Is.EqualTo(sourceFileContent)); + } + [Test] public void MockFile_Move_ShouldThrowIOExceptionWhenTargetAlreadyExists() { diff --git a/TestingHelpers/MockFile.cs b/TestingHelpers/MockFile.cs index affc56da4..b89ba4fee 100644 --- a/TestingHelpers/MockFile.cs +++ b/TestingHelpers/MockFile.cs @@ -323,7 +323,16 @@ public override void Move(string sourceFileName, string destFileName) mockFileDataAccessor.PathVerifier.IsLegalAbsoluteOrRelative(destFileName, "destFileName"); if (mockFileDataAccessor.GetFile(destFileName) != null) - throw new IOException("A file can not be created if it already exists."); + { + if (destFileName.Equals(sourceFileName)) + { + return; + } else + { + throw new IOException("A file can not be created if it already exists."); + } + } + var sourceFile = mockFileDataAccessor.GetFile(sourceFileName); diff --git a/TestingHelpers/MockFileInfo.cs b/TestingHelpers/MockFileInfo.cs index a4ffedf9f..8a21a8d6d 100644 --- a/TestingHelpers/MockFileInfo.cs +++ b/TestingHelpers/MockFileInfo.cs @@ -209,7 +209,7 @@ public override FileSecurity GetAccessControl(AccessControlSections includeSecti public override void MoveTo(string destFileName) { - var movedFileInfo = CopyTo(destFileName); + var movedFileInfo = CopyTo(destFileName, true); Delete(); path = movedFileInfo.FullName; } From 7b075cfa3663502737499ac75d901104e6327cb8 Mon Sep 17 00:00:00 2001 From: Alexander Pruss Date: Mon, 21 Nov 2016 16:03:10 +0100 Subject: [PATCH 3/7] Returning early if destfileName == FullName to prevent side effects. --- TestingHelpers/MockFileInfo.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/TestingHelpers/MockFileInfo.cs b/TestingHelpers/MockFileInfo.cs index 8a21a8d6d..16cdf69c6 100644 --- a/TestingHelpers/MockFileInfo.cs +++ b/TestingHelpers/MockFileInfo.cs @@ -209,7 +209,11 @@ public override FileSecurity GetAccessControl(AccessControlSections includeSecti public override void MoveTo(string destFileName) { - var movedFileInfo = CopyTo(destFileName, true); + if (destFileName == FullName) + { + return; + } + var movedFileInfo = CopyTo(destFileName); Delete(); path = movedFileInfo.FullName; } From 6ddc87dbc4f9b5c595009e64557e3b86065d0922 Mon Sep 17 00:00:00 2001 From: Alexander Pruss Date: Mon, 21 Nov 2016 16:46:24 +0100 Subject: [PATCH 4/7] #181: Moved existence and same target-source check further down to make sure all related calls to Move and Copy made the same checks. Copying and moving nonexistent sources now correctly throws an exception. --- TestHelpers.Tests/MockFileCopyTests.cs | 20 ++++++++++++ TestHelpers.Tests/MockFileInfoTests.cs | 45 ++++++++++++++++++++++++++ TestHelpers.Tests/MockFileMoveTests.cs | 20 ++++++++++++ TestingHelpers/MockFile.cs | 3 ++ TestingHelpers/MockFileInfo.cs | 15 +++++---- 5 files changed, 97 insertions(+), 6 deletions(-) diff --git a/TestHelpers.Tests/MockFileCopyTests.cs b/TestHelpers.Tests/MockFileCopyTests.cs index 7dffee39f..af446a884 100644 --- a/TestHelpers.Tests/MockFileCopyTests.cs +++ b/TestHelpers.Tests/MockFileCopyTests.cs @@ -271,5 +271,25 @@ public void MockFile_Copy_ShouldThrowArgumentExceptionWhenTargetIsEmpty_Message( Assert.That(exception.Message, Is.StringStarting("Empty file name is not legal.")); } + + [Test] + [ExpectedException(typeof(FileNotFoundException))] + public void MockFile_Copy_ShouldThrowExceptionWhenSourceDoesNotExist() + { + string sourceFilePath = XFS.Path(@"c:\something\demo.txt"); + var fileSystem = new MockFileSystem(); + + fileSystem.File.Copy(sourceFilePath, XFS.Path(@"c:\something\demo2.txt")); + } + + [Test] + [ExpectedException(typeof(FileNotFoundException))] + public void MockFile_Copy_ShouldThrowExceptionWhenSourceDoesNotExist_EvenWhenCopyingToItself() + { + string sourceFilePath = XFS.Path(@"c:\something\demo.txt"); + var fileSystem = new MockFileSystem(); + + fileSystem.File.Copy(sourceFilePath, XFS.Path(@"c:\something\demo.txt")); + } } } \ No newline at end of file diff --git a/TestHelpers.Tests/MockFileInfoTests.cs b/TestHelpers.Tests/MockFileInfoTests.cs index e9752e3ab..d7c8a1dcf 100644 --- a/TestHelpers.Tests/MockFileInfoTests.cs +++ b/TestHelpers.Tests/MockFileInfoTests.cs @@ -491,5 +491,50 @@ public void MockFileInfo_MoveTo_SameSourceAndTargetIsANoOp() Assert.AreEqual(fileInfo.FullName, destination); } + + [Test] + [ExpectedException(typeof(FileNotFoundException))] + public void MockFileInfo_MoveTo_SameSourceAndTargetThrowsExceptionIfSourceDoesntExist() + { + // Arrange + var fileSystem = new MockFileSystem(); + var fileInfo = fileSystem.FileInfo.FromFileName(XFS.Path(@"c:\temp\file.txt")); + + // Act + string destination = XFS.Path(XFS.Path(@"c:\temp\file.txt")); + fileInfo.MoveTo(destination); + + Assert.AreEqual(fileInfo.FullName, destination); + } + + [Test] + [ExpectedException(typeof(FileNotFoundException))] + public void MockFileInfo_MoveTo_ThrowsExceptionIfSourceDoesntExist() + { + // Arrange + var fileSystem = new MockFileSystem(); + var fileInfo = fileSystem.FileInfo.FromFileName(XFS.Path(@"c:\temp\file.txt")); + + // Act + string destination = XFS.Path(XFS.Path(@"c:\temp\file2.txt")); + fileInfo.MoveTo(destination); + + Assert.AreEqual(fileInfo.FullName, destination); + } + + [Test] + [ExpectedException(typeof(FileNotFoundException))] + public void MockFileInfo_CopyTo_ThrowsExceptionIfSourceDoesntExist() + { + // Arrange + var fileSystem = new MockFileSystem(); + var fileInfo = fileSystem.FileInfo.FromFileName(XFS.Path(@"c:\temp\file.txt")); + + // Act + string destination = XFS.Path(XFS.Path(@"c:\temp\file2.txt")); + fileInfo.CopyTo(destination); + + Assert.AreEqual(fileInfo.FullName, destination); + } } } \ No newline at end of file diff --git a/TestHelpers.Tests/MockFileMoveTests.cs b/TestHelpers.Tests/MockFileMoveTests.cs index c86283849..71da040d5 100644 --- a/TestHelpers.Tests/MockFileMoveTests.cs +++ b/TestHelpers.Tests/MockFileMoveTests.cs @@ -305,5 +305,25 @@ public void MockFile_Move_ShouldThrowDirectoryNotFoundExceptionWhenSourcePathDoe Assert.That(() => fileSystem.File.Move(sourceFilePath, destFilePath), Throws.InstanceOf().With.Message.StartsWith(@"Could not find a part of the path")); } + + [Test] + [ExpectedException(typeof(FileNotFoundException))] + public void MockFile_Move_ShouldThrowExceptionWhenSourceDoesNotExist() + { + string sourceFilePath = XFS.Path(@"c:\something\demo.txt"); + var fileSystem = new MockFileSystem(); + + fileSystem.File.Move(sourceFilePath, XFS.Path(@"c:\something\demo2.txt")); + } + + [Test] + [ExpectedException(typeof(FileNotFoundException))] + public void MockFile_Move_ShouldThrowExceptionWhenSourceDoesNotExist_EvenWhenCopyingToItself() + { + string sourceFilePath = XFS.Path(@"c:\something\demo.txt"); + var fileSystem = new MockFileSystem(); + + fileSystem.File.Move(sourceFilePath, XFS.Path(@"c:\something\demo.txt")); + } } } \ No newline at end of file diff --git a/TestingHelpers/MockFile.cs b/TestingHelpers/MockFile.cs index b89ba4fee..3b9c49cbc 100644 --- a/TestingHelpers/MockFile.cs +++ b/TestingHelpers/MockFile.cs @@ -99,6 +99,9 @@ public override void Copy(string sourceFileName, string destFileName) public override void Copy(string sourceFileName, string destFileName, bool overwrite) { + if (!Exists(sourceFileName)) + { + } if (sourceFileName == null) { throw new ArgumentNullException("sourceFileName", Properties.Resources.FILENAME_CANNOT_BE_NULL); diff --git a/TestingHelpers/MockFileInfo.cs b/TestingHelpers/MockFileInfo.cs index 16cdf69c6..e17da6416 100644 --- a/TestingHelpers/MockFileInfo.cs +++ b/TestingHelpers/MockFileInfo.cs @@ -161,12 +161,19 @@ public override StreamWriter AppendText() public override FileInfoBase CopyTo(string destFileName) { - new MockFile(mockFileSystem).Copy(FullName, destFileName); - return mockFileSystem.FileInfo.FromFileName(destFileName); + return CopyTo(destFileName, false); } public override FileInfoBase CopyTo(string destFileName, bool overwrite) { + if (!Exists) + { + throw new FileNotFoundException("The file does not exist and can't be moved or copied.", FullName); + } + if (destFileName == FullName) + { + return this; + } new MockFile(mockFileSystem).Copy(FullName, destFileName, overwrite); return mockFileSystem.FileInfo.FromFileName(destFileName); } @@ -209,10 +216,6 @@ public override FileSecurity GetAccessControl(AccessControlSections includeSecti public override void MoveTo(string destFileName) { - if (destFileName == FullName) - { - return; - } var movedFileInfo = CopyTo(destFileName); Delete(); path = movedFileInfo.FullName; From 3c8da79b7f478dc71b45cb6c0fe2c4ccdb5ced73 Mon Sep 17 00:00:00 2001 From: Alexander Pruss Date: Mon, 21 Nov 2016 16:48:37 +0100 Subject: [PATCH 5/7] Added missing change. --- TestingHelpers/MockFile.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/TestingHelpers/MockFile.cs b/TestingHelpers/MockFile.cs index 3b9c49cbc..1de801363 100644 --- a/TestingHelpers/MockFile.cs +++ b/TestingHelpers/MockFile.cs @@ -101,6 +101,7 @@ public override void Copy(string sourceFileName, string destFileName, bool overw { if (!Exists(sourceFileName)) { + throw new FileNotFoundException(); } if (sourceFileName == null) { From eca251965c6d0aea6b2f9416fbf99cbc35f0478f Mon Sep 17 00:00:00 2001 From: Alexander Pruss Date: Mon, 21 Nov 2016 17:12:47 +0100 Subject: [PATCH 6/7] Rearranged order of checks. --- TestingHelpers/MockFile.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/TestingHelpers/MockFile.cs b/TestingHelpers/MockFile.cs index 1de801363..75dd86935 100644 --- a/TestingHelpers/MockFile.cs +++ b/TestingHelpers/MockFile.cs @@ -99,10 +99,7 @@ public override void Copy(string sourceFileName, string destFileName) public override void Copy(string sourceFileName, string destFileName, bool overwrite) { - if (!Exists(sourceFileName)) - { - throw new FileNotFoundException(); - } + //This is the order that checks are executed in MSCORLIB. if (sourceFileName == null) { throw new ArgumentNullException("sourceFileName", Properties.Resources.FILENAME_CANNOT_BE_NULL); @@ -112,9 +109,12 @@ public override void Copy(string sourceFileName, string destFileName, bool overw { throw new ArgumentNullException("destFileName", Properties.Resources.FILENAME_CANNOT_BE_NULL); } - mockFileDataAccessor.PathVerifier.IsLegalAbsoluteOrRelative(sourceFileName, "sourceFileName"); mockFileDataAccessor.PathVerifier.IsLegalAbsoluteOrRelative(destFileName, "destFileName"); + if (!Exists(sourceFileName)) + { + throw new FileNotFoundException(); + } var directoryNameOfDestination = mockPath.GetDirectoryName(destFileName); if (!mockFileDataAccessor.Directory.Exists(directoryNameOfDestination)) From de8908731221e1dfe849c6dae6fbfd78f7812cd9 Mon Sep 17 00:00:00 2001 From: Alexander Pruss Date: Tue, 22 Nov 2016 10:24:11 +0100 Subject: [PATCH 7/7] Caught another edge case. --- TestHelpers.Tests/MockFileInfoTests.cs | 1 + TestingHelpers/MockFileInfo.cs | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/TestHelpers.Tests/MockFileInfoTests.cs b/TestHelpers.Tests/MockFileInfoTests.cs index d7c8a1dcf..6c57e73b7 100644 --- a/TestHelpers.Tests/MockFileInfoTests.cs +++ b/TestHelpers.Tests/MockFileInfoTests.cs @@ -490,6 +490,7 @@ public void MockFileInfo_MoveTo_SameSourceAndTargetIsANoOp() fileInfo.MoveTo(destination); Assert.AreEqual(fileInfo.FullName, destination); + Assert.True(fileInfo.Exists); } [Test] diff --git a/TestingHelpers/MockFileInfo.cs b/TestingHelpers/MockFileInfo.cs index e17da6416..b4144f7e3 100644 --- a/TestingHelpers/MockFileInfo.cs +++ b/TestingHelpers/MockFileInfo.cs @@ -217,6 +217,10 @@ public override FileSecurity GetAccessControl(AccessControlSections includeSecti public override void MoveTo(string destFileName) { var movedFileInfo = CopyTo(destFileName); + if (destFileName == FullName) + { + return; + } Delete(); path = movedFileInfo.FullName; }