MockRedisLuaExtension.wrap is broken by its use of prepend

[ajvondrak]

Context: I'm using mock_redis + this extension in my minitest suite. I'm trying to stub MockRedis#evalsha to raise an error, but minitest's stubbing is not working - it still just calls the MockRedisLuaExtension#evalsha method.

After enough digging, I believe it's due to the injection of MockRedisLuaExtension onto the front of the ancestor chain, which screws with the method lookup order. Relevant line:

mock_redis_lua_extension/lib/mock_redis_lua_extension.rb

Line 33 in ce41d51

prepend(MockRedisLuaExtension)

Here's a minimal way of demonstrating the moving pieces:

1. The relevant code structure.

   class MockRedis
     def evalsha
       puts 'MockRedis#evalsha'
     end
   end
   
   module MockRedisLuaExtension
     def self.wrap(mock_redis)
       class << mock_redis
         prepend MockRedisLuaExtension
         ancestors # check out the return value when I call .wrap below...
       end
     end
   
     def evalsha
       puts 'MockRedisLuaExtension#evalsha'
     end
   end

2. A plain MockRedis stubs as expected.

   [3] pry(main)> require 'minitest/mock'
   => true
   [4] pry(main)> redis = MockRedis.new
   => #<MockRedis:0x00007fc4781ee288>
   [5] pry(main)> redis.evalsha
   MockRedis#evalsha
   => nil
   [6] pry(main)> redis.stub(:evalsha, -> { puts 'stubbed' }) { redis.evalsha }
   stubbed
   => nil
   

3. Prepended with MockRedisLuaExtension, stubs won't budge.

   [7] pry(main)> MockRedisLuaExtension.wrap(redis)
   => [MockRedisLuaExtension, #<Class:#<MockRedis:0x00007fc4781ee288>>, MockRedis, Object, PP::ObjectMixin, Kernel, BasicObject]
   [8] pry(main)> redis.evalsha
   MockRedisLuaExtension#evalsha
   => nil
   [9] pry(main)> redis.stub(:evalsha, -> { puts 'stubbed' }) { redis.evalsha }
   MockRedisLuaExtension#evalsha
   => nil
   

minitest is sending define_method to the MockRedis instance's metaclass to do its clobbering: https://github.com/seattlerb/minitest/blob/e6bc4485730403faff6966c1671cf5de72b2d233/lib/minitest/mock.rb#L225 As seen from the above output, MockRedisLuaExtension is first in the ancestor chain, so the stubbed method on the MockRedis metaclass will fail to be called - it gets beaten to the punch by MockRedisLuaExtension.

From what I can tell, there's no reason to prepend at all. Using an include clobbers #evalsha and so on just fine, it seems:

[1] pry(main)> require 'mock_redis'
=> true
[2] pry(main)> require 'mock_redis_lua_extension'
=> true
[3] pry(main)> redis = MockRedis.new
=> #<MockRedis:0x00007fea6b123650
 @db=
  #<MockRedis::PipelinedWrapper:0x00007fea6b122ed0
   @db=
    #<MockRedis::TransactionWrapper:0x00007fea6b122f70
     @db=
      #<MockRedis::ExpireWrapper:0x00007fea6b122fe8
       @db=
        #<MockRedis::MultiDbWrapper:0x00007fea6b123380
         @databases={0=>#<MockRedis::Database:0x00007fea6b1234e8 @base=#<MockRedis:0x00007fea6b123650 ...>, @data={}, @expire_times=[]>},
         @db_index=0,
         @prototype_db=#<MockRedis::Database:0x00007fea6b1232b8 @base=#<MockRedis:0x00007fea6b123650 ...>, @data={}, @expire_times=[]>>>,
     @in_multi=false,
     @multi_block_given=false,
     @transaction_futures=[]>,
   @in_pipeline=false,
   @pipelined_futures=[]>,
 @options={:scheme=>"redis", :host=>"127.0.0.1", :port=>6379, :path=>nil, :timeout=>5.0, :password=>nil, :db=>0, :time_class=>Time}>
[4] pry(main)> class << redis
[4] pry(main)*   include MockRedisLuaExtension
[4] pry(main)* end
=> #<Class:#<MockRedis:0x00007fea6b123650>>
[5] pry(main)> require 'minitest/mock'
=> true
[6] pry(main)> show-source redis.evalsha

From: ~/.rbenv/versions/2.5.3/lib/ruby/gems/2.5.0/gems/mock_redis_lua_extension-0.2.0/lib/mock_redis_lua_extension.rb @ line 73:
Owner: MockRedisLuaExtension
Visibility: public
Number of lines: 7

def evalsha(sha, keys=nil, argv=nil, **args)
  if script(:exists, sha)
    eval(script_catalog[sha], keys, argv, **args)
  else
    raise ArgumentError, "NOSCRIPT No matching script. Please use EVAL."
  end
end
[7] pry(main)> redis.stub(:evalsha, -> { puts 'stubbed' }) { redis.evalsha }
stubbed
=> nil
[8] pry(main)> redis.evalsha('blah')
ArgumentError: NOSCRIPT No matching script. Please use EVAL.
from ~/.rbenv/versions/2.5.3/lib/ruby/gems/2.5.0/gems/mock_redis_lua_extension-0.2.0/lib/mock_redis_lua_extension.rb:77:in `evalsha'
[9] pry(main)> class << redis
[9] pry(main)*   ancestors
[9] pry(main)* end
=> [#<Class:#<MockRedis:0x00007fea6b123650>>,
 MockRedisLuaExtension,
 MockRedis,
 MockRedis::UndefRedisMethods,
 Object,
 JSON::Ext::Generator::GeneratorMethods::Object,
 PP::ObjectMixin,
 Kernel,
 BasicObject]

At the very least, using include is a workaround. A more proper fix would probably be for this gem to just be incorporated into mock_redis directly, where we can avoid awkward metaprogramming. It seems that the mock_redis folks are open to this, going by sds/mock_redis#135 and sds/mock_redis#130 (comment)