Skip to content

system/idempotence tests: also assert on line endings - #7233

Draft
matthewhughes934 wants to merge 1 commit into
rust-lang:mainfrom
matthewhughes934:eol-testing
Draft

matthewhughes934 wants to merge 1 commit into
rust-lang:mainfrom
matthewhughes934:eol-testing

Conversation

@matthewhughes934

Copy link
Copy Markdown
Contributor

While working on #7220 I got curious about how the integration tests handle line endings, specifically CRLF, and it turns out they don't. Given we have features that explicitly add/remove these line endings we should ensure the tests correctly cover this case to avoid regressions.

We use diff::lines to generate diffs, and this calls str::lines under the hood, which strip any trailing \r\n or \n. So the only way we can tell that line-endings differ is by comparing the two contents, previously we performed this comparison in a way that would ignore if one file had \r\n endings and another had \n endings. This meant we never actually asserted that we saw the expected line endings.

Note: we can only tell the files have differing line endings, not the lines where this occurs. It could be a future improvement to make this more specific, but I think the approach is reasonable since there aren't any tests (or really any use case) for mixed line-endings in a single file. This reflects the behaviour of how we report such errors with e.g. emitter::diff::DiffEmitter::emit_formatted_file

This required updating some test files that used newline_style=Windows but never actually added those line endings in the corresponding target. In these cases carriage returns are added to the target files, but not the source ones so these tests also assert that we do indeed convert the line endings.

The files
tests/{target,source}/configs/enum_discrim_align_threshold/40.rs had CRLF line endings, but not corresponding newline_style option and the feature does not depend on line endings, so these were converted back to used Unix line endings

  • I did not use an LLM to create a change in this PR.
  • I used an LLM to create a change in this PR, and I have explained below how it was used.

NB: this diff might not be very helpful to view in the GitHub UI, since it doesn't show line ending changes

@rustbot rustbot added the S-waiting-on-author Status: awaiting some action (such as code changes or more information) from the author. label Nov 5, 2026
@matthewhughes934 matthewhughes934 mentioned this pull request Nov 5, 2026
1 of 2 tasks
@matthewhughes934
matthewhughes934 force-pushed the eol-testing branch 2 times, most recently from 2554230 to cdb119e Compare November 5, 2026 21:18
While working on rust-lang#7220 I got curious about how the
integration tests handle line endings, specifically CRLF, and it turns
out they don't. Given we have features that explicitly add/remove these
line endings we should ensure the tests correctly cover this case to
avoid regressions.

We use `diff::lines` to generate diffs, and this calls `str::lines`
under the hood, which strip any trailing `\r\n` or `\n`. So the only way
we can tell that line-endings differ is by comparing the two contents,
previously we performed this comparison in a way that would ignore if
one file had `\r\n` endings and another had `\n` endings. This meant we
never actually asserted that we saw the expected line endings.

Note: we can only tell the files have differing line endings, not the
lines where this occurs. It _could_ be a future improvement to make this
more specific, but I think the approach is reasonable since there aren't
any tests (or really any use case) for mixed line-endings in a single
file. This reflects the behaviour of how we report such errors with e.g.
`emitter::diff::DiffEmitter::emit_formatted_file`

This required updating some test files that used `newline_style=Windows`
but never actually added those line endings in the corresponding
`target`. In these cases carriage returns are added to the `target`
files, but not the `source` ones so these tests also assert that we do
indeed convert the line endings.

The files
`tests/{target,source}/configs/enum_discrim_align_threshold/40.rs` had
CRLF line endings, but not corresponding `newline_style` option and the
feature does not depend on line endings, so these were converted back to
used Unix line endings
@matthewhughes934

matthewhughes934 commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor Author

🫠 wrestling with line endings + git is not the most run (git add --renormalize is my friend)

EDIT: why is ./tests/target/empty_file.rs not empty (it contains a newline character)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: awaiting some action (such as code changes or more information) from the author.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants