-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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 few test cases to consistenthash and lru packages #84
base: master
Are you sure you want to change the base?
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
I signed it! |
CLAs look good, thanks! |
@@ -34,6 +34,13 @@ func TestHashing(t *testing.T) { | |||
return uint32(i) | |||
}) | |||
|
|||
// Get a value before any key is added to the hash | |||
// Should yield an empty string | |||
empty_value := hash.Get("empty key") |
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 don't use underscores in variable names in Go.
Also, what do you mean by "empty key"? The key isn't empty. Maybe just "foo"?
// Should yield an empty string | ||
empty_value := hash.Get("empty key") | ||
if empty_value != "" { | ||
t.Errorf("Expecting empty string, got %s", empty_value) |
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.
Go style for errors is:
Func(input) = got; want foo
Also, use %q for strings when the string isn't what you expect. %q shows empty strings and strings with binary gibberish like newlines well.
I'd write this all as:
if got := hash.Get("foo"); got != "" {
t.Errorf("Get on empty hash = %q; want empty string", got)
}
Please squash these all together into one commit. As you make code review changes, squash again. We don't need to see corrections in the history. (Some projects like that, but not Go ones) |
Have updated to use go style coding practices. |
@bradfitz Sorry, didn't see your comment before commenting. |
I will go ahead and write more test cases for the lru package and then will squash them all into a single commit. |
In consistenthash package: Add test case for Getting from empty Hash (before any keys are added); should return an empty string. In lru package: Test adding to an LRU with a MaxEntries value set. The Length of the cache should remain constant. Also add tests to add/get from a cleared empty cache.
I have added more test cases and squashed the commits into one. |
In consistenthash, getting from an empty hash (before keys are added)
trying to Get a value should return an empty string.
Now has 100% coverage on consistenthash package
Also add a test case for adding keys to the lru cache.