Test legacy line-ending rewriting across chunk boundaries - #34
Merged
Merged
Conversation
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
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.a, CR and LF up to length 6 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.Test plan
LosslessFileWriter.cs: 61 -> 53 missed lines (92.5% -> 93.4%); the only line left in that function is the unreachable default arm🤖 Generated with Claude Code