Skip to content

Conversation

mekegi
Copy link
Contributor

@mekegi mekegi commented May 13, 2019

#98

https://github.com/google/go-cmp/ is more usefull then reflect.DeepEqual

I changed reflect.DeepEqual to cmp.Equal
and add to t.Errorf cmp.Diff for more detailed view of diff between got and want
for exmaple how it looks like

=== RUN   TestAddress2addressResultSlice/two_diff_addr
    --- FAIL: TestAddress2addressResultSlice/two_diff_addr (0.00s)
        helpers_test.go:161: Address2addressResultSlice() = [address:<UID:"ololo1" >  address:<UID:"trololo" > ], want [address:<UID:"ololo" >  address:<UID:"trololo" > ]
             diff =   []*address_api.AddressResult{
              	&{
              		Precision:            s"other",
            - 		Address:              s`UID:"ololo1" `,
            + 		Address:              s`UID:"ololo" `,
              		Distance:             0,
              		... // 2 identical fields
              	},
              	&{Address: s`UID:"trololo" `},
              }
FAIL

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 96.411% when pulling 3cb5a24 on mekegi:go-cmp into afa0a37 on cweill:develop.

@coveralls
Copy link

coveralls commented May 13, 2019

Coverage Status

Coverage increased (+0.05%) to 96.411% when pulling cec4af4 on mekegi:go-cmp into afa0a37 on cweill:develop.

@cweill
Copy link
Owner

cweill commented Jun 30, 2019

@mekegi: This looks good to me, except can you please add a flag use_go_cmp=true. That way people who want the old behavior (minus a dependency) can use it.

@cweill cweill self-requested a review November 13, 2019 21:45
@smitt04
Copy link

smitt04 commented Jun 5, 2020

I see it has been a year since this PR was created. I would love to use cmp instead of reflect. Is going to get merged or is it dead?

@sljeff
Copy link

sljeff commented Jun 19, 2020

+1

Now gopls checks DeepEqual with errors.

17:03:20 ➜ gopls check app/grpc/handlers_test.go
/Users/jeff/proj/learning/app/grpc/handlers_test.go:2073:9-49: avoid using reflect.DeepEqual with errors
/Users/jeff/proj/learning/app/grpc/handlers_test.go:3163:7-55: avoid using reflect.DeepEqual with errors
/Users/jeff/proj/learning/app/grpc/handlers_test.go:4498:9-49: avoid using reflect.DeepEqual with errors
/Users/jeff/proj/learning/app/grpc/handlers_test.go:5076:9-49: avoid using reflect.DeepEqual with errors
/Users/jeff/proj/learning/app/grpc/handlers_test.go:5291:9-49: avoid using reflect.DeepEqual with errors

@cweill
Copy link
Owner

cweill commented Dec 31, 2020

If OP or someone else can update this PR to resolve branch conflicts, I will approve it.

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

Successfully merging this pull request may close these issues.

5 participants