-
Notifications
You must be signed in to change notification settings - Fork 52
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
Go modules support and missing unit tests #18
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.
It looks great!
Though, why did you remove the integration test?
Since the unit tests fully cover all aspects of dotsql by utilizing a mocked db layer, it is redundant to try to cover the same parts of code with integration tests whose only difference is real db usage instead of a mocked one. In this case, tests are not only slower, but they also introduce an unnecessary dependency (sqllite). |
I see your point, but I still do think that testing against a real database gives you more confidence than mocks |
Additionally, the fact that they run "slow" shouldn't be a problem cause they by default they don't run, unless you |
Extra layer of testing sounds reasonable, though I feel that integration tests wouldn't add any benefit mostly because neither the input (SQL queries) nor the execution (db implementation / driver) are influenced in any way by dotsql, we only need to ensure that those queries are delivered correctly to the db and that is easily checked via mocks. |
But having those tests doesn't hurt nobody (sqlite will only be added as a dependency to go.mod and the less dependencies the better), so I could return integration tests. |
I modified the commits of this branch to preserve the integration tests. Everything else is the same. |
This PR adds go modules support and missing unit tests,
completely removes integration tests. I think it would be appropriate, after merging this branch into master, to tag the latest commit as v1.0.0