From 47183e9e6698ee107a76f6930e6bbc9ded0a3ea2 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Sat, 9 Oct 2021 06:12:59 +0200 Subject: [PATCH 01/12] FileSystemEntry.Unix: ensure attributes are available when file is deleted. When the file no longer exists, we create attributes based on what we know. The test for this was passing because it cached the attributes before the item was deleted due to enumerating with skipping FileAttributes.Hidden. --- .../tests/Enumeration/AttributeTests.cs | 4 +- .../IO/Enumeration/FileSystemEntry.Unix.cs | 68 ++++++++++++------- .../src/System/IO/FileStatus.Unix.cs | 6 +- 3 files changed, 50 insertions(+), 28 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs index 92f9a829d76d25..fc32449a502fe8 100644 --- a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs +++ b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs @@ -55,7 +55,7 @@ public void FileAttributesAreExpected() fileOne.Attributes &= ~(FileAttributes.Archive | FileAttributes.NotContentIndexed); } - using (var enumerator = new DefaultFileAttributes(testDirectory.FullName, new EnumerationOptions())) + using (var enumerator = new DefaultFileAttributes(testDirectory.FullName, new EnumerationOptions() { AttributesToSkip = 0 })) { Assert.True(enumerator.MoveNext()); Assert.Equal(fileOne.Name, enumerator.Current); @@ -104,7 +104,7 @@ public void DirectoryAttributesAreExpected() subDirectory.Attributes &= ~FileAttributes.NotContentIndexed; } - using (var enumerator = new DefaultDirectoryAttributes(testDirectory.FullName, new EnumerationOptions())) + using (var enumerator = new DefaultDirectoryAttributes(testDirectory.FullName, new EnumerationOptions() { AttributesToSkip = 0 })) { Assert.True(enumerator.MoveNext()); Assert.Equal(subDirectory.Name, enumerator.Current); diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEntry.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEntry.Unix.cs index 61ee454c6b3d03..1be3942011b2fd 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEntry.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEntry.Unix.cs @@ -10,7 +10,7 @@ namespace System.IO.Enumeration /// public unsafe ref partial struct FileSystemEntry { - internal Interop.Sys.DirectoryEntry _directoryEntry; + private Interop.Sys.DirectoryEntry _directoryEntry; private FileStatus _status; private Span _pathBuffer; private ReadOnlySpan _fullPath; @@ -32,38 +32,34 @@ internal static FileAttributes Initialize( entry._pathBuffer = pathBuffer; entry._fullPath = ReadOnlySpan.Empty; entry._fileName = ReadOnlySpan.Empty; - entry._status.InvalidateCaches(); + entry._status.InitiallyDirectory = false; bool isDirectory = directoryEntry.InodeType == Interop.Sys.NodeType.DT_DIR; bool isSymlink = directoryEntry.InodeType == Interop.Sys.NodeType.DT_LNK; bool isUnknown = directoryEntry.InodeType == Interop.Sys.NodeType.DT_UNKNOWN; - // Some operating systems don't have the inode type in the dirent structure, - // so we use DT_UNKNOWN as a sentinel value. As such, check if the dirent is a - // symlink or a directory. - if (isUnknown) + if (isDirectory) { - isSymlink = entry.IsSymbolicLink; - // Need to fail silently in case we are enumerating - isDirectory = entry._status.IsDirectory(entry.FullPath, continueOnError: true); + entry._status.InitiallyDirectory = true; } - // Same idea as the directory check, just repeated for (and tweaked due to the - // nature of) symlinks. - // Whether we had the dirent structure or not, we treat a symlink to a directory as a directory, - // so we need to reflect that in our isDirectory variable. else if (isSymlink) { - // Need to fail silently in case we are enumerating - isDirectory = entry._status.IsDirectory(entry.FullPath, continueOnError: true); + entry._status.InitiallyDirectory = entry._status.IsDirectory(entry.FullPath, continueOnError: true); + } + else if (isUnknown) + { + entry._status.InitiallyDirectory = entry._status.IsDirectory(entry.FullPath, continueOnError: true); + if (entry._status.IsSymbolicLink(entry.FullPath, continueOnError: true)) + { + entry._directoryEntry.InodeType = Interop.Sys.NodeType.DT_LNK; + } } - - entry._status.InitiallyDirectory = isDirectory; FileAttributes attributes = default; - if (isSymlink) + if (entry.IsSymbolicLink) attributes |= FileAttributes.ReparsePoint; - if (isDirectory) + if (entry.IsDirectory) attributes |= FileAttributes.Directory; return attributes; @@ -119,15 +115,41 @@ public ReadOnlySpan FileName // Windows never fails getting attributes, length, or time as that information comes back // with the native enumeration struct. As such we must not throw here. - public FileAttributes Attributes => _status.GetAttributes(FullPath, FileName); + public FileAttributes Attributes + { + get + { + FileAttributes attributes = _status.GetAttributes(FullPath, FileName, continueOnError: true); + if (attributes != (FileAttributes)(-1)) + { + return attributes; + } + + // File was removed before we retrieved attributes. + // Return what we know. + attributes = default; + + if (IsSymbolicLink) + attributes |= FileAttributes.ReparsePoint; + + if (IsDirectory) + attributes |= FileAttributes.Directory; + + if (FileStatus.IsNameHidden(FileName)) + attributes |= FileAttributes.Hidden; + + return attributes != default ? attributes : FileAttributes.Normal; + } + } public long Length => _status.GetLength(FullPath, continueOnError: true); public DateTimeOffset CreationTimeUtc => _status.GetCreationTime(FullPath, continueOnError: true); public DateTimeOffset LastAccessTimeUtc => _status.GetLastAccessTime(FullPath, continueOnError: true); public DateTimeOffset LastWriteTimeUtc => _status.GetLastWriteTime(FullPath, continueOnError: true); + public bool IsHidden => _status.IsHidden(FullPath, FileName, continueOnError: true); + internal bool IsReadOnly => _status.IsReadOnly(FullPath, continueOnError: true); + public bool IsDirectory => _status.InitiallyDirectory; - public bool IsHidden => _status.IsHidden(FullPath, FileName); - internal bool IsReadOnly => _status.IsReadOnly(FullPath); - internal bool IsSymbolicLink => _status.IsSymbolicLink(FullPath); + internal bool IsSymbolicLink => _directoryEntry.InodeType == Interop.Sys.NodeType.DT_LNK; public FileSystemInfo ToFileSystemInfo() { diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs index ff4d8a86cb18c0..dccb25d614ed44 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs @@ -123,7 +123,7 @@ internal bool IsHidden(ReadOnlySpan path, ReadOnlySpan fileName, boo return HasHiddenFlag; } - internal bool IsNameHidden(ReadOnlySpan fileName) => fileName.Length > 0 && fileName[0] == '.'; + internal static bool IsNameHidden(ReadOnlySpan fileName) => fileName.Length > 0 && fileName[0] == '.'; // Returns true if the path points to a directory, or if the path is a symbolic link // that points to a directory @@ -139,9 +139,9 @@ internal bool IsSymbolicLink(ReadOnlySpan path, bool continueOnError = fal return HasSymbolicLinkFlag; } - internal FileAttributes GetAttributes(ReadOnlySpan path, ReadOnlySpan fileName) + internal FileAttributes GetAttributes(ReadOnlySpan path, ReadOnlySpan fileName, bool continueOnError = false) { - EnsureCachesInitialized(path); + EnsureCachesInitialized(path, continueOnError); if (!_exists) return (FileAttributes)(-1); From 8a0edcd2bf1e2fd8b872a2656f4da4e3299a6abf Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Sat, 9 Oct 2021 06:48:46 +0200 Subject: [PATCH 02/12] GetLength: fix reading from uninitialized cache. --- .../System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs index dccb25d614ed44..4fe9794e20575f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs @@ -327,7 +327,7 @@ private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool internal long GetLength(ReadOnlySpan path, bool continueOnError = false) { EnsureCachesInitialized(path, continueOnError); - return _fileCache.Size; + return IsFileCacheInitialized ? _fileCache.Size : 0; } // Tries to refresh the lstat cache (_fileCache). From ed16197f05cbf9391aafc3dac075ff36c59c5831 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 11 Oct 2021 13:00:07 +0200 Subject: [PATCH 03/12] Update test --- .../tests/Enumeration/AttributeTests.cs | 188 +++++++++++------- 1 file changed, 117 insertions(+), 71 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs index fc32449a502fe8..7c279ec3cc3bf0 100644 --- a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs +++ b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs @@ -9,12 +9,26 @@ namespace System.IO.Tests.Enumeration { public class AttributeTests : FileSystemTest { - private class DefaultFileAttributes : FileSystemEnumerator + private class FileSystemEntryProperties { - public DefaultFileAttributes(string directory, EnumerationOptions options) + public string FileName { get; init; } + public FileAttributes Attributes { get; init; } + public DateTimeOffset CreationTimeUtc { get; init; } + public bool IsDirectory { get; init; } + public bool IsHidden { get; init; } + public DateTimeOffset LastAccessTimeUtc { get; init; } + public DateTimeOffset LastWriteTimeUtc { get; init; } + public long Length { get; init; } + public string Directory { get; init; } + public string FullPath { get; init; } + public string SpecifiedFullPath { get; init; } + } + + private class GetPropertiesEnumerator : FileSystemEnumerator + { + public GetPropertiesEnumerator(string directory, EnumerationOptions options) : base(directory, options) - { - } + { } protected override bool ContinueOnError(int error) { @@ -22,93 +36,125 @@ protected override bool ContinueOnError(int error) return false; } - protected override bool ShouldIncludeEntry(ref FileSystemEntry entry) - => !entry.IsDirectory; - - protected override string TransformEntry(ref FileSystemEntry entry) + protected override FileSystemEntryProperties TransformEntry(ref FileSystemEntry entry) { - string path = entry.ToFullPath(); - File.Delete(path); - - // Attributes require a stat call on Unix- ensure that we have the right attributes - // even if the returned file is deleted. - Assert.Equal(FileAttributes.Normal, entry.Attributes); - Assert.Equal(path, entry.ToFullPath()); - return new string(entry.FileName); + return new FileSystemEntryProperties + { + FileName = new string(entry.FileName), + Attributes = entry.Attributes, + CreationTimeUtc = entry.CreationTimeUtc, + IsDirectory = entry.IsDirectory, + IsHidden = entry.IsHidden, + LastAccessTimeUtc = entry.LastAccessTimeUtc, + LastWriteTimeUtc = entry.LastWriteTimeUtc, + Length = entry.Length, + Directory = new string(entry.Directory), + FullPath = entry.ToFullPath(), + SpecifiedFullPath = entry.ToSpecifiedFullPath() + }; } } - [Fact] - public void FileAttributesAreExpected() + // The test is performed using two items with different properties (file/dir, file length) + // to check cached values from the previous entry don't leak into the non-existing entry. + [InlineData("dir1", "dir2")] + [InlineData("file1", "file2")] + [InlineData("dir1", "file1")] + [InlineData("file1", "dir1")] + [Theory] + public void PropertiesWhenItemNoLongerExists(string item1, string item2) { DirectoryInfo testDirectory = Directory.CreateDirectory(GetTestFilePath()); - FileInfo fileOne = new FileInfo(Path.Combine(testDirectory.FullName, GetTestFileName())); - - fileOne.Create().Dispose(); - if (PlatformDetection.IsWindows) - { - // Archive should always be set on a new file. Clear it and other expected flags to - // see that we get "Normal" as the default when enumerating. + FileSystemInfo item1Info = CreateItem(testDirectory, item1); + FileSystemInfo item2Info = CreateItem(testDirectory, item2); - Assert.True((fileOne.Attributes & FileAttributes.Archive) != 0); - fileOne.Attributes &= ~(FileAttributes.Archive | FileAttributes.NotContentIndexed); - } - - using (var enumerator = new DefaultFileAttributes(testDirectory.FullName, new EnumerationOptions() { AttributesToSkip = 0 })) + using (var enumerator = new GetPropertiesEnumerator(testDirectory.FullName, new EnumerationOptions() { AttributesToSkip = 0 })) { + // Move to the first item. Assert.True(enumerator.MoveNext()); - Assert.Equal(fileOne.Name, enumerator.Current); - Assert.False(enumerator.MoveNext()); - } - } + FileSystemEntryProperties entry = enumerator.Current; - private class DefaultDirectoryAttributes : FileSystemEnumerator - { - public DefaultDirectoryAttributes(string directory, EnumerationOptions options) - : base(directory, options) - { - } + Assert.True(entry.FileName == item1 || entry.FileName == item2, "Unexpected item"); - protected override bool ShouldIncludeEntry(ref FileSystemEntry entry) - => entry.IsDirectory; + // Delete both items. + DeleteItem(testDirectory, item1); + DeleteItem(testDirectory, item2); - protected override bool ContinueOnError(int error) - { - Assert.False(true, $"Should not have errored {error}"); - return false; - } + // Move to the second item. + FileSystemInfo expected = entry.FileName == item1 ? item2Info : item1Info; + Assert.True(enumerator.MoveNext()); + entry = enumerator.Current; + Assert.Equal(expected.Name, entry.FileName); + + // Values determined during enumeration. + Assert.Equal(expected is DirectoryInfo, entry.IsDirectory); + Assert.Equal(expected.Attributes, entry.Attributes); + Assert.Equal(testDirectory.FullName, entry.Directory); + Assert.Equal(expected.FullName, entry.FullPath); + Assert.Equal(expected.FullName, entry.SpecifiedFullPath); + + if (PlatformDetection.IsWindows) + { + Assert.Equal((expected.Attributes & FileAttributes.Hidden) != 0, entry.IsHidden); + Assert.Equal(expected.CreationTimeUtc, entry.CreationTimeUtc); + Assert.Equal(expected.LastAccessTimeUtc, entry.LastAccessTimeUtc); + Assert.Equal(expected.LastWriteTimeUtc, entry.LastWriteTimeUtc); + Assert.Equal((expected.Attributes & FileAttributes.Hidden) != 0, entry.IsHidden); + if (expected is FileInfo fileInfo) + { + Assert.Equal(fileInfo.Length, entry.Length); + } + } + else + { + // Because the item is deleted, we can no longer retrieve these values and defaults are returned instead. + Assert.Equal(entry.FileName.StartsWith('.'), entry.IsHidden); + Assert.Equal(new DateTimeOffset(DateTime.FromFileTimeUtc(0)), entry.CreationTimeUtc); + Assert.Equal(new DateTimeOffset(DateTime.FromFileTimeUtc(0)), entry.LastAccessTimeUtc); + Assert.Equal(new DateTimeOffset(DateTime.FromFileTimeUtc(0)), entry.LastWriteTimeUtc); + if (expected is FileInfo fileInfo) + { + Assert.Equal(0, entry.Length); + } + } - protected override string TransformEntry(ref FileSystemEntry entry) - { - string path = entry.ToFullPath(); - Directory.Delete(path); - - // Attributes require a stat call on Unix- ensure that we have the right attributes - // even if the returned directory is deleted. - Assert.Equal(FileAttributes.Directory, entry.Attributes); - Assert.Equal(path, entry.ToFullPath()); - return new string(entry.FileName); + Assert.False(enumerator.MoveNext()); } - } - - [Fact] - public void DirectoryAttributesAreExpected() - { - DirectoryInfo testDirectory = Directory.CreateDirectory(GetTestFilePath()); - DirectoryInfo subDirectory = Directory.CreateDirectory(Path.Combine(testDirectory.FullName, GetTestFileName())); - if (PlatformDetection.IsWindows) + static FileSystemInfo CreateItem(DirectoryInfo testDirectory, string item) { - // Clear possible extra flags to see that we get Directory - subDirectory.Attributes &= ~FileAttributes.NotContentIndexed; + string fullPath = Path.Combine(testDirectory.FullName, item); + if (item.StartsWith("dir")) + { + Directory.CreateDirectory(fullPath); + var info = new DirectoryInfo(fullPath); + info.Refresh(); + return info; + } + else + { + // use the last char to have different lengths for different files. + Assert.True(item == "file1" || item == "file2"); + int length = (int)item[item.Length - 1]; + File.WriteAllBytes(fullPath, new byte[length]); + var info = new FileInfo(fullPath); + info.Refresh(); + return info; + } } - using (var enumerator = new DefaultDirectoryAttributes(testDirectory.FullName, new EnumerationOptions() { AttributesToSkip = 0 })) + static void DeleteItem(DirectoryInfo testDirectory, string item) { - Assert.True(enumerator.MoveNext()); - Assert.Equal(subDirectory.Name, enumerator.Current); - Assert.False(enumerator.MoveNext()); + string fullPath = Path.Combine(testDirectory.FullName, item); + if (item.StartsWith("dir")) + { + Directory.Delete(fullPath); + } + else + { + File.Delete(fullPath); + } } } From 794f2b2822f34a2970676f8b7ec117160a68a20e Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 11 Oct 2021 13:05:54 +0200 Subject: [PATCH 04/12] Add a comment to GetLength about the value it returns for symlinks. --- .../System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs index 4fe9794e20575f..df2175b38ccee4 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStatus.Unix.cs @@ -326,6 +326,9 @@ private unsafe void SetAccessOrWriteTime(string path, DateTimeOffset time, bool internal long GetLength(ReadOnlySpan path, bool continueOnError = false) { + // For symbolic links, on Windows, Length returns zero and not the target file size. + // On Unix, it returns the length of the path stored in the link. + EnsureCachesInitialized(path, continueOnError); return IsFileCacheInitialized ? _fileCache.Size : 0; } From 5322a6b13834306cd97712f99d43f64f0d9815d6 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 11 Oct 2021 13:11:55 +0200 Subject: [PATCH 05/12] Always assert entry.Length on Unix --- .../System.IO.FileSystem/tests/Enumeration/AttributeTests.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs index 7c279ec3cc3bf0..292d1897c2f2db 100644 --- a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs +++ b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs @@ -113,10 +113,7 @@ public void PropertiesWhenItemNoLongerExists(string item1, string item2) Assert.Equal(new DateTimeOffset(DateTime.FromFileTimeUtc(0)), entry.CreationTimeUtc); Assert.Equal(new DateTimeOffset(DateTime.FromFileTimeUtc(0)), entry.LastAccessTimeUtc); Assert.Equal(new DateTimeOffset(DateTime.FromFileTimeUtc(0)), entry.LastWriteTimeUtc); - if (expected is FileInfo fileInfo) - { - Assert.Equal(0, entry.Length); - } + Assert.Equal(0, entry.Length); } Assert.False(enumerator.MoveNext()); From 2031b6454474e3ad6042e38f1dcdf175139dd8dc Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 11 Oct 2021 13:15:48 +0200 Subject: [PATCH 06/12] Minor refactoringcd --- .../tests/Enumeration/AttributeTests.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs index 292d1897c2f2db..1af0087b325903 100644 --- a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs +++ b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs @@ -110,9 +110,10 @@ public void PropertiesWhenItemNoLongerExists(string item1, string item2) { // Because the item is deleted, we can no longer retrieve these values and defaults are returned instead. Assert.Equal(entry.FileName.StartsWith('.'), entry.IsHidden); - Assert.Equal(new DateTimeOffset(DateTime.FromFileTimeUtc(0)), entry.CreationTimeUtc); - Assert.Equal(new DateTimeOffset(DateTime.FromFileTimeUtc(0)), entry.LastAccessTimeUtc); - Assert.Equal(new DateTimeOffset(DateTime.FromFileTimeUtc(0)), entry.LastWriteTimeUtc); + DateTimeOffset defaultOffset = new DateTimeOffset(DateTime.FromFileTimeUtc(0)); + Assert.Equal(defaultOffset, entry.CreationTimeUtc); + Assert.Equal(defaultOffset, entry.LastAccessTimeUtc); + Assert.Equal(defaultOffset, entry.LastWriteTimeUtc); Assert.Equal(0, entry.Length); } From 4e6f8fcd15c365c4d633a6e916892b032e1d118c Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 11 Oct 2021 15:22:29 +0200 Subject: [PATCH 07/12] Add message to each assert. --- .../TestUtilities/System/AssertExtensions.cs | 22 ++++++++++ .../tests/Enumeration/AttributeTests.cs | 43 +++++++++---------- 2 files changed, 43 insertions(+), 22 deletions(-) diff --git a/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs b/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs index 8f97aa5ea31ced..e0fed5dfb97d95 100644 --- a/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs +++ b/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs @@ -269,6 +269,28 @@ public static void Contains(string value, string substring) Assert.Contains(substring, value, StringComparison.Ordinal); } + /// + /// Validate that a given value is equal to another value. + /// + /// The expected value. + /// The value to be compared against. + public static void EqualTo(T expected, T actual, string userMessage = null) + { + if (expected == null && actual == null) + return; + + bool equal = expected != null && expected switch + { + IEquatable equatable => equatable.Equals(actual), + IComparable comparableT => comparableT.CompareTo(actual) == 0, + IComparable comparable => comparable.CompareTo(actual) == 0, + _ => throw new NotSupportedException($"{nameof(EqualTo)} is not supported for {typeof(T)}.") + }; + + if (!equal) + throw new XunitException(AddOptionalUserMessage($"Expected: {expected} is not equal to {actual}", userMessage)); + } + /// /// Validate that a given value is greater than another value. /// diff --git a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs index 1af0087b325903..14dd3cae58cd16 100644 --- a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs +++ b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs @@ -72,7 +72,7 @@ public void PropertiesWhenItemNoLongerExists(string item1, string item2) using (var enumerator = new GetPropertiesEnumerator(testDirectory.FullName, new EnumerationOptions() { AttributesToSkip = 0 })) { // Move to the first item. - Assert.True(enumerator.MoveNext()); + Assert.True(enumerator.MoveNext(), "Move first"); FileSystemEntryProperties entry = enumerator.Current; Assert.True(entry.FileName == item1 || entry.FileName == item2, "Unexpected item"); @@ -83,41 +83,40 @@ public void PropertiesWhenItemNoLongerExists(string item1, string item2) // Move to the second item. FileSystemInfo expected = entry.FileName == item1 ? item2Info : item1Info; - Assert.True(enumerator.MoveNext()); + Assert.True(enumerator.MoveNext(), "Move second"); entry = enumerator.Current; - Assert.Equal(expected.Name, entry.FileName); + AssertExtensions.EqualTo(expected.Name, entry.FileName, "Name"); // Values determined during enumeration. - Assert.Equal(expected is DirectoryInfo, entry.IsDirectory); - Assert.Equal(expected.Attributes, entry.Attributes); - Assert.Equal(testDirectory.FullName, entry.Directory); - Assert.Equal(expected.FullName, entry.FullPath); - Assert.Equal(expected.FullName, entry.SpecifiedFullPath); + AssertExtensions.EqualTo(expected is DirectoryInfo, entry.IsDirectory, "IsDirectory"); + AssertExtensions.EqualTo(expected.Attributes, entry.Attributes, "Attributes"); + AssertExtensions.EqualTo(testDirectory.FullName, entry.Directory, "Directory"); + AssertExtensions.EqualTo(expected.FullName, entry.FullPath, "FullPath"); + AssertExtensions.EqualTo(expected.FullName, entry.SpecifiedFullPath, "SpecifiedFullPath"); if (PlatformDetection.IsWindows) { - Assert.Equal((expected.Attributes & FileAttributes.Hidden) != 0, entry.IsHidden); - Assert.Equal(expected.CreationTimeUtc, entry.CreationTimeUtc); - Assert.Equal(expected.LastAccessTimeUtc, entry.LastAccessTimeUtc); - Assert.Equal(expected.LastWriteTimeUtc, entry.LastWriteTimeUtc); - Assert.Equal((expected.Attributes & FileAttributes.Hidden) != 0, entry.IsHidden); + AssertExtensions.EqualTo((expected.Attributes & FileAttributes.Hidden) != 0, entry.IsHidden, "IsHidden"); + AssertExtensions.EqualTo(expected.CreationTimeUtc, entry.CreationTimeUtc, "CreationTimeUtc"); + AssertExtensions.EqualTo(expected.LastAccessTimeUtc, entry.LastAccessTimeUtc, "LastAccessTimeUtc"); + AssertExtensions.EqualTo(expected.LastWriteTimeUtc, entry.LastWriteTimeUtc, "LastWriteTimeUtc"); if (expected is FileInfo fileInfo) { - Assert.Equal(fileInfo.Length, entry.Length); + AssertExtensions.EqualTo(fileInfo.Length, entry.Length, "Length"); } } else { // Because the item is deleted, we can no longer retrieve these values and defaults are returned instead. - Assert.Equal(entry.FileName.StartsWith('.'), entry.IsHidden); - DateTimeOffset defaultOffset = new DateTimeOffset(DateTime.FromFileTimeUtc(0)); - Assert.Equal(defaultOffset, entry.CreationTimeUtc); - Assert.Equal(defaultOffset, entry.LastAccessTimeUtc); - Assert.Equal(defaultOffset, entry.LastWriteTimeUtc); - Assert.Equal(0, entry.Length); + AssertExtensions.EqualTo(entry.FileName.StartsWith('.'), entry.IsHidden, "IsHidden"); + DateTimeOffset defaultTime = new DateTimeOffset(DateTime.FromFileTimeUtc(0)); + AssertExtensions.EqualTo(defaultTime, entry.CreationTimeUtc, "CreationTimeUtc"); + AssertExtensions.EqualTo(defaultTime, entry.LastAccessTimeUtc, "LastAccessTimeUtc"); + AssertExtensions.EqualTo(defaultTime, entry.LastWriteTimeUtc, "LastWriteTimeUtc"); + AssertExtensions.EqualTo(0, entry.Length, "Length"); } - Assert.False(enumerator.MoveNext()); + Assert.False(enumerator.MoveNext(), "Move final"); } static FileSystemInfo CreateItem(DirectoryInfo testDirectory, string item) @@ -133,7 +132,7 @@ static FileSystemInfo CreateItem(DirectoryInfo testDirectory, string item) else { // use the last char to have different lengths for different files. - Assert.True(item == "file1" || item == "file2"); + Assert.True(item == "file1" || item == "file2", "File names"); int length = (int)item[item.Length - 1]; File.WriteAllBytes(fullPath, new byte[length]); var info = new FileInfo(fullPath); From 4fd4f6b5dd2c14f19ba2d3b3f69f45be8ee22c0a Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 11 Oct 2021 21:58:17 +0200 Subject: [PATCH 08/12] test: update expected values for Browser platform --- .../tests/Enumeration/AttributeTests.cs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs index 14dd3cae58cd16..1eda52537d0877 100644 --- a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs +++ b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs @@ -88,8 +88,18 @@ public void PropertiesWhenItemNoLongerExists(string item1, string item2) AssertExtensions.EqualTo(expected.Name, entry.FileName, "Name"); // Values determined during enumeration. - AssertExtensions.EqualTo(expected is DirectoryInfo, entry.IsDirectory, "IsDirectory"); - AssertExtensions.EqualTo(expected.Attributes, entry.Attributes, "Attributes"); + if (PlatformDetection.IsBrowser) + { + // For Browser, all items are typed as DT_UNKNOWN. + AssertExtensions.EqualTo(false, entry.IsDirectory, "IsDirectory"); + AssertExtensions.EqualTo(entry.FileName.StartsWith('.') ? FileAttributes.Hidden : FileAttributes.Normal, entry.Attributes, "Attributes"); + } + else + { + AssertExtensions.EqualTo(expected is DirectoryInfo, entry.IsDirectory, "IsDirectory"); + AssertExtensions.EqualTo(expected.Attributes, entry.Attributes, "Attributes"); + } + AssertExtensions.EqualTo(testDirectory.FullName, entry.Directory, "Directory"); AssertExtensions.EqualTo(expected.FullName, entry.FullPath, "FullPath"); AssertExtensions.EqualTo(expected.FullName, entry.SpecifiedFullPath, "SpecifiedFullPath"); From 27ef95c9e75637418327434a1018bfcd1d686477 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 11 Oct 2021 22:17:00 +0200 Subject: [PATCH 09/12] Reorder test asserts and add some comments. --- .../tests/Enumeration/AttributeTests.cs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs index 1eda52537d0877..00da7f30c19c18 100644 --- a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs +++ b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs @@ -85,7 +85,12 @@ public void PropertiesWhenItemNoLongerExists(string item1, string item2) FileSystemInfo expected = entry.FileName == item1 ? item2Info : item1Info; Assert.True(enumerator.MoveNext(), "Move second"); entry = enumerator.Current; + + // Names and paths. AssertExtensions.EqualTo(expected.Name, entry.FileName, "Name"); + AssertExtensions.EqualTo(testDirectory.FullName, entry.Directory, "Directory"); + AssertExtensions.EqualTo(expected.FullName, entry.FullPath, "FullPath"); + AssertExtensions.EqualTo(expected.FullName, entry.SpecifiedFullPath, "SpecifiedFullPath"); // Values determined during enumeration. if (PlatformDetection.IsBrowser) @@ -100,10 +105,6 @@ public void PropertiesWhenItemNoLongerExists(string item1, string item2) AssertExtensions.EqualTo(expected.Attributes, entry.Attributes, "Attributes"); } - AssertExtensions.EqualTo(testDirectory.FullName, entry.Directory, "Directory"); - AssertExtensions.EqualTo(expected.FullName, entry.FullPath, "FullPath"); - AssertExtensions.EqualTo(expected.FullName, entry.SpecifiedFullPath, "SpecifiedFullPath"); - if (PlatformDetection.IsWindows) { AssertExtensions.EqualTo((expected.Attributes & FileAttributes.Hidden) != 0, entry.IsHidden, "IsHidden"); @@ -117,7 +118,8 @@ public void PropertiesWhenItemNoLongerExists(string item1, string item2) } else { - // Because the item is deleted, we can no longer retrieve these values and defaults are returned instead. + // On Unix, these values were not determined during enumeration. + // Because the file was deleted, the values can no longer be retrieved and sensible defaults are returned. AssertExtensions.EqualTo(entry.FileName.StartsWith('.'), entry.IsHidden, "IsHidden"); DateTimeOffset defaultTime = new DateTimeOffset(DateTime.FromFileTimeUtc(0)); AssertExtensions.EqualTo(defaultTime, entry.CreationTimeUtc, "CreationTimeUtc"); From c1fe08ffe220c996f0627c3e36153d0a72ca46fb Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 12 Oct 2021 06:38:20 +0200 Subject: [PATCH 10/12] Fix indentation. --- .../Common/tests/TestUtilities/System/AssertExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs b/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs index e0fed5dfb97d95..0cd19df620db3b 100644 --- a/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs +++ b/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs @@ -277,7 +277,7 @@ public static void Contains(string value, string substring) public static void EqualTo(T expected, T actual, string userMessage = null) { if (expected == null && actual == null) - return; + return; bool equal = expected != null && expected switch { From 4cd2e99efc78c7a4f6fd657f798cc83887317e62 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 3 Nov 2021 11:23:22 +0100 Subject: [PATCH 11/12] Also use symbolic links in the test --- .../tests/Enumeration/AttributeTests.cs | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs index 00da7f30c19c18..5351f0de87afb0 100644 --- a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs +++ b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs @@ -58,9 +58,14 @@ protected override FileSystemEntryProperties TransformEntry(ref FileSystemEntry // The test is performed using two items with different properties (file/dir, file length) // to check cached values from the previous entry don't leak into the non-existing entry. [InlineData("dir1", "dir2")] + [InlineData("dir1", "file2")] + [InlineData("dir1", "link2")] [InlineData("file1", "file2")] - [InlineData("dir1", "file1")] - [InlineData("file1", "dir1")] + [InlineData("file1", "dir2")] + [InlineData("file1", "link2")] + [InlineData("link1", "file2")] + [InlineData("link1", "dir2")] + [InlineData("link1", "link2")] [Theory] public void PropertiesWhenItemNoLongerExists(string item1, string item2) { @@ -134,6 +139,11 @@ public void PropertiesWhenItemNoLongerExists(string item1, string item2) static FileSystemInfo CreateItem(DirectoryInfo testDirectory, string item) { string fullPath = Path.Combine(testDirectory.FullName, item); + + // use the last char to have different lengths for different files. + Assert.True(item.EndsWith('1') || item.EndsWith('2')); + int length = (int)item[item.Length - 1]; + if (item.StartsWith("dir")) { Directory.CreateDirectory(fullPath); @@ -141,11 +151,15 @@ static FileSystemInfo CreateItem(DirectoryInfo testDirectory, string item) info.Refresh(); return info; } + else if (item.StartsWith("link")) + { + File.CreateSymbolicLink(fullPath, new string('_', length)); + var info = new FileInfo(fullPath); + info.Refresh(); + return info; + } else { - // use the last char to have different lengths for different files. - Assert.True(item == "file1" || item == "file2", "File names"); - int length = (int)item[item.Length - 1]; File.WriteAllBytes(fullPath, new byte[length]); var info = new FileInfo(fullPath); info.Refresh(); From 3a7356e1e6ca4817d8f30966e122e2f78df62c22 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 18 Nov 2021 05:55:57 +0100 Subject: [PATCH 12/12] PR feedback --- .../TestUtilities/System/AssertExtensions.cs | 22 ------------ .../tests/Enumeration/AttributeTests.cs | 36 +++++++++---------- 2 files changed, 18 insertions(+), 40 deletions(-) diff --git a/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs b/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs index 0cd19df620db3b..8f97aa5ea31ced 100644 --- a/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs +++ b/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs @@ -269,28 +269,6 @@ public static void Contains(string value, string substring) Assert.Contains(substring, value, StringComparison.Ordinal); } - /// - /// Validate that a given value is equal to another value. - /// - /// The expected value. - /// The value to be compared against. - public static void EqualTo(T expected, T actual, string userMessage = null) - { - if (expected == null && actual == null) - return; - - bool equal = expected != null && expected switch - { - IEquatable equatable => equatable.Equals(actual), - IComparable comparableT => comparableT.CompareTo(actual) == 0, - IComparable comparable => comparable.CompareTo(actual) == 0, - _ => throw new NotSupportedException($"{nameof(EqualTo)} is not supported for {typeof(T)}.") - }; - - if (!equal) - throw new XunitException(AddOptionalUserMessage($"Expected: {expected} is not equal to {actual}", userMessage)); - } - /// /// Validate that a given value is greater than another value. /// diff --git a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs index 5351f0de87afb0..ea9d91345ff902 100644 --- a/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs +++ b/src/libraries/System.IO.FileSystem/tests/Enumeration/AttributeTests.cs @@ -92,45 +92,45 @@ public void PropertiesWhenItemNoLongerExists(string item1, string item2) entry = enumerator.Current; // Names and paths. - AssertExtensions.EqualTo(expected.Name, entry.FileName, "Name"); - AssertExtensions.EqualTo(testDirectory.FullName, entry.Directory, "Directory"); - AssertExtensions.EqualTo(expected.FullName, entry.FullPath, "FullPath"); - AssertExtensions.EqualTo(expected.FullName, entry.SpecifiedFullPath, "SpecifiedFullPath"); + Assert.Equal(expected.Name, entry.FileName); + Assert.Equal(testDirectory.FullName, entry.Directory); + Assert.Equal(expected.FullName, entry.FullPath); + Assert.Equal(expected.FullName, entry.SpecifiedFullPath); // Values determined during enumeration. if (PlatformDetection.IsBrowser) { // For Browser, all items are typed as DT_UNKNOWN. - AssertExtensions.EqualTo(false, entry.IsDirectory, "IsDirectory"); - AssertExtensions.EqualTo(entry.FileName.StartsWith('.') ? FileAttributes.Hidden : FileAttributes.Normal, entry.Attributes, "Attributes"); + Assert.False(entry.IsDirectory); + Assert.Equal(entry.FileName.StartsWith('.') ? FileAttributes.Hidden : FileAttributes.Normal, entry.Attributes); } else { - AssertExtensions.EqualTo(expected is DirectoryInfo, entry.IsDirectory, "IsDirectory"); - AssertExtensions.EqualTo(expected.Attributes, entry.Attributes, "Attributes"); + Assert.Equal(expected is DirectoryInfo, entry.IsDirectory); + Assert.Equal(expected.Attributes, entry.Attributes); } if (PlatformDetection.IsWindows) { - AssertExtensions.EqualTo((expected.Attributes & FileAttributes.Hidden) != 0, entry.IsHidden, "IsHidden"); - AssertExtensions.EqualTo(expected.CreationTimeUtc, entry.CreationTimeUtc, "CreationTimeUtc"); - AssertExtensions.EqualTo(expected.LastAccessTimeUtc, entry.LastAccessTimeUtc, "LastAccessTimeUtc"); - AssertExtensions.EqualTo(expected.LastWriteTimeUtc, entry.LastWriteTimeUtc, "LastWriteTimeUtc"); + Assert.Equal((expected.Attributes & FileAttributes.Hidden) != 0, entry.IsHidden); + Assert.Equal(expected.CreationTimeUtc, entry.CreationTimeUtc); + Assert.Equal(expected.LastAccessTimeUtc, entry.LastAccessTimeUtc); + Assert.Equal(expected.LastWriteTimeUtc, entry.LastWriteTimeUtc); if (expected is FileInfo fileInfo) { - AssertExtensions.EqualTo(fileInfo.Length, entry.Length, "Length"); + Assert.Equal(fileInfo.Length, entry.Length); } } else { // On Unix, these values were not determined during enumeration. // Because the file was deleted, the values can no longer be retrieved and sensible defaults are returned. - AssertExtensions.EqualTo(entry.FileName.StartsWith('.'), entry.IsHidden, "IsHidden"); + Assert.Equal(entry.FileName.StartsWith('.'), entry.IsHidden); DateTimeOffset defaultTime = new DateTimeOffset(DateTime.FromFileTimeUtc(0)); - AssertExtensions.EqualTo(defaultTime, entry.CreationTimeUtc, "CreationTimeUtc"); - AssertExtensions.EqualTo(defaultTime, entry.LastAccessTimeUtc, "LastAccessTimeUtc"); - AssertExtensions.EqualTo(defaultTime, entry.LastWriteTimeUtc, "LastWriteTimeUtc"); - AssertExtensions.EqualTo(0, entry.Length, "Length"); + Assert.Equal(defaultTime, entry.CreationTimeUtc); + Assert.Equal(defaultTime, entry.LastAccessTimeUtc); + Assert.Equal(defaultTime, entry.LastWriteTimeUtc); + Assert.Equal(0, entry.Length); } Assert.False(enumerator.MoveNext(), "Move final");