Issue 281 v5 sadd srem return value

Gemfile

@@ -6,8 +6,13 @@ gemspec
 # Run all pre-commit hooks via Overcommit during CI runs
 gem 'overcommit', '0.61.0'
 
+# Pin redis-rb version since v5 introduced breaking changes
+gem 'redis', '>= 5.0.0'
 # Pin tool versions (which are executed by Overcommit) for CI builds
 gem 'rubocop', '1.44.1'
 
 gem 'simplecov', '~> 0.22.0'
 gem 'simplecov-lcov', '~> 0.8.0'
+
+# TODO: gems added for development, remove before merge
+# gem 'pry'

lib/mock_redis/set_methods.rb

@@ -7,22 +7,16 @@ module SetMethods
     include UtilityMethods
 
     def sadd(key, members)
-      members_class = members.class
       members = Array(members).map(&:to_s)
       assert_has_args(members, 'sadd')
 
       with_set_at(key) do |s|
-        size_before = s.size
         if members.size > 1
-          members.reverse_each { |m| s << m }
-          s.size - size_before
+          size_after(s) { members.reverse_each { |m| s << m } }
+        elsif redis_gem_v5?
+          size_after(s) { s.add?(members.first) }
         else
-          added = !!s.add?(members.first)
-          if members_class == Array
-            s.size - size_before
-          else
-            added
-          end
+          !!s.add?(members.first)
         end
       end
     end
@@ -125,14 +119,13 @@ def srandmember(key, count = nil)
     end
 
     def srem(key, members)
+      members = Array(members).map(&:to_s)
+
       with_set_at(key) do |s|
-        if members.is_a?(Array)
-          orig_size = s.size
-          members = members.map(&:to_s)
-          s.delete_if { |m| members.include?(m) }
-          orig_size - s.size
+        if members.size > 1 || redis_gem_v5?
+          size_after(s) { s.delete_if { |m| members.include?(m) } }
         else
-          !!s.delete?(members.to_s)
+          !!s.delete?(members.first)
         end
       end
     end
@@ -195,9 +188,12 @@ def sety?(key)
     def assert_sety(key)
       unless sety?(key)
         # Not the most helpful error, but it's what redis-rb barfs up
-        raise Redis::CommandError,
-              'WRONGTYPE Operation against a key holding the wrong kind of value'
+        raise wrong_type_error, 'WRONGTYPE Operation against a key holding the wrong kind of value'
       end
     end
+
+    def wrong_type_error
+      redis_gem_v5? ? Redis::WrongTypeError : Redis::CommandError
+    end
   end
 end

lib/mock_redis/utility_methods.rb

@@ -74,5 +74,15 @@ def left_pad(str, size)
 
       str
     end
+
+    def redis_gem_v5?
+      Redis::VERSION.to_i == 5
+    end
+
+    def size_after(obj, &_blk)
+      size_before = obj.size
+      yield
+      (obj.size - size_before).abs
+    end
   end
 end

spec/commands/sadd_spec.rb

@@ -4,13 +4,19 @@
   before { @key = 'mock-redis-test:sadd' }
 
   context 'sadd' do
-    it 'returns true if the set did not already contain member' do
-      expect(@redises.sadd(@key, 1)).to eq(true)
-    end
+    context 'adapts to redis-rd version 4 and 5 outputs' do
+      include MockRedis::UtilityMethods
 
-    it 'returns false if the set did already contain member' do
-      @redises.sadd(@key, 1)
-      expect(@redises.sadd(@key, 1)).to eq(false)
+      let(:positive_response) { redis_gem_v5? ? 1 : true }
+      let(:negative_response) { redis_gem_v5? ? 0 : false }
+      it 'returns true if the set did not already contain member' do
+        expect(@redises.sadd(@key, 1)).to eq(positive_response)
+      end
+
+      it 'returns false if the set did already contain member' do
+        @redises.sadd(@key, 1)
+        expect(@redises.sadd(@key, 1)).to eq(negative_response)
+      end
     end
 
     it 'adds member to the set' do

spec/commands/srem_spec.rb

@@ -8,24 +8,31 @@
     @redises.sadd(@key, 'ernie')
   end
 
-  it 'returns true if member is in the set' do
-    expect(@redises.srem(@key, 'bert')).to eq(true)
-  end
+  context 'adapts to redis-rd version 4 and 5 outputs' do
+    include MockRedis::UtilityMethods
+
+    let(:positive_response) { redis_gem_v5? ? 1 : true }
+    let(:negative_response) { redis_gem_v5? ? 0 : false }
 
-  it 'returns false if member is not in the set' do
-    expect(@redises.srem(@key, 'cookiemonster')).to eq(false)
+    it 'returns positive response if member is in the set' do
+      expect(@redises.srem(@key, 'bert')).to eq(positive_response)
+    end
+
+    it 'returns negative response if member is not in the set' do
+      expect(@redises.srem(@key, 'cookiemonster')).to eq(negative_response)
+    end
+
+    it 'stringifies member' do
+      @redises.sadd(@key, '1')
+      expect(@redises.srem(@key, 1)).to eq(positive_response)
+    end
   end
 
   it 'removes member from the set' do
     @redises.srem(@key, 'ernie')
     expect(@redises.smembers(@key)).to eq(['bert'])
   end
 
-  it 'stringifies member' do
-    @redises.sadd(@key, '1')
-    expect(@redises.srem(@key, 1)).to eq(true)
-  end
-
   it 'cleans up empty sets' do
     @redises.smembers(@key).each { |m| @redises.srem(@key, m) }
     expect(@redises.get(@key)).to be_nil

spec/support/redis_multiplexer.rb

@@ -10,7 +10,7 @@ class RedisMultiplexer < BlankSlate
   def initialize(*a)
     super()
     @mock_redis = MockRedis.new(*a)
-    Redis.exists_returns_integer = true
+    configure_redis
     @real_redis = Redis.new(*a)
     _gsub_clear
   end
@@ -121,4 +121,15 @@ def catch_errors
   rescue StandardError => e
     [nil, e]
   end
+
+  private
+
+  def configure_redis
+    case Redis::VERSION.to_i
+    when 5
+      # do nothing
+    else
+      Redis.exists_returns_integer = true
+    end
+  end
 end

spec/support/shared_examples/does_not_cleanup_empty_strings.rb

@@ -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)
     expect(@redises.get(key)).to eq('')
   end
 end

spec/support/shared_examples/only_operates_on_sets.rb

@@ -8,7 +8,7 @@
     @redises.set(key, 1)
     expect do
       @redises.send(method, *args)
-    end.to raise_error(RuntimeError)
+    end.to raise_error(Redis::BaseError)
   end
 
   it_should_behave_like 'does not remove empty strings on error'