Repository navigation
Provide option to not trim output strings #77
Description
Activity
By the way, I'm not curious if this way of trimming behaves the same under Windows as it does on Unix-like systems.
Thanks for the feedback. While trimming is a help to most people, I can see how some cases need exact handling.
By the way, I'm not curious if this way of trimming behaves the same under Windows as it does on Unix-like systems.
You are not curious? Then you probably don't want to read this: (:trollface:)
Returns a string slice with leading and trailing whitespace removed.
'Whitespace' is defined according to the terms of the Unicode Derived Core Property White_Space.
– https://doc.rust-lang.org/1.21.0/std/primitive.str.html#method.trim
Huhu, @killercup! That was supposed to be a "now".
Yeah, I think I wasn't clear about that comment. I meant the
"\n"part of that line, not thetrim()part. Does "\n" become two chars in Windows? (I haven't used Rust in Windows, that's why I'm asking.)Hi Behnam! 👋 I have no way to test this on Windows but
\r\nis stripped on playpen.Do you feel like digging into libstd and friends? 'Cause I feel like it. Let's look at the code! This leads me here which in turn brings me to this and finally to this impl of
trim_matches. The last three steps are not even necessary, I just wanted to make sure there is nothing platform specific there. The most interesting this isself.trim_matches(|c: char| c.is_whitespace())
So, what is
is_whitespace? So glad you ask! It's this simple fn. So every thing from 0x09 to 0x0d is whitespace—and there we finally have it: 0xa is LF (\n), 0x0d is CR (\r)! Let's check to make sure… yep. That's it.(Sorry, you probably knew all that. But I kinda wanted to write it down cause I like this kind of thing 😅)
@killercup, nice writing, but for the newline matters, I'm not talking about Rust's
trim()method anymore, but instead howdifferenceis used here to match output that may have Unix-like newlines or Windows newlines.I looked at the documentation for
Changeset::new(), and looks like theseparatorargument is only used for interpreting the diff, if there's any, but it doesn't affect the matching.And I don't see any newline conversion being done outside of
matches_exact()or other functions.So, if I write an
echocommand and test it like this:Assert::main_binary() .with_args(&["Hello"]) .stdout().is("Hello") //. .stdout().contains("Hello\n") //. .unwrap();
it will be passing on Unix, but will fail on Windows.
So, if the goal is to be able to support cross-platform matching for multiline outputs either a newline-less pattern should be supported (like
.has_lines(&["First line.", "Second line."])), or different newline sequences need to be treated equally when checking the diff.What do you think, @killercup? Is this a goal to support any matching string (via
is()orcontains()) that contains a newline char?Oh, I see, thanks for clarifying, @behnam!
I see two options here: We just note in the docs that you need to take care of platform-specific line endings, or our matchers deal with that automatically. I'm in favor of the latter, because I don't want to deal with that kind of stuff as a user.
but instead how difference is used here to match output that may have Unix-like newlines or Windows newlines.
Adding this to the difference crate is most likely the best way to do it (even if just to avoid the
str.lines().map(str::trim).collect()stuff) and they have an issue that kinda goes into that direction: johannhof/difference.rs#10. (It's called "Edit distance assert should treat all whitespace the same" but Unix and Windows line endings have a different number of line ending characters, so I'm not sure it actually covers it.)(Btw, I have the feeling that most people use
.contains()and only ever check for single line content, but this is completely not backe by facts and I would actually have some feedback on whether that is true!)I have the feeling that most people use .contains()
Right. Most people don't need multi-line matching. But for some applications it's necessary.
Adding this to the difference crate is most likely the best way to do it
Agreed. Almost any diff library/command I know has options for ignoring newline differences, and possibly more generally, number of whitespaces (one or more). For CLI testing, I need the former, but think that some applications can use of the latter, as well.
/cc @johannhof
For now, I think I'll have to use
#[cfg]to get my tests pass in Windows.My proposed API for this.
Outputis our user-facing predicate type once #74 or #75 goes in. We could addtrim(bool)- trims value passed into
verify_str - default value: true
- trims value passed into
normalize_newline(bool)- for the value passed into
verify_str, convert CR+LF to LF - default value: true?
- for the value passed into
So the API would look like
Assert::main_binary() .stdout(contains("Music\nMagic").trim(false).normalize_newline(false)) .unwrap()
Reacted by Behnam EsfahbodCorrect. One benefit of changing to this is it would create consistency across different predicates.
FYI, I just learned something about Rust on Windows:
writeln!()andprintln!()explicitly say:On all platforms, the newline is the LINE FEED character (
\n/U+000A) alone (no additional CARRIAGE RETURN (\r/U+000D).From:
So, the example I provided before, it actually does work fine in Windows. But, because my implementation depends on
writeln!(), and if I fix that part (to get it work as expected on Windows), then the test will fail.This could be the reason
difference.rsdoesn't deal with CR already.The responsibility for this is moving to assert_cmd and predicate-rs.
- trimming is an optional feature in the above set of crates
- newline conversion will be addressed in normalize_newlines adapter predicate predicates-rs#45
assert_cli is going to morph into a wider-focused tool, including things like assert_fs. See #41
https://github.com/killercup/assert_cli/blob/3bc97874a188423fcdc3252c5d677b0fb048530e/src/output.rs#L34
Writing an
echo-like tool, I want to be able to test the output for detailed behavior on trailing newline characters. Unfortunately, it's not possible now because of the trimming done on both strings.It would be great to have an option to disable the trimming, as a config or an alternate to
is(), likeis_exactly().