Issue 281 v5 sadd srem return value #293
Conversation
| @@ -8,7 +8,7 @@ | |||
| @redises.set(key, '') | |||
| expect do | |||
| @redises.send(method, *args) | |||
| end.to raise_error(defined?(default_error) ? default_error : RuntimeError) | |||
| end.to raise_error(defined?(default_error) ? default_error : Redis::BaseError) | |||
There was a problem hiding this comment.
the version 5 BaseError now inherits from StandardError instead of RuntimeError, it felt more appropriate to narrow the scope of this test to Redis::BaseError. This prevents needing to test the version and also limits the tests from passing on a different StandardError or RuntimeError.
| it 'stringifies member' do | ||
| @redises.sadd(@key, '1') | ||
| expect(@redises.srem(@key, 1)).to eq(true) | ||
| end | ||
|
|
There was a problem hiding this comment.
moved to the version specific block
lib/mock_redis/utility_methods.rb
Outdated
| def size_after(obj, &blk) | ||
| size_before = obj.size | ||
| yield | ||
| (obj.size - size_before).abs | ||
| end |
There was a problem hiding this comment.
this just felt like it cleaned up the tests a little, happy to revert if the abstraction is undesired.
Gemfile
Outdated
|
|
||
| # TODO: gems added for development, remove before merge | ||
| # gem 'pry' | ||
| gem 'redis', '>= 5.0.0' |
There was a problem hiding this comment.
force the tests to run in version 5
| @@ -125,14 +119,13 @@ def srandmember(key, count = nil) | |||
| end | |||
|
|
|||
| def srem(key, members) | |||
| members = Array(members).map(&:to_s) | |||
There was a problem hiding this comment.
mimicing the behavior from sadd
| expect(@redises.sadd(@key, 1)).to eq(true) | ||
| end | ||
| context "adapts to redis-rd version 4 and 5 outputs" do | ||
| include MockRedis::UtilityMethods |
There was a problem hiding this comment.
just trying to reuse the redis_gem_v5? method
|
Hey @CoderMiguel, thanks for your effort on this. As mentioned in this comment, very supportive of dropping support for If you rebase this PR on top of the latest |
CoderMiguel
force-pushed
the
issue_281_v5_sadd_srem_return_value
branch
from
f8faef2
to
6da7580
Compare
|
@sds this should be rebased now, I will revisit soon to look into the test failures and remove the version checking code I added for testing some things out. |
attempting to fix #281