diff --git a/README.md b/README.md index fd59a9a..2401053 100644 --- a/README.md +++ b/README.md @@ -90,6 +90,8 @@ Accepting through the owner rather than here is what keeps one writer per source Staged snapshots live in the intermediate (`obj`) directory of the test project, so, as with [recorded pairings](#recorded-pairings), the working directory has to contain `obj`. +A staged patch names the source file its snapshot belongs to, which is a separate question from where it was staged, so two kinds are passed over rather than listed. One whose source file is outside the working directory, since reviewing a directory does not reach out of it, which is the rule the queue already gets. And one whose source file has gone, since the literal is located by searching that file, so nothing can be accepted into it. `obj` keeps what a run staged until something clears it, and a deleted test leaves patches of the second kind behind. + ### Snapshots that cannot be accepted A snapshot that refuses says why, and does not stop the rest of the run being processed. Two cases: diff --git a/src/Directory.Packages.props b/src/Directory.Packages.props index eb925c7..95f724b 100644 --- a/src/Directory.Packages.props +++ b/src/Directory.Packages.props @@ -3,7 +3,7 @@ true - + @@ -14,9 +14,9 @@ - - - + + + diff --git a/src/Verify.Terminal.IntegrationTests/Harness.cs b/src/Verify.Terminal.IntegrationTests/Harness.cs index 4276613..0983d30 100644 --- a/src/Verify.Terminal.IntegrationTests/Harness.cs +++ b/src/Verify.Terminal.IntegrationTests/Harness.cs @@ -44,18 +44,21 @@ public Harness(string name) public DirectoryPath Directory { get; } // Verify writes maps to this test project's obj directory, but a real run scans a root that - // contains obj. So copy this scenario's maps under the harness directory to match that layout. - // Returns how many were copied, so a test can assert the map path is actually set up rather than - // silently falling back. + // contains obj. So this scenario's maps are moved under the harness directory to match that + // layout. Returns how many were moved, so a test can assert the map path is actually set up + // rather than silently falling back. + // + // Moved rather than copied: the paths in them are this scenario's temp directory, which goes + // when it does, so a copy leaves a map naming nothing behind in obj for every scenario ever run. public int PublishMaps() { - var copied = 0; + var moved = 0; var source = System.IO.Path.Combine( AttributeReader.GetIntermediateDirectory(typeof(Harness).Assembly), "VerifyReceived"); if (!System.IO.Directory.Exists(source)) { - return copied; + return moved; } var target = System.IO.Path.Combine(_directory, "obj", "VerifyReceived"); @@ -67,12 +70,12 @@ public int PublishMaps() if (lines.Length > 0 && lines[0].StartsWith(_directory, StringComparison.OrdinalIgnoreCase)) { - File.Copy(file, System.IO.Path.Combine(target, System.IO.Path.GetFileName(file)), true); - copied++; + File.Move(file, System.IO.Path.Combine(target, System.IO.Path.GetFileName(file)), true); + moved++; } } - return copied; + return moved; } public VerifySettings CreateSettings() @@ -121,18 +124,22 @@ public string ReadSource() => File.ReadAllText(System.IO.Path.Combine(_directory, "SampleTests.cs")); // Verify stages inline snapshots in this test project's obj directory, but a real run scans a - // root that contains obj. So copy this scenario's staged files under the harness directory to - // match that layout. Returns how many were copied, so a test can assert that staging really + // root that contains obj. So this scenario's staged files are moved under the harness directory + // to match that layout. Returns how many were moved, so a test can assert that staging really // happened rather than silently finding nothing. + // + // Moved rather than copied, for the reason PublishMaps is: these patches name a source file in + // this scenario's temp directory, so a copy leaves a trio naming a file that has gone behind in + // obj, once per scenario per run, for whatever scans this project next. public int PublishInline() { - var copied = 0; + var moved = 0; var source = System.IO.Path.Combine( AttributeReader.GetIntermediateDirectory(typeof(Harness).Assembly), InlineSnapshotFinder.StagingDirectoryName); if (!System.IO.Directory.Exists(source)) { - return copied; + return moved; } var target = System.IO.Path.Combine(_directory, "obj", InlineSnapshotFinder.StagingDirectoryName); @@ -151,16 +158,16 @@ public int PublishInline() var stem = System.IO.Path.GetFileNameWithoutExtension(patch); foreach (var file in System.IO.Directory.GetFiles(source, $"{stem}.*")) { - File.Copy( + File.Move( file, System.IO.Path.Combine(target, System.IO.Path.GetFileName(file)), true); } - copied++; + moved++; } - return copied; + return moved; } // What the harness directory holds once PublishInline has run, which is what a scan reads and @@ -255,6 +262,8 @@ public void Dispose() System.Environment.SetEnvironmentVariable(DiffEngine.DiffRunner.InlineViewerVariable, _inlineViewer); System.Environment.SetEnvironmentVariable(MaxInstancesVariable, _maxInstances); + ClearIntermediate(); + try { System.IO.Directory.Delete(_directory, recursive: true); @@ -264,4 +273,56 @@ public void Dispose() // Best effort cleanup of the temp directory. } } + + // Verify writes into this test project's obj as it runs: a map for every received file it + // leaves, and a staged trio for an inline snapshot nothing owned. What a scenario published was + // moved, but one that published nothing, or wrote more afterwards, leaves the rest naming a + // temp directory that goes with this harness. Taken here rather than left to accumulate a + // run's worth per run, for whatever scans this project next. + private void ClearIntermediate() + { + var intermediate = AttributeReader.GetIntermediateDirectory(typeof(Harness).Assembly); + + // A map holds the received path on its first line, so the file itself says whose it is. + ClearNaming( + System.IO.Path.Combine(intermediate, "VerifyReceived"), + "*", + _ => System.IO.Path.GetFileName(_)); + + // Only the patch of a staged trio names the source; the two texts beside it are snapshot + // content, and are found by the name they share with it. + ClearNaming( + System.IO.Path.Combine(intermediate, InlineSnapshotFinder.StagingDirectoryName), + "*.inlinepatch", + _ => $"{System.IO.Path.GetFileNameWithoutExtension(_)}.*"); + } + + private void ClearNaming(string directory, string pattern, Func companions) + { + if (!System.IO.Directory.Exists(directory)) + { + return; + } + + foreach (var file in System.IO.Directory.GetFiles(directory, pattern)) + { + try + { + if (!File.ReadAllText(file).Contains(_directory, StringComparison.OrdinalIgnoreCase)) + { + continue; + } + + foreach (var owned in System.IO.Directory.GetFiles(directory, companions(file))) + { + File.Delete(owned); + } + } + catch (Exception exception) + when (exception is IOException or UnauthorizedAccessException) + { + // Best effort: a file left here is noise in obj rather than a failed scenario. + } + } + } } diff --git a/src/Verify.Terminal.Tests/InlineSnapshotFinderTests.cs b/src/Verify.Terminal.Tests/InlineSnapshotFinderTests.cs index 7420a53..232eba0 100644 --- a/src/Verify.Terminal.Tests/InlineSnapshotFinderTests.cs +++ b/src/Verify.Terminal.Tests/InlineSnapshotFinderTests.cs @@ -124,6 +124,31 @@ public void Should_Flag_A_Conflicted_Queued_Snapshot() result.Conflict.ShouldBe("net8.0 / net10.0"); } + [Fact] + public void Should_Ignore_A_Staged_Snapshot_Outside_The_Root() + { + // Staging sits under a project's obj, but the patch inside it names wherever the source + // was. Reviewing a directory must not reach out of it, which is the rule the queue gets. + var fileSystem = CreateFileSystem(); + InlineTestData.Stage( + fileSystem, + InlineTestData.Patch("new snapshot", source: "/Other/src/SampleTests.cs")); + + Find(fileSystem).ShouldBeEmpty(); + } + + [Fact] + public void Should_Ignore_A_Staged_Snapshot_Whose_Source_Is_Gone() + { + // The literal is located by searching the source file, so a patch naming one that has gone + // can never apply. Offering it is offering something no accept can take. + var fileSystem = CreateFileSystem(); + InlineTestData.Stage(fileSystem, InlineTestData.Patch("new snapshot")); + fileSystem.GetFile(InlineTestData.SourceFile).Delete(); + + Find(fileSystem).ShouldBeEmpty(); + } + [Fact] public void Should_Ignore_Queued_Snapshots_Outside_The_Root() { diff --git a/src/Verify.Terminal.Tests/InlineSnapshotManagerTests.cs b/src/Verify.Terminal.Tests/InlineSnapshotManagerTests.cs index 557266c..9c3ec31 100644 --- a/src/Verify.Terminal.Tests/InlineSnapshotManagerTests.cs +++ b/src/Verify.Terminal.Tests/InlineSnapshotManagerTests.cs @@ -1,4 +1,4 @@ -namespace Verify.Terminal.Tests; +namespace Verify.Terminal.Tests; public sealed class InlineSnapshotManagerTests { @@ -77,10 +77,10 @@ public Task Sample() => var fileSystem = CreateFileSystem(); var patch = InlineTestData.Patch("new snapshot", line: 4, source: source.Path); - InlineTestData.Stage(fileSystem, patch); + var staged = InlineTestData.Stage(fileSystem, patch); var queue = new FakeInlineQueueOwner { AcceptOutcome = InlineAcceptOutcome.Unknown }; - var snapshot = new InlineSnapshot(patch, isQueued: true, Staged(fileSystem, patch)); + var snapshot = new InlineSnapshot(patch, isQueued: true, [staged]); Create(queue, fileSystem).Accept(snapshot).Succeeded.ShouldBeTrue(); @@ -95,10 +95,10 @@ public void Should_Clear_The_Staged_Files_When_The_Owner_Accepts() { var fileSystem = CreateFileSystem(); var patch = InlineTestData.Patch("new snapshot"); - InlineTestData.Stage(fileSystem, patch); + var staged = InlineTestData.Stage(fileSystem, patch); var queue = new FakeInlineQueueOwner { AcceptOutcome = InlineAcceptOutcome.Accepted }; - var snapshot = new InlineSnapshot(patch, isQueued: true, Staged(fileSystem, patch)); + var snapshot = new InlineSnapshot(patch, isQueued: true, [staged]); Create(queue, fileSystem).Accept(snapshot).Succeeded.ShouldBeTrue(); @@ -119,9 +119,9 @@ public Task Sample() => var fileSystem = CreateFileSystem(); var patch = InlineTestData.Patch("new snapshot", line: 4, source: source.Path); - InlineTestData.Stage(fileSystem, patch); + var staged = InlineTestData.Stage(fileSystem, patch); - var snapshot = new InlineSnapshot(patch, isQueued: false, Staged(fileSystem, patch)); + var snapshot = new InlineSnapshot(patch, isQueued: false, [staged]); Create(fileSystem: fileSystem).Accept(snapshot).Succeeded.ShouldBeTrue(); @@ -146,9 +146,9 @@ public Task Sample() => var fileSystem = CreateFileSystem(); var patch = InlineTestData.Patch("new snapshot", line: 4, source: source.Path); - InlineTestData.Stage(fileSystem, patch); + var staged = InlineTestData.Stage(fileSystem, patch); - var snapshot = new InlineSnapshot(patch, isQueued: false, Staged(fileSystem, patch)); + var snapshot = new InlineSnapshot(patch, isQueued: false, [staged]); var result = Create(fileSystem: fileSystem).Accept(snapshot); @@ -211,7 +211,7 @@ public void Should_Treat_A_Discard_The_Owner_Could_Not_Be_Asked_About_As_Done() // staging is cleared and the reject stands rather than reporting a failure it cannot back. var fileSystem = CreateFileSystem(); var patch = InlineTestData.Patch("new snapshot"); - InlineTestData.Stage(fileSystem, patch); + var staged = InlineTestData.Stage(fileSystem, patch); var queue = new FakeInlineQueueOwner { @@ -220,7 +220,7 @@ public void Should_Treat_A_Discard_The_Owner_Could_Not_Be_Asked_About_As_Done() StillPendingResult = null, }; - var snapshot = new InlineSnapshot(patch, isQueued: true, Staged(fileSystem, patch)); + var snapshot = new InlineSnapshot(patch, isQueued: true, [staged]); Create(queue, fileSystem).Reject(snapshot).Succeeded.ShouldBeTrue(); @@ -232,9 +232,9 @@ public void Should_Clear_The_Staged_Files_On_Reject() { var fileSystem = CreateFileSystem(); var patch = InlineTestData.Patch("new snapshot"); - InlineTestData.Stage(fileSystem, patch); + var staged = InlineTestData.Stage(fileSystem, patch); - var snapshot = new InlineSnapshot(patch, isQueued: false, Staged(fileSystem, patch)); + var snapshot = new InlineSnapshot(patch, isQueued: false, [staged]); Create(fileSystem: fileSystem).Reject(snapshot).Succeeded.ShouldBeTrue(); @@ -249,24 +249,10 @@ private static InlineSnapshotManager Create( FakeFileSystem? fileSystem = null) => new(fileSystem ?? CreateFileSystem(), queue ?? new FakeInlineQueueOwner()); - // The staged trio as the finder builds it, for a test that starts from the manager instead. - private static IReadOnlyList Staged(FakeFileSystem fileSystem, InlinePatch patch) - { - var globber = new Globber(fileSystem, new FakeEnvironment(PlatformFamily.Linux)); - var finder = new InlineSnapshotFinder( - globber, - new FakeEnvironment(PlatformFamily.Linux), - fileSystem, - new FakeInlineQueueOwner()); - - // Through the finder rather than by hand, so the manager is handed what it is handed in a - // real run: the patch as it was read back off disk, beside the files it was read from. - return finder - .Find(new("/Working")) - .SelectMany(_ => _.Staged) - .ToList(); - } - + // The staged trio comes from InlineTestData, which writes it and hands back what a scan would + // report. Not through the finder: the sources below are real files in a temp directory, so an + // accept has something to rewrite, and a scan declines a patch whose source is outside the root + // it was pointed at. private static IReadOnlyList StagedFiles(FakeFileSystem fileSystem) { var globber = new Globber(fileSystem, new FakeEnvironment(PlatformFamily.Linux)); diff --git a/src/Verify.Terminal.Tests/Utilities/InlineTestData.cs b/src/Verify.Terminal.Tests/Utilities/InlineTestData.cs index 5edccb4..d19a1c2 100644 --- a/src/Verify.Terminal.Tests/Utilities/InlineTestData.cs +++ b/src/Verify.Terminal.Tests/Utilities/InlineTestData.cs @@ -21,8 +21,9 @@ public static InlinePatch Patch( }; // Verify names the staged files `{type}.{method}.{hash}.{runtime}`, and writes the two texts - // beside the patch. - public static void Stage( + // beside the patch. Returns the trio as a scan would report it, for a test that starts from + // the manager rather than from a scan. + public static StagedInline Stage( FakeFileSystem fileSystem, InlinePatch patch, string runtime = "DotNet10_0", @@ -30,13 +31,22 @@ public static void Stage( { var name = $"SampleTests.Sample.a1b2c3d4.{runtime}"; + // The source the patch names, which a run stages beside rather than instead of: the + // snapshot it holds is a literal in that file, and an accept rewrites it there. + if (!fileSystem.Exist(new FilePath(patch.SourceFile))) + { + fileSystem + .CreateFile(new FilePath(patch.SourceFile)) + .SetTextContent("// the test this snapshot belongs to"); + } + fileSystem .CreateFile($"{StagingDirectory}/{name}.inlinepatch") .SetTextContent(InlinePatchFile.Build(patch)); if (!withTexts) { - return; + return new(patch, new($"{StagingDirectory}/{name}.inlinepatch"), null, null, runtime); } fileSystem @@ -45,5 +55,12 @@ public static void Stage( fileSystem .CreateFile($"{StagingDirectory}/{name}.expected.txt") .SetTextContent(patch.OriginalValue ?? string.Empty); + + return new( + patch, + new($"{StagingDirectory}/{name}.inlinepatch"), + new($"{StagingDirectory}/{name}.received.txt"), + new($"{StagingDirectory}/{name}.expected.txt"), + runtime); } } diff --git a/src/Verify.Terminal/InlineSnapshotFinder.cs b/src/Verify.Terminal/InlineSnapshotFinder.cs index 370dc11..61bf37e 100644 --- a/src/Verify.Terminal/InlineSnapshotFinder.cs +++ b/src/Verify.Terminal/InlineSnapshotFinder.cs @@ -108,7 +108,8 @@ private Dictionary> Staged(DirectoryPath root) foreach (var path in paths) { if (!TryReadPatch(path, out var patch) || - !IsReviewable(patch)) + !IsReviewable(patch) || + !HasReviewableSource(root, patch)) { continue; } @@ -151,6 +152,24 @@ private static InlineSnapshot FromStaged(List staged) private static bool IsReviewable(InlinePatch patch) => patch.Mode != InlinePatchMode.Remove; + /// + /// Whether the source file a staged patch names is one this scan can offer. + /// + /// + /// Staging sits under a project's obj, but the patch inside it names wherever the source was, + /// which is not the same question. A queued snapshot is asked both of these by the owner and by + /// ; a staged one was asked neither, so a directory full of patches from + /// runs whose sources have since gone was offered as pending, and none of it could be accepted. + /// + /// Under the root, because reviewing a directory must not reach out of it, which is the rule the + /// queue already gets. And still on disk, because the literal is located by searching that file: + /// a patch naming one that has gone can never apply, whatever is done to it here. + /// + /// + private bool HasReviewableSource(DirectoryPath root, InlinePatch patch) => + IsUnder(root, patch.SourceFile) && + _fileSystem.File.Exists(new FilePath(patch.SourceFile).MakeAbsolute(_environment)); + private bool IsUnder(DirectoryPath root, string path) { var prefix = root.FullPath.TrimEnd('/') + "/";