Fix the test framework is failing in CRLF env. #2028
Conversation
wada314
changed the title
There was a problem hiding this comment.
Thanks for the PR! I think the comment is out of date now though.
I am a bit worried about the performance of the second approach since it requires re-allocating the entire text for both the formatted and unformatted text. I expect that != just loops over s.chars() and compares them, so it would be fairly straightforward to write your own function which compares two strings, but treats \r\n and \n as equivalent.
|
Thank you for reviewing! Yes I agree for your concern. I wrote a function to compare bytes automatically converting |
tests/system.rs
Outdated
|
|
||
| #[test] | ||
| fn string_eq_ignore_newline_repr_test() { | ||
| assert!(string_eq_ignore_newline_repr("a\nb\nc\rd", "a\nb\r\nc\rd")); |
There was a problem hiding this comment.
Could you add a few more test cases please? For example, where the left or right are empty, where they match trivially, where they don't match, and where the left and right are different lengths.
tests/system.rs
Outdated
|
|
||
| // Compare strings without distinguishing between CRLF and LF | ||
| fn string_eq_ignore_newline_repr(left: &str, right: &str) -> bool { | ||
| let left = BytesIgnoreNewlineRepr(left.bytes().peekable()); |
There was a problem hiding this comment.
I think it would be better to use chars rather than bytes here, and iterate over chars (I'm not sure if it is possible, but I feel like, there might be some subtle bug if b'\r' is a byte in a longer char
|
Yeah I somehow messed my repository by rebasing 😆 I needed to squash my commits to fix it, hope it doesn't matter.
Personally I lean into bytes but I don't care actually so changed to chars.
added some. |
|
Great, thanks for the changes! |
Fixes #1318 .
This just fixes the framework error: Many tests are still failing under Windows machine because of CRLF newline. Maybe I follow up with another pull requests.
Note: I cannot use str::lines() method to compare because that method does not respects the last empty line in the file.
https://doc.rust-lang.org/std/string/struct.String.html#method.lines