-
Notifications
You must be signed in to change notification settings - Fork 570
feat(templates): unit tests for indexed type #1105
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Working well for me 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! 🍻
| _, err := srv.Create<%= title(TypeName) %>(ctx, &types.MsgCreate<%= title(TypeName) %>{Creator: creator, Index: index}) | ||
| require.NoError(t, err) | ||
|
|
||
| _, err = srv.Update<%= title(TypeName) %>(ctx, tc.request) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we also Get and check if it's really updated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unfortunately there is nothing to check. Creator can't be updated, and Index is a unique identifier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm. I think, checking with a Get still might be relavent to see that nothing has changed. Also, once users start extending the underlying MsgUpdate type, they'll need to add this right. What do you think?
|
|
||
| _, err := srv.Create<%= title(TypeName) %>(ctx, &types.MsgCreate<%= title(TypeName) %>{Creator: creator, Index: index}) | ||
| require.NoError(t, err) | ||
| _, err = srv.Delete<%= title(TypeName) %>(ctx, tc.request) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we also Get and check if it's really deleted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was pretty sure that i had the line with Get, lost it somehow :) thanks for catching this
3ee8ea3 to
0d14550
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! 🦖
* feat(templates): unit tests for indexed type * Improve msg server tests
related to: #715