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
joelnordell
force-pushed
the
eris-fix_bug_in_shrink_if_needed
branch
from
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? |
|
2 participants
@expires_at@data lengthis more than 1 greater than@max_size