diff --git a/docs/CLI.md b/docs/CLI.md index 3dade29..3df64b3 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -124,6 +124,12 @@ combined with conversion options. | `-Verbose` | Include error details and a result breakdown. | | `-MaxParallelism ` | Maximum simultaneous files. Default: the smaller of CPU count and 8. | +In a preview (`-WhatIf`, `-Plan`, or the GUI's preview), the per-file CSV, whether +on stdout, written with `-Report`, or exported from the GUI, calls a file that would +be converted `WouldConvert`, and the `-Verbose` breakdown counts it the same way. +`Converted` appears only for a file EC wrote. An `Error` row may still have been +replaced; the journal records which. + `-Quiet` and `-Verbose` cannot be combined. After cancellation, a requested journal is still saved with `Interrupted: true` diff --git a/docs/DEFECT-BACKLOG.md b/docs/DEFECT-BACKLOG.md index 86428c8..d35bb04 100644 --- a/docs/DEFECT-BACKLOG.md +++ b/docs/DEFECT-BACKLOG.md @@ -4,9 +4,9 @@ This is the current ledger for defects and review findings in EncodingChecker. It is organised by status, not discovery date, so the open work is visible in one place. Longer evidence and history follow the ledger. - + -**Derived count: 87 findings — 75 fixed, 8 open, 1 not reproduced, 1 withdrawn, +**Derived count: 87 findings — 76 fixed, 7 open, 1 not reproduced, 1 withdrawn, 1 intentional behavior, and 1 design decision.** Recompute and check these figures with: @@ -56,7 +56,6 @@ the 2026-09-08 reformat to findings that previously had only a sentence. | BL-19 | ASCII with many NUL bytes can be reported as UTF-16 | Open | Medium | Rare | [BL-19](#bl-19) | | BL-20 | Two hard-link names for one file are converted separately | Open | Low | Rare | [BL-20](#bl-20) | | BL-21 | Detection can accept a cut-off final character that conversion rejects | Open | Low | Rare | [BL-21](#bl-21) | -| BL-42 | A preview's CSV report calls files Converted | Open | Low | Common | [BL-42](#bl-42) | | BL-43 | A file name with an unpaired UTF-16 surrogate cannot be planned or backed up | Open | Low | Rare | [BL-43](#bl-43) | ### Fixed and resolved findings @@ -94,6 +93,7 @@ the 2026-09-08 reformat to findings that previously had only a sentence. | BL-39 | A source chosen in a cancelled GUI review carried into the next review | Fixed | — | — | [BL-39](#bl-39) | | BL-40 | The GUI's Validate status read the same when every file passed and when none was checked | Fixed | — | — | [BL-40](#bl-40) | | BL-41 | GUI rows kept an earlier run's icon and never showed why a file failed | Fixed | — | — | [BL-41](#bl-41) | +| BL-42 | A preview's CSV report called files Converted | Fixed | — | — | [BL-42](#bl-42) | | EC-32 | A cancellation timeout could hide repeated refusals to press Cancel | Fixed | — | — | [EC-32](#ec-32) | | EC-15 | Plans and journals showed fixed safety flags as if they recorded checks | Fixed | — | — | [EC-15](#ec-15) | | BL-18 | BOM-less UTF-16 could be detected and converted as UTF-32 | Fixed | — | — | [BL-18](#bl-18) | @@ -1462,15 +1462,29 @@ result, and the window showing a scanned row's reason; four mutations fail them. ### BL-42 -**A preview's CSV report calls files `Converted`.** `-WhatIf`, and the GUI's -preview followed by a CSV export, write `Converted` for each file that a real run -would convert, although nothing was written. Only the journal marks a preview, as -`NotAttempted`. Found by the 2026-09-20 review with the CLI's `-WhatIf` output. -No file is at risk: this is what the report says, not what EC does. - -Left open deliberately. The CSV `Result` column is read by scripts, so renaming -the value, or adding a column, changes a contract. It needs a decision on the -wording before any change. +**A preview's CSV report now says `WouldConvert`.** `-WhatIf`, `-Plan`, and the +GUI's preview followed by a CSV export wrote `Converted` for each file that a real +run would convert, although nothing was written. On the command line, only the +journal marked a preview, as `NotAttempted`. Found by the 2026-09-20 review and reproduced on +2026-10-01 with the v3.15.0 CLI: the file's bytes were unchanged and no backup +existed, while the report said `Converted`. No file was at risk: this was what the +report said, not what EC did. + +The `Result` column is read by scripts, so the wording was decided before the +change: a file a preview only decided to convert is `WouldConvert`, and +`Converted` now appears only for a file EC wrote; the `-Verbose` breakdown counts +the same way. A script that counted `Converted` rows in a preview to learn what +would change must now match `WouldConvert`, and a script that checks for known +`Result` values will see a new one. An unreached row still reads `NotAttempted`. +The journal is unchanged. The reverse does not hold: an `Error` row may still have +been replaced, which the journal records. + +Tests read the CSV from `-WhatIf -Report` and `-WhatIf` stdout, `-Plan` stdout, a +real run's report, and the orchestrator's rows after a GUI-style preview followed +by a real conversion, and by a run on which the file had become unreadable. A +writer test checks that `NotAttempted` takes precedence, and a `-Verbose` test +checks the breakdown. Ignoring the preview marker, not clearing it at the start of +a pass, or letting it outrank `NotAttempted` each fails them. ### BL-43 diff --git a/sources/EncodingChecker.Tests/ConversionOrchestrationTests.cs b/sources/EncodingChecker.Tests/ConversionOrchestrationTests.cs index 66b77bf..4bd8bb7 100644 --- a/sources/EncodingChecker.Tests/ConversionOrchestrationTests.cs +++ b/sources/EncodingChecker.Tests/ConversionOrchestrationTests.cs @@ -96,6 +96,13 @@ private OrchestrationResult Convert( private static ConfirmationResponse Proceed(ConversionPlan _) => ConfirmationResponse.Proceed; + // A run that wrote nothing exports every row as NotAttempted: not Converted, and not + // WouldConvert either, because the run meant to write. + private static void AssertEveryCsvRowNotAttempted(string csv) => + Assert.All( + csv.Split(Environment.NewLine).Skip(1).Where(line => line.Length > 0), + row => Assert.Equal("NotAttempted", row.Split(',')[5])); + [Fact] public void ACompletedRunEmitsExactlyOneTerminalResultPerSelectedFile() { @@ -550,7 +557,7 @@ public void ChoosingAnEncodingForNoFilesChangesNothing_EvenForFilesAlreadyMarked using var csv = new StringWriter(); ConversionReport.WriteCsv(entries, csv); - Assert.DoesNotContain("Converted", csv.ToString()); + AssertEveryCsvRowNotAttempted(csv.ToString()); } // ------------------------------------------------------- nothing gets modified @@ -581,7 +588,7 @@ public void CancellingTheConfirmation_ModifiesNothing() using var csv = new StringWriter(); ConversionReport.WriteCsv(entries, csv); - Assert.DoesNotContain("Converted", csv.ToString()); + AssertEveryCsvRowNotAttempted(csv.ToString()); } [Fact] @@ -608,7 +615,7 @@ public void CancellingDuringTheDecidePass_MarksEntriesUnattemptedBeforePropagati using var csv = new StringWriter(); ConversionReport.WriteCsv(entries, csv); - Assert.DoesNotContain("Converted", csv.ToString()); + AssertEveryCsvRowNotAttempted(csv.ToString()); } [Fact] @@ -648,7 +655,7 @@ public void AFileChangedAfterTheConfirmation_StopsTheWholeRun() using var csv = new StringWriter(); ConversionReport.WriteCsv(entries, csv); - Assert.DoesNotContain("Converted", csv.ToString()); + AssertEveryCsvRowNotAttempted(csv.ToString()); } [Fact] diff --git a/sources/EncodingChecker.Tests/PreviewCsvResultTests.cs b/sources/EncodingChecker.Tests/PreviewCsvResultTests.cs new file mode 100644 index 0000000..92ce963 --- /dev/null +++ b/sources/EncodingChecker.Tests/PreviewCsvResultTests.cs @@ -0,0 +1,215 @@ +using System.Text; +using static EncodingChecker.Tests.CliRunner; +using static EncodingChecker.Tests.ExpectedExitCode; + +namespace EncodingChecker.Tests; + +/// +/// A CSV report says WouldConvert for a file a preview only decided to convert, and +/// Converted only for a file that was written. +/// +/// +/// A preview's CSV used to say Converted for files it never touched, and on the +/// command line only the journal marked the run as a preview. The CLI cases read the CSV +/// and summary the CLI writes. The GUI cases run the orchestrator the window uses and +/// write its rows with the export's writer. The last case checks the writer's precedence +/// directly. +/// +public sealed class PreviewCsvResultTests : IDisposable +{ + private readonly string _root = + Directory.CreateTempSubdirectory("ec_preview_csv_").FullName; + + private readonly string _outputs = + Directory.CreateTempSubdirectory("ec_preview_csv_out_").FullName; + + public void Dispose() + { + foreach (string directory in new[] { _root, _outputs }) + { + try + { + Directory.Delete(directory, recursive: true); + } + catch (IOException) + { + // Best-effort cleanup. + } + } + } + + // UTF-16 with a BOM, which converting to UTF-8 rewrites. + private string WriteConvertible() + { + string path = Path.Combine(_root, "a.txt"); + File.WriteAllText(path, "Hello 世界\r\n", Encoding.Unicode); + + return path; + } + + // The Result cell of the row for this file. + private static string ResultFor(string csv, string path) + { + string row = Assert.Single( + csv.Split(Environment.NewLine), + line => line.StartsWith(path + ",", StringComparison.Ordinal)); + + return row.Split(',')[5]; + } + + private string Report => Path.Combine(_outputs, "report.csv"); + + [Fact] + public void APreviewReportSaysWouldConvertAndLeavesTheFileAlone() + { + string path = WriteConvertible(); + byte[] before = File.ReadAllBytes(path); + + (int exit, string output, _) = RunCaptured( + "-BasePath", _root, "-Target", "utf-8", "-WhatIf", "-Backup", "-Report", Report); + + Assert.Equal(ExpectedClean, exit); + Assert.Equal("WouldConvert", ResultFor(File.ReadAllText(Report), path)); + Assert.Equal("WouldConvert", ResultFor(output, path)); + Assert.Equal(before, File.ReadAllBytes(path)); + Assert.False(File.Exists(path + ".bak")); + } + + [Fact] + public void PlanningSaysWouldConvert() + { + string path = WriteConvertible(); + + (int exit, string output, _) = RunCaptured( + "-BasePath", _root, "-Target", "utf-8", "-Plan", Path.Combine(_outputs, "plan.json")); + + Assert.Equal(ExpectedClean, exit); + Assert.Equal("WouldConvert", ResultFor(output, path)); + } + + [Fact] + public void ARealRunStillSaysConverted() + { + string path = WriteConvertible(); + + Assert.Equal( + ExpectedClean, + Run("-BasePath", _root, "-Target", "utf-8", "-Report", Report, "-Quiet")); + + Assert.Equal("Converted", ResultFor(File.ReadAllText(Report), path)); + Assert.False(TestContent.StillHasBom(path)); + } + + [Fact] + public void AGuiPreviewSaysWouldConvertAndALaterConversionOfTheSameRowsSaysConverted() + { + string path = WriteConvertible(); + byte[] before = File.ReadAllBytes(path); + + var scanned = new EntrySink(); + ScanEngine.ScanDirectory( + new ScanDirectoryOptions + { + BaseDirectory = _root, + IncludeSubdirectories = true, + IncludePatterns = ["*"], + Action = ScanAction.Detect, + }, + scanned.Add, + CancellationToken.None); + + List rows = [.. scanned]; + + OrchestrationResult Run(bool preview) => + new ConversionOrchestrator(_ => ConfirmationResponse.Proceed).Run( + rows, _root, "utf-8", targetWriteBom: false, + backup: false, preview: preview, + ScanEngine.DefaultMaxParallelism, + _ => { }, + CancellationToken.None); + + Assert.Equal(OrchestrationOutcome.Previewed, Run(preview: true).Outcome); + Assert.Equal("WouldConvert", ResultFor(ConversionReport.ToCsvString(rows), path)); + Assert.Equal(before, File.ReadAllBytes(path)); + + // The real run's own deciding pass marks the row again; its write pass must clear it. + Assert.Equal(OrchestrationOutcome.Converted, Run(preview: false).Outcome); + Assert.Equal("Converted", ResultFor(ConversionReport.ToCsvString(rows), path)); + Assert.Equal( + new UTF8Encoding(false).GetBytes("Hello 世界\r\n"), File.ReadAllBytes(path)); + } + + [Fact] + public void APreviewedRowThatLaterFailsSaysErrorNotWouldConvert() + { + // The later pass ends before the code that decides a conversion, so only the start + // of the pass can clear the earlier preview's mark. + string path = WriteConvertible(); + List rows = ViewRows(); + + Assert.Equal(OrchestrationOutcome.Previewed, Preview(rows).Outcome); + Assert.Equal("WouldConvert", ResultFor(ConversionReport.ToCsvString(rows), path)); + + File.Delete(path); + Preview(rows); + + ConversionReportEntry row = Assert.Single(rows); + Assert.Equal(ConversionRowResult.Error, row.Result); + Assert.False(row.ConversionOnlyDecided); + Assert.Equal("Error", ResultFor(ConversionReport.ToCsvString(rows), path)); + } + + [Fact] + public void TheVerboseBreakdownCountsAPreviewAsWouldConvert() + { + WriteConvertible(); + + (_, string preview, _) = RunCaptured( + "-BasePath", _root, "-Target", "utf-8", "-WhatIf", "-Verbose"); + Assert.Contains("Converted: 0 WouldConvert: 1 ", preview); + + (_, string real, _) = RunCaptured("-BasePath", _root, "-Target", "utf-8", "-Verbose"); + Assert.Contains("Converted: 1 WouldConvert: 0 ", real); + } + + private List ViewRows() + { + var scanned = new EntrySink(); + ScanEngine.ScanDirectory( + new ScanDirectoryOptions + { + BaseDirectory = _root, + IncludeSubdirectories = true, + IncludePatterns = ["*"], + Action = ScanAction.Detect, + }, + scanned.Add, + CancellationToken.None); + + return [.. scanned]; + } + + private OrchestrationResult Preview(List rows) => + new ConversionOrchestrator(_ => ConfirmationResponse.Proceed).Run( + rows, _root, "utf-8", targetWriteBom: false, + backup: false, preview: true, + ScanEngine.DefaultMaxParallelism, + _ => { }, + CancellationToken.None); + + [Fact] + public void AnUnreachedRowStillSaysNotAttempted() + { + var entry = new ConversionReportEntry + { + FilePath = @"C:\scan\a.txt", + SourceEncoding = "utf-16", + TargetEncoding = "utf-8", + Result = ConversionRowResult.Converted, + ConversionOnlyDecided = true, + NotAttempted = true, + }; + + Assert.Equal("NotAttempted", ResultFor(ConversionReport.ToCsvString([entry]), entry.FilePath)); + } +} diff --git a/sources/EncodingChecker/ConversionReport.cs b/sources/EncodingChecker/ConversionReport.cs index 96773d2..1470a79 100644 --- a/sources/EncodingChecker/ConversionReport.cs +++ b/sources/EncodingChecker/ConversionReport.cs @@ -174,11 +174,25 @@ internal static void MarkUnattempted( /// Whether the run ended before it reached this file. Internal state; not in CSV. /// /// - /// An entry keeps the deciding pass's result until the write pass overwrites it, - /// so a file the run never got to still reads as Converted. This says otherwise. + /// An entry keeps the deciding pass's result until the write pass overwrites it, so a + /// file the run never reached still carries Converted. Without this the journal would + /// report it converted and the CSV would call it WouldConvert. This says neither + /// happened. /// internal bool NotAttempted { get; set; } + /// + /// Whether the last deciding pass chose to convert this file and no write pass has + /// reached it since: a preview, a plan, or a GUI review not yet carried out. + /// Internal state. + /// + /// + /// Cleared at the start of every pass. Meaningful only while is + /// Converted; the CSV then writes WouldConvert. + /// outranks it. + /// + internal bool ConversionOnlyDecided { get; set; } + /// /// Whether the converted file reached its destination, or /// when the outcome is unknown or nothing was attempted. @@ -214,6 +228,7 @@ internal void ResetAttemptEvidence() ResolvedSourceLabel = null; JournalSourceSha256 = null; NotAttempted = false; + ConversionOnlyDecided = false; ReplacementCommitted = null; OutputSha256 = null; BackupPath = null; @@ -317,7 +332,11 @@ internal static void WriteCsv( WriteField(writer, entry.TargetHasBom ? "Yes" : "No"); writer.Write(Delimiter); - writer.Write(entry.NotAttempted ? "NotAttempted" : entry.Result.ToString()); + writer.Write( + entry.NotAttempted ? "NotAttempted" + : entry is { Result: ConversionRowResult.Converted, ConversionOnlyDecided: true } + ? "WouldConvert" + : entry.Result.ToString()); writer.Write(Delimiter); WriteField(writer, entry.ReasonCode); diff --git a/sources/EncodingChecker/Program.CliReporting.cs b/sources/EncodingChecker/Program.CliReporting.cs index 1a37c0f..1d14d8c 100644 --- a/sources/EncodingChecker/Program.CliReporting.cs +++ b/sources/EncodingChecker/Program.CliReporting.cs @@ -16,13 +16,20 @@ private static void PrintVerboseSummary( .GroupBy(e => e.Result) .ToDictionary(g => g.Key, g => g.Count()); + // In a preview, Converted rows were only decided; count them as the CSV names them. + int wouldConvert = entries.Count(e => e is + { + Result: ConversionRowResult.Converted, ConversionOnlyDecided: true, + }); + Console.Out.WriteLine(); Console.Out.WriteLine( $"Total: {entries.Count} " + $"Unchanged: {byResult.GetValueOrDefault(ConversionRowResult.Unchanged)} " + $"Skipped: {byResult.GetValueOrDefault(ConversionRowResult.Skipped)} " + $"Refused: {byResult.GetValueOrDefault(ConversionRowResult.Refused)} " + - $"Converted: {byResult.GetValueOrDefault(ConversionRowResult.Converted)} " + + $"Converted: {byResult.GetValueOrDefault(ConversionRowResult.Converted) - wouldConvert} " + + $"WouldConvert: {wouldConvert} " + $"Invalid: {byResult.GetValueOrDefault(ConversionRowResult.Invalid)} " + $"Error: {byResult.GetValueOrDefault(ConversionRowResult.Error)}"); } diff --git a/sources/EncodingChecker/ScanEngine.cs b/sources/EncodingChecker/ScanEngine.cs index fb10286..21522fe 100644 --- a/sources/EncodingChecker/ScanEngine.cs +++ b/sources/EncodingChecker/ScanEngine.cs @@ -1006,7 +1006,9 @@ automaticallyDetected is not null && return; } - entry.Result = ConversionRowResult.Converted; // "would be converted" + // Converted here means "would be converted"; the flag says so in reports. + entry.Result = ConversionRowResult.Converted; + entry.ConversionOnlyDecided = true; return; }