Should iterable jobs respect fake tests and ignore Redis altogether?

[araishikeiwai]

I find that even with Sidekiq::Testing.fake!, iterable jobs will still be using redis (https://github.com/sidekiq/sidekiq/blob/main/lib/sidekiq/job/iterable.rb)

This is problematic for test environments that don't have Redis running.

I managed to make it work in RSpec with MockRedis allow(Sidekiq).to receive(:redis).and_yield(MockRedis.new), but this didn't work in Minitest + Mocha Sidekiq.stubs(:redis).yields(MockRedis.new) because mocha's yields does not return the block result as its return value, instead it returns nil in this line which causes NoMethodError: undefined method `empty?' for nil:NilClass

I know Mike does not support mocking out Redis but with the way iterable jobs are implemented, it's not possible to run tests on them without having Redis running (which may not be available in everyone's CI setup). Is it possible to prepend the iterable job module with a faked out methods that bypass Redis similar to how Sidekiq::Client's atomic_push behaviour is changed in fake?

Or maybe the actual implementation itself could be changed not to rely on the block value return? i.e. among other Redis calls in the file:

- state = Sidekiq.redis { |conn| conn.hgetall(iteration_key) }
+ state = nil
+ Sidekiq.redis { |conn| state = conn.hgetall(iteration_key) }

which I found work with Mocha stubbing above.

[mperham]

Sidekiq does not support environments without Redis. Why don't you want to install Redis locally?

[araishikeiwai]

I have Redis locally, only to find my CI env does not when I pushed.
So far we haven't needed Redis in CI apparently and I'm not sure if we want to add it only for iterable job tests.

I guess this is coming from the idea of fake! not needing Redis so I thought it would be natural for iterable jobs to be able to run without Redis.

For now I have this small monkeypatch in a helper method for Minitest which works, but this is really brittle (unless I want to add gem version enforcement in case Sidekiq is upgraded)

module IterableJobHelper
  def with_iterable_jobs
    Sidekiq.stubs(:redis).yields(MockRedis.new)
    Sidekiq::Job::Iterable.prepend(MochaCompatibleIterableJob)

    yield
  ensure
    Sidekiq.unstub(:redis)
  end
end

# This is a monkey patch to make Sidekiq iterable jobs work with MockRedis
# Mocha's yields executes the block but does not return the result
# so we need to override wherever the block return value is used
module MochaCompatibleIterableJob
  def cancel!
    return @_cancelled if cancelled?

    key = "it-#{jid}"
    result = nil
    Sidekiq.redis do |c|
      c.pipelined do |p|
        p.hsetnx(key, 'cancelled', Time.now.to_i)
        result = p.hget(key, 'cancelled')
        p.expire(key, Sidekiq::Job::Iterable::STATE_TTL, 'nx')
      end
    end
    @_cancelled = result.to_i
  end

  def is_cancelled? # rubocop:disable Naming/PredicateName
    Sidekiq.redis { |c| @_cancelled = c.hget("it-#{jid}", 'cancelled') }
  end

  def fetch_previous_iteration_state
    state = nil
    Sidekiq.redis { |conn| state = conn.hgetall(iteration_key) }

    unless state.empty?
      @_executions = state['ex'].to_i
      @_cursor = Sidekiq.load_json(state['c'])
      @_runtime = state['rt'].to_f
    end
  end

  def flush_state
    key = iteration_key
    state = {
      'ex' => @_executions,
      'c' => Sidekiq.dump_json(@_cursor),
      'rt' => @_runtime
    }

    returns = []
    Sidekiq.redis do |conn|
      returns = conn.multi do |pipe|
        pipe.hset(key, state)
        pipe.expire(key, STATE_TTL)
        pipe.hget(key, 'cancelled')
      end
    end
    returns
  end
end

[mperham]

I'm not going to add any further support for running Sidekiq without Redis in any mode of operation. It's trivial to enable Redis within CI.

sidekiq/.github/workflows/ci.yml

Lines 38 to 42 in a2594b6

	- name: Start Redis
	uses: supercharge/redis-github-action@1.8.0
	with:
	redis-image: ${{ matrix.database.image }}
	redis-version: ${{ matrix.database.version }}

[stevenharman]

It is surprising that Sidekiq::Testing.fake! doesn't work with Iterable jobs. Though perhaps the definition of "works" is the problem in this case? That is, the jobs will still live in the in-memory queue (rather than Redis), but if you actually try to run the job (via drain, for example) the iteration won't work unless there's a Redis to talk to. While it might be nice if fake! + Iterable jobs still worked without a real Redis, but I understand not wanting to add code/complexity to make that work.

Perhaps we could at least add some mentions about this "not working" without a real Redis to both the job iteration and testing wiki pages?