-
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
Fix bug in #shrink_if_needed. #5
base: master
Are you sure you want to change the base?
Fix bug in #shrink_if_needed. #5
Conversation
Can you provide a test that proves this is a bug? |
Just added a test. Unfortunately, as the bug is purely internal to the FastCache::Cache class, the test has to resort to using My other pull request, #3, actually is what exposed the bug -- one of its tests fails without this fix. Failures:
1) FastCache::Cache non-empty cache cleanup block is called when an item is removed when full
Failure/Error: subject[:d] = 4
NoMethodError:
undefined method `value' for nil:NilClass
# ./lib/fast_cache/cache.rb:217:in `shrink_if_needed'
# ./lib/fast_cache/cache.rb:210:in `store_entry'
# ./lib/fast_cache/cache.rb:200:in `store'
# ./lib/fast_cache/cache.rb:81:in `[]='
# ./spec/lib/fast_cache/cache_spec.rb:140:in `block (4 levels) in <top (required)> |
Higher level question: would you prefer if I consolidated all three of my pull requests into one? I had originally done them all together in one branch, but split them up because they are separate concerns. However, there is a clear dependency order:
|
Separate PRs are best: you made the right decision. I merged RSpec PR. You can pick up the changes. |
great, I'll rebase this one now so it will have a passing travis status |
- shifted entry was not being removed from @expires_at - replace if with while, in case somehow the @DaTa length is more than 1 greater than @max_size
f7983b1
to
0486328
Compare
after this one is merged, I can rebase #3 and it should be ready to merge too. |
@ssimeonov what's the status of this -- are you going to merge it? |
|
@expires_at
@data length
is more than 1 greater than@max_size