More readable assert_eq proposal

[lpil]

Rendered

Improve assert_eq failure message formatting to increase legibility

A continuation of #1864.

[p-kraszewski]

This could be a breaking change if people are parsing this text. I feel the format of this text shouldn't be relied upon, so this is probably OK.

Perhaps it might be feasible to extend --message-format=json influence to test output? CI systems would be very beneficial from this change.

[jethrogb]

I think panics in general could benefit from a multi-line scheme

[joshtriplett]

@jethrogb While I wouldn't want to block this focused PR over that, yes, in general I'd love to see panics handled more gracefully. I don't want to encourage people to use .unwrap(), but right now even .expect(...) produces a fairly user-hostile message.

[ExpHP]

Do note that textual diffs (mis)take the map for the territory; a == b does not imply that a and b have the same Debug output. (e.g. -0. == 0.)

I don't think should be a deal breaker; but it is a limitation which spells diminishing returns for text-based strategies. Compared to textual diffs, the linebreaks and 1-space alignment in the RFC provide quite a bit more bang to the buck.

[lpil]

I think we should only do string based diffing for strings, other types could have their own algorithms.

[ExpHP]

I think we should only do string based diffing for strings, other types could have their own algorithms.

Unfortunately that would require a new trait bound for assert_eq! (or an addition to the Debug trait), which seems to me like a nonstarter...

This is something that could be experimented with in a third party crate though.

[cjxgm]

Should we generally not put too many words into the panic message?
That is, print out details then panic with a short message. Something like this:

---- assertion `(left == right)` failed ----
left:  `"Syntax Error: a.rb:1: syntax error, unexpected end-of-input\n\n"`
right: `"Syntax error: a.rb:1: syntax error, unexpected end-of-input\n\n"`
         ~~~~~~ ^ ~~~~

---- log_packet::tests::syntax_error_test stdout ----
        thread 'log_packet::tests::syntax_error_test' panicked at 'assertion failure', src/log_packet.rs:102
note: Run with `RUST_BACKTRACE=1` for a backtrace.

[aturon]

Punting to @BurntSushi, sorry for the very long delay :(

[lpil]

Thank you :)

[BurntSushi]

@rfcbot fcp merge

[BurntSushi]

@lpil Thanks so much for this! I think the libs team is generally in favor of this, and we think this could probably could have squeak its way through as a PR. However, we should just move forward with the RFC since it is already written. :-)

As the stdlib does not contain any terminal colour manipulation features

rustc certainly emits colored error messages, so there must be some support for that.

[rfcbot]

Team member @BurntSushi has proposed to merge this. The next step is review by the rest of the tagged teams:

• @BurntSushi
• @Kimundi
• @alexcrichton
• @aturon
• @brson
• @sfackler

No concerns currently listed.

Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[mgattozzi]

Might be a bit late but I think this would be great and really plays into the ergonomics goal of Rust in 2017. I'm in favor of it. If it could look like the Elixir example given in the RFC I'd prefer that personally but LLVM style arrows is good too if that's not possible yet.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[colin-kiegel]

The RFC should also address assert_ne (as suggested in #1864 (comment)).

How would those LLVM style arrows look like, if both left/right expressions wrap around several lines and the difference is somewhere in the middle? Would that still be readable and look good?

Just as a sidenote: git diff --word-diff uses a format, which is also readable without colors.

The quick [-red-]{+brown+} fox jumps over the lazy dog

Maybe printing a diff in this style could be a colorless alternative to LLVM style arrows, but it might become less readable or even ambiguous if the underlying text contains a lot of special characters, too.

Alternatively, inspecting/printing the diff of the debug outputs could be left to a third party crate like pretty_assertions. As already pointed out, diffing the debug output is nothing more than a heuristic anyway, since debug output equality can be completely unrelated to the values equality.

[eddyb]

I've had a chance to use pretty_assertions recently and it made a painful search for a bug trivial 🎉
It would be useful for developing rustc itself, as we now have output diff tests for which line diff output can be very annoying to decipher (most of the lines look identical anyway, with small changes sprinkled).

[CajetanP]

It clearly is a major improvement which will surely make using Rust even more comfortable than it currently is. I don't see any reason why it shouldn't be approved. As mentioned above, colours would be great but we can settle for LLVM style arrows if that's currently not possible.

[colin-kiegel]

I think assert_ne should also switch to multi-line display. But highlighting differences only makes sense for assert_eq, not assert_ne. After all assert_ne is expected to fail when both variables are identical (w.r.t PartialEq).

[dlight]

Unfortunately that would require a new trait bound for assert_eq! (or an addition to the Debug trait), which seems to me like a nonstarter...

Specialization provides a backwards compatible manner to add this feature: create a new trait AssertFormat for the richer formatting, and use trait specialization to either print using just Debug info (for types that don't implement AssertFormat) or Debug and AssertFormat.

[Eh2406]

Note that this has been discussed before: https://users.rust-lang.org/t/suggestions-for-better-test-output/7494

[rfcbot]

The final comment period is now complete.

[aturon]

This RFC has now been merged! Discussion should move to the tracking issue.

I added a couple of unresolved questions, one of them for assert_ne; these should be resolved during the implementation, and I encourage folks to discuss them on the tracking issue.

For some reason github isn't recognizing the merge, so I'm going to manually close, but the official RFC is here.

[lpil]

Thank you :)

[vitiral]

This is not strickly related to the RFC -- I think this is a great first step! -- but do you think we could have a new traits related to the Eq* variety, like maybe DebugEq and DebugPartialEq that would help with the debugging of more complex types like Array's and HashMaps. These types would return either Ok(()) for equal or Err(String) for not-equal with helpful information.

I ask because this is really great for comparing strings but does almost nothing in those extremely common cases.

Again, super happy to see this change!