Link roles to users in user service #86
Conversation
Signed-off-by: codegold79 <codegold79@gmail.com>
Signed-off-by: codegold79 <codegold79@gmail.com>
Signed-off-by: codegold79 <codegold79@gmail.com>
Signed-off-by: codegold79 <codegold79@gmail.com>
codegold79
force-pushed
the
Link-roles-to-users-in-User-Service
branch
from
5056aca
to
c25d599
Compare
Signed-off-by: codegold79 <codegold79@gmail.com>
Signed-off-by: codegold79 <codegold79@gmail.com>
codegold79
changed the title
codegold79
changed the title
Signed-off-by: codegold79 <codegold79@gmail.com>
Signed-off-by: codegold79 <codegold79@gmail.com>
codegold79
changed the title
|
@codegold79 Are the TODOs either covered here or by new issues? |
| got.Roles[i].Id = "" | ||
| got.Roles[i].XXX_unrecognized = []byte{} | ||
| got.Roles[i].XXX_sizecache = 0 | ||
| unsetUntestedFields(got.Roles[i]) | ||
| } | ||
|
|
||
| if reflect.DeepEqual(got, tc.want) { |
There was a problem hiding this comment.
Should the boolean logic here be flipped?
There was a problem hiding this comment.
Great catch, @daved! I flipped it and I got something unexpected. Why is it that the got and tc.want are not considered equal?
There was a problem hiding this comment.
Because *pb.User implements the fmt.Stringer interface, the string formatting provided by Fatalf results in output that deceptively looks identical (likely anyway). You can try dereferencing the values or using the %#v verb in the formatting as demoed here: https://play.golang.org/p/UkGo7MVxbRQ
| @@ -526,3 +506,33 @@ func postSvcDelPostFn(ctx context.Context, conn *grpc.ClientConn, clnt pb.PostCl | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| func unsetUntestedFields(item interface{}) { | |||
There was a problem hiding this comment.
I suggest using a type switch. Example: https://play.golang.org/p/9QxbVKyAZz0
Also, please consider alternately using reflection for setting fields when present (use exceptional caution); and I'll defer to your preference of changing this: https://play.golang.org/p/J54OCQ6e_Xq
The Slug field is the only field below that is not common. I think it makes sense to set that within the test itself since any given test may require it to be different values, but the XXX_ prefixed, timestamp, and Id fields will likely always need handling.
There was a problem hiding this comment.
The type switch makes so much more sense than what I was doing.
I'm not sure what you mean by using the reflect.ValueOf(). It seems like I'm using a lot more characters to do the same thing:
I must be missing something.
There was a problem hiding this comment.
As per chat during meeting: https://play.golang.org/p/4Yz_xNLM5kZ
There was a problem hiding this comment.
I liked your hunch that protobuf was messing with the formatting with a String() method when I was outputting using t.Fatalf(). I tried out your advice to pass in pointers so the String() method won't be called, but I see the want and got are still identical. I have no idea why reflect.DeepEqual() still shows as not being equal. I was wondering if you would pull the branch and see if you can find anything @daved? Much thanks in advance.
There was a problem hiding this comment.
Regarding unsetting, here is some logic that might help: https://play.golang.org/p/GoJvQwi5f-h
There was a problem hiding this comment.
Thank you again, @daved. I used what you showed to unset several fields of two different types. This PR is ready for review again.
- Flip the boolean logic in reflect.DeepEqual() - Use reflect to unset fields instead of switch by type Signed-off-by: codegold79 <codegold79@gmail.com>
codegold79
force-pushed
the
Link-roles-to-users-in-User-Service
branch
from
0e0d0c1
to
6633e1e
Compare
Closes #79
Changes
Todo