-
Notifications
You must be signed in to change notification settings - Fork 44
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
test: Introduce basic testing for the version
module
#1111
Conversation
The CI fails because the PR title ends with `. IMO it should be an acceptable character. For now I'm changing it to something else |
version
version
module
802772f
to
5419bec
Compare
Codecov Report
@@ Coverage Diff @@
## develop #1111 +/- ##
===========================================
+ Coverage 67.50% 67.80% +0.29%
===========================================
Files 181 181
Lines 16551 16551
===========================================
+ Hits 11173 11222 +49
+ Misses 4438 4390 -48
+ Partials 940 939 -1
|
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. One question though before merge if that's alright :)
version/version_test.go
Outdated
assert.NotEmpty(t, dv.NetProtocol) | ||
assert.NotEmpty(t, dv.DocKeyVersions) | ||
|
||
// because these are set by the build system, we expect them to be empty in this unit 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.
question: Will this test not be run on release once these variables have been set? And will this not cause the build/tests to fail for anyone running them on a release branch/tag/whatever?
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.
The variables are set in the Makefile
via BUILD_FLAGS
and they are only passed for building defradb and not testing defradb.
For now, this allows to run the tests outside of using make.
I suggest (to myself) adding a comment indicating that assumption.
We might want to run these tests at some point in the future using BUILD_FLAGS, I'm unsure.
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.
Got it - thanks Orpheus :) New comment looks good too
Relevant issue(s)
Resolves #1110
Description
Just a few basic tests for the module.
Tasks
How has this been tested?
make test
Specify the platform(s) on which this was tested: