-
Notifications
You must be signed in to change notification settings - Fork 114
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
Fix update of expired items #124
base: v3
Are you sure you want to change the base?
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.
Thanks for the contribution.
Providing some feedback, however hadn't had the time to investigate this issue thoroughly, will need to come back and re-check this.
cache_test.go
Outdated
@@ -1212,3 +1212,29 @@ func addToCache(c *Cache[string, string], ttl time.Duration, keys ...string) { | |||
c.items.expQueue.push(elem) | |||
} | |||
} | |||
|
|||
func TestUpdateExpired(t *testing.T) { |
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.
This is not how we write unit tests. The test cases should be incorporated into the functions named after the methods that were changed.
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.
To be honest I don't see much value in this test at all, it's too specific. But this test was crucial for me to reproduce issue I was suspecting and validate solution. Should I relocate it or should I just drop 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.
Test_Cache_set
should be expanded to include a test for overwriting an expired item.
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
2ffa4e8
to
a34b07b
Compare
Signed-off-by: Vladislav Yarmak <vladislav-ex-src@vm-0.com>
Signed-off-by: Vladislav Yarmak <vladislav-ex-src@vm-0.com>
a34b07b
to
9e82ac0
Compare
Fix #123
Extends
get
method to return expired items toset
method in order to avoid second insertion into expiration queue.