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

Comparing interface{} #161

Closed
heynemann opened this issue Jul 6, 2016 · 11 comments · Fixed by #409
Closed

Comparing interface{} #161

heynemann opened this issue Jul 6, 2016 · 11 comments · Fixed by #409

Comments

@heynemann
Copy link

When comparing two items for equivalence I get this error:

  Expected
      <[]interface {} | len:2, cap:2>: [1, 2]
  to be equivalent to
      <[]interface {} | len:2, cap:2>: [1, 2]

I only get this when deserializing a []int{1, 2} from messagepack. I'm sure the types are at fault here, but the message could improve IMHO.

@rlubke
Copy link

rlubke commented Nov 1, 2017

I agree, it took me a bit to clue in on this. I received this when using json.

@williammartin
Copy link
Sponsor Collaborator

Hey @heynemann and @rlubke I've just started trying to go through abandoned issues. I could take a guess at what this is about but it would be really awesome if either of you had some simple reproduction code. Thanks!

@nodo
Copy link
Collaborator

nodo commented Feb 9, 2018

I have tried to reproduce the problem with json:

	It("succeeds", func() {
		j := `[1,2]`
		var slice []interface{}
		json.Unmarshal([]byte(j), &slice)
		same := []interface{}{1, 2}
		Expect(slice).To(BeEquivalentTo(same))
	})

The output is:

  Expected
      <[]interface {} | len:2, cap:2>: [1, 2]
  to be equivalent to
      <[]interface {} | len:2, cap:2>: [1, 2]

The example above fails because json.Unmarshal automatically converts int into float64 (I am using go1.9.4). Therefore, the two slices are actually different. To verify that you can add:

fmt.Printf("%T vs %T\n", same[0], slice[0])

You should see: int vs float64.

I have tried the same with the package msgpack (github.com/vmihailenco/msgpack) with similar results:

	It("succeeds msgpack", func() {
		in := []int{1, 2}
		b, _ := msgpack.Marshal(in)
		var slice []interface{}
		msgpack.Unmarshal(b, &slice)
		same := []interface{}{1, 2}

		fmt.Printf("%T vs %T\n", same[0], slice[0])
		Expect(slice).To(BeEquivalentTo(same))
	})

outputs:

int vs int8
[...]
      <[]interface {} | len:2, cap:2>: [1, 2]
  to be equivalent to
      <[]interface {} | len:2, cap:2>: [1, 2]

In this case msgpack converts integers into int8. Since they are different types, the expectation fails.

Hope it helps @heynemann

@williammartin
Copy link
Sponsor Collaborator

Great investigation @nodo. Wondering if there's any way we can provide better feedback to the message, it sure does suck when things are visually the same but not actually.

@nodo
Copy link
Collaborator

nodo commented Feb 11, 2018

Yes, it does! I think you are right @williammartin, it would be a good idea to change the message.

We could add some logic when printing the types of the objects: https://github.com/onsi/gomega/blob/master/format/format.go#L191

For instance:

case reflect.Slice:
  v := reflect.ValueOf(object)
  output := fmt.Sprintf("%T | len:%d, cap:%d", object, v.Len(), v.Cap())
  if v.Len() > 0 {
    a := object.([]interface{})
    output = fmt.Sprintf("%s | Slice of %T", output, a[0])
  }
  return output

I think the message would be clearer:

  Expected
      <[]interface {} | len:2, cap:2 | Slice of int8>: [1, 2]
  to be equivalent to
      <[]interface {} | len:2, cap:2 | Slice of int>: [1, 2]

What do you think? Too hacky?

@williammartin
Copy link
Sponsor Collaborator

No I think that would provide some useful clarity @nodo. Can you PR?

@gcapizzi
Copy link
Collaborator

gcapizzi commented Jun 7, 2018

Expected
     <[]interface {} | len:2, cap:2 | Slice of int8>: [1, 2]
 to be equivalent to
     <[]interface {} | len:2, cap:2 | Slice of int>: [1, 2]

Maybe []int8/[]int instead of spelling Slice of ...? Or maybe just use the actual type at the beginning instead of the reference type ([]interface{})?

Expected
    <[]int8 | len:2, cap:2>: [1, 2]
to be equivalent to
    <[]int | len:2, cap:2>: [1, 2]

@nodo
Copy link
Collaborator

nodo commented Jun 7, 2018

Sure, I can give it a go!

@nodo
Copy link
Collaborator

nodo commented Jun 20, 2018

I am still not sure on how to format the concrete types of the elements of the slice.

Do you think we should list them? It does not feel right to me.

@williammartin
Copy link
Sponsor Collaborator

@nodo Did you have any more thoughts on this?

@williammartin
Copy link
Sponsor Collaborator

@nodo Any thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants