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

Diffs for arrays of multiline strings don't make much sense #212

Closed
myronmarston opened this issue Feb 19, 2013 · 9 comments
Closed

Diffs for arrays of multiline strings don't make much sense #212

myronmarston opened this issue Feb 19, 2013 · 9 comments

Comments

@myronmarston
Copy link
Member

Given:

expect(["a\nb", "c\nd"]).to eq([])

This is the failure output:

     Failure/Error: expect(["a\nb", "c\nd"]).to eq([])

       expected: []
            got: ["a\nb", "c\nd"]

       (compared using ==)

       Diff:
       @@ -1 +1,4 @@
       +a
       +b,c
       +d

The diff doesn't make much sense here. I think the solution is to diff the arrays as objects, not strings, but my first attempt to make that change (e.g. by removing the flatten calls in fail_with) broke some other things. Overall, all the flattening in fail_with feels hacky and is confusing, but I'm not sure what the right fix is yet.

@fables-tales
Copy link
Member

@myronmarston could the strings be escaped before being passed into diff when expecting with collections/arrays? That is the diff would look like:

Diff:
@@ -1, +1,3 @@
+"a\nb"
+"c\nd"

(and similar for sets)

@myronmarston
Copy link
Member Author

Maybe. Want to take a stab at it?

@fables-tales
Copy link
Member

I can't immediately, but I'm going to have more time in a couple of weeks, so I could probably take a look then.

@fables-tales
Copy link
Member

@myronmarston so I got bored in a lecture and hacked this together:


  1) faces fails
     Failure/Error: expect(["a\nb", "c\nd"]).to eq([])

       expected: []
            got: ["a\nb", "c\nd"]

       (compared using ==)

       Diff:
       @@ -1 +1,3 @@
       +"a\nb"
       +"c\nd"

  2) faces bacon
     Failure/Error: expect(["a\nb", ["c\nd"]]).to eq([])

       expected: []
            got: ["a\nb", ["c\nd"]]

       (compared using ==)

       Diff:
       @@ -1 +1,3 @@
       +"a\nb"
       +["c\nd"]


is this what we want?

@fables-tales
Copy link
Member

Another example:

  3) faces cows
     Failure/Error: expect(["a\nb", ["c,\nd", "asdf"]]).to eq([])

       expected: []
            got: ["a\nb", ["c,\nd", "asdf"]]

       (compared using ==)

       Diff:
       @@ -1 +1,3 @@
       +a\nb
       +["c,\nd", "asdf"]

@myronmarston
Copy link
Member Author

That's definitely a big improvement. We can always start there and iterate on it.

@fables-tales
Copy link
Member

Aight, I'll build some specs and fire off a pr.

@fables-tales
Copy link
Member

This should be closed now right?

@myronmarston
Copy link
Member Author

Yep. As usual, I forgot to close it...

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

No branches or pull requests

2 participants