From 355cca55d1ea4c4cd14696d78b76667b73d9b9d5 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 06:27:47 +0000 Subject: [PATCH 1/2] Show the files Deduplicate will delete before asking to confirm Deduplicate printed only a count and its keep-the-shortest-name policy before the "Proceed with deletion? (y/N)" prompt; the paths appeared afterwards, one per file already deleted. Scan and DryRun both print the full per-group KEEP/DELETE listing, so the one verb where seeing it matters was the one that omitted it, and the confirmation gate asked the user to approve an outcome they could not see. Extract that listing into DuplicateReport.PlanDeletions, which computes the keeper with the same Deduplicator.SelectFileToKeep the delete path uses, and call it from Deduplicate before the prompt and from DryRun in place of its own copy. FormatBytes was duplicated in four verbs and moves there too. DryRun's and Scan's output is unchanged byte for byte. The listing is not capped or paged: the point is that every path about to be deleted is on screen before the question is asked. Fixes #114 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01By4ipNgZKvDwoS2HUmjupA --- .../DeduplicateConfirmationTests.cs | 150 ++++++++++++++++++ FileDeduplicator.Test/DuplicateReportTests.cs | 96 +++++++++++ FileDeduplicator/DuplicateReport.cs | 105 ++++++++++++ FileDeduplicator/Verbs/Deduplicate.cs | 26 +-- FileDeduplicator/Verbs/DryRun.cs | 37 +---- FileDeduplicator/Verbs/Scan.cs | 12 +- FileDeduplicator/Verbs/Stats.cs | 14 +- README.md | 2 +- 8 files changed, 376 insertions(+), 66 deletions(-) create mode 100644 FileDeduplicator.Test/DeduplicateConfirmationTests.cs create mode 100644 FileDeduplicator.Test/DuplicateReportTests.cs create mode 100644 FileDeduplicator/DuplicateReport.cs diff --git a/FileDeduplicator.Test/DeduplicateConfirmationTests.cs b/FileDeduplicator.Test/DeduplicateConfirmationTests.cs new file mode 100644 index 0000000..e61f05c --- /dev/null +++ b/FileDeduplicator.Test/DeduplicateConfirmationTests.cs @@ -0,0 +1,150 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.FileDeduplicator.Test; + +using ktsu.FileDeduplicator.Verbs; +using ktsu.Semantics.Paths; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Tests that the Deduplicate verb shows which copies it is about to delete before it asks for +/// permission to delete them. +/// +/// +/// The confirmation prompt is the only gate in front of an irreversible deletion, and "keep the +/// shortest filename" is a blunt enough policy to pick the wrong survivor -- r.pdf over +/// report-final-DO-NOT-DELETE.pdf. A prompt that states only a count asks the user to approve an +/// outcome they cannot see, which makes the gate decorative. These tests pin the ordering: the +/// paths come first, the question second. +/// +/// They drive the verb through with the console redirected, because +/// the ordering is a property of that method and of nothing else -- a unit test of the listing +/// helper alone would still pass if the call were left out or moved below the prompt. +/// +[TestClass] +[DoNotParallelize] +public sealed class DeduplicateConfirmationTests +{ + /// + /// Runs the Deduplicate verb over a tree, answering "n" at the confirmation prompt, and + /// returns everything it wrote. + /// + /// The directory to deduplicate. + /// The verb's console output. + private static string RunDeclining(AbsoluteDirectoryPath root) + { + TextWriter originalOut = Console.Out; + TextReader originalIn = Console.In; + + try + { + using StringWriter captured = new(); + using StringReader answers = new("n"); + Console.SetOut(captured); + Console.SetIn(answers); + + Deduplicate verb = new() { PathString = root.WeakString }; + verb.Run(); + + return captured.ToString(); + } + finally + { + Console.SetOut(originalOut); + Console.SetIn(originalIn); + } + } + + /// + /// Collapses runs of whitespace so the assertions describe the listing's content rather than + /// the column its markers happen to sit in. + /// + /// The captured console output. + /// The output with each line trimmed and its internal whitespace collapsed. + private static string Normalize(string output) => + string.Join('\n', output.Split('\n').Select(line => string.Join(' ', line.Split((char[]?)null, StringSplitOptions.RemoveEmptyEntries)))); + + /// + /// Every copy that would be deleted has to be named before the prompt, not after the deletion. + /// + [TestMethod] + public void EveryPathToBeDeletedIsNamedBeforeTheConfirmationPrompt() + { + // Arrange -- the blunt-policy case: the descriptive name loses to the short one + using TempTree tree = new(); + AbsoluteFilePath keeper = tree.Write("r.pdf", "the only copy that survives"); + AbsoluteFilePath doomed = tree.Write("report-final-DO-NOT-DELETE.pdf", "the only copy that survives"); + AbsoluteFilePath alsoDoomed = tree.Write("archive/report-2026-backup.pdf", "the only copy that survives"); + + // Act + string output = Normalize(RunDeclining(tree.Root)); + int prompt = output.IndexOf("Proceed with deletion?", StringComparison.Ordinal); + string beforePrompt = prompt < 0 ? string.Empty : output[..prompt]; + + // Assert + Assert.AreNotEqual(-1, prompt, $"The verb never reached the confirmation prompt. Output was:\n{output}"); + Assert.Contains($"DELETE: {doomed}", beforePrompt, $"The prompt was shown without naming {doomed}. Output before it was:\n{beforePrompt}"); + Assert.Contains($"DELETE: {alsoDoomed}", beforePrompt, $"The prompt was shown without naming {alsoDoomed}. Output before it was:\n{beforePrompt}"); + Assert.Contains($"KEEP: {keeper}", beforePrompt, $"The prompt was shown without naming the copy being kept. Output before it was:\n{beforePrompt}"); + } + + /// + /// The listing has to cover every group, not just the first one -- a user scrolling past a + /// truncated listing would approve deletions they never saw. + /// + [TestMethod] + public void EveryDuplicateGroupAppearsInTheListing() + { + // Arrange + using TempTree tree = new(); + List doomed = + [ + tree.Write("group1/aa.txt", "alpha"), + tree.Write("group2/bb.txt", "beta"), + tree.Write("group3/nested/cc.txt", "gamma"), + ]; + _ = tree.Write("group1/a.txt", "alpha"); + _ = tree.Write("group2/b.txt", "beta"); + _ = tree.Write("group3/c.txt", "gamma"); + + // Act + string output = Normalize(RunDeclining(tree.Root)); + string beforePrompt = output[..output.IndexOf("Proceed with deletion?", StringComparison.Ordinal)]; + + // Assert + foreach (AbsoluteFilePath file in doomed) + { + Assert.Contains($"DELETE: {file}", beforePrompt, $"{file} was not listed before the prompt."); + } + } + + /// + /// Showing the listing must not have turned the preview into the deletion: declining still + /// leaves every file on disk. + /// + [TestMethod] + public void DecliningTheConfirmationLeavesEveryFileOnDisk() + { + // Arrange + using TempTree tree = new(); + List all = + [ + tree.Write("a.txt", "alpha"), + tree.Write("aa.txt", "alpha"), + tree.Write("b.txt", "beta"), + tree.Write("bb.txt", "beta"), + ]; + + // Act + string output = RunDeclining(tree.Root); + + // Assert + Assert.Contains("Aborted.", output); + + foreach (AbsoluteFilePath file in all) + { + Assert.IsTrue(TempTree.Exists(file), $"{file} was deleted despite the confirmation being declined."); + } + } +} diff --git a/FileDeduplicator.Test/DuplicateReportTests.cs b/FileDeduplicator.Test/DuplicateReportTests.cs new file mode 100644 index 0000000..5255671 --- /dev/null +++ b/FileDeduplicator.Test/DuplicateReportTests.cs @@ -0,0 +1,96 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.FileDeduplicator.Test; + +using ktsu.Semantics.Paths; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Tests the listing and totals that Deduplicate and DryRun both print. +/// +/// +/// Both verbs now read their numbers from here, so an error in this one place is an error the +/// preview and the confirmation would agree on -- which is exactly the kind of wrong that nobody +/// catches by reading the output. +/// +[TestClass] +public sealed class DuplicateReportTests +{ + private static IReadOnlyList DuplicatesIn(TempTree tree) => + Deduplicator.FindDuplicates(Deduplicator.GroupByHash(FileHasher.HashFiles(FileScanner.ScanForFiles(tree.Root)))); + + /// + /// The plan counts every copy except each group's keeper, and the bytes those copies occupy. + /// + [TestMethod] + public void PlanCountsEveryCopyExceptEachGroupsKeeper() + { + // Arrange -- 3 copies of a 5-byte content and 2 of a 4-byte one, so 2 + 1 deletions + using TempTree tree = new(); + _ = tree.Write("a.txt", "alpha"); + _ = tree.Write("aa.txt", "alpha"); + _ = tree.Write("aaa.txt", "alpha"); + _ = tree.Write("b.txt", "beta"); + _ = tree.Write("bb.txt", "beta"); + _ = tree.Write("unique.txt", "gamma-and-then-some"); + + // Act + DeletionPlan plan = DuplicateReport.PlanDeletions(DuplicatesIn(tree)); + + // Assert + Assert.AreEqual(3, plan.FileCount); + Assert.AreEqual((2 * 5L) + 4L, plan.BytesReclaimable); + } + + /// + /// The listing names one keeper and every other copy in each group, so the two sets together + /// account for every file the group holds. + /// + [TestMethod] + public void ListingNamesOneKeeperAndEveryOtherCopyPerGroup() + { + // Arrange + using TempTree tree = new(); + _ = tree.Write("a.txt", "alpha"); + _ = tree.Write("aa.txt", "alpha"); + _ = tree.Write("nested/aaa.txt", "alpha"); + + IReadOnlyList duplicates = DuplicatesIn(tree); + + // Act + DeletionPlan plan = DuplicateReport.PlanDeletions(duplicates); + List keeps = [.. plan.Listing.Where(l => l.Contains("KEEP:", StringComparison.Ordinal))]; + List deletes = [.. plan.Listing.Where(l => l.Contains("DELETE:", StringComparison.Ordinal))]; + + // Assert + Assert.HasCount(1, duplicates); + Assert.HasCount(1, keeps); + Assert.HasCount(2, deletes); + + AbsoluteFilePath keeper = Deduplicator.SelectFileToKeep(duplicates[0].Files); + Assert.Contains(keeper.WeakString, keeps[0]); + + foreach (AbsoluteFilePath file in duplicates[0].Files.Where(f => f != keeper)) + { + Assert.IsTrue( + deletes.Exists(l => l.Contains(file.WeakString, StringComparison.Ordinal)), + $"{file} was not listed for deletion."); + } + } + + /// + /// A run with nothing to delete produces nothing to read. + /// + [TestMethod] + public void PlanForNoDuplicatesIsEmpty() + { + // Act + DeletionPlan plan = DuplicateReport.PlanDeletions([]); + + // Assert + Assert.IsEmpty(plan.Listing); + Assert.AreEqual(0, plan.FileCount); + Assert.AreEqual(0L, plan.BytesReclaimable); + } +} diff --git a/FileDeduplicator/DuplicateReport.cs b/FileDeduplicator/DuplicateReport.cs new file mode 100644 index 0000000..f14d4b9 --- /dev/null +++ b/FileDeduplicator/DuplicateReport.cs @@ -0,0 +1,105 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.FileDeduplicator; + +using System.Collections.Generic; + +using ktsu.Semantics.Paths; + +/// +/// Renders what deduplication would do to a set of duplicate groups, without doing any of it. +/// +/// +/// Every verb that talks about duplicates has to answer the same two questions -- which copy +/// survives each group, and which copies do not -- and answer them identically, because the user +/// reads one verb's answer and then acts on another's. Building the listing here, from the same +/// the delete path uses, is what keeps the preview and +/// the deletion from drifting apart. +/// +internal static class DuplicateReport +{ + /// + /// Works out which copies would be deleted, and renders the per-group KEEP/DELETE listing. + /// + /// The duplicate groups to describe. + /// The listing, plus the totals that go with it. + internal static DeletionPlan PlanDeletions(IReadOnlyList duplicates) + { + List listing = []; + int fileCount = 0; + long bytesReclaimable = 0; + + foreach (DuplicateGroup group in duplicates) + { + AbsoluteFilePath keeper = Deduplicator.SelectFileToKeep(group.Files); + + listing.Add($" Hash: {group.Hash[..12]}... ({FormatBytes(group.FileSize)}, {group.Files.Count} copies)"); + listing.Add($" KEEP: {keeper}"); + + foreach (AbsoluteFilePath file in group.Files) + { + if (file == keeper) + { + continue; + } + + listing.Add($" DELETE: {file}"); + fileCount++; + bytesReclaimable += group.FileSize; + } + + listing.Add(string.Empty); + } + + return new DeletionPlan(listing, fileCount, bytesReclaimable); + } + + /// + /// Writes a listing produced by to the console. + /// + /// The lines to write. + internal static void WriteListing(IReadOnlyList listing) + { + foreach (string line in listing) + { + Console.WriteLine(line); + } + } + + /// + /// Renders a byte count in the largest unit that leaves it above one. + /// + /// The count to render. + /// The rendered size. + internal static string FormatBytes(long bytes) => bytes switch + { + < 1024L => $"{bytes} B", + < 1024L * 1024 => $"{bytes / 1024.0:F1} KB", + < 1024L * 1024 * 1024 => $"{bytes / (1024.0 * 1024.0):F1} MB", + _ => $"{bytes / (1024.0 * 1024.0 * 1024.0):F1} GB", + }; +} + +/// +/// What deduplication would delete, described but not yet done. +/// +/// The per-group KEEP/DELETE lines, ready to print. +/// How many copies would be deleted. +/// How many bytes deleting them would free. +internal sealed class DeletionPlan(IReadOnlyList listing, int fileCount, long bytesReclaimable) +{ + /// + /// Gets the per-group KEEP/DELETE lines, in the order they should be printed. + /// + internal IReadOnlyList Listing { get; } = listing; + + /// + /// Gets the number of copies that would be deleted. + /// + internal int FileCount { get; } = fileCount; + + /// + /// Gets the number of bytes deleting them would free. + /// + internal long BytesReclaimable { get; } = bytesReclaimable; +} diff --git a/FileDeduplicator/Verbs/Deduplicate.cs b/FileDeduplicator/Verbs/Deduplicate.cs index de3e770..6341f03 100644 --- a/FileDeduplicator/Verbs/Deduplicate.cs +++ b/FileDeduplicator/Verbs/Deduplicate.cs @@ -60,11 +60,23 @@ internal override void Run(Deduplicate options) return; } + // Step 4: Show exactly which copies go and which one stays. "Shortest filename wins" is + // a blunt enough policy -- report-final-DO-NOT-DELETE.pdf loses to r.pdf -- that seeing + // the paths is the only way to catch a bad outcome while it is still reversible. Scan + // and DryRun already print this; the verb that actually deletes is the one that needs it. + DeletionPlan plan = DuplicateReport.PlanDeletions(duplicates); + Console.WriteLine($"Found {duplicates.Count} group(s) of duplicate files."); Console.WriteLine("Keeping the copy with the shortest filename in each group."); Console.WriteLine(); - // Step 4: Confirm with user + DuplicateReport.WriteListing(plan.Listing); + + Console.WriteLine($"Files to delete: {plan.FileCount}"); + Console.WriteLine($"Space to reclaim: {DuplicateReport.FormatBytes(plan.BytesReclaimable)}"); + Console.WriteLine(); + + // Step 5: Confirm with user Console.Write("Proceed with deletion? (y/N): "); string? confirmation = Console.ReadLine()?.Trim(); if (!string.Equals(confirmation, "y", StringComparison.OrdinalIgnoreCase)) @@ -75,13 +87,13 @@ internal override void Run(Deduplicate options) Console.WriteLine(); - // Step 5: Delete duplicates + // Step 6: Delete duplicates Console.WriteLine("Deleting duplicates..."); DeduplicationResult result = Deduplicator.DeleteDuplicates(duplicates); Console.WriteLine(); Console.WriteLine($"Deleted {result.DeletedCount} file(s)."); - Console.WriteLine($"Reclaimed {FormatBytes(result.BytesReclaimed)} of disk space."); + Console.WriteLine($"Reclaimed {DuplicateReport.FormatBytes(result.BytesReclaimed)} of disk space."); if (result.SkippedFiles.Count > 0) { @@ -100,12 +112,4 @@ internal override void Run(Deduplicate options) PathString = "."; } - - private static string FormatBytes(long bytes) => bytes switch - { - < 1024L => $"{bytes} B", - < 1024L * 1024 => $"{bytes / 1024.0:F1} KB", - < 1024L * 1024 * 1024 => $"{bytes / (1024.0 * 1024.0):F1} MB", - _ => $"{bytes / (1024.0 * 1024.0 * 1024.0):F1} GB", - }; } diff --git a/FileDeduplicator/Verbs/DryRun.cs b/FileDeduplicator/Verbs/DryRun.cs index 0b82186..f241d47 100644 --- a/FileDeduplicator/Verbs/DryRun.cs +++ b/FileDeduplicator/Verbs/DryRun.cs @@ -60,47 +60,18 @@ internal override void Run(DryRun options) return; } - long totalReclaimable = 0; - int totalDeletions = 0; + DeletionPlan plan = DuplicateReport.PlanDeletions(duplicates); Console.WriteLine($"Found {duplicates.Count} group(s) of duplicate files:"); Console.WriteLine(); - foreach (DuplicateGroup group in duplicates) - { - AbsoluteFilePath keeper = Deduplicator.SelectFileToKeep(group.Files); - - Console.WriteLine($" Hash: {group.Hash[..12]}... ({FormatBytes(group.FileSize)}, {group.Files.Count} copies)"); - Console.WriteLine($" KEEP: {keeper}"); - - foreach (AbsoluteFilePath file in group.Files) - { - if (file == keeper) - { - continue; - } - - Console.WriteLine($" DELETE: {file}"); - totalDeletions++; - totalReclaimable += group.FileSize; - } - - Console.WriteLine(); - } + DuplicateReport.WriteListing(plan.Listing); Console.WriteLine("--- Dry Run Summary ---"); Console.WriteLine($"Duplicate groups: {duplicates.Count}"); - Console.WriteLine($"Files to delete: {totalDeletions}"); - Console.WriteLine($"Space to reclaim: {FormatBytes(totalReclaimable)}"); + Console.WriteLine($"Files to delete: {plan.FileCount}"); + Console.WriteLine($"Space to reclaim: {DuplicateReport.FormatBytes(plan.BytesReclaimable)}"); PathString = "."; } - - private static string FormatBytes(long bytes) => bytes switch - { - < 1024L => $"{bytes} B", - < 1024L * 1024 => $"{bytes / 1024.0:F1} KB", - < 1024L * 1024 * 1024 => $"{bytes / (1024.0 * 1024.0):F1} MB", - _ => $"{bytes / (1024.0 * 1024.0 * 1024.0):F1} GB", - }; } diff --git a/FileDeduplicator/Verbs/Scan.cs b/FileDeduplicator/Verbs/Scan.cs index 1a68f00..62bedc6 100644 --- a/FileDeduplicator/Verbs/Scan.cs +++ b/FileDeduplicator/Verbs/Scan.cs @@ -71,7 +71,7 @@ internal override void Run(Scan options) long wastedBytes = group.FileSize * (group.Files.Count - 1); totalWastedBytes += wastedBytes; - Console.WriteLine($" Hash: {group.Hash[..12]}... ({FormatBytes(group.FileSize)}, {group.Files.Count} copies)"); + Console.WriteLine($" Hash: {group.Hash[..12]}... ({DuplicateReport.FormatBytes(group.FileSize)}, {group.Files.Count} copies)"); foreach (AbsoluteFilePath file in group.Files) { @@ -83,18 +83,10 @@ internal override void Run(Scan options) } Console.WriteLine($"Total duplicate groups: {duplicates.Count}"); - Console.WriteLine($"Total wasted space: {FormatBytes(totalWastedBytes)}"); + Console.WriteLine($"Total wasted space: {DuplicateReport.FormatBytes(totalWastedBytes)}"); Console.WriteLine(); Console.WriteLine("Run the 'Deduplicate' command to remove duplicates."); PathString = "."; } - - private static string FormatBytes(long bytes) => bytes switch - { - < 1024L => $"{bytes} B", - < 1024L * 1024 => $"{bytes / 1024.0:F1} KB", - < 1024L * 1024 * 1024 => $"{bytes / (1024.0 * 1024.0):F1} MB", - _ => $"{bytes / (1024.0 * 1024.0 * 1024.0):F1} GB", - }; } diff --git a/FileDeduplicator/Verbs/Stats.cs b/FileDeduplicator/Verbs/Stats.cs index 9b256eb..2ad684f 100644 --- a/FileDeduplicator/Verbs/Stats.cs +++ b/FileDeduplicator/Verbs/Stats.cs @@ -62,7 +62,7 @@ internal override void Run(Stats options) Console.WriteLine("=== FileDeduplicator Statistics ==="); Console.WriteLine(); Console.WriteLine($"Total files: {files.Count}"); - Console.WriteLine($"Total size: {FormatBytes(totalSize)}"); + Console.WriteLine($"Total size: {DuplicateReport.FormatBytes(totalSize)}"); Console.WriteLine($"Unique files: {uniqueFiles}"); Console.WriteLine($"Duplicate files: {duplicateFiles}"); Console.WriteLine($"Duplicate groups: {duplicates.Count}"); @@ -70,7 +70,7 @@ internal override void Run(Stats options) if (duplicates.Count > 0) { long wastedSpace = duplicates.Sum(g => g.FileSize * (g.Files.Count - 1)); - Console.WriteLine($"Wasted space: {FormatBytes(wastedSpace)}"); + Console.WriteLine($"Wasted space: {DuplicateReport.FormatBytes(wastedSpace)}"); Console.WriteLine(); // Extension breakdown @@ -101,18 +101,10 @@ internal override void Run(Stats options) foreach (DuplicateGroup group in largestGroups) { long wasted = group.FileSize * (group.Files.Count - 1); - Console.WriteLine($" {group.Hash[..12]}... - {group.Files.Count} copies, {FormatBytes(group.FileSize)} each, {FormatBytes(wasted)} wasted"); + Console.WriteLine($" {group.Hash[..12]}... - {group.Files.Count} copies, {DuplicateReport.FormatBytes(group.FileSize)} each, {DuplicateReport.FormatBytes(wasted)} wasted"); } } PathString = "."; } - - private static string FormatBytes(long bytes) => bytes switch - { - < 1024L => $"{bytes} B", - < 1024L * 1024 => $"{bytes / 1024.0:F1} KB", - < 1024L * 1024 * 1024 => $"{bytes / (1024.0 * 1024.0):F1} MB", - _ => $"{bytes / (1024.0 * 1024.0 * 1024.0):F1} GB", - }; } diff --git a/README.md b/README.md index 3cccd2e..1523afe 100644 --- a/README.md +++ b/README.md @@ -76,7 +76,7 @@ Identical to Scan but formatted as a deletion preview with a summary of how many ### Deduplicate -Performs the actual deduplication. After scanning and displaying results, prompts for confirmation (`y/N`) before deleting any files. Reports the number of files deleted and disk space reclaimed. +Performs the actual deduplication. Prints the same per-group `KEEP`/`DELETE` listing DryRun does, so every path that is about to be removed is on screen before the confirmation (`y/N`) prompt is asked. Reports the number of files deleted and disk space reclaimed. ### Stats From b9652a0b8ebd758e69c5691b36212bd39d091881 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 21 Sep 2026 06:38:46 +0000 Subject: [PATCH 2/2] Cover the verbs whose output this change moved SonarCloud's quality gate failed the PR at 68.3% coverage on new code against a required 80%. The uncovered lines were real gaps, not noise: Scan, DryRun and Stats had no test at all, so the FormatBytes call sites this change rewrote in each of them were never executed, and the Deduplicate tests all declined at the prompt, so the delete path below it was never reached either. Add VerbOutputTests, which runs Scan, DryRun and Stats through the console and pins what each reports, including that the two read-only verbs leave every file on disk. Add the confirming case to DeduplicateConfirmationTests -- what the listing named is deleted, what it did not name is not -- plus the totals shown above the prompt, and a FormatBytes row per unit. Console redirection moves to a shared ConsoleCapture helper. Every line this PR adds or changes is now executed by a test. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01By4ipNgZKvDwoS2HUmjupA --- FileDeduplicator.Test/ConsoleCapture.cs | 54 ++++++ .../DeduplicateConfirmationTests.cs | 103 ++++++----- FileDeduplicator.Test/DuplicateReportTests.cs | 15 ++ FileDeduplicator.Test/VerbOutputTests.cs | 163 ++++++++++++++++++ 4 files changed, 293 insertions(+), 42 deletions(-) create mode 100644 FileDeduplicator.Test/ConsoleCapture.cs create mode 100644 FileDeduplicator.Test/VerbOutputTests.cs diff --git a/FileDeduplicator.Test/ConsoleCapture.cs b/FileDeduplicator.Test/ConsoleCapture.cs new file mode 100644 index 0000000..e8b5971 --- /dev/null +++ b/FileDeduplicator.Test/ConsoleCapture.cs @@ -0,0 +1,54 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.FileDeduplicator.Test; + +using ktsu.FileDeduplicator.Verbs; + +/// +/// Runs a verb with the console redirected, and hands back everything it wrote. +/// +/// +/// The verbs have no return value and no output abstraction -- what they do is what they print -- +/// so the console is the only surface a test can assert against. Redirecting it is global state, +/// which is why every class using this is marked [DoNotParallelize]. +/// +internal static class ConsoleCapture +{ + /// + /// Runs a verb, feeding it the given answers on stdin. + /// + /// The verb to run. + /// Lines the verb's prompts will read, or nothing. + /// Everything the verb wrote to the console. + internal static string Run(BaseVerb verb, string stdin = "") + { + TextWriter originalOut = Console.Out; + TextReader originalIn = Console.In; + + try + { + using StringWriter captured = new(); + using StringReader answers = new(stdin); + Console.SetOut(captured); + Console.SetIn(answers); + + verb.Run(); + + return captured.ToString(); + } + finally + { + Console.SetOut(originalOut); + Console.SetIn(originalIn); + } + } + + /// + /// Collapses runs of whitespace so assertions describe what a line says rather than the column + /// its markers happen to sit in, and so a CRLF platform reads the same as an LF one. + /// + /// The captured output. + /// The output with each line trimmed and its internal whitespace collapsed. + internal static string Normalize(string output) => + string.Join('\n', output.Split('\n').Select(line => string.Join(' ', line.Split((char[]?)null, StringSplitOptions.RemoveEmptyEntries)))); +} diff --git a/FileDeduplicator.Test/DeduplicateConfirmationTests.cs b/FileDeduplicator.Test/DeduplicateConfirmationTests.cs index e61f05c..25a9bde 100644 --- a/FileDeduplicator.Test/DeduplicateConfirmationTests.cs +++ b/FileDeduplicator.Test/DeduplicateConfirmationTests.cs @@ -9,7 +9,7 @@ namespace ktsu.FileDeduplicator.Test; /// /// Tests that the Deduplicate verb shows which copies it is about to delete before it asks for -/// permission to delete them. +/// permission to delete them, and that the answer it gets is the one it acts on. /// /// /// The confirmation prompt is the only gate in front of an irreversible deletion, and "keep the @@ -26,45 +26,19 @@ namespace ktsu.FileDeduplicator.Test; [DoNotParallelize] public sealed class DeduplicateConfirmationTests { - /// - /// Runs the Deduplicate verb over a tree, answering "n" at the confirmation prompt, and - /// returns everything it wrote. - /// - /// The directory to deduplicate. - /// The verb's console output. - private static string RunDeclining(AbsoluteDirectoryPath root) - { - TextWriter originalOut = Console.Out; - TextReader originalIn = Console.In; - - try - { - using StringWriter captured = new(); - using StringReader answers = new("n"); - Console.SetOut(captured); - Console.SetIn(answers); + private static string RunDeclining(AbsoluteDirectoryPath root) => + ConsoleCapture.Normalize(ConsoleCapture.Run(new Deduplicate { PathString = root.WeakString }, "n")); - Deduplicate verb = new() { PathString = root.WeakString }; - verb.Run(); + private static string RunConfirming(AbsoluteDirectoryPath root) => + ConsoleCapture.Normalize(ConsoleCapture.Run(new Deduplicate { PathString = root.WeakString }, "y")); - return captured.ToString(); - } - finally - { - Console.SetOut(originalOut); - Console.SetIn(originalIn); - } + private static string BeforeThePrompt(string output) + { + int prompt = output.IndexOf("Proceed with deletion?", StringComparison.Ordinal); + Assert.AreNotEqual(-1, prompt, $"The verb never reached the confirmation prompt. Output was:\n{output}"); + return output[..prompt]; } - /// - /// Collapses runs of whitespace so the assertions describe the listing's content rather than - /// the column its markers happen to sit in. - /// - /// The captured console output. - /// The output with each line trimmed and its internal whitespace collapsed. - private static string Normalize(string output) => - string.Join('\n', output.Split('\n').Select(line => string.Join(' ', line.Split((char[]?)null, StringSplitOptions.RemoveEmptyEntries)))); - /// /// Every copy that would be deleted has to be named before the prompt, not after the deletion. /// @@ -78,12 +52,9 @@ public void EveryPathToBeDeletedIsNamedBeforeTheConfirmationPrompt() AbsoluteFilePath alsoDoomed = tree.Write("archive/report-2026-backup.pdf", "the only copy that survives"); // Act - string output = Normalize(RunDeclining(tree.Root)); - int prompt = output.IndexOf("Proceed with deletion?", StringComparison.Ordinal); - string beforePrompt = prompt < 0 ? string.Empty : output[..prompt]; + string beforePrompt = BeforeThePrompt(RunDeclining(tree.Root)); // Assert - Assert.AreNotEqual(-1, prompt, $"The verb never reached the confirmation prompt. Output was:\n{output}"); Assert.Contains($"DELETE: {doomed}", beforePrompt, $"The prompt was shown without naming {doomed}. Output before it was:\n{beforePrompt}"); Assert.Contains($"DELETE: {alsoDoomed}", beforePrompt, $"The prompt was shown without naming {alsoDoomed}. Output before it was:\n{beforePrompt}"); Assert.Contains($"KEEP: {keeper}", beforePrompt, $"The prompt was shown without naming the copy being kept. Output before it was:\n{beforePrompt}"); @@ -109,8 +80,7 @@ public void EveryDuplicateGroupAppearsInTheListing() _ = tree.Write("group3/c.txt", "gamma"); // Act - string output = Normalize(RunDeclining(tree.Root)); - string beforePrompt = output[..output.IndexOf("Proceed with deletion?", StringComparison.Ordinal)]; + string beforePrompt = BeforeThePrompt(RunDeclining(tree.Root)); // Assert foreach (AbsoluteFilePath file in doomed) @@ -119,6 +89,28 @@ public void EveryDuplicateGroupAppearsInTheListing() } } + /// + /// The totals printed with the listing describe the same deletions the listing names. + /// + [TestMethod] + public void TheTotalsMatchTheListingShownAboveThem() + { + // Arrange -- two groups of two 5-byte files, so 2 deletions and 10 bytes + using TempTree tree = new(); + _ = tree.Write("a.txt", "alpha"); + _ = tree.Write("aa.txt", "alpha"); + _ = tree.Write("b.txt", "bravo"); + _ = tree.Write("bb.txt", "bravo"); + + // Act + string beforePrompt = BeforeThePrompt(RunDeclining(tree.Root)); + + // Assert + Assert.AreEqual(2, beforePrompt.Split("DELETE:").Length - 1, $"Expected two DELETE lines in:\n{beforePrompt}"); + Assert.Contains("Files to delete: 2", beforePrompt); + Assert.Contains("Space to reclaim: 10 B", beforePrompt); + } + /// /// Showing the listing must not have turned the preview into the deletion: declining still /// leaves every file on disk. @@ -147,4 +139,31 @@ public void DecliningTheConfirmationLeavesEveryFileOnDisk() Assert.IsTrue(TempTree.Exists(file), $"{file} was deleted despite the confirmation being declined."); } } + + /// + /// Confirming deletes exactly the copies the listing named, and no others -- the listing is a + /// promise about what the next step does, so it has to be kept. + /// + [TestMethod] + public void ConfirmingDeletesExactlyTheCopiesTheListingNamed() + { + // Arrange + using TempTree tree = new(); + AbsoluteFilePath keeper = tree.Write("a.txt", "alpha"); + AbsoluteFilePath doomed = tree.Write("aa.txt", "alpha"); + AbsoluteFilePath unique = tree.Write("b.txt", "beta"); + + // Act + string output = RunConfirming(tree.Root); + string beforePrompt = BeforeThePrompt(output); + + // Assert -- what the listing named is gone; what it did not name is not + Assert.Contains($"DELETE: {doomed}", beforePrompt); + Assert.IsFalse(TempTree.Exists(doomed), $"{doomed} was listed for deletion but survived."); + Assert.IsTrue(TempTree.Exists(keeper), $"{keeper} was listed as the kept copy but was deleted."); + Assert.IsTrue(TempTree.Exists(unique), $"{unique} has no duplicate and should never have been touched."); + + Assert.Contains("Deleted 1 file(s).", output); + Assert.Contains("Reclaimed 5 B of disk space.", output); + } } diff --git a/FileDeduplicator.Test/DuplicateReportTests.cs b/FileDeduplicator.Test/DuplicateReportTests.cs index 5255671..8d8c3e9 100644 --- a/FileDeduplicator.Test/DuplicateReportTests.cs +++ b/FileDeduplicator.Test/DuplicateReportTests.cs @@ -79,6 +79,21 @@ public void ListingNamesOneKeeperAndEveryOtherCopyPerGroup() } } + /// + /// Sizes are rendered in the largest unit that leaves the number above one, so a listing of + /// large files does not ask the reader to count digits. + /// + /// The count to render. + /// What it should read as. + [TestMethod] + [DataRow(0L, "0 B")] + [DataRow(1023L, "1023 B")] + [DataRow(1024L, "1.0 KB")] + [DataRow(1024L * 1024, "1.0 MB")] + [DataRow((1024L * 1024 * 1024) + (512L * 1024 * 1024), "1.5 GB")] + public void FormatBytesUsesTheLargestUnitThatFits(long bytes, string expected) => + Assert.AreEqual(expected, DuplicateReport.FormatBytes(bytes)); + /// /// A run with nothing to delete produces nothing to read. /// diff --git a/FileDeduplicator.Test/VerbOutputTests.cs b/FileDeduplicator.Test/VerbOutputTests.cs new file mode 100644 index 0000000..42d8f80 --- /dev/null +++ b/FileDeduplicator.Test/VerbOutputTests.cs @@ -0,0 +1,163 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.FileDeduplicator.Test; + +using ktsu.FileDeduplicator.Verbs; +using ktsu.Semantics.Paths; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Tests what the read-only verbs report, and that they stay read-only. +/// +/// +/// Scan, DryRun and Stats exist to be read before anything is deleted, so what they print is their +/// entire contract -- and Deduplicate's listing is now built from the same helper DryRun uses, which +/// makes "DryRun still says what it used to" something worth holding still rather than assuming. +/// +[TestClass] +[DoNotParallelize] +public sealed class VerbOutputTests +{ + /// + /// Writes a tree with one group of three copies and one unique file. + /// + /// The tree to write into. + /// The copy the shortest-name policy will keep. + /// The copies it will not. + private static void WriteOneGroup(TempTree tree, out AbsoluteFilePath keeper, out List doomed) + { + keeper = tree.Write("a.txt", "alpha"); + doomed = + [ + tree.Write("aa.txt", "alpha"), + tree.Write("nested/aaa.txt", "alpha"), + ]; + _ = tree.Write("unique.txt", "beta"); + } + + /// + /// DryRun names the keeper and every other copy, totals them, and deletes nothing. + /// + [TestMethod] + public void DryRunListsTheGroupAndLeavesEveryFileOnDisk() + { + // Arrange + using TempTree tree = new(); + WriteOneGroup(tree, out AbsoluteFilePath keeper, out List doomed); + + // Act + string output = ConsoleCapture.Normalize(ConsoleCapture.Run(new DryRun { PathString = tree.Root.WeakString })); + + // Assert + Assert.Contains("Found 1 group(s) of duplicate files:", output); + Assert.Contains($"KEEP: {keeper}", output); + Assert.Contains("--- Dry Run Summary ---", output); + Assert.Contains("Duplicate groups: 1", output); + Assert.Contains("Files to delete: 2", output); + Assert.Contains("Space to reclaim: 10 B", output); + + foreach (AbsoluteFilePath file in doomed) + { + Assert.Contains($"DELETE: {file}", output); + Assert.IsTrue(TempTree.Exists(file), $"DryRun deleted {file}."); + } + + Assert.IsTrue(TempTree.Exists(keeper), $"DryRun deleted {keeper}."); + } + + /// + /// Scan marks each copy in a group, and points at the verb that acts on them. + /// + [TestMethod] + public void ScanMarksEveryCopyKeepOrDelete() + { + // Arrange + using TempTree tree = new(); + WriteOneGroup(tree, out AbsoluteFilePath keeper, out List doomed); + + // Act + string output = ConsoleCapture.Normalize(ConsoleCapture.Run(new Scan { PathString = tree.Root.WeakString })); + + // Assert + Assert.Contains($"{keeper} [KEEP]", output); + Assert.Contains("Total duplicate groups: 1", output); + Assert.Contains("Total wasted space: 10 B", output); + Assert.Contains("Run the 'Deduplicate' command to remove duplicates.", output); + + foreach (AbsoluteFilePath file in doomed) + { + Assert.Contains($"{file} [DELETE]", output); + Assert.IsTrue(TempTree.Exists(file), $"Scan deleted {file}."); + } + } + + /// + /// Stats counts the tree and breaks the duplicates down without touching anything. + /// + [TestMethod] + public void StatsReportsTotalsForTheTree() + { + // Arrange + using TempTree tree = new(); + WriteOneGroup(tree, out AbsoluteFilePath keeper, out List doomed); + + // Act + string output = ConsoleCapture.Normalize(ConsoleCapture.Run(new Stats { PathString = tree.Root.WeakString })); + + // Assert -- three 5-byte copies plus a 4-byte unique file, so 19 B held and 10 B wasted + Assert.Contains("Total files: 4", output); + Assert.Contains("Total size: 19 B", output); + Assert.Contains("Unique files: 2", output); + Assert.Contains("Duplicate files: 2", output); + Assert.Contains("Duplicate groups: 1", output); + Assert.Contains("Wasted space: 10 B", output); + Assert.Contains("Duplicate files by extension:", output); + Assert.Contains(".txt: 3 file(s)", output); + Assert.Contains("Largest duplicate groups (by wasted space):", output); + Assert.Contains("3 copies, 5 B each, 10 B wasted", output); + + Assert.IsTrue(TempTree.Exists(keeper), $"Stats deleted {keeper}."); + Assert.IsTrue(doomed.TrueForAll(TempTree.Exists), "Stats deleted a file."); + } + + /// + /// A tree with nothing duplicated says so, in every verb that looks for duplicates. + /// + [TestMethod] + public void NoDuplicatesIsReportedByEveryVerb() + { + // Arrange + using TempTree tree = new(); + _ = tree.Write("a.txt", "alpha"); + _ = tree.Write("b.txt", "beta"); + + // Act + string scan = ConsoleCapture.Run(new Scan { PathString = tree.Root.WeakString }); + string dryRun = ConsoleCapture.Run(new DryRun { PathString = tree.Root.WeakString }); + string deduplicate = ConsoleCapture.Run(new Deduplicate { PathString = tree.Root.WeakString }, "y"); + + // Assert + Assert.Contains("No duplicate files found.", scan); + Assert.Contains("No duplicate files found.", dryRun); + Assert.Contains("No duplicate files found.", deduplicate); + Assert.DoesNotContain("Proceed with deletion?", deduplicate, "Nothing was found, so nothing should have been asked."); + } + + /// + /// An empty directory stops before hashing, rather than reporting on nothing. + /// + [TestMethod] + public void AnEmptyDirectoryStopsAfterDiscovery() + { + // Arrange + using TempTree tree = new(); + + // Act + string output = ConsoleCapture.Run(new Scan { PathString = tree.Root.WeakString }); + + // Assert + Assert.Contains("Found 0 file(s).", output); + Assert.DoesNotContain("Hashing files...", output); + } +}