-
Notifications
You must be signed in to change notification settings - Fork 589
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
Add test standards and test templates to CONTRIBUTING.md #1977
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.
Lgtm! Great additions on testing!
p.s. be sure to fix the markdown lint errors :)
Thoughts on making the examples collapsible, something like this: Example #1: [Message-Related Test]
|
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.
Awesome work on this. I really like the examples at multiple levels of abstraction
CONTRIBUTING.md
Outdated
### Example #1: [Message-Related Test](https://github.com/osmosis-labs/osmosis/blob/f0270d04bd77cc5e1c23f7913118b3c2ba737e97/x/tokenfactory/keeper/admins_test.go#L122-L181) | ||
This type of test is mainly for functions that would be triggered by incoming messages (we interact directly with the message server since all other metadata is stripped from a message by the time it hits the msg_server): | ||
|
||
``` |
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.
nit: might be just my personal view so please feel free to ignore it if you disagree but I feel like having a link is sufficient.
If there is a link, I usually skip code blocks in READMEs
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.
Since our goal is to make it as straightforward (and fast) as possible to copy-paste test templates, I personally prefer to keep the code blocks in there. This also lets us update/change/move the tests being linked to without having to worry about keeping the templates simple and relevant for copy-pasting
Co-authored-by: Roman <roman@osmosis.team>
Closes: #1954
What is the purpose of the change
This PR introduces testing standards and templates to our
CONTRIBUTING.md
to align expectations on testing while we continue to refactor and clean up our old tests.Brief Changelog
Add section for writing tests to
CONTRIBUTING.md
that covers:Testing and Verifying
This change is a trivial rework / code cleanup without any test coverage.
Documentation and Release Note
Unreleased
section inCHANGELOG.md
? (no)