Composable matcher diffing

[maxlinc]

The new composable matchers are awesome and I'm hoping to make use of them. I think it makes sense to format the expectations using the description rather than inspect, but the output (particularly diffable output) doesn't always make sense.

Consider the case of one missing object. The contain_exactly output is pretty good:

expect(['aa', 'bb', 'cc']).to contain_exactly(
  a_string_ending_with('a'),
  a_string_ending_with('b'),
  a_string_ending_with('c'),
  a_string_ending_with('d')
) #=>
# RSpec::Expectations::ExpectationNotMetError: expected collection contained:  [(a string ending with "a"), (a string ending with "b"), (a string ending with "c"), (a string ending with "d")]
# actual collection contained:    ["aa", "bb", "cc"]
# the missing elements were:      [(a string ending with "d")]

The include and match output doesn't make as much sense:

expect(['aa', 'bb', 'cc']).to include(
  a_string_ending_with('a'),
  a_string_ending_with('b'),
  a_string_ending_with('c'),
  a_string_ending_with('d')
) #=>
# RSpec::Expectations::ExpectationNotMetError: expected ["aa", "bb", "cc"] to match [(a string ending with "a"), (a string ending with "b"), (a string ending with "c"), (a string ending with "d")]
# Diff:
# @@ -1,5 +1,2 @@
# -[(a string ending with "a"),
# - (a string ending with "b"),
# - (a string ending with "c"),
# - (a string ending with "d")]
# +["aa", "bb", "cc"]

It's displaying a diff, but the diff didn't give me any information. It makes it look like all of my matchers failed, when really the problem is just the last one.

Hope the feedback helps. Thanks for the new features in rspec3 beta!

[myronmarston]

Hmm. I wonder if we should just disable diffs when composed matchers are involved.

Thoughts from others?

[JonRowe]

Makes sense to me

[AlexisKAndersen]

The diff half the reason I like the composed matchers. Is there any way to keep that feature? Or is the way the diff is performed too ignorant of the idea of composed matchers to make it work?

[myronmarston]

The diff half the reason I like the composed matchers. Is there any way to keep that feature? Or is the way the diff is performed too ignorant of the idea of composed matchers to make it work?

Well, we haven't made this change yet. But diffs are meant to show you where two nearly identical strings are different. In the case of an object vs a matcher that represents some aspect of that object, the string output is often entirely different, and in some cases a sub-element of a collection matches a matcher but in the diff it'll claim they are different since the output is different...so it seems confusing to diff them and I still lean towards making this change. Can you explain why you like the diff for composed matchers? Maybe some examples where it's helped you would help.

[nevinera]

This behavior still behaves as it did then, and still seems just as pointless:

  1) testing test 1
     Failure/Error:
       expect(['aa', 'bb', 'cc']).to include(
         a_string_ending_with('a'),
         a_string_ending_with('b'),
         a_string_ending_with('c'),
         a_string_ending_with('d')
       )

       expected ["aa", "bb", "cc"] to include (a string ending with "d")
       Diff:
       @@ -1,5 +1,2 @@
       -[(a string ending with "a"),
       - (a string ending with "b"),
       - (a string ending with "c"),
       - (a string ending with "d")]
       +["aa", "bb", "cc"]

I think I could "just disable diffs when composed matchers are involved" if that were still the desire, and I have trouble seeing how such a diff could be useful - the entirety of expected and actual will always differ, since we're not evaluating the matchers, right? I tried a few variations, and couldn't produce any combination that show a diff with any lines that didn't change..

[JonRowe]

Eventually we'd like a configurable differ, and one that is more intelligent, the current one has no choice but to produce diffs as strings which causes this behaviour, I'd rather improve the situation than disable the diff, in an ideal world we'd have a diff that highlighted the missing a string ending with "d"

[nevinera]

I think that perhaps we should leave the door open by encapsulating the Differ nicely, and letting it be responsible for the decision about what things it can usefully diff? For example, the current differ believes that it can't usefully diff single-line strings, or arrays of them. The caller could just call the differ, and if it gets back nil (or an empty string, which is what it's using for that currently), know that the differ thinks it's not equipped to provide a useful diff, so one shouldn't be displayed. That is essentially how it works now - the Differ as it exists already makes all the decisions about whether or not to display a diff, as far as I can tell.

So I think that if we implemented an AwesomeDiffer (™) of some sort, it would necessarily come with its own set of decisions about what it could usefully diff.. we'd then need to work out if some of those decisions actually don't belong in the Differ? But I guess I feel like disabling the diff in particular cases here shouldn't really impact whether it's disabled under AwesomeDiffer.