From 6a0deaeaafe38f50d3d9c8fea4350eedef857cb7 Mon Sep 17 00:00:00 2001 From: amrali-eg <32075105+amrali-eg@users.noreply.github.com> Date: Sat, 19 Sep 2026 08:32:29 +0300 Subject: [PATCH 1/3] Test legacy line-ending rewriting across chunk boundaries A coverage run showed four parts of LosslessFileWriter.NormalizeBytes, the raw-byte path for legacy encodings, never ran under the suite: the LF and CR target arms, a CR followed by another CR across a chunk boundary, and text after the last line ending. Legacy files are read in 64 KiB chunks and a trailing CR is carried to the next chunk to see whether an LF follows. A mistake there changes the number of lines in a file without any error, so the result must not depend on where the cut falls. Three kinds of test: every string of 'a', CR and LF up to length 6 is run under no split, one split and two splits at every position for all three targets and compared with a plain reference (about 105,000 cases); a hand-written CR-then-CR case across two chunks; and a Windows-1252 file with the line ending straddling the real 65536-byte boundary, followed by unterminated text, for all three targets. The code was correct, so this pins behavior rather than fixing a defect. Mutation-tested in two rounds by breaking the LF swallow, the unterminated-tail copy, the CR-CR carry and the LF arm: each made the matching tests fail. LosslessFileWriter.cs 61 -> 53 missed lines. Full suite 324/324, up from 316. Co-Authored-By: Claude Sonnet 5 --- .../LegacyLineEndingBoundaryTests.cs | 216 ++++++++++++++++++ 1 file changed, 216 insertions(+) create mode 100644 sources/LineEndingNormalizer.Tests/LegacyLineEndingBoundaryTests.cs diff --git a/sources/LineEndingNormalizer.Tests/LegacyLineEndingBoundaryTests.cs b/sources/LineEndingNormalizer.Tests/LegacyLineEndingBoundaryTests.cs new file mode 100644 index 0000000..4795ccb --- /dev/null +++ b/sources/LineEndingNormalizer.Tests/LegacyLineEndingBoundaryTests.cs @@ -0,0 +1,216 @@ +using System.Text; + +namespace LineEndingNormalizer.Tests; + +/// +/// Legacy files are rewritten as raw bytes in 64 KiB chunks, so a line ending can be cut in two +/// by a chunk boundary and the result must not depend on where the cut falls. +/// +/// +/// A CR at the end of one chunk cannot be resolved until the next chunk shows whether an LF +/// follows: CR then LF is one line ending, CR then anything else is two things. Getting that +/// wrong changes the number of lines in a file without any error. The direct test feeds every +/// small input to NormalizeBytes under every possible chunk split and compares each +/// result with a plain reference; the end-to-end test puts a real CRLF across the real +/// 65536-byte read boundary. Both also cover the LF and CR target arms and text after the last +/// line ending, none of which the rest of the suite reached. +/// +public sealed class LegacyLineEndingBoundaryTests +{ + private delegate int NormalizeBytesDelegate( + ReadOnlySpan input, + Span output, + LineEnding target, + ref bool pendingCr, + bool isFinal); + + private static readonly NormalizeBytesDelegate NormalizeBytes = + (NormalizeBytesDelegate)Delegate.CreateDelegate( + typeof(NormalizeBytesDelegate), + typeof(NewLineNormalizer).Assembly + .GetType("LineEndingNormalizer.LosslessFileWriter")! + .GetMethod( + "NormalizeBytes", + System.Reflection.BindingFlags.NonPublic | + System.Reflection.BindingFlags.Static)!); + + private static readonly LineEnding[] Targets = + [LineEnding.Crlf, LineEnding.Lf, LineEnding.Cr]; + + private static byte[] Replacement(LineEnding target) => target switch + { + LineEnding.Crlf => [0x0D, 0x0A], + LineEnding.Lf => [0x0A], + _ => [0x0D], + }; + + // CRLF, lone CR and lone LF are each one line ending; everything else is copied unchanged. + private static byte[] Reference(byte[] input, LineEnding target) + { + var output = new List(); + byte[] replacement = Replacement(target); + + for (int i = 0; i < input.Length; i++) + { + if (input[i] == 0x0D || input[i] == 0x0A) + { + if (input[i] == 0x0D && i + 1 < input.Length && input[i + 1] == 0x0A) + { + i++; + } + + output.AddRange(replacement); + } + else + { + output.Add(input[i]); + } + } + + return [.. output]; + } + + private static byte[] RunInChunks(byte[] input, int[] cuts, LineEnding target) + { + byte[] buffer = new byte[input.Length * 2 + 2]; + var result = new List(); + bool pendingCr = false; + int start = 0; + + foreach (int end in cuts.Append(input.Length)) + { + int written = NormalizeBytes( + input.AsSpan(start, end - start), buffer, target, ref pendingCr, isFinal: false); + result.AddRange(buffer.AsSpan(0, written).ToArray()); + start = end; + } + + int final = NormalizeBytes([], buffer, target, ref pendingCr, isFinal: true); + result.AddRange(buffer.AsSpan(0, final).ToArray()); + + return [.. result]; + } + + // Every string of 'a', CR and LF up to the given length. + private static IEnumerable AllInputs(int maxLength) + { + byte[] alphabet = [(byte)'a', 0x0D, 0x0A]; + var current = new List { Array.Empty() }; + + for (int length = 0; length <= maxLength; length++) + { + foreach (byte[] item in current) + { + yield return item; + } + + current = [.. current.SelectMany(prefix => alphabet.Select(b => (byte[])[.. prefix, b]))]; + } + } + + [Fact] + public void EveryChunkSplitOfEverySmallInputGivesTheSameResultAsAWholeRead() + { + int checkedCases = 0; + + foreach (LineEnding target in Targets) + { + foreach (byte[] input in AllInputs(6)) + { + byte[] expected = Reference(input, target); + + // No cut, one cut at every position, and two cuts at every pair of positions. + Assert.Equal(expected, RunInChunks(input, [], target)); + checkedCases++; + + for (int first = 0; first <= input.Length; first++) + { + Assert.Equal(expected, RunInChunks(input, [first], target)); + checkedCases++; + + for (int second = first; second <= input.Length; second++) + { + Assert.Equal(expected, RunInChunks(input, [first, second], target)); + checkedCases++; + } + } + } + } + + // Guards against the loops silently doing nothing. + Assert.True(checkedCases > 100_000, $"only {checkedCases} cases ran"); + } + + [Fact] + public void ACrAtTheEndOfOneChunkFollowedByACrInTheNextIsTwoLineEndings() + { + // Not a pair: the first CR must be resolved as its own line ending when the second CR + // arrives, rather than being merged with it or dropped. + bool pendingCr = false; + byte[] buffer = new byte[16]; + + int first = NormalizeBytes([(byte)'a', 0x0D], buffer, LineEnding.Lf, ref pendingCr, isFinal: false); + Assert.Equal([(byte)'a'], buffer[..first]); + Assert.True(pendingCr); + + // The carried CR is resolved as a line ending; the new CR is now pending in its place. + int second = NormalizeBytes([0x0D], buffer, LineEnding.Lf, ref pendingCr, isFinal: false); + Assert.Equal([0x0A], buffer[..second]); + Assert.True(pendingCr); + + int last = NormalizeBytes([(byte)'b'], buffer, LineEnding.Lf, ref pendingCr, isFinal: false); + Assert.Equal([0x0A, (byte)'b'], buffer[..last]); + Assert.False(pendingCr); + + Assert.Equal(0, NormalizeBytes([], buffer, LineEnding.Lf, ref pendingCr, isFinal: true)); + Assert.False(pendingCr); + } + + // Second byte LF: CR then LF is one line ending. Second byte CR: two line endings. + [Theory] + [InlineData("Crlf", 0x0A)] + [InlineData("Crlf", 0x0D)] + [InlineData("Lf", 0x0A)] + [InlineData("Lf", 0x0D)] + [InlineData("Cr", 0x0A)] + [InlineData("Cr", 0x0D)] + public void ALegacyFileWithALineEndingAcrossTheRealReadBoundaryIsRewrittenExactly( + string targetName, byte secondByte) + { + LineEnding target = Enum.Parse(targetName); + Encoding.RegisterProvider(CodePagesEncodingProvider.Instance); + Encoding cp1252 = Encoding.GetEncoding(1252); + + // 65535 bytes of accented text, then CR as the last byte of the first 64 KiB read and + // the second byte as the first byte of the next read, then text with no terminator. + byte[] head = [.. Enumerable.Repeat((byte)0xE9, 65535)]; + byte[] tail = cp1252.GetBytes("end of file"); + byte[] source = [.. head, 0x0D, secondByte, .. tail]; + Assert.Equal(0x0D, source[65535]); + Assert.Equal(secondByte, source[65536]); + + using var dir = new TempDirectory(); + string path = dir.WriteFile("boundary.txt", source); + + var info = new FileInfo(path); + var metadata = new FileMetadata( + info.Attributes, + info.Length, + info.CreationTimeUtc, + info.LastWriteTimeUtc, + info.LastAccessTimeUtc); + + using (FileStream handle = File.OpenRead(path)) + { + LosslessFileWriter.ConvertFile(handle, path, cp1252, target, metadata, createBackup: false); + } + + byte[] converted = File.ReadAllBytes(path); + + Assert.Equal(Reference(source, target), converted); + + // The accented bytes must come through untouched, and the unterminated tail intact. + Assert.Equal(head, converted[..head.Length]); + Assert.Equal(tail, converted[^tail.Length..]); + } +} From ecfc09b5e332d6c7063c62e94ce35d3cc420d1d5 Mon Sep 17 00:00:00 2001 From: amrali-eg <32075105+amrali-eg@users.noreply.github.com> Date: Sat, 19 Sep 2026 09:07:33 +0300 Subject: [PATCH 2/3] Broaden the legacy line-ending tests after review Follow-up from review of this PR. - Resolve the private members lazily and throw a message that names the member, so a rename no longer fails every test with an opaque type-initializer error; read the real BufferSize instead of assuming 65536; register the code page provider in a static constructor. - Report the target, input and cuts when the exhaustive test fails, and replace the >100,000 guard with the exact expected case count. - Add 0x85 to the exhaustive alphabet and to the end-to-end filler: NEL is a line separator in Unicode but an ordinary byte in a legacy file. - End-to-end cases: the CR at, one byte before and one byte after the read boundary for both CR+LF and CR+CR; a file of three reads with a line ending at each boundary and a trailing CR; files ending in a CR (short, exactly one read, only a CR); all-CR files that fill the output buffer to its worst case. - Test the hash path: the hashes WriteConvertedFileBytes records match independently computed XxHash3 and SHA-256 values, and VerifyConvertedFileBytes accepts matching output and rejects a changed byte and a truncation. - Correct the class comment: the CR+CR rows are not a CRLF, the exhaustive test goes to two cuts not every split, and drop the claim about the rest of the suite. Mutation-tested with eight breaks of NormalizeBytes, the hash, the verify step and the output buffer; each fails the matching tests. The output-buffer break only fails because the pool rounds a request up to a power of two, so the test needs a buffer of exactly BufferSize to catch it. 38 tests, up from 8; full suite 354. LosslessFileWriter.cs 92.5% -> 93.7%. Co-Authored-By: Claude Sonnet 5 --- .../LegacyLineEndingBoundaryTests.cs | 384 +++++++++++++++--- 1 file changed, 320 insertions(+), 64 deletions(-) diff --git a/sources/LineEndingNormalizer.Tests/LegacyLineEndingBoundaryTests.cs b/sources/LineEndingNormalizer.Tests/LegacyLineEndingBoundaryTests.cs index 4795ccb..d3e2363 100644 --- a/sources/LineEndingNormalizer.Tests/LegacyLineEndingBoundaryTests.cs +++ b/sources/LineEndingNormalizer.Tests/LegacyLineEndingBoundaryTests.cs @@ -1,3 +1,7 @@ +using System.IO.Hashing; +using System.Reflection; +using System.Runtime.ExceptionServices; +using System.Security.Cryptography; using System.Text; namespace LineEndingNormalizer.Tests; @@ -7,16 +11,36 @@ namespace LineEndingNormalizer.Tests; /// by a chunk boundary and the result must not depend on where the cut falls. /// /// -/// A CR at the end of one chunk cannot be resolved until the next chunk shows whether an LF -/// follows: CR then LF is one line ending, CR then anything else is two things. Getting that -/// wrong changes the number of lines in a file without any error. The direct test feeds every -/// small input to NormalizeBytes under every possible chunk split and compares each -/// result with a plain reference; the end-to-end test puts a real CRLF across the real -/// 65536-byte read boundary. Both also cover the LF and CR target arms and text after the last -/// line ending, none of which the rest of the suite reached. +/// A CR at the end of one chunk cannot be resolved until the next chunk shows what follows it: +/// CR then LF is one line ending, CR then any other byte is a line ending followed by that byte, +/// and a CR still pending at the end of the input is a line ending of its own. Getting that wrong +/// changes the number of lines in a file without any error. +/// +/// The direct tests feed every input of up to six bytes over {a, 0x85, CR, LF} to +/// NormalizeBytes under no cut, every single cut and every pair of cuts (two cuts at one +/// position leave an empty chunk) and compare each result with a plain reference. The end-to-end +/// tests put CR+LF and CR+CR across the real read-buffer boundary and one byte either side of it, +/// across three chunks, at the end of a file, and fill the output buffer to its worst case. The +/// byte-level path also records its own hashes, so the last tests check the hash it records for +/// the output and that verification rejects damaged output. +/// +/// +/// 0x85 is NEL in a Unicode file but an ordinary byte in a legacy one, so it must pass through +/// untouched, as must the other control bytes 0x0B and 0x0C. +/// /// public sealed class LegacyLineEndingBoundaryTests { + static LegacyLineEndingBoundaryTests() + { + // Program.Main registers this; tests run in a separate process and must register it + // themselves to resolve code page 1252 at all. + Encoding.RegisterProvider(CodePagesEncodingProvider.Instance); + } + + private const byte Cr = 0x0D; + private const byte Lf = 0x0A; + private delegate int NormalizeBytesDelegate( ReadOnlySpan input, Span output, @@ -24,24 +48,56 @@ private delegate int NormalizeBytesDelegate( ref bool pendingCr, bool isFinal); - private static readonly NormalizeBytesDelegate NormalizeBytes = - (NormalizeBytesDelegate)Delegate.CreateDelegate( - typeof(NormalizeBytesDelegate), - typeof(NewLineNormalizer).Assembly - .GetType("LineEndingNormalizer.LosslessFileWriter")! - .GetMethod( - "NormalizeBytes", - System.Reflection.BindingFlags.NonPublic | - System.Reflection.BindingFlags.Static)!); + // The members under test are private, so they are found by reflection. Resolving lazily and + // throwing a plain exception keeps a rename from surfacing as an opaque type-initializer + // failure that names neither the member nor the cause. + private static Type FindWriterType() => + typeof(NewLineNormalizer).Assembly.GetType("LineEndingNormalizer.LosslessFileWriter") + ?? throw new InvalidOperationException( + "LosslessFileWriter was not found; update these tests if it was renamed or moved."); + + private static MethodInfo FindWriterMethod(string name) => + FindWriterType().GetMethod(name, BindingFlags.NonPublic | BindingFlags.Static) + ?? throw new InvalidOperationException( + $"LosslessFileWriter.{name} was not found; update these tests if it was renamed."); + + private static readonly Lazy NormalizeBytesLazy = new(() => + { + try + { + return (NormalizeBytesDelegate)Delegate.CreateDelegate( + typeof(NormalizeBytesDelegate), FindWriterMethod("NormalizeBytes")); + } + catch (ArgumentException ex) + { + throw new InvalidOperationException( + "LosslessFileWriter.NormalizeBytes no longer has the signature these tests call; " + + "update the delegate in this file.", + ex); + } + }); + + private static NormalizeBytesDelegate NormalizeBytes => NormalizeBytesLazy.Value; + + private static int BufferSize => + (int)(FindWriterType() + .GetField("BufferSize", BindingFlags.NonPublic | BindingFlags.Static) + ?.GetRawConstantValue() + ?? throw new InvalidOperationException( + "LosslessFileWriter.BufferSize was not found; update these tests if it was renamed.")); private static readonly LineEnding[] Targets = [LineEnding.Crlf, LineEnding.Lf, LineEnding.Cr]; + public static TheoryData TargetNames => ["Crlf", "Lf", "Cr"]; + + private static LineEnding Parse(string targetName) => Enum.Parse(targetName); + private static byte[] Replacement(LineEnding target) => target switch { - LineEnding.Crlf => [0x0D, 0x0A], - LineEnding.Lf => [0x0A], - _ => [0x0D], + LineEnding.Crlf => [Cr, Lf], + LineEnding.Lf => [Lf], + _ => [Cr], }; // CRLF, lone CR and lone LF are each one line ending; everything else is copied unchanged. @@ -52,9 +108,9 @@ private static byte[] Reference(byte[] input, LineEnding target) for (int i = 0; i < input.Length; i++) { - if (input[i] == 0x0D || input[i] == 0x0A) + if (input[i] == Cr || input[i] == Lf) { - if (input[i] == 0x0D && i + 1 < input.Length && input[i + 1] == 0x0A) + if (input[i] == Cr && i + 1 < input.Length && input[i + 1] == Lf) { i++; } @@ -70,6 +126,14 @@ private static byte[] Reference(byte[] input, LineEnding target) return [.. output]; } + // Bytes that must never be treated as line endings on the byte-level path, none of them CR + // or LF: an accented letter, NEL, vertical tab, form feed and an ASCII letter. + private static byte[] Filler(int length) + { + byte[] pattern = [0xE9, 0x85, 0x0B, 0x0C, (byte)'a']; + return [.. Enumerable.Range(0, length).Select(i => pattern[i % pattern.Length])]; + } + private static byte[] RunInChunks(byte[] input, int[] cuts, LineEnding target) { byte[] buffer = new byte[input.Length * 2 + 2]; @@ -91,11 +155,23 @@ private static byte[] RunInChunks(byte[] input, int[] cuts, LineEnding target) return [.. result]; } - // Every string of 'a', CR and LF up to the given length. + // A failure here would otherwise show two byte arrays and nothing about which of the + // hundreds of thousands of inputs and cuts produced them. + private static void AssertSameBytes(byte[] expected, byte[] actual, LineEnding target, byte[] input, int[] cuts) + { + if (!actual.AsSpan().SequenceEqual(expected)) + { + Assert.Fail( + $"target {target}, input {Convert.ToHexString(input)}, cuts [{string.Join(',', cuts)}]: " + + $"expected {Convert.ToHexString(expected)} but got {Convert.ToHexString(actual)}"); + } + } + + // Every string over the alphabet of each length up to the maximum, shortest first. private static IEnumerable AllInputs(int maxLength) { - byte[] alphabet = [(byte)'a', 0x0D, 0x0A]; - var current = new List { Array.Empty() }; + byte[] alphabet = [(byte)'a', 0x85, Cr, Lf]; + List current = [[]]; for (int length = 0; length <= maxLength; length++) { @@ -104,41 +180,60 @@ private static IEnumerable AllInputs(int maxLength) yield return item; } - current = [.. current.SelectMany(prefix => alphabet.Select(b => (byte[])[.. prefix, b]))]; + if (length < maxLength) + { + current = [.. current.SelectMany(prefix => alphabet.Select(b => (byte[])[.. prefix, b]))]; + } } } + // Inputs of length n, times the no-cut, single-cut and pair-of-cuts runs for each. + private static long ExpectedCases(int maxLength, int alphabetSize, int targets) + { + long total = 0; + + for (int n = 0; n <= maxLength; n++) + { + long runs = 1 + (n + 1) + (long)(n + 1) * (n + 2) / 2; + total += (long)Math.Pow(alphabetSize, n) * runs; + } + + return total * targets; + } + [Fact] - public void EveryChunkSplitOfEverySmallInputGivesTheSameResultAsAWholeRead() + public void EveryCutOfEverySmallInputGivesTheSameResultAsAWholeRead() { - int checkedCases = 0; + const int MaxLength = 6; + long checkedCases = 0; foreach (LineEnding target in Targets) { - foreach (byte[] input in AllInputs(6)) + foreach (byte[] input in AllInputs(MaxLength)) { byte[] expected = Reference(input, target); // No cut, one cut at every position, and two cuts at every pair of positions. - Assert.Equal(expected, RunInChunks(input, [], target)); + AssertSameBytes(expected, RunInChunks(input, [], target), target, input, []); checkedCases++; for (int first = 0; first <= input.Length; first++) { - Assert.Equal(expected, RunInChunks(input, [first], target)); + AssertSameBytes(expected, RunInChunks(input, [first], target), target, input, [first]); checkedCases++; for (int second = first; second <= input.Length; second++) { - Assert.Equal(expected, RunInChunks(input, [first, second], target)); + int[] cuts = [first, second]; + AssertSameBytes(expected, RunInChunks(input, cuts, target), target, input, cuts); checkedCases++; } } } } - // Guards against the loops silently doing nothing. - Assert.True(checkedCases > 100_000, $"only {checkedCases} cases ran"); + // Guards against the loops silently doing nothing or the input generator changing shape. + Assert.Equal(ExpectedCases(MaxLength, alphabetSize: 4, targets: Targets.Length), checkedCases); } [Fact] @@ -149,48 +244,31 @@ public void ACrAtTheEndOfOneChunkFollowedByACrInTheNextIsTwoLineEndings() bool pendingCr = false; byte[] buffer = new byte[16]; - int first = NormalizeBytes([(byte)'a', 0x0D], buffer, LineEnding.Lf, ref pendingCr, isFinal: false); + int first = NormalizeBytes([(byte)'a', Cr], buffer, LineEnding.Lf, ref pendingCr, isFinal: false); Assert.Equal([(byte)'a'], buffer[..first]); Assert.True(pendingCr); // The carried CR is resolved as a line ending; the new CR is now pending in its place. - int second = NormalizeBytes([0x0D], buffer, LineEnding.Lf, ref pendingCr, isFinal: false); - Assert.Equal([0x0A], buffer[..second]); + int second = NormalizeBytes([Cr], buffer, LineEnding.Lf, ref pendingCr, isFinal: false); + Assert.Equal([Lf], buffer[..second]); Assert.True(pendingCr); int last = NormalizeBytes([(byte)'b'], buffer, LineEnding.Lf, ref pendingCr, isFinal: false); - Assert.Equal([0x0A, (byte)'b'], buffer[..last]); + Assert.Equal([Lf, (byte)'b'], buffer[..last]); Assert.False(pendingCr); Assert.Equal(0, NormalizeBytes([], buffer, LineEnding.Lf, ref pendingCr, isFinal: true)); Assert.False(pendingCr); } - // Second byte LF: CR then LF is one line ending. Second byte CR: two line endings. - [Theory] - [InlineData("Crlf", 0x0A)] - [InlineData("Crlf", 0x0D)] - [InlineData("Lf", 0x0A)] - [InlineData("Lf", 0x0D)] - [InlineData("Cr", 0x0A)] - [InlineData("Cr", 0x0D)] - public void ALegacyFileWithALineEndingAcrossTheRealReadBoundaryIsRewrittenExactly( - string targetName, byte secondByte) - { - LineEnding target = Enum.Parse(targetName); - Encoding.RegisterProvider(CodePagesEncodingProvider.Instance); + // Converts through the same entry point the tool uses, as Windows-1252 (a legacy code page), + // and returns the bytes left in the file. + private static byte[] ConvertLegacy(byte[] source, LineEnding target) + { Encoding cp1252 = Encoding.GetEncoding(1252); - // 65535 bytes of accented text, then CR as the last byte of the first 64 KiB read and - // the second byte as the first byte of the next read, then text with no terminator. - byte[] head = [.. Enumerable.Repeat((byte)0xE9, 65535)]; - byte[] tail = cp1252.GetBytes("end of file"); - byte[] source = [.. head, 0x0D, secondByte, .. tail]; - Assert.Equal(0x0D, source[65535]); - Assert.Equal(secondByte, source[65536]); - using var dir = new TempDirectory(); - string path = dir.WriteFile("boundary.txt", source); + string path = dir.WriteFile("legacy.txt", source); var info = new FileInfo(path); var metadata = new FileMetadata( @@ -205,12 +283,190 @@ public void ALegacyFileWithALineEndingAcrossTheRealReadBoundaryIsRewrittenExactl LosslessFileWriter.ConvertFile(handle, path, cp1252, target, metadata, createBackup: false); } - byte[] converted = File.ReadAllBytes(path); + return File.ReadAllBytes(path); + } + + private static void AssertConvertsExactly(byte[] source, LineEnding target) + { + byte[] converted = ConvertLegacy(source, target); + + // Compare lengths first so a mismatch on a file of hundreds of kilobytes reports a + // readable number instead of two enormous arrays. + byte[] expected = Reference(source, target); + Assert.True( + expected.Length == converted.Length, + $"target {target}: expected {expected.Length} bytes but the file holds {converted.Length}"); + Assert.True( + expected.AsSpan().SequenceEqual(converted), + $"target {target}: same length ({expected.Length}) but different bytes"); + } + + // The pair starts one byte before, at, and one byte after the last byte of the first read. + public static IEnumerable BoundaryCases() + { + foreach (string target in new[] { "Crlf", "Lf", "Cr" }) + { + foreach (int offset in new[] { -1, 0, 1 }) + { + foreach (byte second in new[] { Lf, Cr }) + { + yield return [target, offset, second]; + } + } + } + } + + [Theory] + [MemberData(nameof(BoundaryCases))] + public void ALineEndingAtTheReadBoundaryOrBesideItIsRewrittenExactly( + string targetName, int offset, byte secondByte) + { + // The CR is the last byte of the first read when the offset is 0, so CR+LF is split + // across two reads and CR+CR must stay two line endings. At -1 the pair sits inside the + // first read, and at +1 inside the second. + int crIndex = BufferSize - 1 + offset; + byte[] source = [.. Filler(crIndex), Cr, secondByte, .. Filler(11)]; + Assert.Equal(Cr, source[crIndex]); + Assert.Equal(secondByte, source[crIndex + 1]); + + AssertConvertsExactly(source, Parse(targetName)); + } + + [Theory] + [MemberData(nameof(TargetNames))] + public void AFileOfThreeReadsWithLineEndingsAtBothBoundariesIsRewrittenExactly(string targetName) + { + int size = BufferSize; + + var source = new List(); + source.AddRange(Filler(size - 1)); + source.AddRange([Cr, Lf]); // CR+LF across the first boundary + source.AddRange(Filler(2 * size - 1 - source.Count)); + source.AddRange([Cr, Cr]); // CR+CR across the second boundary + source.AddRange(Filler(3 * size - 1 - source.Count)); + source.Add(Cr); // the last byte of an exact multiple + + Assert.Equal(3 * size, source.Count); + + AssertConvertsExactly([.. source], Parse(targetName)); + } + + public static IEnumerable FilesEndingInCr() + { + foreach (string target in new[] { "Crlf", "Lf", "Cr" }) + { + foreach (string kind in new[] { "short", "one read", "only a CR" }) + { + yield return [target, kind]; + } + } + } + + [Theory] + [MemberData(nameof(FilesEndingInCr))] + public void AFileThatEndsInACrHasThatCrRewritten(string targetName, string kind) + { + // A trailing CR stays pending until the end of the input and is then flushed on its own, + // so it goes through a different write and hash step from every other line ending. + byte[] source = kind switch + { + "short" => [.. Filler(10), Cr], + "one read" => [.. Filler(BufferSize - 1), Cr], + _ => [Cr], + }; + + AssertConvertsExactly(source, Parse(targetName)); + } + + [Theory] + [InlineData(1)] + [InlineData(2)] + public void AllCrFilesFillTheOutputBufferToItsWorstCase(int reads) + { + // Every CR becomes two bytes, so a full read of CRs doubles in size: the output buffer + // has to hold twice the read, plus a carried CR, without overflowing. + byte[] source = [.. Enumerable.Repeat(Cr, reads * BufferSize + 1)]; + + AssertConvertsExactly(source, LineEnding.Crlf); + } + + // Runs the byte-level writer directly so the hashes it records can be checked. + private static (byte[] VerificationHash, byte[] SourceSha256) WriteBytes( + string sourcePath, LineEnding target, string tempPath) + { + using FileStream source = File.OpenRead(sourcePath); + + object result = FindWriterMethod("WriteConvertedFileBytes") + .Invoke(null, [source, target, tempPath, CancellationToken.None])!; + + Type type = result.GetType(); + + return ( + (byte[])type.GetProperty("VerificationHash")!.GetValue(result)!, + (byte[])type.GetProperty("SourceSha256")!.GetValue(result)!); + } + + private static void VerifyBytes(string tempPath, byte[] expectedHash) + { + try + { + FindWriterMethod("VerifyConvertedFileBytes") + .Invoke(null, [tempPath, expectedHash, CancellationToken.None]); + } + catch (TargetInvocationException ex) when (ex.InnerException is not null) + { + ExceptionDispatchInfo.Capture(ex.InnerException).Throw(); + } + } + + [Theory] + [MemberData(nameof(TargetNames))] + public void TheHashesRecordedWhileWritingDescribeTheBytesActuallyWritten(string targetName) + { + LineEnding target = Parse(targetName); + + // More than one read, a line ending in the middle, and a trailing CR that is flushed + // separately after the read loop ends. + byte[] source = [.. Filler(BufferSize + 10), Cr, Lf, .. Filler(5), Cr]; + + using var dir = new TempDirectory(); + string sourcePath = dir.WriteFile("source.txt", source); + string tempPath = dir.CombinePath("output.tmp"); + + var (verificationHash, sourceSha256) = WriteBytes(sourcePath, target, tempPath); + + byte[] expected = Reference(source, target); + Assert.Equal(expected, File.ReadAllBytes(tempPath)); + Assert.Equal(XxHash3.Hash(expected), verificationHash); + Assert.Equal(SHA256.HashData(source), sourceSha256); + + // Verification accepts output that matches its recorded hash. + VerifyBytes(tempPath, verificationHash); + } + + [Fact] + public void VerificationRejectsOutputThatNoLongerMatchesItsRecordedHash() + { + byte[] source = [.. Filler(100), Cr, Lf, .. Filler(100)]; + + using var dir = new TempDirectory(); + string sourcePath = dir.WriteFile("source.txt", source); + string tempPath = dir.CombinePath("output.tmp"); + + var (verificationHash, _) = WriteBytes(sourcePath, LineEnding.Crlf, tempPath); + byte[] written = File.ReadAllBytes(tempPath); + + // The control: untouched output verifies. + VerifyBytes(tempPath, verificationHash); - Assert.Equal(Reference(source, target), converted); + // One byte changed, same length. + byte[] changed = [.. written]; + changed[0] ^= 0x01; + File.WriteAllBytes(tempPath, changed); + Assert.Throws(() => VerifyBytes(tempPath, verificationHash)); - // The accented bytes must come through untouched, and the unterminated tail intact. - Assert.Equal(head, converted[..head.Length]); - Assert.Equal(tail, converted[^tail.Length..]); + // Truncated by one byte. + File.WriteAllBytes(tempPath, written[..^1]); + Assert.Throws(() => VerifyBytes(tempPath, verificationHash)); } } From b630df817a790fc51107fb7400535b4faaad3efa Mon Sep 17 00:00:00 2001 From: amrali-eg <32075105+amrali-eg@users.noreply.github.com> Date: Sat, 19 Sep 2026 09:14:39 +0300 Subject: [PATCH 3/3] Add an independent oracle and an empty-file case; correct two comments Follow-up from the second review of this PR. - Check the reference model against a regular expression replace (CRLF | CR | LF, CRLF first) over every exhaustive input. The reference follows the same rule as the code, so a shared misunderstanding would have passed both. Latin-1 maps each byte to one character, so the bytes survive the round trip. - Add an empty file through ConvertFile for all three targets: the read loop never runs, so only the final flush and the hash of nothing are involved. - Rename the all-CR test to AllCrFilesDoubleInSizeExactly and say what it cannot see. It never filled the buffer to its worst case: a read of CRs emits at most twice its size because the last CR stays pending, and the pool rounds the request up to a larger array, so only a capacity of exactly one read fails. Rename its parameter to fullReads, since the file spans one read more than that. - Say that 0x85 is an ellipsis in Windows-1252, not NEL. - Unwrap TargetInvocationException in both reflective helpers through one InvokeWriter, so a writer failure is not reported as a reflection error. Mutation-tested: a reference that treats CRLF as two endings fails the oracle test; a final flush that emits a line ending with nothing pending fails the three empty-file cases among others; an output buffer of exactly one read fails the renamed test. 42 tests, up from 38; full suite 358. Co-Authored-By: Claude Sonnet 5 --- .../LegacyLineEndingBoundaryTests.cs | 79 ++++++++++++++----- 1 file changed, 59 insertions(+), 20 deletions(-) diff --git a/sources/LineEndingNormalizer.Tests/LegacyLineEndingBoundaryTests.cs b/sources/LineEndingNormalizer.Tests/LegacyLineEndingBoundaryTests.cs index d3e2363..85810d2 100644 --- a/sources/LineEndingNormalizer.Tests/LegacyLineEndingBoundaryTests.cs +++ b/sources/LineEndingNormalizer.Tests/LegacyLineEndingBoundaryTests.cs @@ -3,6 +3,7 @@ using System.Runtime.ExceptionServices; using System.Security.Cryptography; using System.Text; +using System.Text.RegularExpressions; namespace LineEndingNormalizer.Tests; @@ -127,7 +128,8 @@ private static byte[] Reference(byte[] input, LineEnding target) } // Bytes that must never be treated as line endings on the byte-level path, none of them CR - // or LF: an accented letter, NEL, vertical tab, form feed and an ASCII letter. + // or LF: an accented letter, 0x85 (NEL as U+0085 in Unicode, an ellipsis in Windows-1252), + // vertical tab, form feed and an ASCII letter. private static byte[] Filler(int length) { byte[] pattern = [0xE9, 0x85, 0x0B, 0x0C, (byte)'a']; @@ -187,7 +189,8 @@ private static IEnumerable AllInputs(int maxLength) } } - // Inputs of length n, times the no-cut, single-cut and pair-of-cuts runs for each. + // The runs per input (no cut, every single cut, every pair of cuts) times the inputs of each + // length from 0 to the maximum, times the targets. private static long ExpectedCases(int maxLength, int alphabetSize, int targets) { long total = 0; @@ -236,6 +239,28 @@ public void EveryCutOfEverySmallInputGivesTheSameResultAsAWholeRead() Assert.Equal(ExpectedCases(MaxLength, alphabetSize: 4, targets: Targets.Length), checkedCases); } + [Fact] + public void TheReferenceModelAgreesWithARegularExpressionReplace() + { + // The reference follows the same rule as the code, so a shared misunderstanding would + // pass both. A regular expression states the rule differently: an alternation that tries + // CRLF first. Latin-1 maps every byte to one character, so bytes survive the round trip. + foreach (LineEnding target in Targets) + { + string replacement = Encoding.Latin1.GetString(Replacement(target)); + + foreach (byte[] input in AllInputs(6)) + { + byte[] viaRegex = Encoding.Latin1.GetBytes( + Regex.Replace(Encoding.Latin1.GetString(input), @"\r\n|\r|\n", replacement)); + + Assert.True( + Reference(input, target).AsSpan().SequenceEqual(viaRegex), + $"target {target}, input {Convert.ToHexString(input)}"); + } + } + } + [Fact] public void ACrAtTheEndOfOneChunkFollowedByACrInTheNextIsTwoLineEndings() { @@ -301,6 +326,14 @@ private static void AssertConvertsExactly(byte[] source, LineEnding target) $"target {target}: same length ({expected.Length}) but different bytes"); } + [Theory] + [MemberData(nameof(TargetNames))] + public void AnEmptyFileStaysEmpty(string targetName) + { + // The read loop never runs, so only the final flush and the hash of nothing are involved. + AssertConvertsExactly([], Parse(targetName)); + } + // The pair starts one byte before, at, and one byte after the last byte of the first read. public static IEnumerable BoundaryCases() { @@ -381,23 +414,39 @@ public void AFileThatEndsInACrHasThatCrRewritten(string targetName, string kind) [Theory] [InlineData(1)] [InlineData(2)] - public void AllCrFilesFillTheOutputBufferToItsWorstCase(int reads) + public void AllCrFilesDoubleInSizeExactly(int fullReads) { - // Every CR becomes two bytes, so a full read of CRs doubles in size: the output buffer - // has to hold twice the read, plus a carried CR, without overflowing. - byte[] source = [.. Enumerable.Repeat(Cr, reads * BufferSize + 1)]; + // Every CR becomes two bytes, so a read of CRs doubles in size. This checks the result + // stays exact at that size. It cannot see an output buffer that is only slightly too + // small: the pool rounds the request up to a larger array, so only a capacity of exactly + // one read fails here. + byte[] source = [.. Enumerable.Repeat(Cr, fullReads * BufferSize + 1)]; AssertConvertsExactly(source, LineEnding.Crlf); } + // Invokes a private static method and rethrows what it threw, not the reflection wrapper. + private static object? InvokeWriter(string name, params object[] arguments) + { + try + { + return FindWriterMethod(name).Invoke(null, arguments); + } + catch (TargetInvocationException ex) when (ex.InnerException is not null) + { + ExceptionDispatchInfo.Capture(ex.InnerException).Throw(); + throw; + } + } + // Runs the byte-level writer directly so the hashes it records can be checked. private static (byte[] VerificationHash, byte[] SourceSha256) WriteBytes( string sourcePath, LineEnding target, string tempPath) { using FileStream source = File.OpenRead(sourcePath); - object result = FindWriterMethod("WriteConvertedFileBytes") - .Invoke(null, [source, target, tempPath, CancellationToken.None])!; + object result = InvokeWriter( + "WriteConvertedFileBytes", source, target, tempPath, CancellationToken.None)!; Type type = result.GetType(); @@ -406,18 +455,8 @@ private static (byte[] VerificationHash, byte[] SourceSha256) WriteBytes( (byte[])type.GetProperty("SourceSha256")!.GetValue(result)!); } - private static void VerifyBytes(string tempPath, byte[] expectedHash) - { - try - { - FindWriterMethod("VerifyConvertedFileBytes") - .Invoke(null, [tempPath, expectedHash, CancellationToken.None]); - } - catch (TargetInvocationException ex) when (ex.InnerException is not null) - { - ExceptionDispatchInfo.Capture(ex.InnerException).Throw(); - } - } + private static void VerifyBytes(string tempPath, byte[] expectedHash) => + InvokeWriter("VerifyConvertedFileBytes", tempPath, expectedHash, CancellationToken.None); [Theory] [MemberData(nameof(TargetNames))]