-
Notifications
You must be signed in to change notification settings - Fork 5
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
Update tests #4
Update tests #4
Conversation
@@ -0,0 +1,12 @@ | |||
github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= |
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.
ci add ci
Ci add cache
@mrsndmn Thank you for your PR! I will review it tomorrow. It looks promising. |
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.
Thank you for your work! Please take a look at my review and we will be ready to merge it!
cdb_test.go
Outdated
if reader.Size() != 7 { | ||
t.Errorf("Expected size %d, got %d", 7, reader.Size()) | ||
// generate test records | ||
const testRecordsCount = 10 |
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.
Do we need this const
here? I guess we can remove it
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.
fidex
cdb_test.go
Outdated
{"key7", "value7"}, | ||
} | ||
writer := suite.getCDBWriter() | ||
defer writer.Close() |
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.
We should check the result of Close
function
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.
Good point! Thank you!
cdb_test.go
Outdated
defer os.Remove("test.cdb") | ||
func (suite *CDBTestSuite) TestShouldReturnNilOnNonExistingKeys() { | ||
writer := suite.getCDBWriter() | ||
writer.Close() |
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.
Please add here check suite.Nilf
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.
Done.
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.
Thank you for review! Fixed.
cdb_test.go
Outdated
if reader.Size() != 7 { | ||
t.Errorf("Expected size %d, got %d", 7, reader.Size()) | ||
// generate test records | ||
const testRecordsCount = 10 |
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.
fidex
cdb_test.go
Outdated
{"key7", "value7"}, | ||
} | ||
writer := suite.getCDBWriter() | ||
defer writer.Close() |
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.
Good point! Thank you!
cdb_test.go
Outdated
defer os.Remove("test.cdb") | ||
func (suite *CDBTestSuite) TestShouldReturnNilOnNonExistingKeys() { | ||
writer := suite.getCDBWriter() | ||
writer.Close() |
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.
Done.
I also added |
It is better to create a separate PR:) Thank you for your work! |
I've refactored tests with testify. Seems it looks nice now.
Unfortunately, I cant do anything with benchmarks. There is no support of benchmarks in
testify/suite
.