Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Diff string arrays like other arrays #519

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dmolesUC
Copy link

Fixes #518.

@pirj
Copy link
Member

pirj commented Oct 20, 2021

I suggest to either add checks for Diff::LCS::VERSION or re-target this to 4-0-dev branch. 4.0 is pretty much ready, but the release date is not defined. The fact that a sub-build's specs are failing can affect your decision.

@dmolesUC
Copy link
Author

@pirj OK, I checked in what I hope is a fix for thediff-lcs issue. But it looks like the tests for rspec-mocks may explicitly depend on the old behavior?

I've created another branch rebased against 4-0-dev if you think that's better, but unfortunately I don't have much time to devote to this right now so if there's more R&D or changes that need to be made in other modules, someone else may have to pick it up.

@pirj
Copy link
Member

pirj commented Oct 20, 2021

It's an interesting special case:

#<Double "double"> received :foo with unexpected arguments
  expected: ("some string", "some other string")
       got: ("this other string", "some string")
Diff:
@@ -1,3 +1,3 @@
-some other string
 some string
+this other string

Unrelated to your change

receive's with is order-dependent, and for:

expect(d).to receive(:foo).with("a", "b")
d.foo("c", "a")

it may be confusing to see

- "b"
+ "c"

because changing the code

- d.foo("c", "a")
+ d.foo("b", "a")

won't fix this spec.

It doesn't really matter if it's a list of single-line string arguments or a multi-line ones, or anything else.

@pirj pirj requested a review from JonRowe October 20, 2021 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Differ does not report differences in string arrays
2 participants