go-test / deep

Golang deep variable equality test that returns human-readable differences
MIT License
743 stars 54 forks source link

FLAG_IGNORE_SLICE_ORDER - panic: runtime error: hash of unhashable type map[string]interface {} #60

Open sschulz-t opened 9 months ago

sschulz-t commented 9 months ago

Please see the example 5) from this playground: https://go.dev/play/p/M-VZFkXte9T

With FLAG_IGNORE_SLICE_ORDER set, it causes a panic.

It happens at:

func (c *cmp) equals(a, b reflect.Value, level int) {
...
am[a.Index(i).Interface()] += 1
...
}

where the code tries to use the map[string]interface{} as a key when the child is a slice.

When reading the comments in the code:

    // FLAG_IGNORE_SLICE_ORDER causes Equal to ignore slice order so that
    // []int{1, 2} and []int{2, 1} are equal. Only slices of primitive scalars
    // like numbers and strings are supported. Slices of complex types,
    // like []T where T is a struct, are undefined because Equal does not
    // recurse into the slice value when this flag is enabled.

This states it is unsupported to use it with this type of data. I was wondering if it is possible to add this feature in the future. At least this ticket could save others some time when they do a web search for the cause of the error and did not read the documentation ;)

daniel-nichter commented 1 week ago

That is a tough one. My first thought is "It shouldn't panic", but then I also don't know what it should report in a case like this, which makes me think that maybe this is a rare case when panicking is the right behaviour?

To compare this reliably (so there are no false negatives, i.e. deep says "they're equal" when they're not), deep would have to recurse into the inner map[string]interface{}. That is no problem, but the problem is how to compare that inner map in a vs. b (the canonical vars being diff'ed) when it's doing the ignore order slice comparison...

There'd need to be code/logic to divert from the slice cmp temporarily, then do a "simpler" inner a.map vs. inner b.map cmp. If the two are equal, then they're both omitted (somehow) from the slice cmp. And of course, if they're the not equal, that's logged and reported later.

In this example, that might be feasible code/logic because the inner maps are at the same slice index (3):

    a := map[string]interface{}{"array_test": []interface{}{1, 2, 3, map[string]interface{}{"a": 234}}}
    b := map[string]interface{}{"array_test": []interface{}{1, 2, 3, map[string]interface{}{"a": 234}}}

But if they were not, like,

    a := map[string]interface{}{"array_test": []interface{}{map[string]interface{}{"a": 234}, 1, 2, 3}}
    a := map[string]interface{}{"array_test": []interface{}{1, 2, 3, map[string]interface{}{"a": 234}}}

I'm not sure this case (immediately above) is feasible to solve because when deep encounters the inner a.map at [0], I don't think there's a way to find the equivalent inner b.map at [3]. We know the two maps are related, but programatically since these are interface{}, there's no logical relationship between elements--they can be anything, anywhere. For example:

    a := ...{"array_test": []interface{}{map[string]interface{}{"a": 234}, 1, 2, 3}}
    a := ...{"array_test": []interface{}{map[string]interface{}{"a": 234}, 2, 3, map[string]interface{}{"a": 234}}}

Which two inner maps should be compared? Maybe it doesn't matter. If deep picks the first maps (at index [0] in both a and b), it'll have a leftover map (b index [3]`) that'll make them not equal. But "pick the first maps" is contrary to "ignore slice order".

Regardless of this flag, arbitrarily picking values/elements to compare seems like the wrong approach.


Maybe this is a solution: equal if addresses (of any non-primitive scalars) are the same? That means this test case will fail (not equal) because they're two different maps, but perhaps that's better than panicking? But it means this should pass (no diff):

    x := map[string]interface{}{"a": 234}
    a := map[string]interface{}{"array_test": []interface{}{1, 2, 3, x}}
    b := map[string]interface{}{"array_test": []interface{}{1, 2, 3, x}}

Other ideas?