Array diffing behaviour questionable

[jarednorman]

Currently, if you have an example like expect(array1).to eq(array2) a diff of the arrays will only show if both arrays once flattened contain only strings, and at least one of those strings contains a newline.

The behaviour got this way through reasonable looking changes that did nice things, but this case doesn't make much sense any more.

I'm going to work on making this do something a little more expected (and figuring out how this got exactly the way it is) tonight. If anyone has any feedback on what exactly the desired behaviour is, or has any other info that would be helpful, please let me know.

👽 The truth is out there. 🚀

[JonRowe]

Hmm, definitely sounds like a regression happened, as it used to be ok, please do work up a spec illustrating this example :) (if you could attach it to this issue using hub etc that'd be cool)

[jarednorman]

Here are some entertaining examples of the symptoms of this problem. The only specs that test fail_with with Arrays, only test them with Arrays that contain strings with newlines in them, which is the only case in which Array diffing happens.

This has a little bit to do with this change and also with the fact that we are checking to see if there are any multiline strings in the flattened expected/actual, before coercing them to strings instead of after.

[nevinera]

This is very old, but still present - I'm guessing it's not obvious anything is wrong here, since both behaviors are actually reasonable (you still get a useful failure message without the diff in the 'wrong' case. But the problematic logic is here. It looks almost intentional, but I suspect we just need another branch here, for the 'if no multiline-strings present' case.

At a glance, just removing the any_multiline_strings? check accomplishes the objective, but has some other impact. It was added in this commit (before the split out to rspec-support), for this issue.

I think what we probably want is for the diff to not be displayed if the strings we would diff only have one line each, right? I can gin that up fairly readily, I just want to make sure it'll be the right thing.

[nevinera]

Ah, I misunderstood our current behavior slightly - if there are 'multiline strings' in the array, we make them one-line strings, and then print the diff; otherwise we don't print a diff at all. I can see why that would bother someone :-)

So what we probably want is to skip the diff in the case that we get one-line strings for our two arguments, but keep it if we have anything else.

[nevinera]

Okay, I think I have a solution over here (in rspec-support)