leandromoreira / redlock-rb

Redlock is a redis-based distributed lock implementation in Ruby. More than 20M downloads.
BSD 2-Clause "Simplified" License
693 stars 81 forks source link

`Client#initialize` specs: are they working as. expected? #154

Open tagliala opened 1 day ago

tagliala commented 1 day ago

Hello,

while implementing #153 I've noticed that the newly instantiated redlock client is not being used by the spec, leading to false positives

https://github.com/leandromoreira/redlock-rb/blob/f176e5748a41e8b35a9a986fb6cc8eeae203a64f/spec/client_spec.rb#L48-L66

Example

    it 'accepts hashes responding to call like ActiveSupport::OrderedOptions' do
      callable_hash = Class.new(Hash) do
        def call; end
      end

      config = callable_hash.new.merge(url: "redis://#{redis1_host}:#{redis1_port}")
      _redlock = Redlock::Client.new([config])

      lock_info = lock_manager.lock(resource_key, ttl)
      expect(lock_info).to be_a(Hash)
      expect(resource_key).to_not be_lockable(lock_manager, ttl)
      lock_manager.unlock(lock_info)
    end

This was not failing, because lock_manager was using its own client, and the previous initialization was not used at all

  let(:lock_manager) {
    Redlock::Client.new(redis_urls_or_clients, lock_manager_opts)
  }
leandromoreira commented 1 day ago

and it should be _redlock right?

tagliala commented 1 day ago

and it should be _redlock right?

Yes, according to the intent of what I see. But at that point _redlock does not make much sense because it is a used variable, so I would either go for redlock or redefine lock_manager.

I've used redlock in #153