-
Notifications
You must be signed in to change notification settings - Fork 3
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
Db/integration tests #311
Db/integration tests #311
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.
Had some feedback,
-
We should handle all errors instead of ignoring them with
_
. If the error is really unusual for a test and it is tedious to have the calling function handle them, can we justpanic()
iferr != nil
? -
Tests would be much more easy to read if each object had multiple
Test_
functions rather than onlyTest_...CRUD
which does everything. -
We could add a
cmd_test
package (I don't hold this opinion strongly)
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 revision, I have another recommendation to consider before merging
Issues
CLI integration tests
Changelog
Add integration tests on CRUD operations in cli. Creates, Updates, Deletes actual test resources
changie
entry, N/A testing onlyTophatting
task test-integration