Skip to content

Reject carriage returns in verified files instead of normalizing#1762

Merged
SimonCropp merged 4 commits into
mainfrom
Reject-carriage-returns-in-verified-files-instead-of-normalizing
Jun 29, 2026
Merged

Reject carriage returns in verified files instead of normalizing#1762
SimonCropp merged 4 commits into
mainfrom
Reject-carriage-returns-in-verified-files-instead-of-normalizing

Conversation

@SimonCropp

Copy link
Copy Markdown
Member

No description provided.

The text comparison previously read the verified file via
ReadStringBuilderWithFixedLines, silently converting \r\n and \r to \n.
Now it reads the file directly and throws if it contains a \r, linking
to the text-file-settings docs. Verified files are LF-only (enforced by
.gitattributes), so this surfaces misconfigured line endings rather than
masking them.

Drop the now-unused ReadStringBuilderWithFixedLines(string) overload and
rewrite the two net9 NewLineTests to assert the throw.
@SimonCropp SimonCropp added this to the 31.21.0 milestone Jun 29, 2026
@SimonCropp
SimonCropp merged commit 7253acb into main Jun 29, 2026
3 of 5 checks passed
@SimonCropp
SimonCropp deleted the Reject-carriage-returns-in-verified-files-instead-of-normalizing branch June 29, 2026 07:37
This was referenced Jul 6, 2026
SimonCropp added a commit that referenced this pull request Jul 16, 2026
* Improve the invalid line ending failure

The verified file line ending check threw before received was written, so a
CRLF verified file produced no output at all. That reads as "verify did
nothing" rather than a line ending problem, and the cause was buried:
VerifyEngine wraps every exception in "Failed to compare files", so the outer
message runners surface said nothing about line endings.

Write received before throwing, so the failure produces a snapshot, and
accepting it rewrites verified with \n endings. Throw a dedicated
VerifiedLineEndingException that VerifyEngine rethrows unwrapped, and name the
likely cause (git checking the file out with \r\n) and the .gitattributes fix
in the message.

Surfaced by VerifyTests/Verify.DiffPlex#474, where the missing output was
misattributed to Verify.DiffPlex.

Also correct two dead guards. NET10 is not a symbol the SDK defines (it is
NET10_0), so #if NET10 was always false, and TrailingNewlinesRaw was
additionally nested inside the #if NET9_0 above it, requiring both symbols.
TrailingNewlinesRaw, SecondsFractionUpperLong and SecondsFractionUpperShort had
never compiled; all three pass once the guards are corrected. Notably
TrailingNewlinesRaw covers the trailing newline half of #1762, which until now
had no running test.

TrailingNewlinesRaw hand-manages its verified file (delete, write, verify,
delete), so the tracked NewLineTests.TrailingNewlinesRaw.verified.txt was a
leftover that every run deletes, leaving a dirty tree. Untrack it, matching
StringWithDifferingNewline which uses the same pattern. Also remove the
TrailingNewlinesObject snapshot, whose test is commented out.

* Use the failing file's extension in the line ending message

The suggested .gitattributes line hardcoded *.verified.txt, but the verified
file may be json, xml, or any other registered text extension, so the
suggestion did not match the file it was reported against.

Interpolate FilePair.Extension instead, and cover it with a json case. The
extension being unread would now also fail the build (CS9113).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant