diff --git a/docs/naming.md b/docs/naming.md index 08bc9c461..125f7333d 100644 --- a/docs/naming.md +++ b/docs/naming.md @@ -789,7 +789,7 @@ public static string NameWithParent(this Type type) return type.Name; } ``` -snippet source | anchor +snippet source | anchor Any path calculated in `DerivePathInfo` should be fully qualified to remove the inconsistency of the current directory. diff --git a/docs/temp-file.md b/docs/temp-file.md index a5bad7648..0bc13ecf8 100644 --- a/docs/temp-file.md +++ b/docs/temp-file.md @@ -34,7 +34,7 @@ public void Usage() // file automatically deleted here } ``` -snippet source | anchor +snippet source | anchor @@ -62,7 +62,7 @@ public void PathProperty() Assert.True(Path.IsPathRooted(path)); } ``` -snippet source | anchor +snippet source | anchor @@ -91,7 +91,7 @@ public void StringConversion() Trace.WriteLine(content); } ``` -snippet source | anchor +snippet source | anchor @@ -114,7 +114,7 @@ public void FileInfoConversion() Trace.WriteLine(directoryName); } ``` -snippet source | anchor +snippet source | anchor @@ -135,7 +135,7 @@ public void InfoProperty() Trace.WriteLine(directoryName); } ``` -snippet source | anchor +snippet source | anchor @@ -151,7 +151,7 @@ public void RootDirectory() => // Accessing the root directory for all TempDirectory instances Trace.WriteLine(TempFile.RootDirectory); ``` -snippet source | anchor +snippet source | anchor @@ -186,7 +186,7 @@ public async Task VerifyFileInstance() await VerifyFile(file); } ``` -snippet source | anchor +snippet source | anchor @@ -207,7 +207,7 @@ public async Task Scrubbing() }); } ``` -snippet source | anchor +snippet source | anchor Result: @@ -237,7 +237,7 @@ File.WriteAllText(temp, "content"); // file automatically deleted here ``` -snippet source | anchor +snippet source | anchor @@ -252,7 +252,7 @@ using var temp = TempFile.Create(".txt"); File.WriteAllText(temp, "content"); ``` -snippet source | anchor +snippet source | anchor @@ -267,7 +267,7 @@ using var temp = TempFile.Create(".txt", Encoding.UTF8); File.Exists(temp.Path); ``` -snippet source | anchor +snippet source | anchor @@ -283,7 +283,7 @@ using var temp = await TempFile.CreateText("Hello, World!"); var content = await File.ReadAllTextAsync(temp); Assert.Equal("Hello, World!", content); ``` -snippet source | anchor +snippet source | anchor @@ -304,7 +304,7 @@ using var temp = await TempFile.CreateText(json, ".json"); var content = await File.ReadAllTextAsync(temp); Assert.Equal(json, content); ``` -snippet source | anchor +snippet source | anchor @@ -323,7 +323,7 @@ using var temp = await TempFile.CreateText( var content = await File.ReadAllTextAsync(temp, Encoding.UTF8); Assert.Equal("Content with special chars: äöü", content); ``` -snippet source | anchor +snippet source | anchor @@ -341,7 +341,7 @@ using var temp = await TempFile.CreateBinary(data); var readData = await File.ReadAllBytesAsync(temp); Assert.Equal(data, readData); ``` -snippet source | anchor +snippet source | anchor @@ -353,7 +353,7 @@ Assert.Equal(data, readData); byte[] data = [0x01, 0x02, 0x03, 0x04]; using var temp = await TempFile.CreateBinary(data, ".bin"); ``` -snippet source | anchor +snippet source | anchor @@ -384,7 +384,7 @@ public void NoUsing() Debug.WriteLine(temp); } ``` -snippet source | anchor +snippet source | anchor The file can then be manually inspected. @@ -408,7 +408,7 @@ public void OpenExplorerAndDebug() temp.OpenExplorerAndDebug(); } ``` -snippet source | anchor +snippet source | anchor This method is designed to help debug tests by enabling the inspection of the contents of the temporary file while the test is paused. It performs two actions: diff --git a/src/Verify.Tests/CopyConstructorTests.cs b/src/Verify.Tests/CopyConstructorTests.cs new file mode 100644 index 000000000..81b632423 --- /dev/null +++ b/src/Verify.Tests/CopyConstructorTests.cs @@ -0,0 +1,58 @@ +public class CopyConstructorTests +{ + [Fact] + public void PreservesThrowException() + { + var settings = new VerifySettings(); + settings.AutoVerify(throwException: true); + + var copy = new VerifySettings(settings); + + Assert.True(copy.throwException); + } + + [Fact] + public void ClonesAppendedFiles() + { + var settings = new VerifySettings(); + settings.AppendContentAsFile("base"); + + var copy = new VerifySettings(settings); + copy.AppendContentAsFile("extra"); + + // The base settings must not see the file appended to the copy. + Assert.Single(settings.appendedFiles!); + Assert.Equal(2, copy.appendedFiles!.Count); + } + + [Fact] + public void ClonesExtensionStreamComparers() + { + var settings = new VerifySettings(); + StreamCompare compare = (_, _, _) => Task.FromResult(CompareResult.Equal); + // Extensions unique to this test so the global comparer registry can't + // satisfy the lookup below. + settings.UseStreamComparer(compare, "copyctorext1"); + + var copy = new VerifySettings(settings); + copy.UseStreamComparer(compare, "copyctorext2"); + + // The comparer added to the copy must not leak into the base settings. + Assert.False(settings.TryFindStreamComparer("copyctorext2", out _)); + Assert.True(copy.TryFindStreamComparer("copyctorext2", out _)); + } + + [Fact] + public void ClonesExtensionMappedScrubbers() + { + var settings = new VerifySettings(); + settings.AddScrubber("txt", _ => { }); + + var copy = new VerifySettings(settings); + copy.AddScrubber("txt", _ => { }); + + // The inner list is shared unless deep-copied. + Assert.Single(settings.ExtensionMappedInstanceScrubbers!["txt"]); + Assert.Equal(2, copy.ExtensionMappedInstanceScrubbers!["txt"].Count); + } +} diff --git a/src/Verify.Tests/IoHelpersTests.cs b/src/Verify.Tests/IoHelpersTests.cs index 779af602c..25c029dd1 100644 --- a/src/Verify.Tests/IoHelpersTests.cs +++ b/src/Verify.Tests/IoHelpersTests.cs @@ -18,4 +18,44 @@ public void ResolveDirectoryNameFromSourceFileTests(string sourceFile, string ex Assert.Equal(expectedDirectory, IoHelpers.ResolveDirectoryFromSourceFile(sourceFile)); } + + [Fact] + public async Task WriteStreamHandlesExclusiveFileStream() + { + using var source = new TempFile(".bin"); + await File.WriteAllBytesAsync(source, [1, 2, 3, 4, 5, 6, 7, 8]); + var destination = Path.Combine(Path.GetTempPath(), $"WriteStream{Guid.NewGuid():N}.bin"); + try + { + // Writable + FileShare.None means File.Copy cannot re-open the source + // by path on Windows; WriteStream must fall back to the handle. + using var stream = new FileStream(source, FileMode.Open, FileAccess.ReadWrite, FileShare.None); + await IoHelpers.WriteStream(destination, stream); + + Assert.True(File.Exists(destination)); + Assert.Equal(8, new FileInfo(destination).Length); + } + finally + { + File.Delete(destination); + } + } + + [Fact] + public void ExtensionForExtensionlessFileStream() + { + var directory = Path.Combine(Path.GetTempPath(), $"ExtTest{Guid.NewGuid():N}"); + Directory.CreateDirectory(directory); + var path = Path.Combine(directory, "Dockerfile"); + File.WriteAllText(path, "content"); + try + { + using var stream = File.OpenRead(path); + Assert.Equal("noextension", stream.Extension()); + } + finally + { + Directory.Delete(directory, true); + } + } } \ No newline at end of file diff --git a/src/Verify.Tests/TempDirectoryTests.cs b/src/Verify.Tests/TempDirectoryTests.cs index b88ae3f62..36284a194 100644 --- a/src/Verify.Tests/TempDirectoryTests.cs +++ b/src/Verify.Tests/TempDirectoryTests.cs @@ -427,6 +427,28 @@ public void IgnoreLockedFiles_DefaultIsFalse() } } + [Fact] + public void CleanupDoesNotThrowForUndeletableDirectory() + { + var directoryPath = Path.Combine(TempDirectory.RootDirectory, Path.GetRandomFileName()); + Directory.CreateDirectory(directoryPath); + var filePath = Path.Combine(directoryPath, "locked.txt"); + File.WriteAllText(filePath, "content"); + PreventDeletion(filePath, directoryPath); + Directory.SetLastWriteTime(directoryPath, DateTime.Now.AddDays(-2)); + try + { + // An aged-out orphan that cannot be deleted must not fail the whole + // module-initializer cleanup. + TempDirectory.Cleanup(); + } + finally + { + AllowDeletion(filePath, directoryPath); + Directory.Delete(directoryPath, true); + } + } + // On Windows, ReadOnly on a file prevents deletion. // On Unix, file deletion is controlled by the parent directory's write permission. static void PreventDeletion(string filePath, string directoryPath) diff --git a/src/Verify.Tests/TempFileTests.cs b/src/Verify.Tests/TempFileTests.cs index 0003f07ec..5a74c319f 100644 --- a/src/Verify.Tests/TempFileTests.cs +++ b/src/Verify.Tests/TempFileTests.cs @@ -23,6 +23,55 @@ public void Constructor_WithExtension_AppliesExtension() Assert.EndsWith(".txt", temp.Path); } + [Fact] + public void CleanupDoesNotThrowForUndeletableFile() + { + var filePath = Path.Combine(TempFile.RootDirectory, $"undeletable{Guid.NewGuid():N}.txt"); + File.WriteAllText(filePath, "content"); + // Age the file out before marking it read-only: on .NET Framework the + // timestamp cannot be set on a read-only file. + File.SetLastWriteTime(filePath, DateTime.Now.AddDays(-2)); + // On Windows a read-only file cannot be deleted; on Unix deletion is + // governed by the directory, so the file is deletable there and this + // test is a no-op guard. + var readOnly = RuntimeInformation.IsOSPlatform(OSPlatform.Windows); + if (readOnly) + { + File.SetAttributes(filePath, FileAttributes.ReadOnly); + } + + try + { + TempFile.Cleanup(); + } + finally + { + if (readOnly) + { + File.SetAttributes(filePath, FileAttributes.Normal); + } + + if (File.Exists(filePath)) + { + File.Delete(filePath); + } + } + } + + [Fact] + public async Task ConcurrentInstancesDoNotCorruptState() + { + // Establish the shared async-local path list, then hammer it from many + // child contexts that flow the same list in. + using var parent = new TempFile(); + var tasks = Enumerable.Range(0, 200) + .Select(_ => Task.Run(() => + { + using var temp = new TempFile(); + })); + await Task.WhenAll(tasks); + } + #region VerifyTempFile [Fact] diff --git a/src/Verify/Extensions.cs b/src/Verify/Extensions.cs index deba36960..0873a696e 100644 --- a/src/Verify/Extensions.cs +++ b/src/Verify/Extensions.cs @@ -20,8 +20,17 @@ static class Extensions public static bool IsNumeric(this Type type) => numericTypes.Contains(Nullable.GetUnderlyingType(type) ?? type); - public static string Extension(this FileStream file) => - Path.GetExtension(file.Name)[1..]; + public static string Extension(this FileStream file) + { + var extension = Path.GetExtension(file.Name); + if (extension.Length == 0) + { + // Matches the convention used for extension-less directory entries. + return "noextension"; + } + + return extension[1..]; + } public static Version MajorMinor(this Version version) => new(version.Major, version.Minor); diff --git a/src/Verify/IoHelpers.cs b/src/Verify/IoHelpers.cs index 5fe104ee3..39b07c413 100644 --- a/src/Verify/IoHelpers.cs +++ b/src/Verify/IoHelpers.cs @@ -218,8 +218,18 @@ public static async Task WriteStream(string path, Stream stream) throw new($"Empty data is not allowed. Path: {fileStream.Name}"); } - File.Copy(fileStream.Name, path, true); - return; + if (TryFileCopy(fileStream, path)) + { + return; + } + + // The fast path could not re-open the source by path; fall back to + // copying through the existing handle. Reset first so the whole file + // is written, matching File.Copy semantics. + if (fileStream.CanSeek) + { + fileStream.MoveToStart(); + } } // keep using scope to stream is flushed @@ -233,4 +243,29 @@ public static async Task WriteStream(string path, Stream stream) throw new($"Empty data is not allowed. Path: {path}"); } } + + // Copies a FileStream by path (avoiding a read through the managed stream) + // when safe. Returns false — so the caller falls back to a handle copy — for + // handle-based streams (Name is not a rooted path) or when the source cannot + // be re-opened because it is held with a conflicting mode (writable / + // FileShare.None on Windows). + static bool TryFileCopy(FileStream fileStream, string path) + { + var name = fileStream.Name; + if (!Path.IsPathRooted(name)) + { + return false; + } + + try + { + File.Copy(name, path, true); + return true; + } + catch (Exception exception) + when (exception is IOException or UnauthorizedAccessException) + { + return false; + } + } } \ No newline at end of file diff --git a/src/Verify/TempDirectory.cs b/src/Verify/TempDirectory.cs index 1941f90b9..4b56fa14a 100644 --- a/src/Verify/TempDirectory.cs +++ b/src/Verify/TempDirectory.cs @@ -71,9 +71,12 @@ public static void Init() return; } - foreach (var path in pathsValue) + lock (pathsLock) { - scrubber.Replace(path, "{TempDirectory}"); + foreach (var path in pathsValue) + { + scrubber.Replace(path, "{TempDirectory}"); + } } }); @@ -81,6 +84,7 @@ public static void Init() } static AsyncLocal?> asyncPaths = new(); + static readonly object pathsLock = new(); internal static void Cleanup() { @@ -98,9 +102,12 @@ internal static void Cleanup() { Directory.Delete(dir, recursive: true); } - catch (DirectoryNotFoundException) + catch (Exception exception) + when (exception is IOException or UnauthorizedAccessException) { - //Ignore directory cleanup race condition + // Ignore orphans that can't be deleted (locked, read-only, or a + // cleanup race); they are retried on a later run once released. + // DirectoryNotFoundException derives from IOException. } } } @@ -144,14 +151,17 @@ public TempDirectory(bool ignoreLockedFiles) Path = IoPath.Combine(RootDirectory, IoPath.GetRandomFileName()); Directory.CreateDirectory(Path); - paths = asyncPaths.Value!; - if (paths == null) - { - paths = asyncPaths.Value = [Path]; - } - else + lock (pathsLock) { - paths.Add(Path); + paths = asyncPaths.Value!; + if (paths == null) + { + paths = asyncPaths.Value = [Path]; + } + else + { + paths.Add(Path); + } } } @@ -182,7 +192,10 @@ public void Dispose() } } - paths.Remove(Path); + lock (pathsLock) + { + paths.Remove(Path); + } } /// diff --git a/src/Verify/TempFile.cs b/src/Verify/TempFile.cs index c6dc7305e..612745f37 100644 --- a/src/Verify/TempFile.cs +++ b/src/Verify/TempFile.cs @@ -73,9 +73,12 @@ public static void Init() return; } - foreach (var path in pathsValue) + lock (pathsLock) { - scrubber.Replace(path, "{TempFile}"); + foreach (var path in pathsValue) + { + scrubber.Replace(path, "{TempFile}"); + } } }); @@ -83,6 +86,7 @@ public static void Init() } static AsyncLocal?> asyncPaths = new(); + static readonly object pathsLock = new(); internal static void Cleanup() { @@ -101,9 +105,12 @@ internal static void Cleanup() { File.Delete(file); } - catch (FileNotFoundException) + catch (Exception exception) + when (exception is IOException or UnauthorizedAccessException) { - // Ignore file cleanup race condition + // Ignore orphans that can't be deleted (locked, read-only, or a + // cleanup race). File.Delete is a no-op for a missing file, so + // FileNotFoundException is not the case that needs handling here. } } } @@ -135,14 +142,17 @@ public TempFile(string? extension = null) Path = IoPath.Combine(RootDirectory, fileName); - paths = asyncPaths.Value!; - if (paths == null) + lock (pathsLock) { - paths = asyncPaths.Value = [Path]; - } - else - { - paths.Add(Path); + paths = asyncPaths.Value!; + if (paths == null) + { + paths = asyncPaths.Value = [Path]; + } + else + { + paths.Add(Path); + } } } @@ -324,7 +334,10 @@ public void Dispose() File.Delete(Path); } - paths.Remove(Path); + lock (pathsLock) + { + paths.Remove(Path); + } } /// diff --git a/src/Verify/VerifySettings.cs b/src/Verify/VerifySettings.cs index 7af23e441..006667199 100644 --- a/src/Verify/VerifySettings.cs +++ b/src/Verify/VerifySettings.cs @@ -31,22 +31,28 @@ public VerifySettings(VerifySettings? settings) if (settings.ExtensionMappedInstanceScrubbers != null) { - ExtensionMappedInstanceScrubbers = new(settings.ExtensionMappedInstanceScrubbers); + // Deep copy: the inner lists are mutated in place by AddScrubber. + ExtensionMappedInstanceScrubbers = settings.ExtensionMappedInstanceScrubbers + .ToDictionary(_ => _.Key, _ => _.Value.ToList()); } diffEnabled = settings.diffEnabled; MethodName = settings.MethodName; TypeName = settings.TypeName; strictJson = settings.strictJson; - appendedFiles = settings.appendedFiles; + // Clone the mutable collections below, otherwise per-test fluent config + // (AppendFile/UseStreamComparer/etc.) would mutate a shared base settings + // instance and leak across tests. + appendedFiles = settings.appendedFiles?.ToList(); UniqueDirectory = settings.UniqueDirectory; Directory = settings.Directory; autoVerify = settings.autoVerify; + throwException = settings.throwException; serialization = settings.serialization; stringComparer = settings.stringComparer; streamComparer = settings.streamComparer; - extensionStringComparers = settings.extensionStringComparers; - extensionStreamComparers = settings.extensionStreamComparers; + extensionStringComparers = settings.extensionStringComparers == null ? null : new(settings.extensionStringComparers); + extensionStreamComparers = settings.extensionStreamComparers == null ? null : new(settings.extensionStreamComparers); parameters = settings.parameters; ignoredParameters = settings.ignoredParameters; classArgumentCount = settings.classArgumentCount;