From 059a2ed98fe28813fd48739778670e99c8fed936 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Fri, 18 Sep 2026 15:05:23 +1000 Subject: [PATCH 1/7] Characterize dangling snapshot detection gaps Adds tests covering the scenarios the check currently gets wrong. The snapshots record actual behaviour, so the fix shows up as a diff rather than as new files. False negatives, all caused by IfFileUnique being a substring blacklist over the whole relative path: * Every UniqueFor* snapshot is exempt permanently, not deferred. No run on any framework or OS reports Deleted.DotNet9_0.verified.txt. * `.Net` has no trailing separator, so Tests.NetworkClient.verified.txt and Tests.NetCoreShim.verified.txt are skipped. * One directory whose name contains a token hides every snapshot below it. * Split mode UseUniqueDirectory puts `.verified.` in the directory name, so the `*.verified.*` glob never matches those files at all. False positives, which are what forced the blacklist in the first place: * The architecture and assembly configuration axes are not in the list, so .arm64 and .Debug snapshots from another run are reported. * ResolveDirectory leaves `..` in the path for a relative UseDirectory, so the ordinal comparison misses and every snapshot in that directory is reported. * A directory case difference between the MSBuild project directory and the compiler's caller file path puts everything in the incorrect case bucket. Also covers indexes, target names, parameters, trailing separators, duplicate tracked entries, tracked paths with no file yet, and bin/obj being walked. Extracts FindSnapshotFiles from Run so the scan has a test seam. No behaviour change. --- ...ckTests.DirectoryCaseMismatch.verified.txt | 6 + ...irectoryWithTrailingSeparator.verified.txt | 6 + ...Tests.DuplicateTrackedEntries.verified.txt | 1 + ...shotsCheckTests.NothingOnDisk.verified.txt | 1 + ...ts.OrphanedNamesContainingNet.verified.txt | 1 + ...toryContainingUniquenessToken.verified.txt | 6 + ...hanedWithIndexesAndParameters.verified.txt | 8 + ...thLowerCaseUniquenessSegments.verified.txt | 9 + ...kTests.OrphanedWithUniqueness.verified.txt | 13 + ...ests.StaleParameterOnLiveTest.verified.txt | 6 + ...ts.TrackedFileMissingFromDisk.verified.txt | 1 + ...ests.UniquenessFromAnotherRun.verified.txt | 7 + ...Tests.UnnormalizedTrackedPath.verified.txt | 6 + ...sts.UntrackedAndIncorrectCase.verified.txt | 11 + .../DanglingSnapshotsCheckTests.cs | 321 +++++++++++++++++- .../ConventionCheck/DanglingSnapshotsCheck.cs | 6 +- 16 files changed, 403 insertions(+), 6 deletions(-) create mode 100644 src/Verify.Tests/DanglingSnapshotsCheckTests.DirectoryCaseMismatch.verified.txt create mode 100644 src/Verify.Tests/DanglingSnapshotsCheckTests.DirectoryWithTrailingSeparator.verified.txt create mode 100644 src/Verify.Tests/DanglingSnapshotsCheckTests.DuplicateTrackedEntries.verified.txt create mode 100644 src/Verify.Tests/DanglingSnapshotsCheckTests.NothingOnDisk.verified.txt create mode 100644 src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedNamesContainingNet.verified.txt create mode 100644 src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedUnderDirectoryContainingUniquenessToken.verified.txt create mode 100644 src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithIndexesAndParameters.verified.txt create mode 100644 src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithLowerCaseUniquenessSegments.verified.txt create mode 100644 src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithUniqueness.verified.txt create mode 100644 src/Verify.Tests/DanglingSnapshotsCheckTests.StaleParameterOnLiveTest.verified.txt create mode 100644 src/Verify.Tests/DanglingSnapshotsCheckTests.TrackedFileMissingFromDisk.verified.txt create mode 100644 src/Verify.Tests/DanglingSnapshotsCheckTests.UniquenessFromAnotherRun.verified.txt create mode 100644 src/Verify.Tests/DanglingSnapshotsCheckTests.UnnormalizedTrackedPath.verified.txt create mode 100644 src/Verify.Tests/DanglingSnapshotsCheckTests.UntrackedAndIncorrectCase.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.DirectoryCaseMismatch.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.DirectoryCaseMismatch.verified.txt new file mode 100644 index 0000000000..ea7f01328d --- /dev/null +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.DirectoryCaseMismatch.verified.txt @@ -0,0 +1,6 @@ + +Verify has detected the following issues with snapshot files: + +The following files have been tracked with incorrect case: + + * Nested/Alive.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.DirectoryWithTrailingSeparator.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.DirectoryWithTrailingSeparator.verified.txt new file mode 100644 index 0000000000..0e129bf23f --- /dev/null +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.DirectoryWithTrailingSeparator.verified.txt @@ -0,0 +1,6 @@ + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Deleted.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.DuplicateTrackedEntries.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.DuplicateTrackedEntries.verified.txt new file mode 100644 index 0000000000..e3c88929ff --- /dev/null +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.DuplicateTrackedEntries.verified.txt @@ -0,0 +1 @@ +Nothing reported. \ No newline at end of file diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.NothingOnDisk.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.NothingOnDisk.verified.txt new file mode 100644 index 0000000000..e3c88929ff --- /dev/null +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.NothingOnDisk.verified.txt @@ -0,0 +1 @@ +Nothing reported. \ No newline at end of file diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedNamesContainingNet.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedNamesContainingNet.verified.txt new file mode 100644 index 0000000000..e3c88929ff --- /dev/null +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedNamesContainingNet.verified.txt @@ -0,0 +1 @@ +Nothing reported. \ No newline at end of file diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedUnderDirectoryContainingUniquenessToken.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedUnderDirectoryContainingUniquenessToken.verified.txt new file mode 100644 index 0000000000..55297af44d --- /dev/null +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedUnderDirectoryContainingUniquenessToken.verified.txt @@ -0,0 +1,6 @@ + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Plain/Deleted.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithIndexesAndParameters.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithIndexesAndParameters.verified.txt new file mode 100644 index 0000000000..4ef7d66c20 --- /dev/null +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithIndexesAndParameters.verified.txt @@ -0,0 +1,8 @@ + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Deleted#00.verified.txt + * Deleted#name.verified.txt + * Deleted_param=value.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithLowerCaseUniquenessSegments.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithLowerCaseUniquenessSegments.verified.txt new file mode 100644 index 0000000000..33097e584b --- /dev/null +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithLowerCaseUniquenessSegments.verified.txt @@ -0,0 +1,9 @@ + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Deleted.windows.verified.txt + * Deleted.linux.verified.txt + * Deleted.osx.verified.txt + * Deleted.dotnet9_0.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithUniqueness.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithUniqueness.verified.txt new file mode 100644 index 0000000000..72431a9af3 --- /dev/null +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithUniqueness.verified.txt @@ -0,0 +1,13 @@ + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Deleted.verified.txt + * Deleted.Mono6_12.verified.txt + * Deleted.Android.verified.txt + * Deleted.IOS.verified.txt + * Deleted.x64.verified.txt + * Deleted.arm64.verified.txt + * Deleted.Debug.verified.txt + * Deleted.Release.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.StaleParameterOnLiveTest.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.StaleParameterOnLiveTest.verified.txt new file mode 100644 index 0000000000..664a3e06a2 --- /dev/null +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.StaleParameterOnLiveTest.verified.txt @@ -0,0 +1,6 @@ + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Alive_param=removed.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.TrackedFileMissingFromDisk.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.TrackedFileMissingFromDisk.verified.txt new file mode 100644 index 0000000000..e3c88929ff --- /dev/null +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.TrackedFileMissingFromDisk.verified.txt @@ -0,0 +1 @@ +Nothing reported. \ No newline at end of file diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.UniquenessFromAnotherRun.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.UniquenessFromAnotherRun.verified.txt new file mode 100644 index 0000000000..52dc79e4d2 --- /dev/null +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.UniquenessFromAnotherRun.verified.txt @@ -0,0 +1,7 @@ + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Alive.arm64.verified.txt + * Alive.Debug.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.UnnormalizedTrackedPath.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.UnnormalizedTrackedPath.verified.txt new file mode 100644 index 0000000000..bd321f588a --- /dev/null +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.UnnormalizedTrackedPath.verified.txt @@ -0,0 +1,6 @@ + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Snapshots/Alive.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.UntrackedAndIncorrectCase.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.UntrackedAndIncorrectCase.verified.txt new file mode 100644 index 0000000000..e28ce6de3d --- /dev/null +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.UntrackedAndIncorrectCase.verified.txt @@ -0,0 +1,11 @@ + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Deleted.verified.txt + + +The following files have been tracked with incorrect case: + + * AliveWithCase.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.cs b/src/Verify.Tests/DanglingSnapshotsCheckTests.cs index b98fd95d74..6160da436d 100644 --- a/src/Verify.Tests/DanglingSnapshotsCheckTests.cs +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.cs @@ -1,6 +1,8 @@ #pragma warning disable VerifyDanglingSnapshots public class DanglingSnapshotsCheckTests { + const string root = "path/to"; + [Fact] public Task Untracked() { @@ -13,7 +15,7 @@ public Task Untracked() "path/to/tracked.verified.txt" }; - return Throws(() => DanglingSnapshotsCheck.CheckFiles(filesOnDisk, trackedFiles, "path/to")) + return Throws(() => DanglingSnapshotsCheck.CheckFiles(filesOnDisk, trackedFiles, root)) .IgnoreStackTrace(); } @@ -29,7 +31,7 @@ public Task IncorrectCase() "path/to/tracked.verified.txt" }; - return Throws(() => DanglingSnapshotsCheck.CheckFiles(filesOnDisk, trackedFiles, "path/to")) + return Throws(() => DanglingSnapshotsCheck.CheckFiles(filesOnDisk, trackedFiles, root)) .IgnoreStackTrace(); } @@ -39,6 +41,317 @@ public void AllTracked() var filesOnDisk = new List { "path/to/tracked.verified.txt" }; var trackedFiles = new ConcurrentBag { "path/to/tracked.verified.txt" }; - DanglingSnapshotsCheck.CheckFiles(filesOnDisk, trackedFiles, "path/to"); + DanglingSnapshotsCheck.CheckFiles(filesOnDisk, trackedFiles, root); + } + + /// + /// Every file here is a snapshot for a test that no longer exists: nothing shares its prefix + /// with anything the run tracked. A file is reported only when its whole relative path avoids + /// the IfFileUnique substring list, so the ones carrying a runtime, framework or OS + /// segment are skipped. They are skipped on every framework and every OS, so nothing ever + /// reports them. + /// + [Fact] + public Task OrphanedWithUniqueness() => + Verify( + Report( + [ + "path/to/Deleted.verified.txt", + "path/to/Deleted.DotNet.verified.txt", + "path/to/Deleted.DotNet9_0.verified.txt", + "path/to/Deleted.Net.verified.txt", + "path/to/Deleted.Net4_8.verified.txt", + "path/to/Deleted.Mono.verified.txt", + "path/to/Deleted.Mono6_12.verified.txt", + "path/to/Deleted.Windows.verified.txt", + "path/to/Deleted.Linux.verified.txt", + "path/to/Deleted.OSX.verified.txt", + "path/to/Deleted.Android.verified.txt", + "path/to/Deleted.IOS.verified.txt", + "path/to/Deleted.x64.verified.txt", + "path/to/Deleted.arm64.verified.txt", + "path/to/Deleted.Debug.verified.txt", + "path/to/Deleted.Release.verified.txt" + ], + "path/to/Alive.verified.txt")); + + /// + /// The case the uniqueness skip exists for. A multi targeted project running one framework + /// leaves the snapshots of the other frameworks untouched, and the same holds for the OS, + /// architecture and configuration axes. None of these may be reported. + /// + [Fact] + public Task UniquenessFromAnotherRun() => + Verify( + Report( + [ + "path/to/Alive.DotNet9_0.verified.txt", + "path/to/Alive.DotNet8_0.verified.txt", + "path/to/Alive.Net4_8.verified.txt", + "path/to/Alive.Linux.verified.txt", + "path/to/Alive.arm64.verified.txt", + "path/to/Alive.Debug.verified.txt" + ], + "path/to/Alive.DotNet9_0.verified.txt")); + + /// + /// IfFileUnique matches .Net with no trailing separator, so any name that merely + /// starts a word with Net is skipped. None of these are uniqueness segments, and all of + /// them are snapshots for tests that do not exist. + /// + [Fact] + public Task OrphanedNamesContainingNet() => + Verify( + Report( + [ + "path/to/Tests.NetworkClient.verified.txt", + "path/to/Tests.NetCoreShim.verified.txt", + "path/to/Tests.Nettle.verified.txt", + "path/to/HttpTests.NetworkFailure.verified.txt" + ], + "path/to/Alive.verified.txt")); + + /// + /// The substring list is applied to the path relative to the project, not to the file name, so + /// a single directory whose name contains one of the tokens hides every snapshot below it. + /// + [Fact] + public Task OrphanedUnderDirectoryContainingUniquenessToken() => + Verify( + Report( + [ + "path/to/Shared.NetCore/Deleted.verified.txt", + "path/to/Snapshots.Windows.Only/Deleted.verified.txt", + "path/to/Plain/Deleted.verified.txt" + ], + "path/to/Alive.verified.txt")); + + /// + /// The skip is a case sensitive Contains, so a segment is skipped only in the exact + /// casing Namer produces. Not wrong on its own, but it shows the list is matching text + /// rather than recognising a uniqueness value. + /// + [Fact] + public Task OrphanedWithLowerCaseUniquenessSegments() => + Verify( + Report( + [ + "path/to/Deleted.windows.verified.txt", + "path/to/Deleted.linux.verified.txt", + "path/to/Deleted.osx.verified.txt", + "path/to/Deleted.dotnet9_0.verified.txt" + ], + "path/to/Alive.verified.txt")); + + /// + /// A verified name is {Type}.{Method}{_parameters}{.uniqueness}#{index}. Everything + /// after the type and method varies per run, so none of it identifies the test. + /// + [Fact] + public Task OrphanedWithIndexesAndParameters() => + Verify( + Report( + [ + "path/to/Deleted#00.verified.txt", + "path/to/Deleted#name.verified.txt", + "path/to/Deleted_param=value.verified.txt", + "path/to/Deleted_param=value.DotNet9_0#00.verified.txt", + "path/to/Deleted.DotNet9_0#00.verified.txt" + ], + "path/to/Alive.verified.txt")); + + /// + /// A test that is still there but whose parameter set changed leaves a snapshot behind. The + /// prefix is live, so this is the one case where the run genuinely cannot tell a stale + /// parameter from one produced by a sibling test case that did not run. + /// + [Fact] + public Task StaleParameterOnLiveTest() => + Verify( + Report( + [ + "path/to/Alive_param=current.verified.txt", + "path/to/Alive_param=removed.verified.txt" + ], + "path/to/Alive_param=current.verified.txt")); + + /// + /// ResolveDirectory combines the source file directory with a relative + /// UseDirectory through Path.Combine, which does not normalise, so the tracked + /// path keeps its ... The scan returns the normalised path and the ordinal comparison + /// misses, reporting a live snapshot as dangling. + /// + [Fact] + public Task UnnormalizedTrackedPath() => + Verify( + Report( + ["path/to/Snapshots/Alive.verified.txt"], + "path/to/Tests/../Snapshots/Alive.verified.txt")); + + /// + /// Only the case of the file name is meaningful. The case of the directory comes from whatever + /// string each side happened to build its path from: the scan starts at the project directory + /// recorded by MSBuild, the tracked path starts at the compiler's caller file path. + /// + [Fact] + public Task DirectoryCaseMismatch() => + Verify( + Report( + ["path/to/Nested/Alive.verified.txt"], + "path/to/nested/Alive.verified.txt")); + + /// + /// The project directory recorded by MSBuild ends with a separator, so the relative suffix has + /// to survive both forms. + /// + [Fact] + public Task DirectoryWithTrailingSeparator() => + Verify( + ReportIn( + "path/to/", + ["path/to/Deleted.verified.txt"], + "path/to/Alive.verified.txt")); + + [Fact] + public Task UntrackedAndIncorrectCase() => + Verify( + Report( + [ + "path/to/Deleted.verified.txt", + "path/to/AliveWithCase.verified.txt" + ], + "path/to/alivewithcase.verified.txt")); + + /// + /// A test can track a verified path that has no file yet: the first run of a new test, or a run + /// where every target was new. That is not a dangling file. + /// + [Fact] + public Task TrackedFileMissingFromDisk() => + Verify(Report([], "path/to/NotYetAccepted.verified.txt")); + + /// + /// Several verifications can resolve to one verified path, through UseFileName or a + /// DerivePathInfo that collapses them, so the bag can hold duplicates. + /// + [Fact] + public Task DuplicateTrackedEntries() => + Verify( + Report( + ["path/to/Alive.verified.txt"], + "path/to/Alive.verified.txt", + "path/to/Alive.verified.txt")); + + [Fact] + public Task NothingOnDisk() => + Verify(Report([])); + + /// + /// UseUniqueDirectory in split mode writes to + /// {prefix}.verified\{target}.{extension}, so .verified. is in the directory name + /// rather than the file name and the scan's glob never matches. A deleted test leaves its whole + /// snapshot directory behind. + /// + [Fact] + public void FindSplitModeUniqueDirectory() + { + using var directory = new TempDirectory(); + WriteNested(directory, "Tests.Split.verified", "target.txt"); + WriteNested(directory, "Tests.Split.verified", "target#00.txt"); + WriteNested(directory, "Tests.Split.verified", "nested", "target.txt"); + + Assert.Empty(DanglingSnapshotsCheck.FindSnapshotFiles(directory)); + } + + /// + /// The non split unique directory convention keeps .verified. in the file name, so those + /// files are found. + /// + [Fact] + public void FindNonSplitUniqueDirectory() + { + using var directory = new TempDirectory(); + WriteNested(directory, "Tests.Unique", "target.verified.txt"); + WriteNested(directory, "Tests.Unique", "target#00.verified.txt"); + + Assert.Equal(2, DanglingSnapshotsCheck.FindSnapshotFiles(directory).Count()); + } + + /// + /// The scan walks every directory below the project, build output included. Harmless while + /// nothing copies snapshots there, and a report of files that are not snapshot sources for + /// every project that sets CopyToOutputDirectory on them. + /// + [Fact] + public void FindIncludesBinAndObj() + { + using var directory = new TempDirectory(); + WriteNested(directory, "bin", "Release", "net9.0", "Tests.Copied.verified.txt"); + WriteNested(directory, "obj", "Release", "net9.0", "Tests.Copied.verified.txt"); + + Assert.Equal(2, DanglingSnapshotsCheck.FindSnapshotFiles(directory).Count()); + } + + [Fact] + public void FindExcludesReceived() + { + using var directory = new TempDirectory(); + WriteNested(directory, "Tests.Alive.received.txt"); + WriteNested(directory, "Tests.Alive.verified.txt"); + + var found = DanglingSnapshotsCheck.FindSnapshotFiles(directory) + .ToList(); + + Assert.Single(found); + Assert.EndsWith("Tests.Alive.verified.txt", found[0]); + } + + /// + /// The Win32 glob *.verified.* also matches a name ending in .verified with no + /// extension, which is not a file Verify writes. + /// + [Fact] + public void FindIncludesExtensionlessVerified() + { + using var directory = new TempDirectory(); + WriteNested(directory, "Tests.Stray.verified"); + + Assert.Single(DanglingSnapshotsCheck.FindSnapshotFiles(directory)); + } + + [Fact] + public void FindIncludesNestedDirectories() + { + using var directory = new TempDirectory(); + WriteNested(directory, "Tests.Alive.verified.txt"); + WriteNested(directory, "Snapshots", "Tests.Alive.verified.txt"); + WriteNested(directory, "Snapshots", "Deep", "Tests.Alive.verified.txt"); + + Assert.Equal(3, DanglingSnapshotsCheck.FindSnapshotFiles(directory).Count()); + } + + static string WriteNested(TempDirectory directory, params string[] segments) + { + var path = directory.BuildPath(segments); + Directory.CreateDirectory(Path.GetDirectoryName(path)!); + File.WriteAllText(path, "content"); + return path; + } + + static string Report(IEnumerable filesOnDisk, params string[] tracked) => + ReportIn(root, filesOnDisk, tracked); + + static string ReportIn(string directory, IEnumerable filesOnDisk, params string[] tracked) + { + ConcurrentBag trackedFiles = [..tracked]; + try + { + DanglingSnapshotsCheck.CheckFiles(filesOnDisk, trackedFiles, directory); + return "Nothing reported."; + } + catch (Exception exception) + { + return exception.Message; + } } -} \ No newline at end of file +} diff --git a/src/Verify/ConventionCheck/DanglingSnapshotsCheck.cs b/src/Verify/ConventionCheck/DanglingSnapshotsCheck.cs index d2d1d2bba9..55aed3b06f 100644 --- a/src/Verify/ConventionCheck/DanglingSnapshotsCheck.cs +++ b/src/Verify/ConventionCheck/DanglingSnapshotsCheck.cs @@ -18,10 +18,12 @@ public static void Run() } var directory = AttributeReader.GetProjectDirectory(VerifierSettings.Assembly); - var files = Directory.EnumerateFiles(directory, "*.verified.*", SearchOption.AllDirectories); - CheckFiles(files, trackedVerifiedFiles!, directory); + CheckFiles(FindSnapshotFiles(directory), trackedVerifiedFiles!, directory); } + internal static IEnumerable FindSnapshotFiles(string directory) => + Directory.EnumerateFiles(directory, "*.verified.*", SearchOption.AllDirectories); + internal static void CheckFiles(IEnumerable filesOnDisk, ConcurrentBag trackedFiles, string directory) { static void AppendItems(StringBuilder builder, List list, string title) From 01cb0d524c23958e01a10a087d6e418f53b30ca7 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Fri, 18 Sep 2026 15:18:31 +1000 Subject: [PATCH 2/7] Decide dangling snapshots by manifest union rather than by file name A run only ever knew the files it wrote itself. A multi targeted project runs once per framework, so most of the snapshots on disk belong to a framework that is not running, and telling those from a snapshot whose test was deleted was a guess from the file name: any path containing ".Net", ".DotNet", ".Mono.", ".OSX.", ".Windows." or ".Linux." was skipped. That guess is permanent, not a deferral. No run on any framework reports Deleted.DotNet9_0.verified.txt, because every run skips it. In this repo 203 of 2350 verified files were exempt. Each run now records what it tracked to a manifest in the intermediate directory, named after its target framework. The manifests share one directory across every framework of the project, so a run reads back what the other runs tracked. Once every framework in TargetFrameworks has a manifest the union is the complete set of files the project owns, and anything outside it is dangling whatever its name says. The name based skip is split to match what a manifest can settle: * The runtime and target framework tokens are consulted only until the manifests cover every framework, and are ignored once they do. * The OS tokens are always skipped. Those runs are on other machines with their own intermediate directories, so their manifests are never visible here. Degrades to the previous behaviour whenever the union is incomplete: the first framework to run, a project that does not consume Verify's build props, a manifest that cannot be read, or one predating the assembly running the check, which would otherwise let a stale manifest mask the danglers the check exists for. Adds Verify.TargetFramework and Verify.DanglingDirectory to the stamped assembly metadata. DanglingDirectory is derived from BaseIntermediateOutputPath, which unlike IntermediateOutputPath is shared by every target framework, and is scoped by configuration. Verified end to end: a two framework project with a retired Tests.Retired.DotNet9_0.verified.txt reports nothing on the first framework and reports the file on the second, once both manifests exist. --- docs/dangling-files.md | 16 ++ docs/mdsource/dangling-files.source.md | 16 ++ ...tTests.OrphanSurvivesTheUnion.verified.txt | 10 + src/Verify.Tests/DanglingManifestTests.cs | 234 ++++++++++++++++++ ...ckTests.DirectoryCaseMismatch.verified.txt | 11 +- ...irectoryWithTrailingSeparator.verified.txt | 11 +- ...Tests.DuplicateTrackedEntries.verified.txt | 5 +- ...shotsCheckTests.NothingOnDisk.verified.txt | 5 +- ...ts.OrphanedNamesContainingNet.verified.txt | 13 +- ...toryContainingUniquenessToken.verified.txt | 12 +- ...hanedWithIndexesAndParameters.verified.txt | 15 +- ...thLowerCaseUniquenessSegments.verified.txt | 14 +- ...kTests.OrphanedWithUniqueness.verified.txt | 23 +- ...sts.RetiredFrameworkSnapshots.verified.txt | 9 + ...ests.StaleParameterOnLiveTest.verified.txt | 11 +- ...ts.TrackedFileMissingFromDisk.verified.txt | 5 +- ...ests.UniquenessFromAnotherRun.verified.txt | 12 +- ...Tests.UnnormalizedTrackedPath.verified.txt | 11 +- ...sts.UntrackedAndIncorrectCase.verified.txt | 16 +- .../DanglingSnapshotsCheckTests.cs | 46 +++- .../ConventionCheck/DanglingManifest.cs | 113 +++++++++ .../ConventionCheck/DanglingSnapshotsCheck.cs | 99 +++++++- src/Verify/DerivePaths/AttributeReader.cs | 13 + src/Verify/VerifierSettings_TargetAssembly.cs | 28 +++ src/Verify/buildTransitive/Verify.props | 13 + 25 files changed, 735 insertions(+), 26 deletions(-) create mode 100644 src/Verify.Tests/DanglingManifestTests.OrphanSurvivesTheUnion.verified.txt create mode 100644 src/Verify.Tests/DanglingManifestTests.cs create mode 100644 src/Verify.Tests/DanglingSnapshotsCheckTests.RetiredFrameworkSnapshots.verified.txt create mode 100644 src/Verify/ConventionCheck/DanglingManifest.cs diff --git a/docs/dangling-files.md b/docs/dangling-files.md index 2efb8bf5fb..f960333e9d 100644 --- a/docs/dangling-files.md +++ b/docs/dangling-files.md @@ -19,6 +19,22 @@ A dangling snapshot file are when a `.verified.` file exist with no correspondin * have casing that does not match the test case +## Multi targeted projects + +A multi targeted project runs its tests once per target framework, so most of the snapshots on disk during any one run belong to a framework that is not currently running. A `UniqueForRuntime` or `UniqueForTargetFramework` snapshot of a deleted test is indistinguishable, by name, from one that another framework still owns. + +To tell them apart, each run records what it tracked to a manifest in the intermediate (obj) directory. The manifests are named after the target framework and share one directory across all frameworks of the project, so a run can read what the other runs tracked. Once every target framework has a manifest, the union of them is the complete set of snapshot files the project owns, and anything on disk outside that union is dangling regardless of what its name suggests. + +In a multi targeted run this means the check is at its most accurate on the last framework to run: the earlier runs cannot yet account for the frameworks still to come, and fall back to skipping names that look like they belong to another framework. + +The manifests are scoped to the build configuration, and are ignored if they predate the assembly running the check, so a stale manifest cannot mask a dangling file. They live in obj and are removed by a clean. + +Two axes cannot be settled this way, and snapshot names carrying them are always skipped: + + * `UniqueForOSPlatform`, since the runs that produce those files are on other machines with their own intermediate directories. + * `UniqueForArchitecture` and `UniqueForAssemblyConfiguration`, which are not recognised as uniqueness at all and are reported as dangling if no run tracks them. + + ## Experimental `DanglingSnapshots` is an experimental feature (marked with `[Experimental("VerifyDanglingSnapshots")]`) and is subject to change in minor version. diff --git a/docs/mdsource/dangling-files.source.md b/docs/mdsource/dangling-files.source.md index cdcd7c89d1..76d226af36 100644 --- a/docs/mdsource/dangling-files.source.md +++ b/docs/mdsource/dangling-files.source.md @@ -12,6 +12,22 @@ A dangling snapshot file are when a `.verified.` file exist with no correspondin * have casing that does not match the test case +## Multi targeted projects + +A multi targeted project runs its tests once per target framework, so most of the snapshots on disk during any one run belong to a framework that is not currently running. A `UniqueForRuntime` or `UniqueForTargetFramework` snapshot of a deleted test is indistinguishable, by name, from one that another framework still owns. + +To tell them apart, each run records what it tracked to a manifest in the intermediate (obj) directory. The manifests are named after the target framework and share one directory across all frameworks of the project, so a run can read what the other runs tracked. Once every target framework has a manifest, the union of them is the complete set of snapshot files the project owns, and anything on disk outside that union is dangling regardless of what its name suggests. + +In a multi targeted run this means the check is at its most accurate on the last framework to run: the earlier runs cannot yet account for the frameworks still to come, and fall back to skipping names that look like they belong to another framework. + +The manifests are scoped to the build configuration, and are ignored if they predate the assembly running the check, so a stale manifest cannot mask a dangling file. They live in obj and are removed by a clean. + +Two axes cannot be settled this way, and snapshot names carrying them are always skipped: + + * `UniqueForOSPlatform`, since the runs that produce those files are on other machines with their own intermediate directories. + * `UniqueForArchitecture` and `UniqueForAssemblyConfiguration`, which are not recognised as uniqueness at all and are reported as dangling if no run tracks them. + + ## Experimental `DanglingSnapshots` is an experimental feature (marked with `[Experimental("VerifyDanglingSnapshots")]`) and is subject to change in minor version. diff --git a/src/Verify.Tests/DanglingManifestTests.OrphanSurvivesTheUnion.verified.txt b/src/Verify.Tests/DanglingManifestTests.OrphanSurvivesTheUnion.verified.txt new file mode 100644 index 0000000000..92e34d56ed --- /dev/null +++ b/src/Verify.Tests/DanglingManifestTests.OrphanSurvivesTheUnion.verified.txt @@ -0,0 +1,10 @@ +{ + Type: Exception, + Message: +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Tests.Deleted.DotNet8_0.verified.txt + +} \ No newline at end of file diff --git a/src/Verify.Tests/DanglingManifestTests.cs b/src/Verify.Tests/DanglingManifestTests.cs new file mode 100644 index 0000000000..efa89316dd --- /dev/null +++ b/src/Verify.Tests/DanglingManifestTests.cs @@ -0,0 +1,234 @@ +#pragma warning disable VerifyDanglingSnapshots +public class DanglingManifestTests +{ + static readonly DateTime noStaleFiltering = DateTime.MinValue; + + [Fact] + public void SingleFrameworkIsCoveredByItsOwnManifest() + { + using var directory = new TempDirectory(); + DanglingManifest.Write(directory, "net9.0", ["one.verified.txt", "two.verified.txt"]); + + var (tracked, covered) = DanglingManifest.Read(directory, [], "net9.0", noStaleFiltering); + + Assert.True(covered); + Assert.Equal(["one.verified.txt", "two.verified.txt"], tracked.Order()); + } + + /// + /// The point of the manifests: the run of one framework sees what the runs of the others + /// tracked, so their files are not mistaken for dangling ones. + /// + [Fact] + public void EveryFrameworkPresentIsCovered() + { + using var directory = new TempDirectory(); + DanglingManifest.Write(directory, "net8.0", ["Alive.DotNet8_0.verified.txt"]); + DanglingManifest.Write(directory, "net9.0", ["Alive.DotNet9_0.verified.txt"]); + + var (tracked, covered) = DanglingManifest.Read(directory, ["net8.0", "net9.0"], "net9.0", noStaleFiltering); + + Assert.True(covered); + Assert.Equal(["Alive.DotNet8_0.verified.txt", "Alive.DotNet9_0.verified.txt"], tracked.Order()); + } + + /// + /// The first framework to run has only its own manifest, so it cannot account for the files of + /// the frameworks still to run and has to fall back. + /// + [Fact] + public void MissingFrameworkIsNotCovered() + { + using var directory = new TempDirectory(); + DanglingManifest.Write(directory, "net9.0", ["Alive.DotNet9_0.verified.txt"]); + + var (tracked, covered) = DanglingManifest.Read(directory, ["net8.0", "net9.0"], "net9.0", noStaleFiltering); + + Assert.False(covered); + Assert.Equal(["Alive.DotNet9_0.verified.txt"], tracked); + } + + /// + /// A manifest naming a framework the project no longer targets is read for its paths, but says + /// nothing about coverage. + /// + [Fact] + public void RetiredFrameworkDoesNotSatisfyCoverage() + { + using var directory = new TempDirectory(); + DanglingManifest.Write(directory, "net7.0", ["Alive.DotNet7_0.verified.txt"]); + DanglingManifest.Write(directory, "net9.0", ["Alive.DotNet9_0.verified.txt"]); + + var (_, covered) = DanglingManifest.Read(directory, ["net8.0", "net9.0"], "net9.0", noStaleFiltering); + + Assert.False(covered); + } + + /// + /// A manifest from before the last build may name files that have since been renamed or + /// deleted, and counting those as tracked would hide exactly the danglers the check is for. + /// + [Fact] + public void StaleManifestIsIgnored() + { + using var directory = new TempDirectory(); + DanglingManifest.Write(directory, "net8.0", ["Stale.DotNet8_0.verified.txt"]); + DanglingManifest.Write(directory, "net9.0", ["Alive.DotNet9_0.verified.txt"]); + + // Pinned rather than left to the clock: the two manifests are written milliseconds apart, + // and file timestamp granularity is coarser than that on some file systems. + var buildTime = DateTime.UtcNow; + File.SetLastWriteTimeUtc( + DanglingManifest.PathFor(directory, "net8.0"), + buildTime.AddHours(-1)); + File.SetLastWriteTimeUtc( + DanglingManifest.PathFor(directory, "net9.0"), + buildTime.AddHours(1)); + + var (tracked, covered) = DanglingManifest.Read(directory, ["net8.0", "net9.0"], "net9.0", buildTime); + + Assert.False(covered); + Assert.Equal(["Alive.DotNet9_0.verified.txt"], tracked); + } + + /// + /// A re run of one framework replaces its manifest rather than adding to it, so a file that run + /// no longer tracks stops being tracked. + /// + [Fact] + public void ReRunReplacesManifest() + { + using var directory = new TempDirectory(); + DanglingManifest.Write(directory, "net9.0", ["Before.verified.txt"]); + DanglingManifest.Write(directory, "net9.0", ["After.verified.txt"]); + + var (tracked, _) = DanglingManifest.Read(directory, [], "net9.0", noStaleFiltering); + + Assert.Equal(["After.verified.txt"], tracked); + } + + /// + /// Several verifications can resolve to one verified path, so the bag feeding the manifest can + /// hold duplicates. + /// + [Fact] + public void DuplicatesAreWrittenOnce() + { + using var directory = new TempDirectory(); + DanglingManifest.Write(directory, "net9.0", ["Alive.verified.txt", "Alive.verified.txt"]); + + var lines = File.ReadAllLines(DanglingManifest.PathFor(directory, "net9.0")); + + Assert.Equal(["Alive.verified.txt"], lines); + } + + [Fact] + public void NoDirectoryIsNotCovered() + { + using var directory = new TempDirectory(); + var missing = directory.BuildPath("absent"); + + var (tracked, covered) = DanglingManifest.Read(missing, ["net9.0"], "net9.0", noStaleFiltering); + + Assert.False(covered); + Assert.Empty(tracked); + } + + [Fact] + public void EmptyManifestIsStillCoverage() + { + using var directory = new TempDirectory(); + DanglingManifest.Write(directory, "net9.0", []); + + var (tracked, covered) = DanglingManifest.Read(directory, ["net9.0"], "net9.0", noStaleFiltering); + + Assert.True(covered); + Assert.Empty(tracked); + } + + /// + /// A verified path can be anything the file system accepts, so the manifest has to survive + /// spaces and the characters Verify puts in a name for parameters, targets and indexes. + /// + [Fact] + public void AwkwardPathsRoundTrip() + { + using var directory = new TempDirectory(); + string[] paths = + [ + @"D:\a b\Tests.Method_param=a b.DotNet9_0#00.verified.txt", + "/home/a b/Tests.Method#name.verified.txt", + @"D:\a\Tests.Method_param=Ünïcödé.verified.txt" + ]; + DanglingManifest.Write(directory, "net9.0", paths); + + var (tracked, _) = DanglingManifest.Read(directory, [], "net9.0", noStaleFiltering); + + Assert.Equal(paths.Order(), tracked.Order()); + } + + /// + /// Written through a temporary file and moved into place. A partially written manifest read by + /// another framework's run would drop paths and report live snapshots as dangling. + /// + [Fact] + public void WriteLeavesNoTemporaryFiles() + { + using var directory = new TempDirectory(); + DanglingManifest.Write(directory, "net9.0", ["Alive.verified.txt"]); + + var files = Directory.GetFiles(directory) + .Select(Path.GetFileName) + .Order(); + + Assert.Equal(["net9.0.txt"], files); + } + + /// + /// End to end: two frameworks, one snapshot each, and one left behind by a deleted test. Only + /// the union can tell them apart, and only when it is complete. + /// + [Fact] + public Task OrphanSurvivesTheUnion() + { + using var directory = new TempDirectory(); + var root = directory.Path; + var aliveOnNet8 = Path.Combine(root, "Tests.Alive.DotNet8_0.verified.txt"); + var aliveOnNet9 = Path.Combine(root, "Tests.Alive.DotNet9_0.verified.txt"); + var orphan = Path.Combine(root, "Tests.Deleted.DotNet8_0.verified.txt"); + + DanglingManifest.Write(directory, "net8.0", [aliveOnNet8]); + DanglingManifest.Write(directory, "net9.0", [aliveOnNet9]); + + var (tracked, covered) = DanglingManifest.Read(directory, ["net8.0", "net9.0"], "net9.0", noStaleFiltering); + + Assert.True(covered); + return Throws( + () => DanglingSnapshotsCheck.CheckFiles( + [aliveOnNet8, aliveOnNet9, orphan], + tracked, + root, + covered)) + .IgnoreStackTrace(); + } + + /// + /// The same inputs without full coverage. The orphan carries a framework segment, so the + /// fallback skips it and reports nothing. + /// + [Fact] + public void OrphanIsInvisibleWithoutCoverage() + { + using var directory = new TempDirectory(); + var root = directory.Path; + var aliveOnNet9 = Path.Combine(root, "Tests.Alive.DotNet9_0.verified.txt"); + var orphan = Path.Combine(root, "Tests.Deleted.DotNet8_0.verified.txt"); + + DanglingManifest.Write(directory, "net9.0", [aliveOnNet9]); + + var (tracked, covered) = DanglingManifest.Read(directory, ["net8.0", "net9.0"], "net9.0", noStaleFiltering); + + Assert.False(covered); + DanglingSnapshotsCheck.CheckFiles([aliveOnNet9, orphan], tracked, root, covered); + } +} diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.DirectoryCaseMismatch.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.DirectoryCaseMismatch.verified.txt index ea7f01328d..cc0604d2d7 100644 --- a/src/Verify.Tests/DanglingSnapshotsCheckTests.DirectoryCaseMismatch.verified.txt +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.DirectoryCaseMismatch.verified.txt @@ -1,4 +1,13 @@ - +== Frameworks not covered == + +Verify has detected the following issues with snapshot files: + +The following files have been tracked with incorrect case: + + * Nested/Alive.verified.txt + +== Frameworks covered == + Verify has detected the following issues with snapshot files: The following files have been tracked with incorrect case: diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.DirectoryWithTrailingSeparator.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.DirectoryWithTrailingSeparator.verified.txt index 0e129bf23f..1367861742 100644 --- a/src/Verify.Tests/DanglingSnapshotsCheckTests.DirectoryWithTrailingSeparator.verified.txt +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.DirectoryWithTrailingSeparator.verified.txt @@ -1,4 +1,13 @@ - +== Frameworks not covered == + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Deleted.verified.txt + +== Frameworks covered == + Verify has detected the following issues with snapshot files: The following files have not been tracked: diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.DuplicateTrackedEntries.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.DuplicateTrackedEntries.verified.txt index e3c88929ff..cd471dbf3d 100644 --- a/src/Verify.Tests/DanglingSnapshotsCheckTests.DuplicateTrackedEntries.verified.txt +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.DuplicateTrackedEntries.verified.txt @@ -1 +1,4 @@ -Nothing reported. \ No newline at end of file +== Frameworks not covered == +Nothing reported. +== Frameworks covered == +Nothing reported. \ No newline at end of file diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.NothingOnDisk.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.NothingOnDisk.verified.txt index e3c88929ff..cd471dbf3d 100644 --- a/src/Verify.Tests/DanglingSnapshotsCheckTests.NothingOnDisk.verified.txt +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.NothingOnDisk.verified.txt @@ -1 +1,4 @@ -Nothing reported. \ No newline at end of file +== Frameworks not covered == +Nothing reported. +== Frameworks covered == +Nothing reported. \ No newline at end of file diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedNamesContainingNet.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedNamesContainingNet.verified.txt index e3c88929ff..7a8879ea22 100644 --- a/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedNamesContainingNet.verified.txt +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedNamesContainingNet.verified.txt @@ -1 +1,12 @@ -Nothing reported. \ No newline at end of file +== Frameworks not covered == +Nothing reported. +== Frameworks covered == + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Tests.NetworkClient.verified.txt + * Tests.NetCoreShim.verified.txt + * Tests.Nettle.verified.txt + * HttpTests.NetworkFailure.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedUnderDirectoryContainingUniquenessToken.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedUnderDirectoryContainingUniquenessToken.verified.txt index 55297af44d..4cefaf5b17 100644 --- a/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedUnderDirectoryContainingUniquenessToken.verified.txt +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedUnderDirectoryContainingUniquenessToken.verified.txt @@ -1,6 +1,16 @@ - +== Frameworks not covered == + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Plain/Deleted.verified.txt + +== Frameworks covered == + Verify has detected the following issues with snapshot files: The following files have not been tracked: + * Shared.NetCore/Deleted.verified.txt * Plain/Deleted.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithIndexesAndParameters.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithIndexesAndParameters.verified.txt index 4ef7d66c20..9db089e1ef 100644 --- a/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithIndexesAndParameters.verified.txt +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithIndexesAndParameters.verified.txt @@ -1,4 +1,15 @@ - +== Frameworks not covered == + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Deleted#00.verified.txt + * Deleted#name.verified.txt + * Deleted_param=value.verified.txt + +== Frameworks covered == + Verify has detected the following issues with snapshot files: The following files have not been tracked: @@ -6,3 +17,5 @@ The following files have not been tracked: * Deleted#00.verified.txt * Deleted#name.verified.txt * Deleted_param=value.verified.txt + * Deleted_param=value.DotNet9_0#00.verified.txt + * Deleted.DotNet9_0#00.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithLowerCaseUniquenessSegments.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithLowerCaseUniquenessSegments.verified.txt index 33097e584b..f00e1a207c 100644 --- a/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithLowerCaseUniquenessSegments.verified.txt +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithLowerCaseUniquenessSegments.verified.txt @@ -1,4 +1,16 @@ - +== Frameworks not covered == + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Deleted.windows.verified.txt + * Deleted.linux.verified.txt + * Deleted.osx.verified.txt + * Deleted.dotnet9_0.verified.txt + +== Frameworks covered == + Verify has detected the following issues with snapshot files: The following files have not been tracked: diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithUniqueness.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithUniqueness.verified.txt index 72431a9af3..7f77eb2ef0 100644 --- a/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithUniqueness.verified.txt +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithUniqueness.verified.txt @@ -1,9 +1,30 @@ - +== Frameworks not covered == + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Deleted.verified.txt + * Deleted.Mono6_12.verified.txt + * Deleted.Android.verified.txt + * Deleted.IOS.verified.txt + * Deleted.x64.verified.txt + * Deleted.arm64.verified.txt + * Deleted.Debug.verified.txt + * Deleted.Release.verified.txt + +== Frameworks covered == + Verify has detected the following issues with snapshot files: The following files have not been tracked: * Deleted.verified.txt + * Deleted.DotNet.verified.txt + * Deleted.DotNet9_0.verified.txt + * Deleted.Net.verified.txt + * Deleted.Net4_8.verified.txt + * Deleted.Mono.verified.txt * Deleted.Mono6_12.verified.txt * Deleted.Android.verified.txt * Deleted.IOS.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.RetiredFrameworkSnapshots.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.RetiredFrameworkSnapshots.verified.txt new file mode 100644 index 0000000000..74aa46b481 --- /dev/null +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.RetiredFrameworkSnapshots.verified.txt @@ -0,0 +1,9 @@ +== Frameworks not covered == +Nothing reported. +== Frameworks covered == + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Alive.DotNet8_0.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.StaleParameterOnLiveTest.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.StaleParameterOnLiveTest.verified.txt index 664a3e06a2..ddf144af46 100644 --- a/src/Verify.Tests/DanglingSnapshotsCheckTests.StaleParameterOnLiveTest.verified.txt +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.StaleParameterOnLiveTest.verified.txt @@ -1,4 +1,13 @@ - +== Frameworks not covered == + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Alive_param=removed.verified.txt + +== Frameworks covered == + Verify has detected the following issues with snapshot files: The following files have not been tracked: diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.TrackedFileMissingFromDisk.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.TrackedFileMissingFromDisk.verified.txt index e3c88929ff..cd471dbf3d 100644 --- a/src/Verify.Tests/DanglingSnapshotsCheckTests.TrackedFileMissingFromDisk.verified.txt +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.TrackedFileMissingFromDisk.verified.txt @@ -1 +1,4 @@ -Nothing reported. \ No newline at end of file +== Frameworks not covered == +Nothing reported. +== Frameworks covered == +Nothing reported. \ No newline at end of file diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.UniquenessFromAnotherRun.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.UniquenessFromAnotherRun.verified.txt index 52dc79e4d2..79dd0afb04 100644 --- a/src/Verify.Tests/DanglingSnapshotsCheckTests.UniquenessFromAnotherRun.verified.txt +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.UniquenessFromAnotherRun.verified.txt @@ -1,4 +1,14 @@ - +== Frameworks not covered == + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Alive.arm64.verified.txt + * Alive.Debug.verified.txt + +== Frameworks covered == + Verify has detected the following issues with snapshot files: The following files have not been tracked: diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.UnnormalizedTrackedPath.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.UnnormalizedTrackedPath.verified.txt index bd321f588a..acde8d9292 100644 --- a/src/Verify.Tests/DanglingSnapshotsCheckTests.UnnormalizedTrackedPath.verified.txt +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.UnnormalizedTrackedPath.verified.txt @@ -1,4 +1,13 @@ - +== Frameworks not covered == + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Snapshots/Alive.verified.txt + +== Frameworks covered == + Verify has detected the following issues with snapshot files: The following files have not been tracked: diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.UntrackedAndIncorrectCase.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.UntrackedAndIncorrectCase.verified.txt index e28ce6de3d..ab35b9092e 100644 --- a/src/Verify.Tests/DanglingSnapshotsCheckTests.UntrackedAndIncorrectCase.verified.txt +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.UntrackedAndIncorrectCase.verified.txt @@ -1,4 +1,18 @@ - +== Frameworks not covered == + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Deleted.verified.txt + + +The following files have been tracked with incorrect case: + + * AliveWithCase.verified.txt + +== Frameworks covered == + Verify has detected the following issues with snapshot files: The following files have not been tracked: diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.cs b/src/Verify.Tests/DanglingSnapshotsCheckTests.cs index 6160da436d..5f8559f183 100644 --- a/src/Verify.Tests/DanglingSnapshotsCheckTests.cs +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.cs @@ -15,7 +15,7 @@ public Task Untracked() "path/to/tracked.verified.txt" }; - return Throws(() => DanglingSnapshotsCheck.CheckFiles(filesOnDisk, trackedFiles, root)) + return Throws(() => DanglingSnapshotsCheck.CheckFiles(filesOnDisk, trackedFiles, root, false)) .IgnoreStackTrace(); } @@ -31,7 +31,7 @@ public Task IncorrectCase() "path/to/tracked.verified.txt" }; - return Throws(() => DanglingSnapshotsCheck.CheckFiles(filesOnDisk, trackedFiles, root)) + return Throws(() => DanglingSnapshotsCheck.CheckFiles(filesOnDisk, trackedFiles, root, false)) .IgnoreStackTrace(); } @@ -41,7 +41,7 @@ public void AllTracked() var filesOnDisk = new List { "path/to/tracked.verified.txt" }; var trackedFiles = new ConcurrentBag { "path/to/tracked.verified.txt" }; - DanglingSnapshotsCheck.CheckFiles(filesOnDisk, trackedFiles, root); + DanglingSnapshotsCheck.CheckFiles(filesOnDisk, trackedFiles, root, false); } /// @@ -79,6 +79,10 @@ public Task OrphanedWithUniqueness() => /// The case the uniqueness skip exists for. A multi targeted project running one framework /// leaves the snapshots of the other frameworks untouched, and the same holds for the OS, /// architecture and configuration axes. None of these may be reported. + /// + /// The tracked set is the complete framework union, which is what a covered run has. The + /// architecture and configuration axes are reported either way: no manifest covers them, and + /// they were never in the name based skip either. /// [Fact] public Task UniquenessFromAnotherRun() => @@ -92,6 +96,23 @@ public Task UniquenessFromAnotherRun() => "path/to/Alive.arm64.verified.txt", "path/to/Alive.Debug.verified.txt" ], + "path/to/Alive.DotNet9_0.verified.txt", + "path/to/Alive.DotNet8_0.verified.txt", + "path/to/Alive.Net4_8.verified.txt")); + + /// + /// A project that dropped a target framework keeps the snapshots that framework owned. No run + /// produces them again, so they are dangling, and only a complete union can say so: by name + /// they are indistinguishable from the files of a framework that simply is not running. + /// + [Fact] + public Task RetiredFrameworkSnapshots() => + Verify( + Report( + [ + "path/to/Alive.DotNet9_0.verified.txt", + "path/to/Alive.DotNet8_0.verified.txt" + ], "path/to/Alive.DotNet9_0.verified.txt")); /// @@ -341,12 +362,27 @@ static string WriteNested(TempDirectory directory, params string[] segments) static string Report(IEnumerable filesOnDisk, params string[] tracked) => ReportIn(root, filesOnDisk, tracked); + /// + /// Reports both ways round. Without a manifest from every target framework the check falls back + /// to skipping names that look like they belong to another framework; with one the union decides + /// instead, and the difference between the two halves is what the manifests bought. + /// static string ReportIn(string directory, IEnumerable filesOnDisk, params string[] tracked) { - ConcurrentBag trackedFiles = [..tracked]; + var files = filesOnDisk.ToList(); + var builder = new StringBuilder(); + builder.AppendLine("== Frameworks not covered =="); + builder.AppendLine(Check(files, tracked, directory, false)); + builder.AppendLine("== Frameworks covered =="); + builder.Append(Check(files, tracked, directory, true)); + return builder.ToString(); + } + + static string Check(IReadOnlyCollection filesOnDisk, IReadOnlyCollection tracked, string directory, bool frameworksCovered) + { try { - DanglingSnapshotsCheck.CheckFiles(filesOnDisk, trackedFiles, directory); + DanglingSnapshotsCheck.CheckFiles(filesOnDisk, tracked, directory, frameworksCovered); return "Nothing reported."; } catch (Exception exception) diff --git a/src/Verify/ConventionCheck/DanglingManifest.cs b/src/Verify/ConventionCheck/DanglingManifest.cs new file mode 100644 index 0000000000..477484f8ec --- /dev/null +++ b/src/Verify/ConventionCheck/DanglingManifest.cs @@ -0,0 +1,113 @@ +namespace VerifyTests; + +/// +/// Records the verified files one target framework's test run tracked, so that the runs of the other +/// target frameworks can read it back. +/// +/// Without this a run only knows the files it wrote itself. A multi targeted project runs once per +/// framework, so most of the snapshots on disk belong to a framework that is not running, and the +/// only way to tell those from a snapshot whose test was deleted was to guess from the file name. +/// Unioning the manifests replaces the guess with the answer for that axis. +/// +/// Manifests go in the intermediate (obj) directory, below BaseIntermediateOutputPath, which unlike +/// IntermediateOutputPath is shared by every target framework of the project. They are scoped by +/// configuration, since a Debug run says nothing about the snapshots a Release run owns, and named +/// after the target framework, so a re run overwrites rather than accumulates. +/// +/// Staleness is bounded two ways: obj is wiped by a clean, and a manifest older than the running +/// assembly is from before the last build and is ignored. A run that cannot account for every target +/// framework says so, and the caller falls back to the name based skip. +/// +static class DanglingManifest +{ + const string extension = ".txt"; + + public static string PathFor(string directory, string targetFramework) => + Path.Combine(directory, $"{targetFramework}{extension}"); + + /// + /// Written through a temporary file and moved into place, so a run of another target framework + /// reading concurrently sees either the previous manifest or this one, never half of one. + /// + public static void Write(string directory, string targetFramework, IEnumerable trackedFiles) + { + Directory.CreateDirectory(directory); + var path = PathFor(directory, targetFramework); + var temp = $"{path}.{Guid.NewGuid():N}.tmp"; + try + { + File.WriteAllLines(temp, trackedFiles.Distinct()); + File.Move(temp, path, true); + } + catch + { + IoHelpers.DeleteFile(temp); + throw; + } + } + + /// + /// Every verified file recorded by the manifests in , and whether + /// those manifests account for every framework in . + /// + /// + /// Manifests last written before this are from before the last build and are ignored. The + /// running assembly's timestamp is the caller's value: a build server builds every framework + /// before running any of them, so a manifest from this build is always newer. + /// + public static (HashSet tracked, bool covered) Read( + string directory, + IReadOnlyList targetFrameworks, + string targetFramework, + DateTime staleBefore) + { + HashSet tracked = []; + HashSet frameworksRead = new(StringComparer.OrdinalIgnoreCase); + + if (Directory.Exists(directory)) + { + foreach (var file in Directory.EnumerateFiles(directory, $"*{extension}")) + { + if (File.GetLastWriteTimeUtc(file) < staleBefore) + { + continue; + } + + foreach (var line in File.ReadLines(file)) + { + if (line.Length > 0) + { + tracked.Add(line); + } + } + + frameworksRead.Add(Path.GetFileNameWithoutExtension(file)); + } + } + + return (tracked, IsCovered(targetFrameworks, targetFramework, frameworksRead)); + } + + /// + /// A single targeted project has no TargetFrameworks value, so its own manifest is the whole + /// picture. A multi targeted project needs one manifest per framework: anything less and the + /// files of the frameworks still to run are indistinguishable from dangling ones. + /// + static bool IsCovered(IReadOnlyList targetFrameworks, string targetFramework, HashSet frameworksRead) + { + if (targetFrameworks.Count == 0) + { + return frameworksRead.Contains(targetFramework); + } + + foreach (var framework in targetFrameworks) + { + if (!frameworksRead.Contains(framework)) + { + return false; + } + } + + return true; + } +} diff --git a/src/Verify/ConventionCheck/DanglingSnapshotsCheck.cs b/src/Verify/ConventionCheck/DanglingSnapshotsCheck.cs index 55aed3b06f..719f9c42eb 100644 --- a/src/Verify/ConventionCheck/DanglingSnapshotsCheck.cs +++ b/src/Verify/ConventionCheck/DanglingSnapshotsCheck.cs @@ -17,14 +17,81 @@ public static void Run() return; } - var directory = AttributeReader.GetProjectDirectory(VerifierSettings.Assembly); - CheckFiles(FindSnapshotFiles(directory), trackedVerifiedFiles!, directory); + var assembly = VerifierSettings.Assembly; + var directory = AttributeReader.GetProjectDirectory(assembly); + var (tracked, frameworksCovered) = MergeWithOtherFrameworks(assembly); + CheckFiles(FindSnapshotFiles(directory), tracked, directory, frameworksCovered); } internal static IEnumerable FindSnapshotFiles(string directory) => Directory.EnumerateFiles(directory, "*.verified.*", SearchOption.AllDirectories); - internal static void CheckFiles(IEnumerable filesOnDisk, ConcurrentBag trackedFiles, string directory) + /// + /// Publishes what this run tracked, then reads back what the runs of the other target frameworks + /// tracked. When every framework is accounted for, the union is the complete set of verified + /// files the project owns, and a file outside it is dangling whatever its name says. + /// + static (IReadOnlyCollection tracked, bool frameworksCovered) MergeWithOtherFrameworks(Assembly assembly) + { + var tracked = trackedVerifiedFiles!; + var directory = VerifierSettings.DanglingDir; + var targetFramework = VerifierSettings.TargetFramework; + if (directory is null || + targetFramework is null) + { + // The project does not consume Verify's build props, so there is nowhere to put a + // manifest and no framework list to check it against. + return (tracked, false); + } + + try + { + DanglingManifest.Write(directory, targetFramework, tracked); + return DanglingManifest.Read( + directory, + VerifierSettings.TargetFrameworks, + targetFramework, + AssemblyWriteTime(assembly)); + } + catch (Exception exception) + when (exception is IOException or UnauthorizedAccessException) + { + // The manifest is an optimization: without it the check still runs, it just falls back + // to skipping names that look like they belong to another framework. Failing to read or + // write one must not fail the test run. + return (tracked, false); + } + } + + /// + /// A manifest older than the assembly running it is from before the last build, so whatever it + /// records may since have been renamed or deleted. Unknown timestamp means no filtering, which + /// is what a single file or in memory assembly gets. + /// + static DateTime AssemblyWriteTime(Assembly assembly) + { + var location = assembly.Location; + if (location.Length == 0) + { + return DateTime.MinValue; + } + + try + { + return File.GetLastWriteTimeUtc(location); + } + catch (Exception exception) + when (exception is IOException or UnauthorizedAccessException) + { + return DateTime.MinValue; + } + } + + internal static void CheckFiles( + IEnumerable filesOnDisk, + IReadOnlyCollection trackedFiles, + string directory, + bool frameworksCovered) { static void AppendItems(StringBuilder builder, List list, string title) { @@ -60,7 +127,13 @@ static void AppendItems(StringBuilder builder, List list, string title) var suffix = file[directory.Length..]; suffix = suffix.TrimStart(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); - if (IfFileUnique(suffix)) + if (IsPlatformUnique(suffix)) + { + continue; + } + + if (!frameworksCovered && + IsFrameworkUnique(suffix)) { continue; } @@ -92,10 +165,22 @@ static void AppendItems(StringBuilder builder, List list, string title) throw new(builder.ToString()); } - static bool IfFileUnique(string file) => + /// + /// The runtime and target framework axes. A run of one framework never writes the files of + /// another, so without a manifest from every framework these have to be left alone. With one, + /// they are decided by the union instead and this is not consulted. + /// + static bool IsFrameworkUnique(string file) => file.Contains(".Net") || file.Contains(".DotNet") || - file.Contains(".Mono.") || + file.Contains(".Mono."); + + /// + /// The OS axis. Manifests cannot settle this one: the runs that produce these files are on + /// other machines, with their own intermediate directories, so their manifests are never + /// visible here. + /// + static bool IsPlatformUnique(string file) => file.Contains(".OSX.") || file.Contains(".Windows.") || file.Contains(".Linux."); @@ -108,4 +193,4 @@ internal static void Init() trackedVerifiedFiles = []; } } -} \ No newline at end of file +} diff --git a/src/Verify/DerivePaths/AttributeReader.cs b/src/Verify/DerivePaths/AttributeReader.cs index d89f5fe1e3..e68f034b9b 100644 --- a/src/Verify/DerivePaths/AttributeReader.cs +++ b/src/Verify/DerivePaths/AttributeReader.cs @@ -14,6 +14,19 @@ public static bool TryGetTargetFrameworks([NotNullWhen(true)] out string? target public static bool TryGetTargetFrameworks(Assembly assembly, [NotNullWhen(true)] out string? targetFrameworks) => TryGetValue(assembly, "Verify.TargetFrameworks", out targetFrameworks); + /// + /// The single target framework this assembly was built for, as written in the project file. + /// + public static bool TryGetTargetFramework(Assembly assembly, [NotNullWhen(true)] out string? targetFramework) => + TryGetValue(assembly, "Verify.TargetFramework", out targetFramework); + + /// + /// The directory holding the dangling snapshot manifests, shared by every target framework of + /// the project and scoped to the build configuration. + /// + internal static bool TryGetDanglingDirectory(Assembly assembly, [NotNullWhen(true)] out string? danglingDirectory) => + TryGetValue(assembly, "Verify.DanglingDirectory", out danglingDirectory); + public static string GetProjectDirectory() => GetProjectDirectory(Assembly.GetCallingAssembly()); diff --git a/src/Verify/VerifierSettings_TargetAssembly.cs b/src/Verify/VerifierSettings_TargetAssembly.cs index b1135aa90e..7780dfa184 100644 --- a/src/Verify/VerifierSettings_TargetAssembly.cs +++ b/src/Verify/VerifierSettings_TargetAssembly.cs @@ -16,6 +16,24 @@ public static partial class VerifierSettings internal static bool TargetsMultipleFramework { get; private set; } = true; + /// + /// Every target framework of the project, as written in the project file. Empty when the project + /// does not consume Verify's build props, or targets a single framework. + /// + internal static IReadOnlyList TargetFrameworks { get; private set; } = []; + + /// + /// The single target framework this assembly was built for. Null when the project does not + /// consume Verify's build props. + /// + internal static string? TargetFramework { get; private set; } + + /// + /// The directory holding the dangling snapshot manifests. Null when the project does not consume + /// Verify's build props, in which case no manifests are written or read. + /// + internal static string? DanglingDir { get; private set; } + [Experimental("VerifierSettingsTestAssembly")] public static Assembly Assembly { @@ -56,8 +74,18 @@ public static void AssignTargetAssembly(Assembly assembly) if (AttributeReader.TryGetTargetFrameworks(assembly, out var targetFrameworks)) { TargetsMultipleFramework = targetFrameworks.Contains(';'); + TargetFrameworks = targetFrameworks + .Split(';', StringSplitOptions.RemoveEmptyEntries) + .Select(_ => _.Trim()) + .Where(_ => _.Length > 0) + .ToList(); } + AttributeReader.TryGetTargetFramework(assembly, out var targetFramework); + TargetFramework = targetFramework; + AttributeReader.TryGetDanglingDirectory(assembly, out var danglingDir); + DanglingDir = danglingDir; + DirectoryReplacements.UseAssembly(solutionDir, ProjectDir); VerifierSettings.assembly = assembly; } diff --git a/src/Verify/buildTransitive/Verify.props b/src/Verify/buildTransitive/Verify.props index 2ac2d64015..02ff56cccb 100644 --- a/src/Verify/buildTransitive/Verify.props +++ b/src/Verify/buildTransitive/Verify.props @@ -108,12 +108,25 @@ $([System.IO.Path]::Combine('$(MSBuildProjectDirectory)', '$(IntermediateOutputPath)')) + + $([System.IO.Path]::Combine('$(MSBuildProjectDirectory)', '$(BaseIntermediateOutputPath)', 'VerifyDangling', '$(Configuration)')) <_Parameter1>Verify.TargetFrameworks <_Parameter2>$(TargetFrameworks) + + <_Parameter1>Verify.TargetFramework + <_Parameter2>$(TargetFramework) + + + <_Parameter1>Verify.DanglingDirectory + <_Parameter2>$(VerifyDanglingDirectory) + <_Parameter1>Verify.IntermediateDirectory <_Parameter2>$(VerifyIntermediateDirectory) From 00b399a7d43f050ab7c955e01fb61dc65ee26e60 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Fri, 18 Sep 2026 15:42:06 +1000 Subject: [PATCH 3/7] Identify snapshots by the test that owns them The check compared files, so every question it could not answer from a file name it could not answer at all. It now records, per verification, the verified path up to the end of the type and method, before parameters, uniqueness and index. That is the test's identity, and it is recorded from the constructor, so a test counts as existing even when it produced no file: an inline snapshot, a verification that threw, or one whose targets were all excluded. A file the run did not write now falls into one of two cases. No recorded prefix owns a name starting where it does. The set of tests in an assembly does not vary by framework, OS or architecture, so nothing produces it and no uniqueness in its name can excuse it. Reported unconditionally, and without needing a manifest, which is what finally makes SomeTests.Deleted.DotNet9_0.verified.txt visible. A prefix does own it, so it is a variant of a live test, and what follows the prefix decides it. The segments between the prefix and the verified marker are compared whole against the values Namer produces for this run. A different OS or architecture belongs to another machine and is left alone; a different target framework is left alone only until the manifests cover every framework; anything else is not uniqueness, so the file is reported. Whole segment matching replaces the substring list outright, so Tests.NetworkClient is a test rather than a Net uniqueness segment, a directory named Shared.NetCore no longer hides its contents, and .arm64 stops being reported as dangling. The architecture names are listed rather than reflected off the Architecture enum, which grew over time: reflecting it made recognition depend on the runtime doing the reading, so a .NET Framework run did not recognise an s390x snapshot. Prefixes go in the manifest alongside the files. A test behind an #if NET48 exists only in the net48 run, and without its prefix in the union every other run would report its snapshots. Degrades safely: with no prefixes recorded at all there is no identity to reason from, and nothing is reported. Also enables the three DanglingSnapshots usage projects in CI, which were built but never run, so the check has end to end coverage of the adapter wiring, module initializer, teardown hook, manifest and scan. Making them pass surfaced two pre-existing faults and cost their deliberate fixtures: * The MSTest usage project had no [UsesVerify], so every verification failed with "TestContext is null", and DanglingSnapshots.Run() then failed again on VerifierSettings.Assembly being unset. Run() now returns when no verification ran, rather than burying the real failure under a second one. Applied to the class rather than the assembly: an assembly wide [UsesVerify] also reaches the static Cleanup class, where the generated TestContext property does not compile. * The deliberately dangling and mis-cased snapshots in all three projects are removed, since a project that fails by design cannot run in CI. Detection is covered by the unit tests instead. --- docs/dangling-files.md | 27 +- docs/mdsource/dangling-files.source.md | 27 +- ...d.txt => Tests.IncorrectCase.verified.txt} | 0 src/DanglingSnapshotsMSTestUsage/Tests.cs | 7 +- .../Tests.IncorrectCase.verified.txt} | 0 .../Tests.Incorrectcase.verified.txt | 1 - .../Tests.Dangling.verified.txt | 1 - .../Tests.IncorrectCase.verified.txt} | 0 .../Tests.Incorrectcase.verified.txt | 1 - ...tiredFrameworkSurviveTheUnion.verified.txt | 11 + ...erageOnlyTheOrphanIsReported.verified.txt} | 0 src/Verify.Tests/DanglingManifestTests.cs | 209 ++++++++++----- ...sCheckTests.LongestPrefixWins.verified.txt | 16 ++ ...CheckTests.NoPrefixesRecorded.verified.txt | 4 + ...ts.OrphanedNamesContainingNet.verified.txt | 11 +- ...toryContainingUniquenessToken.verified.txt | 3 + ...hanedWithIndexesAndParameters.verified.txt | 2 + ...kTests.OrphanedWithUniqueness.verified.txt | 11 + ...ts.PrefixIsNotASubstringMatch.verified.txt | 17 ++ ...kTests.PrefixMatchIgnoresCase.verified.txt | 9 + ...sts.RetiredFrameworkSnapshots.verified.txt | 2 +- ...Tests.UniqueDirectoryPrefixes.verified.txt | 15 ++ ...ests.UniquenessFromAnotherRun.verified.txt | 2 - ...sCheckTests.UseFileNamePrefix.verified.txt | 16 ++ .../DanglingSnapshotsCheckTests.cs | 252 +++++++++++++----- src/Verify.Tests/UniquenessSegmentsTests.cs | 186 +++++++++++++ .../ConventionCheck/DanglingManifest.cs | 78 ++++-- .../ConventionCheck/DanglingSnapshotsCheck.cs | 123 ++++++--- .../ConventionCheck/UniquenessSegments.cs | 217 +++++++++++++++ src/Verify/Verifier/InnerVerifier.cs | 8 + src/Verify/VerifierSettings_TargetAssembly.cs | 6 + src/appveyor.yml | 9 +- 32 files changed, 1050 insertions(+), 221 deletions(-) rename src/DanglingSnapshotsMSTestUsage/{Tests.Dangling.verified.txt => Tests.IncorrectCase.verified.txt} (100%) rename src/{DanglingSnapshotsMSTestUsage/Tests.Incorrectcase.verified.txt => DanglingSnapshotsNUnitUsage/Tests.IncorrectCase.verified.txt} (100%) delete mode 100644 src/DanglingSnapshotsNUnitUsage/Tests.Incorrectcase.verified.txt delete mode 100644 src/DanglingSnapshotsXunitV3Usage/Tests.Dangling.verified.txt rename src/{DanglingSnapshotsNUnitUsage/Tests.Dangling.verified.txt => DanglingSnapshotsXunitV3Usage/Tests.IncorrectCase.verified.txt} (100%) delete mode 100644 src/DanglingSnapshotsXunitV3Usage/Tests.Incorrectcase.verified.txt create mode 100644 src/Verify.Tests/DanglingManifestTests.OrphanAndRetiredFrameworkSurviveTheUnion.verified.txt rename src/Verify.Tests/{DanglingManifestTests.OrphanSurvivesTheUnion.verified.txt => DanglingManifestTests.WithoutCoverageOnlyTheOrphanIsReported.verified.txt} (100%) create mode 100644 src/Verify.Tests/DanglingSnapshotsCheckTests.LongestPrefixWins.verified.txt create mode 100644 src/Verify.Tests/DanglingSnapshotsCheckTests.NoPrefixesRecorded.verified.txt create mode 100644 src/Verify.Tests/DanglingSnapshotsCheckTests.PrefixIsNotASubstringMatch.verified.txt create mode 100644 src/Verify.Tests/DanglingSnapshotsCheckTests.PrefixMatchIgnoresCase.verified.txt create mode 100644 src/Verify.Tests/DanglingSnapshotsCheckTests.UniqueDirectoryPrefixes.verified.txt create mode 100644 src/Verify.Tests/DanglingSnapshotsCheckTests.UseFileNamePrefix.verified.txt create mode 100644 src/Verify.Tests/UniquenessSegmentsTests.cs create mode 100644 src/Verify/ConventionCheck/UniquenessSegments.cs diff --git a/docs/dangling-files.md b/docs/dangling-files.md index f960333e9d..bf9549e783 100644 --- a/docs/dangling-files.md +++ b/docs/dangling-files.md @@ -12,27 +12,40 @@ A dangling snapshot file are when a `.verified.` file exist with no correspondin ## How dangling snapshot checks works - * When each test is executed, any snapshots produced are recorded. - * After all tests are executed, all recorded snapshots are checked against the snapshots that exist on disk + * When each test is executed, two things are recorded: the snapshots it produced, and the name it produces them under, up to the end of the type and method. The name is recorded even when the test produces no snapshot at all, so an inline test, or one that threw, still counts as existing. + * After all tests are executed, the snapshots on disk are checked against both. * An exception is thrown if any files: * exist on disk and do not have a corresponding recorded test * have casing that does not match the test case +## Which run a snapshot belongs to + +A snapshot file the current run did not produce is not necessarily redundant. It can belong to another target framework, another OS, or another architecture, none of which are running. The two recorded halves answer different parts of that question. + +A file whose name starts with no recorded test name belongs to no run at all. The set of tests in an assembly does not vary by framework, OS or architecture, so a snapshot no test claims is redundant however much uniqueness its name carries. `SomeTests.Deleted.DotNet9_0.verified.txt` is reported as readily as `SomeTests.Deleted.verified.txt`. + +A file whose name does start with a recorded test name is a variant of a live test, and what follows that name decides it. Everything between the test name and the `.verified.` marker is split into segments and each is compared, whole, against the values [`Namer`](https://github.com/VerifyTests/Verify/blob/main/src/Verify/Naming/Namer.cs) produces for this run. A segment naming a different OS or architecture belongs to a run on another machine and is left alone. A segment naming a different target framework is covered below. Anything else is not uniqueness, so the file is reported. + +Matching whole segments is what separates a uniqueness segment from a name that merely starts the same way: `SomeTests.NetworkClient.verified.txt` is a test called `NetworkClient`, not a `Net` uniqueness segment. + + ## Multi targeted projects A multi targeted project runs its tests once per target framework, so most of the snapshots on disk during any one run belong to a framework that is not currently running. A `UniqueForRuntime` or `UniqueForTargetFramework` snapshot of a deleted test is indistinguishable, by name, from one that another framework still owns. -To tell them apart, each run records what it tracked to a manifest in the intermediate (obj) directory. The manifests are named after the target framework and share one directory across all frameworks of the project, so a run can read what the other runs tracked. Once every target framework has a manifest, the union of them is the complete set of snapshot files the project owns, and anything on disk outside that union is dangling regardless of what its name suggests. +To tell them apart, each run records what it tracked to a manifest in the intermediate (obj) directory. The manifests are named after the target framework and share one directory across all frameworks of the project, so a run can read what the other runs tracked. Once every target framework has a manifest, the union of them is the complete set of snapshot files the project owns, and a target framework segment no longer excuses a file: nothing produces it, so it is dangling. + +Both recorded halves go in the manifest. The test names matter across frameworks as much as the files do: a test behind an `#if NET48` exists only in the net48 run, and without its name in the union every other run would report its snapshots. -In a multi targeted run this means the check is at its most accurate on the last framework to run: the earlier runs cannot yet account for the frameworks still to come, and fall back to skipping names that look like they belong to another framework. +In a multi targeted run this means the check is at its most accurate on the last framework to run: the earlier runs cannot yet account for the frameworks still to come, and leave the target framework segments alone. Snapshots of deleted tests are reported by every run, since no manifest is needed to know that no test claims them. The manifests are scoped to the build configuration, and are ignored if they predate the assembly running the check, so a stale manifest cannot mask a dangling file. They live in obj and are removed by a clean. -Two axes cannot be settled this way, and snapshot names carrying them are always skipped: +Two axes cannot be settled this way: - * `UniqueForOSPlatform`, since the runs that produce those files are on other machines with their own intermediate directories. - * `UniqueForArchitecture` and `UniqueForAssemblyConfiguration`, which are not recognised as uniqueness at all and are reported as dangling if no run tracks them. + * `UniqueForOSPlatform` and `UniqueForArchitecture`, since the runs that produce those files are on other machines with their own intermediate directories. A segment naming an OS or architecture other than the current one is always left alone. + * `UniqueForAssemblyConfiguration`, which has no enumerable set of values and so is not recognised as uniqueness at all. A snapshot only another configuration produces is reported. ## Experimental diff --git a/docs/mdsource/dangling-files.source.md b/docs/mdsource/dangling-files.source.md index 76d226af36..4ba2d37ce5 100644 --- a/docs/mdsource/dangling-files.source.md +++ b/docs/mdsource/dangling-files.source.md @@ -5,27 +5,40 @@ A dangling snapshot file are when a `.verified.` file exist with no correspondin ## How dangling snapshot checks works - * When each test is executed, any snapshots produced are recorded. - * After all tests are executed, all recorded snapshots are checked against the snapshots that exist on disk + * When each test is executed, two things are recorded: the snapshots it produced, and the name it produces them under, up to the end of the type and method. The name is recorded even when the test produces no snapshot at all, so an inline test, or one that threw, still counts as existing. + * After all tests are executed, the snapshots on disk are checked against both. * An exception is thrown if any files: * exist on disk and do not have a corresponding recorded test * have casing that does not match the test case +## Which run a snapshot belongs to + +A snapshot file the current run did not produce is not necessarily redundant. It can belong to another target framework, another OS, or another architecture, none of which are running. The two recorded halves answer different parts of that question. + +A file whose name starts with no recorded test name belongs to no run at all. The set of tests in an assembly does not vary by framework, OS or architecture, so a snapshot no test claims is redundant however much uniqueness its name carries. `SomeTests.Deleted.DotNet9_0.verified.txt` is reported as readily as `SomeTests.Deleted.verified.txt`. + +A file whose name does start with a recorded test name is a variant of a live test, and what follows that name decides it. Everything between the test name and the `.verified.` marker is split into segments and each is compared, whole, against the values [`Namer`](https://github.com/VerifyTests/Verify/blob/main/src/Verify/Naming/Namer.cs) produces for this run. A segment naming a different OS or architecture belongs to a run on another machine and is left alone. A segment naming a different target framework is covered below. Anything else is not uniqueness, so the file is reported. + +Matching whole segments is what separates a uniqueness segment from a name that merely starts the same way: `SomeTests.NetworkClient.verified.txt` is a test called `NetworkClient`, not a `Net` uniqueness segment. + + ## Multi targeted projects A multi targeted project runs its tests once per target framework, so most of the snapshots on disk during any one run belong to a framework that is not currently running. A `UniqueForRuntime` or `UniqueForTargetFramework` snapshot of a deleted test is indistinguishable, by name, from one that another framework still owns. -To tell them apart, each run records what it tracked to a manifest in the intermediate (obj) directory. The manifests are named after the target framework and share one directory across all frameworks of the project, so a run can read what the other runs tracked. Once every target framework has a manifest, the union of them is the complete set of snapshot files the project owns, and anything on disk outside that union is dangling regardless of what its name suggests. +To tell them apart, each run records what it tracked to a manifest in the intermediate (obj) directory. The manifests are named after the target framework and share one directory across all frameworks of the project, so a run can read what the other runs tracked. Once every target framework has a manifest, the union of them is the complete set of snapshot files the project owns, and a target framework segment no longer excuses a file: nothing produces it, so it is dangling. + +Both recorded halves go in the manifest. The test names matter across frameworks as much as the files do: a test behind an `#if NET48` exists only in the net48 run, and without its name in the union every other run would report its snapshots. -In a multi targeted run this means the check is at its most accurate on the last framework to run: the earlier runs cannot yet account for the frameworks still to come, and fall back to skipping names that look like they belong to another framework. +In a multi targeted run this means the check is at its most accurate on the last framework to run: the earlier runs cannot yet account for the frameworks still to come, and leave the target framework segments alone. Snapshots of deleted tests are reported by every run, since no manifest is needed to know that no test claims them. The manifests are scoped to the build configuration, and are ignored if they predate the assembly running the check, so a stale manifest cannot mask a dangling file. They live in obj and are removed by a clean. -Two axes cannot be settled this way, and snapshot names carrying them are always skipped: +Two axes cannot be settled this way: - * `UniqueForOSPlatform`, since the runs that produce those files are on other machines with their own intermediate directories. - * `UniqueForArchitecture` and `UniqueForAssemblyConfiguration`, which are not recognised as uniqueness at all and are reported as dangling if no run tracks them. + * `UniqueForOSPlatform` and `UniqueForArchitecture`, since the runs that produce those files are on other machines with their own intermediate directories. A segment naming an OS or architecture other than the current one is always left alone. + * `UniqueForAssemblyConfiguration`, which has no enumerable set of values and so is not recognised as uniqueness at all. A snapshot only another configuration produces is reported. ## Experimental diff --git a/src/DanglingSnapshotsMSTestUsage/Tests.Dangling.verified.txt b/src/DanglingSnapshotsMSTestUsage/Tests.IncorrectCase.verified.txt similarity index 100% rename from src/DanglingSnapshotsMSTestUsage/Tests.Dangling.verified.txt rename to src/DanglingSnapshotsMSTestUsage/Tests.IncorrectCase.verified.txt diff --git a/src/DanglingSnapshotsMSTestUsage/Tests.cs b/src/DanglingSnapshotsMSTestUsage/Tests.cs index 17e5762e77..3cac29a0d1 100644 --- a/src/DanglingSnapshotsMSTestUsage/Tests.cs +++ b/src/DanglingSnapshotsMSTestUsage/Tests.cs @@ -1,4 +1,9 @@ -[TestClass] +// Without this the source generator never plumbs the TestContext through, and every verification +// fails with "TestContext is null" before it can produce a snapshot. Applied to the class rather +// than the assembly, since an assembly wide attribute also reaches the static Cleanup class, where +// the generated TestContext property does not compile. +[UsesVerify] +[TestClass] public partial class Tests { [TestMethod] diff --git a/src/DanglingSnapshotsMSTestUsage/Tests.Incorrectcase.verified.txt b/src/DanglingSnapshotsNUnitUsage/Tests.IncorrectCase.verified.txt similarity index 100% rename from src/DanglingSnapshotsMSTestUsage/Tests.Incorrectcase.verified.txt rename to src/DanglingSnapshotsNUnitUsage/Tests.IncorrectCase.verified.txt diff --git a/src/DanglingSnapshotsNUnitUsage/Tests.Incorrectcase.verified.txt b/src/DanglingSnapshotsNUnitUsage/Tests.Incorrectcase.verified.txt deleted file mode 100644 index fdf74cdc4b..0000000000 --- a/src/DanglingSnapshotsNUnitUsage/Tests.Incorrectcase.verified.txt +++ /dev/null @@ -1 +0,0 @@ -Foo \ No newline at end of file diff --git a/src/DanglingSnapshotsXunitV3Usage/Tests.Dangling.verified.txt b/src/DanglingSnapshotsXunitV3Usage/Tests.Dangling.verified.txt deleted file mode 100644 index fdf74cdc4b..0000000000 --- a/src/DanglingSnapshotsXunitV3Usage/Tests.Dangling.verified.txt +++ /dev/null @@ -1 +0,0 @@ -Foo \ No newline at end of file diff --git a/src/DanglingSnapshotsNUnitUsage/Tests.Dangling.verified.txt b/src/DanglingSnapshotsXunitV3Usage/Tests.IncorrectCase.verified.txt similarity index 100% rename from src/DanglingSnapshotsNUnitUsage/Tests.Dangling.verified.txt rename to src/DanglingSnapshotsXunitV3Usage/Tests.IncorrectCase.verified.txt diff --git a/src/DanglingSnapshotsXunitV3Usage/Tests.Incorrectcase.verified.txt b/src/DanglingSnapshotsXunitV3Usage/Tests.Incorrectcase.verified.txt deleted file mode 100644 index fdf74cdc4b..0000000000 --- a/src/DanglingSnapshotsXunitV3Usage/Tests.Incorrectcase.verified.txt +++ /dev/null @@ -1 +0,0 @@ -Foo \ No newline at end of file diff --git a/src/Verify.Tests/DanglingManifestTests.OrphanAndRetiredFrameworkSurviveTheUnion.verified.txt b/src/Verify.Tests/DanglingManifestTests.OrphanAndRetiredFrameworkSurviveTheUnion.verified.txt new file mode 100644 index 0000000000..d2ddea70b4 --- /dev/null +++ b/src/Verify.Tests/DanglingManifestTests.OrphanAndRetiredFrameworkSurviveTheUnion.verified.txt @@ -0,0 +1,11 @@ +{ + Type: Exception, + Message: +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Tests.Deleted.DotNet8_0.verified.txt + * Tests.Alive.Mono3_1.verified.txt + +} \ No newline at end of file diff --git a/src/Verify.Tests/DanglingManifestTests.OrphanSurvivesTheUnion.verified.txt b/src/Verify.Tests/DanglingManifestTests.WithoutCoverageOnlyTheOrphanIsReported.verified.txt similarity index 100% rename from src/Verify.Tests/DanglingManifestTests.OrphanSurvivesTheUnion.verified.txt rename to src/Verify.Tests/DanglingManifestTests.WithoutCoverageOnlyTheOrphanIsReported.verified.txt diff --git a/src/Verify.Tests/DanglingManifestTests.cs b/src/Verify.Tests/DanglingManifestTests.cs index efa89316dd..7e20e1e118 100644 --- a/src/Verify.Tests/DanglingManifestTests.cs +++ b/src/Verify.Tests/DanglingManifestTests.cs @@ -1,4 +1,4 @@ -#pragma warning disable VerifyDanglingSnapshots +#pragma warning disable VerifyDanglingSnapshots public class DanglingManifestTests { static readonly DateTime noStaleFiltering = DateTime.MinValue; @@ -7,12 +7,13 @@ public class DanglingManifestTests public void SingleFrameworkIsCoveredByItsOwnManifest() { using var directory = new TempDirectory(); - DanglingManifest.Write(directory, "net9.0", ["one.verified.txt", "two.verified.txt"]); + DanglingManifest.Write(directory, "net9.0", ["one.verified.txt", "two.verified.txt"], ["one", "two"]); - var (tracked, covered) = DanglingManifest.Read(directory, [], "net9.0", noStaleFiltering); + var merged = DanglingManifest.Read(directory, [], "net9.0", noStaleFiltering); - Assert.True(covered); - Assert.Equal(["one.verified.txt", "two.verified.txt"], tracked.Order()); + Assert.True(merged.FrameworksCovered); + Assert.Equal(["one.verified.txt", "two.verified.txt"], merged.Files.Order()); + Assert.Equal(["one", "two"], merged.Prefixes.Order()); } /// @@ -23,57 +24,77 @@ public void SingleFrameworkIsCoveredByItsOwnManifest() public void EveryFrameworkPresentIsCovered() { using var directory = new TempDirectory(); - DanglingManifest.Write(directory, "net8.0", ["Alive.DotNet8_0.verified.txt"]); - DanglingManifest.Write(directory, "net9.0", ["Alive.DotNet9_0.verified.txt"]); + DanglingManifest.Write(directory, "net8.0", ["Alive.DotNet8_0.verified.txt"], ["Alive"]); + DanglingManifest.Write(directory, "net9.0", ["Alive.DotNet9_0.verified.txt"], ["Alive"]); - var (tracked, covered) = DanglingManifest.Read(directory, ["net8.0", "net9.0"], "net9.0", noStaleFiltering); + var merged = DanglingManifest.Read(directory, ["net8.0", "net9.0"], "net9.0", noStaleFiltering); - Assert.True(covered); - Assert.Equal(["Alive.DotNet8_0.verified.txt", "Alive.DotNet9_0.verified.txt"], tracked.Order()); + Assert.True(merged.FrameworksCovered); + Assert.Equal(["Alive.DotNet8_0.verified.txt", "Alive.DotNet9_0.verified.txt"], merged.Files.Order()); + Assert.Equal(["Alive"], merged.Prefixes); + } + + /// + /// A test behind a #if exists only in the runs of the frameworks that compile it. Without + /// its prefix in the union, every other framework's run would see its snapshots as belonging to + /// no test at all, and report them. + /// + [Fact] + public void FrameworkSpecificTestIsKnownToTheOtherFrameworks() + { + using var directory = new TempDirectory(); + DanglingManifest.Write(directory, "net48", ["OnlyOnNet48.verified.txt"], ["OnlyOnNet48"]); + DanglingManifest.Write(directory, "net9.0", ["Shared.verified.txt"], ["Shared"]); + + var merged = DanglingManifest.Read(directory, ["net48", "net9.0"], "net9.0", noStaleFiltering); + + Assert.True(merged.FrameworksCovered); + Assert.Contains("OnlyOnNet48", merged.Prefixes); + Assert.Contains("OnlyOnNet48.verified.txt", merged.Files); } /// /// The first framework to run has only its own manifest, so it cannot account for the files of - /// the frameworks still to run and has to fall back. + /// the frameworks still to run and has to leave the framework axis to the name. /// [Fact] public void MissingFrameworkIsNotCovered() { using var directory = new TempDirectory(); - DanglingManifest.Write(directory, "net9.0", ["Alive.DotNet9_0.verified.txt"]); + DanglingManifest.Write(directory, "net9.0", ["Alive.DotNet9_0.verified.txt"], ["Alive"]); - var (tracked, covered) = DanglingManifest.Read(directory, ["net8.0", "net9.0"], "net9.0", noStaleFiltering); + var merged = DanglingManifest.Read(directory, ["net8.0", "net9.0"], "net9.0", noStaleFiltering); - Assert.False(covered); - Assert.Equal(["Alive.DotNet9_0.verified.txt"], tracked); + Assert.False(merged.FrameworksCovered); + Assert.Equal(["Alive.DotNet9_0.verified.txt"], merged.Files); } /// - /// A manifest naming a framework the project no longer targets is read for its paths, but says - /// nothing about coverage. + /// A manifest naming a framework the project no longer targets is read for its contents, but + /// says nothing about coverage. /// [Fact] public void RetiredFrameworkDoesNotSatisfyCoverage() { using var directory = new TempDirectory(); - DanglingManifest.Write(directory, "net7.0", ["Alive.DotNet7_0.verified.txt"]); - DanglingManifest.Write(directory, "net9.0", ["Alive.DotNet9_0.verified.txt"]); + DanglingManifest.Write(directory, "net7.0", ["Alive.DotNet7_0.verified.txt"], ["Alive"]); + DanglingManifest.Write(directory, "net9.0", ["Alive.DotNet9_0.verified.txt"], ["Alive"]); - var (_, covered) = DanglingManifest.Read(directory, ["net8.0", "net9.0"], "net9.0", noStaleFiltering); + var merged = DanglingManifest.Read(directory, ["net8.0", "net9.0"], "net9.0", noStaleFiltering); - Assert.False(covered); + Assert.False(merged.FrameworksCovered); } /// - /// A manifest from before the last build may name files that have since been renamed or - /// deleted, and counting those as tracked would hide exactly the danglers the check is for. + /// A manifest from before the last build may name tests and files that have since been renamed + /// or deleted, and counting those as tracked would hide exactly the danglers the check is for. /// [Fact] public void StaleManifestIsIgnored() { using var directory = new TempDirectory(); - DanglingManifest.Write(directory, "net8.0", ["Stale.DotNet8_0.verified.txt"]); - DanglingManifest.Write(directory, "net9.0", ["Alive.DotNet9_0.verified.txt"]); + DanglingManifest.Write(directory, "net8.0", ["Stale.DotNet8_0.verified.txt"], ["Stale"]); + DanglingManifest.Write(directory, "net9.0", ["Alive.DotNet9_0.verified.txt"], ["Alive"]); // Pinned rather than left to the clock: the two manifests are written milliseconds apart, // and file timestamp granularity is coarser than that on some file systems. @@ -85,41 +106,47 @@ public void StaleManifestIsIgnored() DanglingManifest.PathFor(directory, "net9.0"), buildTime.AddHours(1)); - var (tracked, covered) = DanglingManifest.Read(directory, ["net8.0", "net9.0"], "net9.0", buildTime); + var merged = DanglingManifest.Read(directory, ["net8.0", "net9.0"], "net9.0", buildTime); - Assert.False(covered); - Assert.Equal(["Alive.DotNet9_0.verified.txt"], tracked); + Assert.False(merged.FrameworksCovered); + Assert.Equal(["Alive.DotNet9_0.verified.txt"], merged.Files); + Assert.Equal(["Alive"], merged.Prefixes); } /// - /// A re run of one framework replaces its manifest rather than adding to it, so a file that run - /// no longer tracks stops being tracked. + /// A re run of one framework replaces its manifest rather than adding to it, so a file or test + /// that run no longer tracks stops being tracked. /// [Fact] public void ReRunReplacesManifest() { using var directory = new TempDirectory(); - DanglingManifest.Write(directory, "net9.0", ["Before.verified.txt"]); - DanglingManifest.Write(directory, "net9.0", ["After.verified.txt"]); + DanglingManifest.Write(directory, "net9.0", ["Before.verified.txt"], ["Before"]); + DanglingManifest.Write(directory, "net9.0", ["After.verified.txt"], ["After"]); - var (tracked, _) = DanglingManifest.Read(directory, [], "net9.0", noStaleFiltering); + var merged = DanglingManifest.Read(directory, [], "net9.0", noStaleFiltering); - Assert.Equal(["After.verified.txt"], tracked); + Assert.Equal(["After.verified.txt"], merged.Files); + Assert.Equal(["After"], merged.Prefixes); } /// - /// Several verifications can resolve to one verified path, so the bag feeding the manifest can - /// hold duplicates. + /// Several verifications can resolve to one verified path, and every case of a parameterised + /// test records the same prefix, so both bags hold duplicates. /// [Fact] public void DuplicatesAreWrittenOnce() { using var directory = new TempDirectory(); - DanglingManifest.Write(directory, "net9.0", ["Alive.verified.txt", "Alive.verified.txt"]); + DanglingManifest.Write( + directory, + "net9.0", + ["Alive.verified.txt", "Alive.verified.txt"], + ["Alive", "Alive", "Alive"]); var lines = File.ReadAllLines(DanglingManifest.PathFor(directory, "net9.0")); - Assert.Equal(["Alive.verified.txt"], lines); + Assert.Equal(["[files]", "Alive.verified.txt", "[prefixes]", "Alive"], lines); } [Fact] @@ -128,22 +155,40 @@ public void NoDirectoryIsNotCovered() using var directory = new TempDirectory(); var missing = directory.BuildPath("absent"); - var (tracked, covered) = DanglingManifest.Read(missing, ["net9.0"], "net9.0", noStaleFiltering); + var merged = DanglingManifest.Read(missing, ["net9.0"], "net9.0", noStaleFiltering); - Assert.False(covered); - Assert.Empty(tracked); + Assert.False(merged.FrameworksCovered); + Assert.Empty(merged.Files); + Assert.Empty(merged.Prefixes); } [Fact] public void EmptyManifestIsStillCoverage() { using var directory = new TempDirectory(); - DanglingManifest.Write(directory, "net9.0", []); + DanglingManifest.Write(directory, "net9.0", [], []); + + var merged = DanglingManifest.Read(directory, ["net9.0"], "net9.0", noStaleFiltering); + + Assert.True(merged.FrameworksCovered); + Assert.Empty(merged.Files); + Assert.Empty(merged.Prefixes); + } + + /// + /// A test that produced no file at all - inline, excluded targets, or one that threw - records a + /// prefix and nothing else. That is the case the prefix half exists for. + /// + [Fact] + public void PrefixWithoutFiles() + { + using var directory = new TempDirectory(); + DanglingManifest.Write(directory, "net9.0", [], ["Inline"]); - var (tracked, covered) = DanglingManifest.Read(directory, ["net9.0"], "net9.0", noStaleFiltering); + var merged = DanglingManifest.Read(directory, [], "net9.0", noStaleFiltering); - Assert.True(covered); - Assert.Empty(tracked); + Assert.Empty(merged.Files); + Assert.Equal(["Inline"], merged.Prefixes); } /// @@ -154,17 +199,24 @@ public void EmptyManifestIsStillCoverage() public void AwkwardPathsRoundTrip() { using var directory = new TempDirectory(); - string[] paths = + string[] files = [ @"D:\a b\Tests.Method_param=a b.DotNet9_0#00.verified.txt", "/home/a b/Tests.Method#name.verified.txt", @"D:\a\Tests.Method_param=Ünïcödé.verified.txt" ]; - DanglingManifest.Write(directory, "net9.0", paths); + string[] prefixes = + [ + @"D:\a b\Tests.Method", + "/home/a b/Tests.Method", + @"D:\a\[files]Tests.Method" + ]; + DanglingManifest.Write(directory, "net9.0", files, prefixes); - var (tracked, _) = DanglingManifest.Read(directory, [], "net9.0", noStaleFiltering); + var merged = DanglingManifest.Read(directory, [], "net9.0", noStaleFiltering); - Assert.Equal(paths.Order(), tracked.Order()); + Assert.Equal(files.Order(), merged.Files.Order()); + Assert.Equal(prefixes.Order(), merged.Prefixes.Order()); } /// @@ -175,7 +227,7 @@ public void AwkwardPathsRoundTrip() public void WriteLeavesNoTemporaryFiles() { using var directory = new TempDirectory(); - DanglingManifest.Write(directory, "net9.0", ["Alive.verified.txt"]); + DanglingManifest.Write(directory, "net9.0", ["Alive.verified.txt"], ["Alive"]); var files = Directory.GetFiles(directory) .Select(Path.GetFileName) @@ -185,50 +237,69 @@ public void WriteLeavesNoTemporaryFiles() } /// - /// End to end: two frameworks, one snapshot each, and one left behind by a deleted test. Only - /// the union can tell them apart, and only when it is complete. + /// End to end: two frameworks, one snapshot each, one left behind by a test that was deleted, + /// and one left behind by a framework that was dropped. The deleted test's file is reported + /// either way, since no prefix owns it. The retired framework's file needs the complete union: + /// its test is still there, so only the absence of a run that claims it says anything. /// [Fact] - public Task OrphanSurvivesTheUnion() + public Task OrphanAndRetiredFrameworkSurviveTheUnion() { using var directory = new TempDirectory(); var root = directory.Path; var aliveOnNet8 = Path.Combine(root, "Tests.Alive.DotNet8_0.verified.txt"); var aliveOnNet9 = Path.Combine(root, "Tests.Alive.DotNet9_0.verified.txt"); - var orphan = Path.Combine(root, "Tests.Deleted.DotNet8_0.verified.txt"); + var deletedTest = Path.Combine(root, "Tests.Deleted.DotNet8_0.verified.txt"); + // A runtime no run of this project produces, so the verdict is the same on every target + // framework and the snapshot can be shared between them. + var retiredFramework = Path.Combine(root, "Tests.Alive.Mono3_1.verified.txt"); + var alivePrefix = Path.Combine(root, "Tests.Alive"); - DanglingManifest.Write(directory, "net8.0", [aliveOnNet8]); - DanglingManifest.Write(directory, "net9.0", [aliveOnNet9]); + DanglingManifest.Write(directory, "net8.0", [aliveOnNet8], [alivePrefix]); + DanglingManifest.Write(directory, "net9.0", [aliveOnNet9], [alivePrefix]); - var (tracked, covered) = DanglingManifest.Read(directory, ["net8.0", "net9.0"], "net9.0", noStaleFiltering); + var merged = DanglingManifest.Read(directory, ["net8.0", "net9.0"], "net9.0", noStaleFiltering); - Assert.True(covered); + Assert.True(merged.FrameworksCovered); return Throws( () => DanglingSnapshotsCheck.CheckFiles( - [aliveOnNet8, aliveOnNet9, orphan], - tracked, + [aliveOnNet8, aliveOnNet9, deletedTest, retiredFramework], + merged.Files, + merged.Prefixes, root, - covered)) + merged.FrameworksCovered)) .IgnoreStackTrace(); } /// - /// The same inputs without full coverage. The orphan carries a framework segment, so the - /// fallback skips it and reports nothing. + /// The same inputs without full coverage. The deleted test's snapshot is still reported, because + /// no prefix owns it whatever framework it names. The retired framework's one is not: its test + /// is alive, and without every manifest there is no way to know no run produces it. /// [Fact] - public void OrphanIsInvisibleWithoutCoverage() + public Task WithoutCoverageOnlyTheOrphanIsReported() { using var directory = new TempDirectory(); var root = directory.Path; var aliveOnNet9 = Path.Combine(root, "Tests.Alive.DotNet9_0.verified.txt"); - var orphan = Path.Combine(root, "Tests.Deleted.DotNet8_0.verified.txt"); + var deletedTest = Path.Combine(root, "Tests.Deleted.DotNet8_0.verified.txt"); + // A runtime no run of this project produces, so the verdict is the same on every target + // framework and the snapshot can be shared between them. + var retiredFramework = Path.Combine(root, "Tests.Alive.Mono3_1.verified.txt"); + var alivePrefix = Path.Combine(root, "Tests.Alive"); - DanglingManifest.Write(directory, "net9.0", [aliveOnNet9]); + DanglingManifest.Write(directory, "net9.0", [aliveOnNet9], [alivePrefix]); - var (tracked, covered) = DanglingManifest.Read(directory, ["net8.0", "net9.0"], "net9.0", noStaleFiltering); + var merged = DanglingManifest.Read(directory, ["net8.0", "net9.0"], "net9.0", noStaleFiltering); - Assert.False(covered); - DanglingSnapshotsCheck.CheckFiles([aliveOnNet9, orphan], tracked, root, covered); + Assert.False(merged.FrameworksCovered); + return Throws( + () => DanglingSnapshotsCheck.CheckFiles( + [aliveOnNet9, deletedTest, retiredFramework], + merged.Files, + merged.Prefixes, + root, + merged.FrameworksCovered)) + .IgnoreStackTrace(); } } diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.LongestPrefixWins.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.LongestPrefixWins.verified.txt new file mode 100644 index 0000000000..20c95507ab --- /dev/null +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.LongestPrefixWins.verified.txt @@ -0,0 +1,16 @@ +== Frameworks not covered == + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Tests.Method.Nested.verified.txt + +== Frameworks covered == + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Tests.Method.Nested.verified.txt + * Tests.Method.Nested.Mono3_1.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.NoPrefixesRecorded.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.NoPrefixesRecorded.verified.txt new file mode 100644 index 0000000000..cd471dbf3d --- /dev/null +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.NoPrefixesRecorded.verified.txt @@ -0,0 +1,4 @@ +== Frameworks not covered == +Nothing reported. +== Frameworks covered == +Nothing reported. \ No newline at end of file diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedNamesContainingNet.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedNamesContainingNet.verified.txt index 7a8879ea22..4b0529a1da 100644 --- a/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedNamesContainingNet.verified.txt +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedNamesContainingNet.verified.txt @@ -1,5 +1,14 @@ == Frameworks not covered == -Nothing reported. + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Tests.NetworkClient.verified.txt + * Tests.NetCoreShim.verified.txt + * Tests.Nettle.verified.txt + * HttpTests.NetworkFailure.verified.txt + == Frameworks covered == Verify has detected the following issues with snapshot files: diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedUnderDirectoryContainingUniquenessToken.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedUnderDirectoryContainingUniquenessToken.verified.txt index 4cefaf5b17..b9769bd09f 100644 --- a/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedUnderDirectoryContainingUniquenessToken.verified.txt +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedUnderDirectoryContainingUniquenessToken.verified.txt @@ -4,6 +4,8 @@ Verify has detected the following issues with snapshot files: The following files have not been tracked: + * Shared.NetCore/Deleted.verified.txt + * Snapshots.Windows.Only/Deleted.verified.txt * Plain/Deleted.verified.txt == Frameworks covered == @@ -13,4 +15,5 @@ Verify has detected the following issues with snapshot files: The following files have not been tracked: * Shared.NetCore/Deleted.verified.txt + * Snapshots.Windows.Only/Deleted.verified.txt * Plain/Deleted.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithIndexesAndParameters.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithIndexesAndParameters.verified.txt index 9db089e1ef..3615ed54b9 100644 --- a/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithIndexesAndParameters.verified.txt +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithIndexesAndParameters.verified.txt @@ -7,6 +7,8 @@ The following files have not been tracked: * Deleted#00.verified.txt * Deleted#name.verified.txt * Deleted_param=value.verified.txt + * Deleted_param=value.DotNet9_0#00.verified.txt + * Deleted.DotNet9_0#00.verified.txt == Frameworks covered == diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithUniqueness.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithUniqueness.verified.txt index 7f77eb2ef0..45e4b1aa05 100644 --- a/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithUniqueness.verified.txt +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.OrphanedWithUniqueness.verified.txt @@ -5,7 +5,15 @@ Verify has detected the following issues with snapshot files: The following files have not been tracked: * Deleted.verified.txt + * Deleted.DotNet.verified.txt + * Deleted.DotNet9_0.verified.txt + * Deleted.Net.verified.txt + * Deleted.Net4_8.verified.txt + * Deleted.Mono.verified.txt * Deleted.Mono6_12.verified.txt + * Deleted.Windows.verified.txt + * Deleted.Linux.verified.txt + * Deleted.OSX.verified.txt * Deleted.Android.verified.txt * Deleted.IOS.verified.txt * Deleted.x64.verified.txt @@ -26,6 +34,9 @@ The following files have not been tracked: * Deleted.Net4_8.verified.txt * Deleted.Mono.verified.txt * Deleted.Mono6_12.verified.txt + * Deleted.Windows.verified.txt + * Deleted.Linux.verified.txt + * Deleted.OSX.verified.txt * Deleted.Android.verified.txt * Deleted.IOS.verified.txt * Deleted.x64.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.PrefixIsNotASubstringMatch.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.PrefixIsNotASubstringMatch.verified.txt new file mode 100644 index 0000000000..f6a23380be --- /dev/null +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.PrefixIsNotASubstringMatch.verified.txt @@ -0,0 +1,17 @@ +== Frameworks not covered == + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * AliveToo.verified.txt + * AliveToo.Mono3_1.verified.txt + +== Frameworks covered == + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * AliveToo.verified.txt + * AliveToo.Mono3_1.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.PrefixMatchIgnoresCase.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.PrefixMatchIgnoresCase.verified.txt new file mode 100644 index 0000000000..de4984a47c --- /dev/null +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.PrefixMatchIgnoresCase.verified.txt @@ -0,0 +1,9 @@ +== Frameworks not covered == +Nothing reported. +== Frameworks covered == + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Nested/Alive.Mono4_2.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.RetiredFrameworkSnapshots.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.RetiredFrameworkSnapshots.verified.txt index 74aa46b481..656b7263cb 100644 --- a/src/Verify.Tests/DanglingSnapshotsCheckTests.RetiredFrameworkSnapshots.verified.txt +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.RetiredFrameworkSnapshots.verified.txt @@ -6,4 +6,4 @@ Verify has detected the following issues with snapshot files: The following files have not been tracked: - * Alive.DotNet8_0.verified.txt + * Alive.Mono4_2.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.UniqueDirectoryPrefixes.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.UniqueDirectoryPrefixes.verified.txt new file mode 100644 index 0000000000..9896216ff2 --- /dev/null +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.UniqueDirectoryPrefixes.verified.txt @@ -0,0 +1,15 @@ +== Frameworks not covered == + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Deleted.Mono3_1/target.verified.txt + +== Frameworks covered == + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Deleted.Mono3_1/target.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.UniquenessFromAnotherRun.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.UniquenessFromAnotherRun.verified.txt index 79dd0afb04..3425df79bc 100644 --- a/src/Verify.Tests/DanglingSnapshotsCheckTests.UniquenessFromAnotherRun.verified.txt +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.UniquenessFromAnotherRun.verified.txt @@ -4,7 +4,6 @@ Verify has detected the following issues with snapshot files: The following files have not been tracked: - * Alive.arm64.verified.txt * Alive.Debug.verified.txt == Frameworks covered == @@ -13,5 +12,4 @@ Verify has detected the following issues with snapshot files: The following files have not been tracked: - * Alive.arm64.verified.txt * Alive.Debug.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.UseFileNamePrefix.verified.txt b/src/Verify.Tests/DanglingSnapshotsCheckTests.UseFileNamePrefix.verified.txt new file mode 100644 index 0000000000..d9731af288 --- /dev/null +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.UseFileNamePrefix.verified.txt @@ -0,0 +1,16 @@ +== Frameworks not covered == + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Unpinned.Mono3_1.verified.txt + +== Frameworks covered == + +Verify has detected the following issues with snapshot files: + +The following files have not been tracked: + + * Pinned.Mono3_1.verified.txt + * Unpinned.Mono3_1.verified.txt diff --git a/src/Verify.Tests/DanglingSnapshotsCheckTests.cs b/src/Verify.Tests/DanglingSnapshotsCheckTests.cs index 5f8559f183..c96f4795e9 100644 --- a/src/Verify.Tests/DanglingSnapshotsCheckTests.cs +++ b/src/Verify.Tests/DanglingSnapshotsCheckTests.cs @@ -1,8 +1,19 @@ -#pragma warning disable VerifyDanglingSnapshots +#pragma warning disable VerifyDanglingSnapshots public class DanglingSnapshotsCheckTests { const string root = "path/to"; + // Runtimes and architectures no run of this project produces, so the classifier reaches the same + // verdict on every target framework and machine and the snapshots can be shared between them. + // A test that hard coded, say, DotNet8_0 would report it as belonging to another run everywhere + // except the net8.0 run, which owns it. + const string foreignRuntime = "Mono3_1"; + const string otherForeignRuntime = "Mono4_2"; + const string foreignArchitecture = "s390x"; + + static readonly string[] alive = ["path/to/Alive.verified.txt"]; + static readonly string[] alivePrefix = ["path/to/Alive"]; + [Fact] public Task Untracked() { @@ -15,7 +26,7 @@ public Task Untracked() "path/to/tracked.verified.txt" }; - return Throws(() => DanglingSnapshotsCheck.CheckFiles(filesOnDisk, trackedFiles, root, false)) + return Throws(() => DanglingSnapshotsCheck.CheckFiles(filesOnDisk, trackedFiles, ["path/to/tracked"], root, false)) .IgnoreStackTrace(); } @@ -31,7 +42,7 @@ public Task IncorrectCase() "path/to/tracked.verified.txt" }; - return Throws(() => DanglingSnapshotsCheck.CheckFiles(filesOnDisk, trackedFiles, root, false)) + return Throws(() => DanglingSnapshotsCheck.CheckFiles(filesOnDisk, trackedFiles, ["path/to/tracked"], root, false)) .IgnoreStackTrace(); } @@ -41,15 +52,14 @@ public void AllTracked() var filesOnDisk = new List { "path/to/tracked.verified.txt" }; var trackedFiles = new ConcurrentBag { "path/to/tracked.verified.txt" }; - DanglingSnapshotsCheck.CheckFiles(filesOnDisk, trackedFiles, root, false); + DanglingSnapshotsCheck.CheckFiles(filesOnDisk, trackedFiles, ["path/to/tracked"], root, false); } /// - /// Every file here is a snapshot for a test that no longer exists: nothing shares its prefix - /// with anything the run tracked. A file is reported only when its whole relative path avoids - /// the IfFileUnique substring list, so the ones carrying a runtime, framework or OS - /// segment are skipped. They are skipped on every framework and every OS, so nothing ever - /// reports them. + /// Every file here is a snapshot for a test that no longer exists: no recorded prefix owns a + /// name starting where theirs do. The set of tests does not vary by framework, OS or + /// architecture, so there is no run left for the uniqueness in the name to belong to, and all of + /// them are reported whether or not the manifests cover the frameworks. /// [Fact] public Task OrphanedWithUniqueness() => @@ -73,52 +83,45 @@ public Task OrphanedWithUniqueness() => "path/to/Deleted.Debug.verified.txt", "path/to/Deleted.Release.verified.txt" ], - "path/to/Alive.verified.txt")); + alive, + alivePrefix)); /// - /// The case the uniqueness skip exists for. A multi targeted project running one framework - /// leaves the snapshots of the other frameworks untouched, and the same holds for the OS, - /// architecture and configuration axes. None of these may be reported. - /// - /// The tracked set is the complete framework union, which is what a covered run has. The - /// architecture and configuration axes are reported either way: no manifest covers them, and - /// they were never in the name based skip either. + /// The axes no manifest can settle. The architecture one is skipped whether or not the + /// frameworks are covered, since the run that owns it is on another machine. The configuration + /// one has no enumerable value set, so it is not recognised as uniqueness at all and is + /// reported. /// [Fact] public Task UniquenessFromAnotherRun() => Verify( Report( [ - "path/to/Alive.DotNet9_0.verified.txt", - "path/to/Alive.DotNet8_0.verified.txt", - "path/to/Alive.Net4_8.verified.txt", - "path/to/Alive.Linux.verified.txt", - "path/to/Alive.arm64.verified.txt", + $"path/to/Alive.{foreignArchitecture}.verified.txt", "path/to/Alive.Debug.verified.txt" ], - "path/to/Alive.DotNet9_0.verified.txt", - "path/to/Alive.DotNet8_0.verified.txt", - "path/to/Alive.Net4_8.verified.txt")); + alive, + alivePrefix)); /// - /// A project that dropped a target framework keeps the snapshots that framework owned. No run - /// produces them again, so they are dangling, and only a complete union can say so: by name - /// they are indistinguishable from the files of a framework that simply is not running. + /// A project that dropped a target framework keeps the snapshots that framework owned. The test + /// is still there, so only a complete union can say the file is stale: by name it is + /// indistinguishable from the file of a framework that simply is not running. /// [Fact] public Task RetiredFrameworkSnapshots() => Verify( Report( [ - "path/to/Alive.DotNet9_0.verified.txt", - "path/to/Alive.DotNet8_0.verified.txt" + $"path/to/Alive.{foreignRuntime}.verified.txt", + $"path/to/Alive.{otherForeignRuntime}.verified.txt" ], - "path/to/Alive.DotNet9_0.verified.txt")); + [$"path/to/Alive.{foreignRuntime}.verified.txt"], + alivePrefix)); /// - /// IfFileUnique matches .Net with no trailing separator, so any name that merely - /// starts a word with Net is skipped. None of these are uniqueness segments, and all of - /// them are snapshots for tests that do not exist. + /// Names that merely start a word with Net. A whole segment match cannot confuse them + /// with a uniqueness segment, and none of them is owned by a live test. /// [Fact] public Task OrphanedNamesContainingNet() => @@ -130,11 +133,12 @@ public Task OrphanedNamesContainingNet() => "path/to/Tests.Nettle.verified.txt", "path/to/HttpTests.NetworkFailure.verified.txt" ], - "path/to/Alive.verified.txt")); + alive, + alivePrefix)); /// - /// The substring list is applied to the path relative to the project, not to the file name, so - /// a single directory whose name contains one of the tokens hides every snapshot below it. + /// A directory whose name contains a uniqueness token holds snapshots like any other. Only the + /// segments after a recorded prefix are uniqueness, so the directory name cannot hide them. /// [Fact] public Task OrphanedUnderDirectoryContainingUniquenessToken() => @@ -145,13 +149,9 @@ public Task OrphanedUnderDirectoryContainingUniquenessToken() => "path/to/Snapshots.Windows.Only/Deleted.verified.txt", "path/to/Plain/Deleted.verified.txt" ], - "path/to/Alive.verified.txt")); + alive, + alivePrefix)); - /// - /// The skip is a case sensitive Contains, so a segment is skipped only in the exact - /// casing Namer produces. Not wrong on its own, but it shows the list is matching text - /// rather than recognising a uniqueness value. - /// [Fact] public Task OrphanedWithLowerCaseUniquenessSegments() => Verify( @@ -162,11 +162,12 @@ public Task OrphanedWithLowerCaseUniquenessSegments() => "path/to/Deleted.osx.verified.txt", "path/to/Deleted.dotnet9_0.verified.txt" ], - "path/to/Alive.verified.txt")); + alive, + alivePrefix)); /// - /// A verified name is {Type}.{Method}{_parameters}{.uniqueness}#{index}. Everything - /// after the type and method varies per run, so none of it identifies the test. + /// A verified name is {Type}.{Method}{_parameters}{.uniqueness}#{index}. Everything after + /// the type and method varies per run, so none of it identifies the test. /// [Fact] public Task OrphanedWithIndexesAndParameters() => @@ -179,12 +180,12 @@ public Task OrphanedWithIndexesAndParameters() => "path/to/Deleted_param=value.DotNet9_0#00.verified.txt", "path/to/Deleted.DotNet9_0#00.verified.txt" ], - "path/to/Alive.verified.txt")); + alive, + alivePrefix)); /// /// A test that is still there but whose parameter set changed leaves a snapshot behind. The - /// prefix is live, so this is the one case where the run genuinely cannot tell a stale - /// parameter from one produced by a sibling test case that did not run. + /// prefix is live and no uniqueness segment excuses the file, so it is reported. /// [Fact] public Task StaleParameterOnLiveTest() => @@ -194,32 +195,130 @@ public Task StaleParameterOnLiveTest() => "path/to/Alive_param=current.verified.txt", "path/to/Alive_param=removed.verified.txt" ], - "path/to/Alive_param=current.verified.txt")); + ["path/to/Alive_param=current.verified.txt"], + alivePrefix)); + + /// + /// A prefix ends at the type and method, so a test whose name merely starts with another's does + /// not lend it its identity. + /// + [Fact] + public Task PrefixIsNotASubstringMatch() => + Verify( + Report( + [ + "path/to/Alive.verified.txt", + "path/to/AliveToo.verified.txt", + $"path/to/AliveToo.{foreignRuntime}.verified.txt" + ], + alive, + alivePrefix)); + + /// + /// Two tests where one name extends the other. The longer prefix has to win, or the shorter + /// one's uniqueness rules would be applied to the longer one's files. + /// + [Fact] + public Task LongestPrefixWins() => + Verify( + Report( + [ + "path/to/Tests.Method.verified.txt", + "path/to/Tests.Method.Nested.verified.txt", + $"path/to/Tests.Method.Nested.{foreignRuntime}.verified.txt" + ], + ["path/to/Tests.Method.verified.txt"], + ["path/to/Tests.Method", "path/to/Tests.Method.Nested"])); + + /// + /// The directory half of a prefix comes from the compiler's caller file path, while the scan + /// starts at the project directory MSBuild recorded. The two disagree on case often enough that + /// matching them ordinally would call every test unknown. + /// + [Fact] + public Task PrefixMatchIgnoresCase() => + Verify( + Report( + [$"path/to/Nested/Alive.{otherForeignRuntime}.verified.txt"], + [$"path/to/Nested/Alive.{foreignRuntime}.verified.txt"], + ["path/to/nested/Alive"])); + + /// + /// The unique directory conventions put the parameters and uniqueness in a directory name, with + /// the snapshots below it, so a prefix is followed by a separator rather than by a dot. + /// + [Fact] + public Task UniqueDirectoryPrefixes() => + Verify( + Report( + [ + "path/to/Alive/target.verified.txt", + $"path/to/Alive.{foreignRuntime}/target.verified.txt", + $"path/to/Deleted.{foreignRuntime}/target.verified.txt" + ], + [ + "path/to/Alive/target.verified.txt", + $"path/to/Alive.{foreignRuntime}/target.verified.txt" + ], + alivePrefix)); + + /// + /// UseFileName pins the verified name, so the recorded prefix is that name rather than + /// the type and method. + /// + [Fact] + public Task UseFileNamePrefix() => + Verify( + Report( + [ + "path/to/Pinned.verified.txt", + $"path/to/Pinned.{foreignRuntime}.verified.txt", + $"path/to/Unpinned.{foreignRuntime}.verified.txt" + ], + ["path/to/Pinned.verified.txt"], + ["path/to/Pinned"])); + + /// + /// Nothing recorded the tests, so there is no identity to reason from and every file would look + /// unowned. Only reachable through a caller that tracks files alone, or a manifest written + /// before prefixes were recorded. + /// + [Fact] + public Task NoPrefixesRecorded() => + Verify( + Report( + [ + "path/to/Deleted.verified.txt", + $"path/to/Deleted.{foreignRuntime}.verified.txt" + ], + alive, + [])); /// /// ResolveDirectory combines the source file directory with a relative /// UseDirectory through Path.Combine, which does not normalise, so the tracked - /// path keeps its ... The scan returns the normalised path and the ordinal comparison - /// misses, reporting a live snapshot as dangling. + /// path keeps its ... The scan returns the normalised path, and neither the file nor the + /// prefix matches it. /// [Fact] public Task UnnormalizedTrackedPath() => Verify( Report( ["path/to/Snapshots/Alive.verified.txt"], - "path/to/Tests/../Snapshots/Alive.verified.txt")); + ["path/to/Tests/../Snapshots/Alive.verified.txt"], + ["path/to/Tests/../Snapshots/Alive"])); /// - /// Only the case of the file name is meaningful. The case of the directory comes from whatever - /// string each side happened to build its path from: the scan starts at the project directory - /// recorded by MSBuild, the tracked path starts at the compiler's caller file path. + /// Only the case of the file name is meaningful. The prefix matches either way, so the file + /// reaches the casing check rather than being reported as an unknown test. /// [Fact] public Task DirectoryCaseMismatch() => Verify( Report( ["path/to/Nested/Alive.verified.txt"], - "path/to/nested/Alive.verified.txt")); + ["path/to/nested/Alive.verified.txt"], + ["path/to/nested/Alive"])); /// /// The project directory recorded by MSBuild ends with a separator, so the relative suffix has @@ -231,7 +330,8 @@ public Task DirectoryWithTrailingSeparator() => ReportIn( "path/to/", ["path/to/Deleted.verified.txt"], - "path/to/Alive.verified.txt")); + alive, + alivePrefix)); [Fact] public Task UntrackedAndIncorrectCase() => @@ -241,7 +341,8 @@ public Task UntrackedAndIncorrectCase() => "path/to/Deleted.verified.txt", "path/to/AliveWithCase.verified.txt" ], - "path/to/alivewithcase.verified.txt")); + ["path/to/alivewithcase.verified.txt"], + ["path/to/alivewithcase"])); /// /// A test can track a verified path that has no file yet: the first run of a new test, or a run @@ -249,7 +350,7 @@ public Task UntrackedAndIncorrectCase() => /// [Fact] public Task TrackedFileMissingFromDisk() => - Verify(Report([], "path/to/NotYetAccepted.verified.txt")); + Verify(Report([], ["path/to/NotYetAccepted.verified.txt"], ["path/to/NotYetAccepted"])); /// /// Several verifications can resolve to one verified path, through UseFileName or a @@ -260,12 +361,12 @@ public Task DuplicateTrackedEntries() => Verify( Report( ["path/to/Alive.verified.txt"], - "path/to/Alive.verified.txt", - "path/to/Alive.verified.txt")); + ["path/to/Alive.verified.txt", "path/to/Alive.verified.txt"], + ["path/to/Alive", "path/to/Alive"])); [Fact] public Task NothingOnDisk() => - Verify(Report([])); + Verify(Report([], alive, alivePrefix)); /// /// UseUniqueDirectory in split mode writes to @@ -359,30 +460,35 @@ static string WriteNested(TempDirectory directory, params string[] segments) return path; } - static string Report(IEnumerable filesOnDisk, params string[] tracked) => - ReportIn(root, filesOnDisk, tracked); + static string Report(IEnumerable filesOnDisk, IReadOnlyCollection tracked, IReadOnlyCollection prefixes) => + ReportIn(root, filesOnDisk, tracked, prefixes); /// - /// Reports both ways round. Without a manifest from every target framework the check falls back - /// to skipping names that look like they belong to another framework; with one the union decides - /// instead, and the difference between the two halves is what the manifests bought. + /// Reports both ways round. Without a manifest from every target framework the check leaves the + /// runtime and framework axis to the name; with one the union decides it instead, and the + /// difference between the two halves is what the manifests bought. /// - static string ReportIn(string directory, IEnumerable filesOnDisk, params string[] tracked) + static string ReportIn(string directory, IEnumerable filesOnDisk, IReadOnlyCollection tracked, IReadOnlyCollection prefixes) { var files = filesOnDisk.ToList(); var builder = new StringBuilder(); builder.AppendLine("== Frameworks not covered =="); - builder.AppendLine(Check(files, tracked, directory, false)); + builder.AppendLine(Check(files, tracked, prefixes, directory, false)); builder.AppendLine("== Frameworks covered =="); - builder.Append(Check(files, tracked, directory, true)); + builder.Append(Check(files, tracked, prefixes, directory, true)); return builder.ToString(); } - static string Check(IReadOnlyCollection filesOnDisk, IReadOnlyCollection tracked, string directory, bool frameworksCovered) + static string Check( + IReadOnlyCollection filesOnDisk, + IReadOnlyCollection tracked, + IReadOnlyCollection prefixes, + string directory, + bool frameworksCovered) { try { - DanglingSnapshotsCheck.CheckFiles(filesOnDisk, tracked, directory, frameworksCovered); + DanglingSnapshotsCheck.CheckFiles(filesOnDisk, tracked, prefixes, directory, frameworksCovered); return "Nothing reported."; } catch (Exception exception) diff --git a/src/Verify.Tests/UniquenessSegmentsTests.cs b/src/Verify.Tests/UniquenessSegmentsTests.cs new file mode 100644 index 0000000000..663494b675 --- /dev/null +++ b/src/Verify.Tests/UniquenessSegmentsTests.cs @@ -0,0 +1,186 @@ +#pragma warning disable VerifyDanglingSnapshots + +/// +/// Written against the values Namer reports for the run executing them, rather than against +/// literals, so the same assertions hold on every target framework, OS and architecture. +/// +public class UniquenessSegmentsTests +{ + // A runtime name that is never the one this run produces. + static readonly string foreignRuntime = Namer.Runtime == "Mono" ? "DotNet" : "Mono"; + + static readonly string foreignRuntimeAndVersion = $"{foreignRuntime}3_1"; + + static readonly string foreignPlatform = Namer.OperatingSystemPlatform == "Linux" ? "Windows" : "Linux"; + + static readonly string foreignArchitecture = Namer.Architecture == "arm64" ? "x64" : "arm64"; + + [Fact] + public void NoUniquenessIsNotAnotherRun() + { + Assert.False(BelongsToAnotherRun(".verified.txt")); + Assert.False(BelongsToAnotherRun("_param=value.verified.txt")); + Assert.False(BelongsToAnotherRun("#00.verified.txt")); + Assert.False(BelongsToAnotherRun("#name.verified.txt")); + } + + [Fact] + public void ThisRuntimeIsNotAnotherRun() + { + Assert.False(BelongsToAnotherRun($".{Namer.Runtime}.verified.txt")); + Assert.False(BelongsToAnotherRun($".{Namer.RuntimeAndVersion}.verified.txt")); + } + + [Fact] + public void AnotherRuntimeIsAnotherRun() + { + Assert.True(BelongsToAnotherRun($".{foreignRuntime}.verified.txt")); + Assert.True(BelongsToAnotherRun($".{foreignRuntimeAndVersion}.verified.txt")); + } + + /// + /// A version on the same runtime as this run, but not this run's version. The version has to be + /// compared, not just the runtime name, or every framework of a multi targeted project would + /// look like this one. + /// + [Fact] + public void AnotherVersionOfThisRuntimeIsAnotherRun() + { + var other = $"{Namer.Runtime}99_9"; + + Assert.NotEqual(Namer.RuntimeAndVersion, other); + Assert.True(BelongsToAnotherRun($".{other}.verified.txt")); + } + + /// + /// The manifests settle the runtime and framework axis, so once they cover every framework the + /// segments naming it say nothing. + /// + [Fact] + public void RuntimeIsIgnoredOnceFrameworksAreCovered() => + Assert.False(BelongsToAnotherRun($".{foreignRuntimeAndVersion}.verified.txt", frameworksCovered: true)); + + [Fact] + public void ThisPlatformIsNotAnotherRun() => + Assert.False(BelongsToAnotherRun($".{Namer.OperatingSystemPlatform}.verified.txt")); + + /// + /// No manifest can settle the OS axis: those runs are on other machines, with their own + /// intermediate directories. So a foreign platform stays foreign even when covered. + /// + [Fact] + public void AnotherPlatformIsAnotherRunEvenWhenCovered() + { + Assert.True(BelongsToAnotherRun($".{foreignPlatform}.verified.txt")); + Assert.True(BelongsToAnotherRun($".{foreignPlatform}.verified.txt", frameworksCovered: true)); + } + + [Fact] + public void ThisArchitectureIsNotAnotherRun() => + Assert.False(BelongsToAnotherRun($".{Namer.Architecture}.verified.txt")); + + [Fact] + public void AnotherArchitectureIsAnotherRunEvenWhenCovered() + { + Assert.True(BelongsToAnotherRun($".{foreignArchitecture}.verified.txt")); + Assert.True(BelongsToAnotherRun($".{foreignArchitecture}.verified.txt", frameworksCovered: true)); + } + + /// + /// The configuration axis has no enumerable value set, so a segment naming one is not treated as + /// uniqueness at all. A snapshot only another configuration produces is reported. + /// + [Fact] + public void ConfigurationIsNotRecognised() + { + Assert.True(!BelongsToAnotherRun(".Debug.verified.txt")); + Assert.True(!BelongsToAnotherRun(".Release.verified.txt")); + } + + /// + /// A whole segment match, so a name that merely starts a word with a uniqueness token is left + /// alone. These are what the old substring list could not tell apart. + /// + [Fact] + public void NamesStartingWithAUniquenessTokenAreNotUniqueness() + { + Assert.False(BelongsToAnotherRun(".NetworkClient.verified.txt")); + Assert.False(BelongsToAnotherRun(".NetCoreShim.verified.txt")); + Assert.False(BelongsToAnotherRun(".Nettle.verified.txt")); + Assert.False(BelongsToAnotherRun(".Windowsill.verified.txt")); + Assert.False(BelongsToAnotherRun(".Linuxish.verified.txt")); + } + + [Fact] + public void CaseIsPartOfTheMatch() + { + Assert.False(BelongsToAnotherRun(".windows.verified.txt")); + Assert.False(BelongsToAnotherRun($".{foreignRuntime.ToLowerInvariant()}.verified.txt")); + } + + /// + /// The first segment is the parameter text, which is never uniqueness. A parameter value that + /// happens to read like a runtime must not be mistaken for one. + /// + [Fact] + public void ParametersAreNotUniqueness() => + Assert.False(BelongsToAnotherRun($"_param={foreignRuntimeAndVersion}.verified.txt")); + + [Fact] + public void UniquenessAfterParameters() => + Assert.True(BelongsToAnotherRun($"_param=value.{foreignRuntimeAndVersion}.verified.txt")); + + [Fact] + public void UniquenessBeforeAnIndex() + { + Assert.True(BelongsToAnotherRun($".{foreignRuntimeAndVersion}#00.verified.txt")); + Assert.True(BelongsToAnotherRun($".{foreignRuntimeAndVersion}#name.verified.txt")); + } + + [Fact] + public void SeveralUniquenessSegments() + { + Assert.True(BelongsToAnotherRun($".{foreignRuntimeAndVersion}.{Namer.OperatingSystemPlatform}.verified.txt")); + Assert.True(BelongsToAnotherRun($".{Namer.RuntimeAndVersion}.{foreignPlatform}.verified.txt")); + Assert.False(BelongsToAnotherRun($".{Namer.RuntimeAndVersion}.{Namer.OperatingSystemPlatform}.verified.txt")); + } + + /// + /// The unique directory conventions put the uniqueness in a directory name. Uniqueness ends + /// where that directory does, so the file name below it is never read as uniqueness. + /// + [Fact] + public void UniqueDirectory() + { + Assert.True(BelongsToAnotherRun($".{foreignRuntimeAndVersion}{Path.DirectorySeparatorChar}target.verified.txt")); + Assert.False(BelongsToAnotherRun($"{Path.DirectorySeparatorChar}{foreignRuntimeAndVersion}.verified.txt")); + } + + /// + /// Split mode names the directory {prefix}{uniqueness}.verified, so uniqueness ends at + /// the verified marker even though it is a directory rather than a file. + /// + [Fact] + public void SplitModeUniqueDirectory() + { + Assert.True(BelongsToAnotherRun($".{foreignRuntimeAndVersion}.verified{Path.DirectorySeparatorChar}target.txt")); + Assert.False(BelongsToAnotherRun($".verified{Path.DirectorySeparatorChar}target.txt")); + } + + /// + /// Only {name}{major}_{minor} is a versioned segment. Anything else keeps its underscores + /// and fails the name lookup whole. + /// + [Fact] + public void OnlyMajorMinorIsAVersion() + { + Assert.False(BelongsToAnotherRun($".{foreignRuntime}_.verified.txt")); + Assert.False(BelongsToAnotherRun($".{foreignRuntime}_x.verified.txt")); + Assert.False(BelongsToAnotherRun($".{foreignRuntime}3_.verified.txt")); + Assert.False(BelongsToAnotherRun($".{foreignRuntime}_1.verified.txt")); + Assert.True(BelongsToAnotherRun($".{foreignRuntime}10_11.verified.txt")); + } + + static bool BelongsToAnotherRun(string tail, bool frameworksCovered = false) => + UniquenessSegments.BelongsToAnotherRun(tail, frameworksCovered); +} diff --git a/src/Verify/ConventionCheck/DanglingManifest.cs b/src/Verify/ConventionCheck/DanglingManifest.cs index 477484f8ec..0674a63290 100644 --- a/src/Verify/ConventionCheck/DanglingManifest.cs +++ b/src/Verify/ConventionCheck/DanglingManifest.cs @@ -1,14 +1,18 @@ namespace VerifyTests; /// -/// Records the verified files one target framework's test run tracked, so that the runs of the other -/// target frameworks can read it back. +/// Records what one target framework's test run tracked, so that the runs of the other target +/// frameworks can read it back. /// -/// Without this a run only knows the files it wrote itself. A multi targeted project runs once per +/// Without this a run only knows what it did itself. A multi targeted project runs once per /// framework, so most of the snapshots on disk belong to a framework that is not running, and the /// only way to tell those from a snapshot whose test was deleted was to guess from the file name. /// Unioning the manifests replaces the guess with the answer for that axis. /// +/// Both halves are recorded. The files say which snapshots exist; the prefixes say which tests do, +/// which matters just as much across frameworks: a test behind a #if NET48 exists only in +/// the net48 run, and without its prefix every other run would call its snapshots unowned. +/// /// Manifests go in the intermediate (obj) directory, below BaseIntermediateOutputPath, which unlike /// IntermediateOutputPath is shared by every target framework of the project. They are scoped by /// configuration, since a Debug run says nothing about the snapshots a Release run owns, and named @@ -16,11 +20,20 @@ namespace VerifyTests; /// /// Staleness is bounded two ways: obj is wiped by a clean, and a manifest older than the running /// assembly is from before the last build and is ignored. A run that cannot account for every target -/// framework says so, and the caller falls back to the name based skip. +/// framework says so, and the caller leaves the framework axis to the name instead. /// static class DanglingManifest { const string extension = ".txt"; + const string filesHeader = "[files]"; + const string prefixesHeader = "[prefixes]"; + + internal readonly struct Merged(IReadOnlyCollection files, IReadOnlyCollection prefixes, bool frameworksCovered) + { + public IReadOnlyCollection Files { get; } = files; + public IReadOnlyCollection Prefixes { get; } = prefixes; + public bool FrameworksCovered { get; } = frameworksCovered; + } public static string PathFor(string directory, string targetFramework) => Path.Combine(directory, $"{targetFramework}{extension}"); @@ -29,14 +42,21 @@ public static string PathFor(string directory, string targetFramework) => /// Written through a temporary file and moved into place, so a run of another target framework /// reading concurrently sees either the previous manifest or this one, never half of one. /// - public static void Write(string directory, string targetFramework, IEnumerable trackedFiles) + public static void Write( + string directory, + string targetFramework, + IEnumerable files, + IEnumerable prefixes) { Directory.CreateDirectory(directory); var path = PathFor(directory, targetFramework); var temp = $"{path}.{Guid.NewGuid():N}.tmp"; try { - File.WriteAllLines(temp, trackedFiles.Distinct()); + // The headers cannot collide with a recorded path: both halves are absolute, so neither + // can be a line that is exactly "[files]" or "[prefixes]". + List lines = [filesHeader, ..files.Distinct(), prefixesHeader, ..prefixes.Distinct()]; + File.WriteAllLines(temp, lines); File.Move(temp, path, true); } catch @@ -47,21 +67,22 @@ public static void Write(string directory, string targetFramework, IEnumerable - /// Every verified file recorded by the manifests in , and whether - /// those manifests account for every framework in . + /// Everything recorded by the manifests in , and whether those + /// manifests account for every framework in . /// /// /// Manifests last written before this are from before the last build and are ignored. The /// running assembly's timestamp is the caller's value: a build server builds every framework /// before running any of them, so a manifest from this build is always newer. /// - public static (HashSet tracked, bool covered) Read( + public static Merged Read( string directory, IReadOnlyList targetFrameworks, string targetFramework, DateTime staleBefore) { - HashSet tracked = []; + HashSet files = []; + HashSet prefixes = []; HashSet frameworksRead = new(StringComparer.OrdinalIgnoreCase); if (Directory.Exists(directory)) @@ -73,19 +94,38 @@ public static (HashSet tracked, bool covered) Read( continue; } - foreach (var line in File.ReadLines(file)) - { - if (line.Length > 0) - { - tracked.Add(line); - } - } - + ReadInto(file, files, prefixes); frameworksRead.Add(Path.GetFileNameWithoutExtension(file)); } } - return (tracked, IsCovered(targetFrameworks, targetFramework, frameworksRead)); + return new(files, prefixes, IsCovered(targetFrameworks, targetFramework, frameworksRead)); + } + + static void ReadInto(string file, HashSet files, HashSet prefixes) + { + var target = files; + foreach (var line in File.ReadLines(file)) + { + if (line.Length == 0) + { + continue; + } + + if (line == filesHeader) + { + target = files; + continue; + } + + if (line == prefixesHeader) + { + target = prefixes; + continue; + } + + target.Add(line); + } } /// diff --git a/src/Verify/ConventionCheck/DanglingSnapshotsCheck.cs b/src/Verify/ConventionCheck/DanglingSnapshotsCheck.cs index 719f9c42eb..c88ef73c98 100644 --- a/src/Verify/ConventionCheck/DanglingSnapshotsCheck.cs +++ b/src/Verify/ConventionCheck/DanglingSnapshotsCheck.cs @@ -7,9 +7,17 @@ namespace VerifyTests; public static class DanglingSnapshotsCheck { static ConcurrentBag? trackedVerifiedFiles; + static ConcurrentBag? trackedPrefixes; internal static void TrackVerifiedFile(string path) => trackedVerifiedFiles?.Add(path); + /// + /// The verified path up to the end of the type and method, before parameters, uniqueness and + /// index. Recorded once per verification, from the constructor, so a test is known to exist + /// even when it produced no file: it was inline, it threw, or every target was excluded. + /// + internal static void TrackPrefix(string prefix) => trackedPrefixes?.Add(prefix); + public static void Run() { if (!BuildServerDetector.Detected) @@ -17,10 +25,17 @@ public static void Run() return; } - var assembly = VerifierSettings.Assembly; + if (VerifierSettings.AssemblyOrNull is not { } assembly) + { + // No verification ran, so nothing is tracked and every snapshot on disk would be + // reported. A run that verified nothing is something the test framework reports; adding + // a second failure here only buries it. + return; + } + var directory = AttributeReader.GetProjectDirectory(assembly); - var (tracked, frameworksCovered) = MergeWithOtherFrameworks(assembly); - CheckFiles(FindSnapshotFiles(directory), tracked, directory, frameworksCovered); + var merged = MergeWithOtherFrameworks(assembly); + CheckFiles(FindSnapshotFiles(directory), merged.Files, merged.Prefixes, directory, merged.FrameworksCovered); } internal static IEnumerable FindSnapshotFiles(string directory) => @@ -31,9 +46,10 @@ internal static IEnumerable FindSnapshotFiles(string directory) => /// tracked. When every framework is accounted for, the union is the complete set of verified /// files the project owns, and a file outside it is dangling whatever its name says. /// - static (IReadOnlyCollection tracked, bool frameworksCovered) MergeWithOtherFrameworks(Assembly assembly) + static DanglingManifest.Merged MergeWithOtherFrameworks(Assembly assembly) { - var tracked = trackedVerifiedFiles!; + var files = trackedVerifiedFiles!; + var prefixes = trackedPrefixes!; var directory = VerifierSettings.DanglingDir; var targetFramework = VerifierSettings.TargetFramework; if (directory is null || @@ -41,12 +57,12 @@ internal static IEnumerable FindSnapshotFiles(string directory) => { // The project does not consume Verify's build props, so there is nowhere to put a // manifest and no framework list to check it against. - return (tracked, false); + return new(files, prefixes, false); } try { - DanglingManifest.Write(directory, targetFramework, tracked); + DanglingManifest.Write(directory, targetFramework, files, prefixes); return DanglingManifest.Read( directory, VerifierSettings.TargetFrameworks, @@ -56,10 +72,9 @@ internal static IEnumerable FindSnapshotFiles(string directory) => catch (Exception exception) when (exception is IOException or UnauthorizedAccessException) { - // The manifest is an optimization: without it the check still runs, it just falls back - // to skipping names that look like they belong to another framework. Failing to read or - // write one must not fail the test run. - return (tracked, false); + // The manifest is an optimization: without it the check still runs, it just cannot + // settle the framework axis. Failing to read or write one must not fail the test run. + return new(files, prefixes, false); } } @@ -90,6 +105,7 @@ static DateTime AssemblyWriteTime(Assembly assembly) internal static void CheckFiles( IEnumerable filesOnDisk, IReadOnlyCollection trackedFiles, + IReadOnlyCollection prefixes, string directory, bool frameworksCovered) { @@ -115,6 +131,11 @@ static void AppendItems(StringBuilder builder, List list, string title) // making the check O(files * tracked). var tracked = new HashSet(trackedFiles); var trackedIgnoreCase = new HashSet(trackedFiles, StringComparer.OrdinalIgnoreCase); + // A prefix identifies a test, and case is not part of that identity. The casing check below + // is about the snapshot's own name; the directory half of a prefix comes from whatever + // string each side built its path from, and the two disagree often enough that an ordinal + // comparison would call every test unknown. + var knownPrefixes = new HashSet(prefixes, StringComparer.OrdinalIgnoreCase); List untracked = []; List incorrectCase = []; @@ -125,19 +146,13 @@ static void AppendItems(StringBuilder builder, List list, string title) continue; } - var suffix = file[directory.Length..]; - suffix = suffix.TrimStart(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); - if (IsPlatformUnique(suffix)) - { - continue; - } - - if (!frameworksCovered && - IsFrameworkUnique(suffix)) + if (!IsDangling(file, knownPrefixes, frameworksCovered)) { continue; } + var suffix = file[directory.Length..]; + suffix = suffix.TrimStart(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); if (trackedIgnoreCase.Contains(file)) { incorrectCase.Add(suffix); @@ -166,24 +181,63 @@ static void AppendItems(StringBuilder builder, List list, string title) } /// - /// The runtime and target framework axes. A run of one framework never writes the files of - /// another, so without a manifest from every framework these have to be left alone. With one, - /// they are decided by the union instead and this is not consulted. + /// A file the run did not write is dangling unless some other run owns it. + /// + /// When no test of this assembly owns a name starting where this one does, no run of the project + /// owns it either: the set of tests does not vary by framework, OS or architecture, so there is + /// nothing left for a name to excuse. That is the whole point of tracking prefixes, and it is + /// what makes a deleted test's snapshot visible however much uniqueness its name carries. + /// + /// When a test does own the name, the file is either a variant this run does not produce, which + /// the uniqueness segments say, or a leftover, which they do not. /// - static bool IsFrameworkUnique(string file) => - file.Contains(".Net") || - file.Contains(".DotNet") || - file.Contains(".Mono."); + static bool IsDangling(string file, HashSet prefixes, bool frameworksCovered) + { + if (prefixes.Count == 0) + { + // Nothing recorded the tests, so there is no identity to reason from. Only reachable + // through a caller that tracks files alone, or a manifest written before prefixes were + // recorded. Treating every file as unowned would report all of them. + return false; + } + + if (!TryMatchPrefix(file, prefixes, out var tailIndex)) + { + return true; + } + + return !UniquenessSegments.BelongsToAnotherRun(file[tailIndex..], frameworksCovered); + } /// - /// The OS axis. Manifests cannot settle this one: the runs that produce these files are on - /// other machines, with their own intermediate directories, so their manifests are never - /// visible here. + /// The longest recorded prefix this file continues from. A verified name continues from the type + /// and method with parameters (_), uniqueness or the verified marker (.), a target + /// name or index (#), or, for the unique directory conventions, a directory separator. + /// Probing every such position finds the prefix without having to know which of them the name + /// uses, and searching from the end takes the longest match. /// - static bool IsPlatformUnique(string file) => - file.Contains(".OSX.") || - file.Contains(".Windows.") || - file.Contains(".Linux."); + static bool TryMatchPrefix(string file, HashSet prefixes, out int tailIndex) + { + for (var index = file.Length - 1; index > 0; index--) + { + var character = file[index]; + if (character is not ('.' or '_' or '#') && + character != Path.DirectorySeparatorChar && + character != Path.AltDirectorySeparatorChar) + { + continue; + } + + if (prefixes.Contains(file[..index])) + { + tailIndex = index; + return true; + } + } + + tailIndex = 0; + return false; + } [ModuleInitializer] internal static void Init() @@ -191,6 +245,7 @@ internal static void Init() if (BuildServerDetector.Detected) { trackedVerifiedFiles = []; + trackedPrefixes = []; } } } diff --git a/src/Verify/ConventionCheck/UniquenessSegments.cs b/src/Verify/ConventionCheck/UniquenessSegments.cs new file mode 100644 index 0000000000..f9e93b214c --- /dev/null +++ b/src/Verify/ConventionCheck/UniquenessSegments.cs @@ -0,0 +1,217 @@ +namespace VerifyTests; + +/// +/// Decides whether what follows a verified prefix names a run other than this one. +/// +/// A verified name is +/// {Type}.{Method}{_parameters}{.uniqueness}{#target}{#index}.verified.{extension}, so the +/// uniqueness segments sit between the parameters and the target name, the index, or the verified +/// marker. Under the unique directory conventions the same segments are in a directory name, with +/// the snapshot files below it, so the segments also end at a directory separator. +/// +/// Segments are matched whole, against the values can actually produce. A +/// substring search cannot tell Tests.NetworkClient from a Net uniqueness segment, and +/// cannot tell a DotNet8_0 written by another target framework from the DotNet9_0 this +/// run writes itself. +/// +static class UniquenessSegments +{ + static HashSet platforms = new(StringComparer.Ordinal) + { + "Windows", + "Linux", + "OSX", + "Android", + "IOS" + }; + + // Namer.Architecture is RuntimeInformation.ProcessArchitecture lowercased. Listed rather than + // reflected off the Architecture enum, which grew over time: reflecting it would make a name + // recognised or not depending on the runtime reading it, so a snapshot an s390x agent produced + // would go unrecognised by a .NET Framework run, which only knows the first four. + static HashSet architectures = new(StringComparer.Ordinal) + { + "x86", + "x64", + "arm", + "arm64", + "wasm", + "s390x", + "loongarch64", + "armv6", + "ppc64le", + "riscv64" + }; + + // Every value Namer.GetRuntimeAndVersion and Namer.GetSimpleFrameworkName can return, before + // the major and minor version is appended. + static HashSet frameworks = new(StringComparer.Ordinal) + { + "Net", + "DotNet", + "Mono" + }; + + /// The verified path from the end of the matched prefix. + /// + /// Whether a manifest is in hand for every target framework. When one is, the union of them has + /// already settled the runtime and framework axis and the segments naming it are not consulted. + /// + public static bool BelongsToAnotherRun(string tail, bool frameworksCovered) + { + var end = EndOfUniqueness(tail); + var start = 0; + // The first segment is the parameter text, which is not uniqueness. Under the file + // convention the tail opens with the separator, making that segment empty. + var isParameters = true; + while (start < end) + { + var next = tail.IndexOf('.', start, end - start); + if (next == -1) + { + next = end; + } + + if (!isParameters && + IsFromAnotherRun(tail[start..next], frameworksCovered)) + { + return true; + } + + isParameters = false; + start = next + 1; + } + + return false; + } + + /// + /// Uniqueness runs out at the target name or index, at the verified marker, or, for the unique + /// directory conventions, where the directory holding the snapshots ends. + /// + static int EndOfUniqueness(string tail) + { + var end = tail.Length; + for (var index = 0; index < tail.Length; index++) + { + var character = tail[index]; + if (character == '#' || + character == Path.DirectorySeparatorChar || + character == Path.AltDirectorySeparatorChar) + { + return index; + } + } + + var marker = tail.IndexOf(".verified", StringComparison.Ordinal); + if (marker != -1 && + marker < end) + { + return marker; + } + + return end; + } + + static bool IsFromAnotherRun(string segment, bool frameworksCovered) + { + if (segment.Length == 0) + { + return false; + } + + if (platforms.Contains(segment)) + { + return !MatchesCurrentPlatform(segment); + } + + if (architectures.Contains(segment)) + { + return !string.Equals(segment, Namer.Architecture, StringComparison.Ordinal); + } + + if (frameworksCovered) + { + return false; + } + + return IsFramework(segment) && + !MatchesCurrentFramework(segment); + } + + /// + /// Namer.OperatingSystemPlatform throws rather than name a platform it does not know. Nothing + /// this run produces can be said to match in that case, so the file is left alone. + /// + static bool MatchesCurrentPlatform(string segment) + { + try + { + return string.Equals(segment, Namer.OperatingSystemPlatform, StringComparison.Ordinal); + } + catch + { + return false; + } + } + + static bool MatchesCurrentFramework(string segment) + { + if (string.Equals(segment, Namer.Runtime, StringComparison.Ordinal) || + string.Equals(segment, Namer.RuntimeAndVersion, StringComparison.Ordinal)) + { + return true; + } + + // The target framework names need a TargetFrameworkAttribute, and throw without one. + try + { + return string.Equals(segment, Namer.TargetFrameworkName, StringComparison.Ordinal) || + string.Equals(segment, Namer.TargetFrameworkNameAndVersion, StringComparison.Ordinal); + } + catch + { + return false; + } + } + + static bool IsFramework(string segment) => + frameworks.Contains(WithoutVersion(segment)); + + /// + /// Strips the {major}_{minor} a versioned uniqueness segment ends with, so that + /// DotNet9_0 is recognised as the same axis as DotNet. Anything that is not + /// exactly that shape is returned whole, and fails the name lookup on its own. + /// + static string WithoutVersion(string segment) + { + var underscore = segment.LastIndexOf('_'); + if (underscore < 1 || + underscore == segment.Length - 1) + { + return segment; + } + + for (var index = underscore + 1; index < segment.Length; index++) + { + if (!char.IsDigit(segment[index])) + { + return segment; + } + } + + var nameEnd = underscore; + while (nameEnd > 0 && + char.IsDigit(segment[nameEnd - 1])) + { + nameEnd--; + } + + if (nameEnd == underscore) + { + return segment; + } + + return segment[..nameEnd]; + } +} diff --git a/src/Verify/Verifier/InnerVerifier.cs b/src/Verify/Verifier/InnerVerifier.cs index 898d834760..9fe06664b8 100644 --- a/src/Verify/Verifier/InnerVerifier.cs +++ b/src/Verify/Verifier/InnerVerifier.cs @@ -1,3 +1,4 @@ +#pragma warning disable VerifyDanglingSnapshots namespace VerifyTests; public partial class InnerVerifier : @@ -102,6 +103,12 @@ public InnerVerifier( Directory.CreateDirectory(directory); + // Recorded here rather than alongside the files, so that a test counts as existing even when + // it produces none: an inline snapshot, a verification that throws, or one whose targets are + // all excluded. Everything the naming appends after this point - parameters, uniqueness, + // target name, index - varies per run and per case, so none of it identifies the test. + DanglingSnapshotsCheck.TrackPrefix(Path.Combine(directory, settings.FileName ?? typeAndMethod)); + if (settings.UniqueDirectory) { if (settings.inline != null) @@ -211,6 +218,7 @@ public InnerVerifier(string directory, string name, VerifySettings? settings = n Directory.CreateDirectory(directory); var prefix = Path.Combine(directory, name); + DanglingSnapshotsCheck.TrackPrefix(prefix); ValidatePrefix(this.settings, prefix); verifiedFiles = MatchingFileFinder.FindVerified(name, directory); diff --git a/src/Verify/VerifierSettings_TargetAssembly.cs b/src/Verify/VerifierSettings_TargetAssembly.cs index 7780dfa184..e1042fe0c3 100644 --- a/src/Verify/VerifierSettings_TargetAssembly.cs +++ b/src/Verify/VerifierSettings_TargetAssembly.cs @@ -48,6 +48,12 @@ public static Assembly Assembly } } + /// + /// Null until the first verification runs. Unlike , does not throw, for + /// callers whose whole job is to do nothing when no verification ran. + /// + internal static Assembly? AssemblyOrNull => assembly; + static Lock locker = new(); public static void AssignTargetAssembly(Assembly assembly) diff --git a/src/appveyor.yml b/src/appveyor.yml index 61df4ad56e..539f3f4507 100644 --- a/src/appveyor.yml +++ b/src/appveyor.yml @@ -58,11 +58,12 @@ build_script: # Run it via the fixie.console tool (restored from src/.config/dotnet-tools.json, which is only discoverable from src). # Use pushd/popd so the `cd` into src does not persist into subsequent AppVeyor script lines. - cmd: pushd src && dotnet fixie Verify.Fixie.Tests --configuration Release --no-build && popd +# The dangling snapshot check only runs on a build server, so these projects are the only place it +# is exercised end to end: adapter wiring, module initializer, teardown hook, manifest and scan. - dotnet build src/VerifyDangling.slnx --configuration Release --verbosity minimal -#- dotnet test %CD%/src/DanglingSnapshotsMSTestUsage --configuration Release --no-build --no-restore -#- dotnet test %CD%/src/DanglingSnapshotsNUnitUsage --configuration Release --no-build --no-restore -#- dotnet test %CD%/src/DanglingSnapshotsXunitUsage --configuration Release --no-build --no-restore -#- dotnet test %CD%/src/DanglingSnapshotsXunitV3Usage --configuration Release --no-build --no-restore +- dotnet test %CD%/src/DanglingSnapshotsMSTestUsage --configuration Release --no-build --no-restore --verbosity minimal +- dotnet test %CD%/src/DanglingSnapshotsNUnitUsage --configuration Release --no-build --no-restore --verbosity minimal +- dotnet test %CD%/src/DanglingSnapshotsXunitV3Usage --configuration Release --no-build --no-restore --verbosity minimal #- pwsh: | From fa954da7f65d9cf7d99992f341d5ce3ea5fca3b0 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Fri, 18 Sep 2026 15:57:19 +1000 Subject: [PATCH 4/7] Fail the run when the NUnit dangling check finds something NUnit's Microsoft.Testing.Platform runner prints a teardown failure but does not count it. The run was summarised as passed and the process exited zero, so a dangling snapshot never failed a build and enabling the NUnit usage project in CI was vacuous. Not specific to the exception, nor to [SetUpFixture]: a bare throw from [OneTimeTearDown] behaves the same, and so does one from a [TestFixture], so there is no teardown hook whose failure the runner surfaces. Assigning Environment.ExitCode during the run does not help either, since the runner's own result replaces it. So the NUnit adapter now sets the exit code from a ProcessExit handler, which runs after the runner has returned and does stick. Registered only once the check has already failed, and only over a zero, so a run that failed for its own reasons keeps the code it earned. The handler also writes the reason, landing after the runner's summary, which is where a passing summary next to a failing exit code needs explaining. Verified end to end against the NUnit usage project with a planted dangling snapshot: running the executable directly gives exit code 1, and through dotnet test, which is how CI invokes it, the run is reported as failed. A clean run is untouched and still exits zero. --- src/Verify.NUnit/DanglingSnapshots.cs | 44 +++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/src/Verify.NUnit/DanglingSnapshots.cs b/src/Verify.NUnit/DanglingSnapshots.cs index dde2e3f066..335e39b778 100644 --- a/src/Verify.NUnit/DanglingSnapshots.cs +++ b/src/Verify.NUnit/DanglingSnapshots.cs @@ -3,6 +3,44 @@ [Experimental("VerifyDanglingSnapshots")] public static class DanglingSnapshots { - public static void Run() => - DanglingSnapshotsCheck.Run(); -} \ No newline at end of file + public static void Run() + { + try + { + DanglingSnapshotsCheck.Run(); + } + catch (Exception exception) + { + FailProcessOnExit(exception); + throw; + } + } + + /// + /// NUnit's Microsoft.Testing.Platform runner prints a teardown failure but does not count it: the + /// run is summarised as passed and the process exits zero, so a dangling snapshot would not fail + /// a build. That holds for [SetUpFixture] and [TestFixture] alike, and there is no + /// teardown hook whose failure the runner does surface, so the exit code is set here instead. + /// + /// Set from ProcessExit, because the runner's own result replaces anything assigned while it is + /// still running. Registered only once the check has already failed, and only over a zero, so a + /// run that failed for its own reasons keeps the code it earned. + /// + static void FailProcessOnExit(Exception exception) => + AppDomain.CurrentDomain.ProcessExit += (_, _) => + { + if (Environment.ExitCode != 0) + { + return; + } + + Environment.ExitCode = 1; + // Written from the handler so it lands after the runner's summary, which is where a + // passing summary sitting next to a failing exit code needs explaining. + Console.Error.WriteLine( + $""" + Verify has failed the dangling snapshot check. The NUnit runner does not count a teardown failure, so it reported the run as passed; the exit code has been set to 1. + {exception.Message} + """); + }; +} From 65c2f5f695d075cb5444f12a981d802e37a01b1d Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Fri, 18 Sep 2026 16:11:19 +1000 Subject: [PATCH 5/7] Add Expecto, Fixie and TUnit dangling snapshot usage projects The three adapters had a DanglingSnapshots.Run() and nothing exercising it, so whether a dangling snapshot actually failed a build was unknown. Each now has a usage project, in the solution, in CI, and as the documented snippet for that framework. Measured, per adapter, by planting a Tests.Retired.Mono3_1.verified.txt and running the project the way CI does. Clean runs all exit zero. * Expecto, in the entry point after the run: exit 127. The check throws out of Main, which reads as a crash rather than a failure, but the report is printed and the build goes red. * Fixie, at the end of IExecution.Run: exit 1. Fixie attributes the failure to every test in the suite, so the run reports as failed. * TUnit, in [After(TestSession)]: exit 10, reported as an unhandled exception in the session hook. So NUnit was the only adapter that swallowed it, and the exit code fix already made for it is the only one needed. The Expecto project needs an explicit IsPackable. The other five are recognised as test projects by their test SDK, which sets it; an Expecto project is a plain console application, so nothing does, and the defaults tried to pack the sample as a NuGet package. --- docs/dangling-files.md | 81 +++++++++++++++++++ docs/mdsource/dangling-files.source.md | 23 ++++++ .../DanglingSnapshots.cs | 7 ++ .../DanglingSnapshotsExpectoUsage.csproj | 23 ++++++ .../Tests.IncorrectCase.verified.txt | 1 + .../Tests.Simple.verified.txt | 1 + src/DanglingSnapshotsExpectoUsage/Tests.cs | 18 +++++ .../DanglingSnapshots.cs | 28 +++++++ .../DanglingSnapshotsFixieUsage.csproj | 17 ++++ .../Tests.IncorrectCase.verified.txt | 1 + .../Tests.Simple.verified.txt | 1 + src/DanglingSnapshotsFixieUsage/Tests.cs | 8 ++ .../DanglingSnapshots.cs | 8 ++ .../DanglingSnapshotsTUnitUsage.csproj | 17 ++++ .../Tests.IncorrectCase.verified.txt | 1 + .../Tests.Simple.verified.txt | 1 + src/DanglingSnapshotsTUnitUsage/Tests.cs | 10 +++ src/VerifyDangling.slnx | 3 + src/appveyor.yml | 3 + 19 files changed, 252 insertions(+) create mode 100644 src/DanglingSnapshotsExpectoUsage/DanglingSnapshots.cs create mode 100644 src/DanglingSnapshotsExpectoUsage/DanglingSnapshotsExpectoUsage.csproj create mode 100644 src/DanglingSnapshotsExpectoUsage/Tests.IncorrectCase.verified.txt create mode 100644 src/DanglingSnapshotsExpectoUsage/Tests.Simple.verified.txt create mode 100644 src/DanglingSnapshotsExpectoUsage/Tests.cs create mode 100644 src/DanglingSnapshotsFixieUsage/DanglingSnapshots.cs create mode 100644 src/DanglingSnapshotsFixieUsage/DanglingSnapshotsFixieUsage.csproj create mode 100644 src/DanglingSnapshotsFixieUsage/Tests.IncorrectCase.verified.txt create mode 100644 src/DanglingSnapshotsFixieUsage/Tests.Simple.verified.txt create mode 100644 src/DanglingSnapshotsFixieUsage/Tests.cs create mode 100644 src/DanglingSnapshotsTUnitUsage/DanglingSnapshots.cs create mode 100644 src/DanglingSnapshotsTUnitUsage/DanglingSnapshotsTUnitUsage.csproj create mode 100644 src/DanglingSnapshotsTUnitUsage/Tests.IncorrectCase.verified.txt create mode 100644 src/DanglingSnapshotsTUnitUsage/Tests.Simple.verified.txt create mode 100644 src/DanglingSnapshotsTUnitUsage/Tests.cs diff --git a/docs/dangling-files.md b/docs/dangling-files.md index bf9549e783..fd69333475 100644 --- a/docs/dangling-files.md +++ b/docs/dangling-files.md @@ -109,6 +109,8 @@ public static class SetUp snippet source | anchor +The NUnit runner prints a teardown failure but does not count it, so the run is summarised as passed. To keep a dangling snapshot from going unnoticed, the NUnit integration sets the process exit code to 1 when the check fails, and writes the reason after the runner's summary. + ### XUnitV3 @@ -147,3 +149,82 @@ public class Tests ``` snippet source | anchor + + +### TUnit + +Use the `[After(TestSession)]` feature: + + + +```cs +#pragma warning disable VerifyDanglingSnapshots + +public static class Cleanup +{ + [After(TestSession)] + public static void Run() => + DanglingSnapshots.Run(); +} +``` +snippet source | anchor + + + +### Expecto + +Expecto test projects are console applications, so the check goes in the entry point, after the run: + + + +```cs +#pragma warning disable VerifyDanglingSnapshots + +var result = Runner.RunTestsInAssemblyWithCLIArgs([], args); + +DanglingSnapshots.Run(); + +return result; +``` +snippet source | anchor + + + +### Fixie + +Fixie already requires an `IExecution` implementation for Verify. Add the check to the end of `Run`: + + + +```cs +#pragma warning disable VerifyDanglingSnapshots + +public class TestProject : + ITestProject, + IExecution +{ + public void Configure(TestConfiguration configuration, TestEnvironment environment) + { + VerifierSettings.AssignTargetAssembly(environment.Assembly); + configuration.Conventions.Add(); + } + + public async Task Run(TestSuite testSuite) + { + foreach (var testClass in testSuite.TestClasses) + { + foreach (var test in testClass.Tests) + { + using (ExecutionState.Set(testClass, test, null)) + { + await test.Run(); + } + } + } + + DanglingSnapshots.Run(); + } +} +``` +snippet source | anchor + diff --git a/docs/mdsource/dangling-files.source.md b/docs/mdsource/dangling-files.source.md index 4ba2d37ce5..4915036d8e 100644 --- a/docs/mdsource/dangling-files.source.md +++ b/docs/mdsource/dangling-files.source.md @@ -75,6 +75,8 @@ use the `[OneTimeTearDown]` feature: snippet: DanglingSnapshotsNUnitUsage/DanglingSnapshots.cs +The NUnit runner prints a teardown failure but does not count it, so the run is summarised as passed. To keep a dangling snapshot from going unnoticed, the NUnit integration sets the process exit code to 1 when the check fails, and writes the reason after the runner's summary. + ### XUnitV3 @@ -85,3 +87,24 @@ snippet: DanglingSnapshotsXUnitV3Usage/DanglingSnapshots.cs Apply that collection to all tests: snippet: XunitV3DanglingCollection + + +### TUnit + +Use the `[After(TestSession)]` feature: + +snippet: DanglingSnapshotsTUnitUsage/DanglingSnapshots.cs + + +### Expecto + +Expecto test projects are console applications, so the check goes in the entry point, after the run: + +snippet: DanglingSnapshotsExpectoUsage/DanglingSnapshots.cs + + +### Fixie + +Fixie already requires an `IExecution` implementation for Verify. Add the check to the end of `Run`: + +snippet: DanglingSnapshotsFixieUsage/DanglingSnapshots.cs diff --git a/src/DanglingSnapshotsExpectoUsage/DanglingSnapshots.cs b/src/DanglingSnapshotsExpectoUsage/DanglingSnapshots.cs new file mode 100644 index 0000000000..e95d0a12d7 --- /dev/null +++ b/src/DanglingSnapshotsExpectoUsage/DanglingSnapshots.cs @@ -0,0 +1,7 @@ +#pragma warning disable VerifyDanglingSnapshots + +var result = Runner.RunTestsInAssemblyWithCLIArgs([], args); + +DanglingSnapshots.Run(); + +return result; diff --git a/src/DanglingSnapshotsExpectoUsage/DanglingSnapshotsExpectoUsage.csproj b/src/DanglingSnapshotsExpectoUsage/DanglingSnapshotsExpectoUsage.csproj new file mode 100644 index 0000000000..92ebfb1620 --- /dev/null +++ b/src/DanglingSnapshotsExpectoUsage/DanglingSnapshotsExpectoUsage.csproj @@ -0,0 +1,23 @@ + + + net11.0 + true + false + + false + true + Exe + False + + + + + + + + + + + diff --git a/src/DanglingSnapshotsExpectoUsage/Tests.IncorrectCase.verified.txt b/src/DanglingSnapshotsExpectoUsage/Tests.IncorrectCase.verified.txt new file mode 100644 index 0000000000..fdf74cdc4b --- /dev/null +++ b/src/DanglingSnapshotsExpectoUsage/Tests.IncorrectCase.verified.txt @@ -0,0 +1 @@ +Foo \ No newline at end of file diff --git a/src/DanglingSnapshotsExpectoUsage/Tests.Simple.verified.txt b/src/DanglingSnapshotsExpectoUsage/Tests.Simple.verified.txt new file mode 100644 index 0000000000..fdf74cdc4b --- /dev/null +++ b/src/DanglingSnapshotsExpectoUsage/Tests.Simple.verified.txt @@ -0,0 +1 @@ +Foo \ No newline at end of file diff --git a/src/DanglingSnapshotsExpectoUsage/Tests.cs b/src/DanglingSnapshotsExpectoUsage/Tests.cs new file mode 100644 index 0000000000..83c525969d --- /dev/null +++ b/src/DanglingSnapshotsExpectoUsage/Tests.cs @@ -0,0 +1,18 @@ +using Expecto; + +public class Tests +{ + [Tests] + public static Test Simple = Runner.TestCase( + nameof(Simple), + () => Verify( + name: nameof(Simple), + target: "Foo")); + + [Tests] + public static Test IncorrectCase = Runner.TestCase( + nameof(IncorrectCase), + () => Verify( + name: nameof(IncorrectCase), + target: "Foo")); +} diff --git a/src/DanglingSnapshotsFixieUsage/DanglingSnapshots.cs b/src/DanglingSnapshotsFixieUsage/DanglingSnapshots.cs new file mode 100644 index 0000000000..649e7ab1cf --- /dev/null +++ b/src/DanglingSnapshotsFixieUsage/DanglingSnapshots.cs @@ -0,0 +1,28 @@ +#pragma warning disable VerifyDanglingSnapshots + +public class TestProject : + ITestProject, + IExecution +{ + public void Configure(TestConfiguration configuration, TestEnvironment environment) + { + VerifierSettings.AssignTargetAssembly(environment.Assembly); + configuration.Conventions.Add(); + } + + public async Task Run(TestSuite testSuite) + { + foreach (var testClass in testSuite.TestClasses) + { + foreach (var test in testClass.Tests) + { + using (ExecutionState.Set(testClass, test, null)) + { + await test.Run(); + } + } + } + + DanglingSnapshots.Run(); + } +} diff --git a/src/DanglingSnapshotsFixieUsage/DanglingSnapshotsFixieUsage.csproj b/src/DanglingSnapshotsFixieUsage/DanglingSnapshotsFixieUsage.csproj new file mode 100644 index 0000000000..b1fc09dd7a --- /dev/null +++ b/src/DanglingSnapshotsFixieUsage/DanglingSnapshotsFixieUsage.csproj @@ -0,0 +1,17 @@ + + + net11.0 + true + false + true + false + + + + + + + + + + diff --git a/src/DanglingSnapshotsFixieUsage/Tests.IncorrectCase.verified.txt b/src/DanglingSnapshotsFixieUsage/Tests.IncorrectCase.verified.txt new file mode 100644 index 0000000000..fdf74cdc4b --- /dev/null +++ b/src/DanglingSnapshotsFixieUsage/Tests.IncorrectCase.verified.txt @@ -0,0 +1 @@ +Foo \ No newline at end of file diff --git a/src/DanglingSnapshotsFixieUsage/Tests.Simple.verified.txt b/src/DanglingSnapshotsFixieUsage/Tests.Simple.verified.txt new file mode 100644 index 0000000000..fdf74cdc4b --- /dev/null +++ b/src/DanglingSnapshotsFixieUsage/Tests.Simple.verified.txt @@ -0,0 +1 @@ +Foo \ No newline at end of file diff --git a/src/DanglingSnapshotsFixieUsage/Tests.cs b/src/DanglingSnapshotsFixieUsage/Tests.cs new file mode 100644 index 0000000000..47e4f9815e --- /dev/null +++ b/src/DanglingSnapshotsFixieUsage/Tests.cs @@ -0,0 +1,8 @@ +public class Tests +{ + public Task Simple() => + Verify("Foo"); + + public Task IncorrectCase() => + Verify("Foo"); +} diff --git a/src/DanglingSnapshotsTUnitUsage/DanglingSnapshots.cs b/src/DanglingSnapshotsTUnitUsage/DanglingSnapshots.cs new file mode 100644 index 0000000000..1ee42328a6 --- /dev/null +++ b/src/DanglingSnapshotsTUnitUsage/DanglingSnapshots.cs @@ -0,0 +1,8 @@ +#pragma warning disable VerifyDanglingSnapshots + +public static class Cleanup +{ + [After(TestSession)] + public static void Run() => + DanglingSnapshots.Run(); +} diff --git a/src/DanglingSnapshotsTUnitUsage/DanglingSnapshotsTUnitUsage.csproj b/src/DanglingSnapshotsTUnitUsage/DanglingSnapshotsTUnitUsage.csproj new file mode 100644 index 0000000000..828f4ad537 --- /dev/null +++ b/src/DanglingSnapshotsTUnitUsage/DanglingSnapshotsTUnitUsage.csproj @@ -0,0 +1,17 @@ + + + net11.0 + true + false + true + $(NoWarn);CA1822 + + + + + + + + + + diff --git a/src/DanglingSnapshotsTUnitUsage/Tests.IncorrectCase.verified.txt b/src/DanglingSnapshotsTUnitUsage/Tests.IncorrectCase.verified.txt new file mode 100644 index 0000000000..fdf74cdc4b --- /dev/null +++ b/src/DanglingSnapshotsTUnitUsage/Tests.IncorrectCase.verified.txt @@ -0,0 +1 @@ +Foo \ No newline at end of file diff --git a/src/DanglingSnapshotsTUnitUsage/Tests.Simple.verified.txt b/src/DanglingSnapshotsTUnitUsage/Tests.Simple.verified.txt new file mode 100644 index 0000000000..fdf74cdc4b --- /dev/null +++ b/src/DanglingSnapshotsTUnitUsage/Tests.Simple.verified.txt @@ -0,0 +1 @@ +Foo \ No newline at end of file diff --git a/src/DanglingSnapshotsTUnitUsage/Tests.cs b/src/DanglingSnapshotsTUnitUsage/Tests.cs new file mode 100644 index 0000000000..3ea3ae5a05 --- /dev/null +++ b/src/DanglingSnapshotsTUnitUsage/Tests.cs @@ -0,0 +1,10 @@ +public class Tests +{ + [Test] + public Task Simple() => + Verify("Foo"); + + [Test] + public Task IncorrectCase() => + Verify("Foo"); +} diff --git a/src/VerifyDangling.slnx b/src/VerifyDangling.slnx index e96e7d8ca1..4fe7e58b21 100644 --- a/src/VerifyDangling.slnx +++ b/src/VerifyDangling.slnx @@ -11,8 +11,11 @@ + + + diff --git a/src/appveyor.yml b/src/appveyor.yml index 539f3f4507..06f9f2aa87 100644 --- a/src/appveyor.yml +++ b/src/appveyor.yml @@ -64,6 +64,9 @@ build_script: - dotnet test %CD%/src/DanglingSnapshotsMSTestUsage --configuration Release --no-build --no-restore --verbosity minimal - dotnet test %CD%/src/DanglingSnapshotsNUnitUsage --configuration Release --no-build --no-restore --verbosity minimal - dotnet test %CD%/src/DanglingSnapshotsXunitV3Usage --configuration Release --no-build --no-restore --verbosity minimal +- dotnet run --project src/DanglingSnapshotsExpectoUsage --configuration Release --no-build --no-restore --verbosity minimal +- dotnet run --project src/DanglingSnapshotsTUnitUsage/DanglingSnapshotsTUnitUsage.csproj --configuration Release --no-build --no-restore --verbosity minimal +- cmd: pushd src && dotnet fixie DanglingSnapshotsFixieUsage --configuration Release --no-build && popd #- pwsh: | From 6ef97d0e10990fbead1831dbf97f7c987871215d Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Fri, 18 Sep 2026 16:14:00 +1000 Subject: [PATCH 6/7] Rename the second usage test from IncorrectCase to Second The name came from a deliberately mis-cased verified file that paired with it. That fixture went when the usage projects were made to pass in CI, so the name described nothing. It is only there to give each project more than one test. --- ...correctCase.verified.txt => Tests.Second.verified.txt} | 0 src/DanglingSnapshotsExpectoUsage/Tests.cs | 8 ++++---- ...correctCase.verified.txt => Tests.Second.verified.txt} | 0 src/DanglingSnapshotsFixieUsage/Tests.cs | 4 ++-- ...correctCase.verified.txt => Tests.Second.verified.txt} | 0 src/DanglingSnapshotsMSTestUsage/Tests.cs | 2 +- ...correctCase.verified.txt => Tests.Second.verified.txt} | 0 src/DanglingSnapshotsNUnitUsage/Tests.cs | 2 +- ...correctCase.verified.txt => Tests.Second.verified.txt} | 0 src/DanglingSnapshotsTUnitUsage/Tests.cs | 4 ++-- ...correctCase.verified.txt => Tests.Second.verified.txt} | 0 src/DanglingSnapshotsXunitV3Usage/Tests.cs | 2 +- 12 files changed, 11 insertions(+), 11 deletions(-) rename src/DanglingSnapshotsExpectoUsage/{Tests.IncorrectCase.verified.txt => Tests.Second.verified.txt} (100%) rename src/DanglingSnapshotsFixieUsage/{Tests.IncorrectCase.verified.txt => Tests.Second.verified.txt} (100%) rename src/DanglingSnapshotsMSTestUsage/{Tests.IncorrectCase.verified.txt => Tests.Second.verified.txt} (100%) rename src/DanglingSnapshotsNUnitUsage/{Tests.IncorrectCase.verified.txt => Tests.Second.verified.txt} (100%) rename src/DanglingSnapshotsTUnitUsage/{Tests.IncorrectCase.verified.txt => Tests.Second.verified.txt} (100%) rename src/DanglingSnapshotsXunitV3Usage/{Tests.IncorrectCase.verified.txt => Tests.Second.verified.txt} (100%) diff --git a/src/DanglingSnapshotsExpectoUsage/Tests.IncorrectCase.verified.txt b/src/DanglingSnapshotsExpectoUsage/Tests.Second.verified.txt similarity index 100% rename from src/DanglingSnapshotsExpectoUsage/Tests.IncorrectCase.verified.txt rename to src/DanglingSnapshotsExpectoUsage/Tests.Second.verified.txt diff --git a/src/DanglingSnapshotsExpectoUsage/Tests.cs b/src/DanglingSnapshotsExpectoUsage/Tests.cs index 83c525969d..87b79b7bcc 100644 --- a/src/DanglingSnapshotsExpectoUsage/Tests.cs +++ b/src/DanglingSnapshotsExpectoUsage/Tests.cs @@ -1,4 +1,4 @@ -using Expecto; +using Expecto; public class Tests { @@ -10,9 +10,9 @@ public class Tests target: "Foo")); [Tests] - public static Test IncorrectCase = Runner.TestCase( - nameof(IncorrectCase), + public static Test Second = Runner.TestCase( + nameof(Second), () => Verify( - name: nameof(IncorrectCase), + name: nameof(Second), target: "Foo")); } diff --git a/src/DanglingSnapshotsFixieUsage/Tests.IncorrectCase.verified.txt b/src/DanglingSnapshotsFixieUsage/Tests.Second.verified.txt similarity index 100% rename from src/DanglingSnapshotsFixieUsage/Tests.IncorrectCase.verified.txt rename to src/DanglingSnapshotsFixieUsage/Tests.Second.verified.txt diff --git a/src/DanglingSnapshotsFixieUsage/Tests.cs b/src/DanglingSnapshotsFixieUsage/Tests.cs index 47e4f9815e..24b18e90dc 100644 --- a/src/DanglingSnapshotsFixieUsage/Tests.cs +++ b/src/DanglingSnapshotsFixieUsage/Tests.cs @@ -1,8 +1,8 @@ -public class Tests +public class Tests { public Task Simple() => Verify("Foo"); - public Task IncorrectCase() => + public Task Second() => Verify("Foo"); } diff --git a/src/DanglingSnapshotsMSTestUsage/Tests.IncorrectCase.verified.txt b/src/DanglingSnapshotsMSTestUsage/Tests.Second.verified.txt similarity index 100% rename from src/DanglingSnapshotsMSTestUsage/Tests.IncorrectCase.verified.txt rename to src/DanglingSnapshotsMSTestUsage/Tests.Second.verified.txt diff --git a/src/DanglingSnapshotsMSTestUsage/Tests.cs b/src/DanglingSnapshotsMSTestUsage/Tests.cs index 3cac29a0d1..ff48076413 100644 --- a/src/DanglingSnapshotsMSTestUsage/Tests.cs +++ b/src/DanglingSnapshotsMSTestUsage/Tests.cs @@ -11,6 +11,6 @@ public Task Simple() => Verify("Foo"); [TestMethod] - public Task IncorrectCase() => + public Task Second() => Verify("Foo"); } \ No newline at end of file diff --git a/src/DanglingSnapshotsNUnitUsage/Tests.IncorrectCase.verified.txt b/src/DanglingSnapshotsNUnitUsage/Tests.Second.verified.txt similarity index 100% rename from src/DanglingSnapshotsNUnitUsage/Tests.IncorrectCase.verified.txt rename to src/DanglingSnapshotsNUnitUsage/Tests.Second.verified.txt diff --git a/src/DanglingSnapshotsNUnitUsage/Tests.cs b/src/DanglingSnapshotsNUnitUsage/Tests.cs index ab6c1d612f..f74ab098fc 100644 --- a/src/DanglingSnapshotsNUnitUsage/Tests.cs +++ b/src/DanglingSnapshotsNUnitUsage/Tests.cs @@ -6,6 +6,6 @@ public Task Simple() => Verify("Foo"); [Test] - public Task IncorrectCase() => + public Task Second() => Verify("Foo"); } \ No newline at end of file diff --git a/src/DanglingSnapshotsTUnitUsage/Tests.IncorrectCase.verified.txt b/src/DanglingSnapshotsTUnitUsage/Tests.Second.verified.txt similarity index 100% rename from src/DanglingSnapshotsTUnitUsage/Tests.IncorrectCase.verified.txt rename to src/DanglingSnapshotsTUnitUsage/Tests.Second.verified.txt diff --git a/src/DanglingSnapshotsTUnitUsage/Tests.cs b/src/DanglingSnapshotsTUnitUsage/Tests.cs index 3ea3ae5a05..fb2911ac01 100644 --- a/src/DanglingSnapshotsTUnitUsage/Tests.cs +++ b/src/DanglingSnapshotsTUnitUsage/Tests.cs @@ -1,10 +1,10 @@ -public class Tests +public class Tests { [Test] public Task Simple() => Verify("Foo"); [Test] - public Task IncorrectCase() => + public Task Second() => Verify("Foo"); } diff --git a/src/DanglingSnapshotsXunitV3Usage/Tests.IncorrectCase.verified.txt b/src/DanglingSnapshotsXunitV3Usage/Tests.Second.verified.txt similarity index 100% rename from src/DanglingSnapshotsXunitV3Usage/Tests.IncorrectCase.verified.txt rename to src/DanglingSnapshotsXunitV3Usage/Tests.Second.verified.txt diff --git a/src/DanglingSnapshotsXunitV3Usage/Tests.cs b/src/DanglingSnapshotsXunitV3Usage/Tests.cs index c588b8ba06..d9dbb6c046 100644 --- a/src/DanglingSnapshotsXunitV3Usage/Tests.cs +++ b/src/DanglingSnapshotsXunitV3Usage/Tests.cs @@ -8,7 +8,7 @@ public Task Simple() => #endregion [Fact] - public Task IncorrectCase() => + public Task Second() => Verify("Foo"); } From eac43e0153390decd21ee373fcdec09b21a17dce Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Fri, 18 Sep 2026 18:41:02 +1000 Subject: [PATCH 7/7] Drop the class level UsesVerify workaround The generator now skips static classes, so the assembly wide attribute no longer reaches the static [TestClass] hosting [AssemblyCleanup], and the class level placement it was moved to is redundant. --- src/DanglingSnapshotsMSTestUsage/AssemblyInfo.cs | 2 +- src/DanglingSnapshotsMSTestUsage/Tests.cs | 7 +------ 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/DanglingSnapshotsMSTestUsage/AssemblyInfo.cs b/src/DanglingSnapshotsMSTestUsage/AssemblyInfo.cs index f247aba7e8..68ac8f25ac 100644 --- a/src/DanglingSnapshotsMSTestUsage/AssemblyInfo.cs +++ b/src/DanglingSnapshotsMSTestUsage/AssemblyInfo.cs @@ -1,5 +1,5 @@ [assembly: Parallelize] // Without this every verification fails with "TestContext is null". Applied to the assembly rather // than the test class so that building this project also covers the generator skipping the static -// [TestClass] that hosts the [AssemblyCleanup] below. +// [TestClass] in DanglingSnapshots.cs, which an assembly wide attribute also reaches. [assembly: UsesVerify] diff --git a/src/DanglingSnapshotsMSTestUsage/Tests.cs b/src/DanglingSnapshotsMSTestUsage/Tests.cs index ff48076413..6524fb29bf 100644 --- a/src/DanglingSnapshotsMSTestUsage/Tests.cs +++ b/src/DanglingSnapshotsMSTestUsage/Tests.cs @@ -1,9 +1,4 @@ -// Without this the source generator never plumbs the TestContext through, and every verification -// fails with "TestContext is null" before it can produce a snapshot. Applied to the class rather -// than the assembly, since an assembly wide attribute also reaches the static Cleanup class, where -// the generated TestContext property does not compile. -[UsesVerify] -[TestClass] +[TestClass] public partial class Tests { [TestMethod]