Show a diff for comparisons of arrays of one-line strings

[nevinera]

A long time ago, a fellow notice a regression in the behavior of diffing. Roughly speaking, expect(["foo"]).to eq(["foo", "baz"]) doesn't print a "Diff" section of output. But expect(["foo"]).to eq(["foo", "baz\n"]) does. And that seems like an odd distinction to make! To the code mines!

I had to trace this across the move from rspec-expectations into rspec-support, but it was originally introduced here, which was motivated by this issue. That seems like a reasonable goal, and we do actually have some tests enforcing it already (though they're a little entangled with the encoding specs - I can add another test specific to this purpose if you like). The effect it had on diffing arrays of strings seems incidental though, a result of the way we flatten the arrays to test them, rather than checking them individually.

So! Break that check out ahead, and make it a bit clearer - add pointless_diff?, which tests that both arguments are Strings, and that neither string is multiline; for that case, skip adding the diff like before, but drop the 'multiline' test that was skipping.

[nevinera]

Ah, and now I think I get to learn why the mono-repo is such a desirous outcome :-)

Hm, these failures are all in rspec-mocks. Only one of them looks like it legitimately intended to test the thing I'm now braeaking - the others are all just specifying failure messages, which are changing (by gaining diffs). I could update all of those specs by making them sufficiently specific regexes, but this last one I'm not sure about:

Diffs printed when arguments don't match with a non matcher object does not print a diff when multiple single-line string arguments are mismatched

That's pretty specific. Let's dive into the history - it was added as part of this PR. It does look like it reflects the intent of the code at the time, and it predates the differ. I'm going to see if I can accommodate that intent easily.

[nevinera]

I'm going to see if I can accommodate that intent easily.

Narrator: "He could not."

It's essentially doing exactly the thing we were trying to change; diffing two arrays of two single-line strings. So - the options are (a) update the specs to reflect a new intent, (b) cancel this plan, or (c)switch plans - the inconsistency could also be resolved by just not diffing arrays of strings at all.

I favor (a), despite the extra PRs it'll involve (go, monorepo!) - the output it produces looks like these, which seems sufficiently useful to me:

     Failure/Error: d.foo("argA", "argB", "ARGTHREE", "arg2")

       #<Double "double"> received :foo with unexpected arguments
         expected: ("arg1", "arg2", "arg3", "arg4")
              got: ("argA", "argB", "ARGTHREE", "arg2")
       Diff:

       @@ -1,5 +1,5 @@
       -arg1
       +argA
       +argB
       +ARGTHREE
        arg2
       -arg3
       -arg4


    Failure/Error: d.foo("arg1", "arg2", "ARGTHREE", "arg4")

       #<Double "double"> received :foo with unexpected arguments
         expected: ("arg1", "arg2", "arg3", "arg4")
              got: ("arg1", "arg2", "ARGTHREE", "arg4")
       Diff:
       @@ -1,5 +1,5 @@
        arg1
        arg2
       -arg3
       +ARGTHREE
        arg4

If I get buy-in, I'll make a PR to update the specs on rspec-mocks to accomodate the change I have here (I'll just remove the one spec, and I'll make the others use regexes that pass on either version of rspec-support). After that releases, we can ship this one, assuming I understand your version-relationships properly :-)

[nevinera]

@pirj / @JonRowe , does that plan seem appropriate?

[pirj]

I’m all for the plan a!