feature suggestion: optional context tag for `sidekiq_retries_exhausted`

[nrpx]

I know that this project already has a helper method within_sidekiq_retries_exhausted_block, but I think there will be those who need a more versatile approach for testing the mentioned Sidekiq feature.

Here's a possibly more generic solution that allows to execute a declared block of code within an RSpec context:

# spec/support/sidekiq_exhausted_retries_spec_helper.rb

module SidekiqExhaustedRetriesSpecHelper
  def self.included(base)
    base.before(:each, :sidekiq_with_exhausted_retries) do
      allow_any_instance_of(Sidekiq::Middleware::Chain).to \
        receive(:invoke).and_wrap_original do |original_method, *args, &block|
        job, msg = args

        begin
          original_method.call(*args, &block)
        rescue => e
          job.class.try(:sidekiq_retries_exhausted_block)&.call(msg, e)
        end
      end
    end
  end
end

Include it into rspec_helper.rb:

# spec/rails_helper.rb
# ...
RSpec.configure do |config|
  # ...
  config.include SidekiqExhaustedRetriesSpecHelper
  # ...
end
# ...

And then simply use it in any specs where you need some result from the mentioned block:

class SimpleJob
  include Sidekiq::Job
  sidekiq_options retry: 5
  sidekiq_retries_exhausted do |job, e|
    # <some logic after all failed retries>
  end

  def perform
    raise "Just for test"
  end
end

class SomeClassWithSidekiqJobs
  def initialize
    SimpleJob.perform_async
  end
end

describe SomeClassWithSidekiqJobs do
  context "when sidekiq job failed", :sidekiq_with_exhausted_retries do
    around { |ex| Sidekiq::Testing.inline!(&ex) }

    it "should run sidekiq_exhausted_retries block" do
      expect { subject }.to change("something that can be checked based on the block result")
    end
  end
end

[wspurgin]

I'm not quite sure what this includable context solves beyond what is already accomplishable through testing the within_sidekiq_retries_exhausted_block... In other words, how is this more versatile as you claim is needed?

In fact, this feels like it's less versatile (needing to remember to use a "magic" context tag and insuring any enqueue jobs are actually run inside those tests).

[nrpx]

Let's imagine a fairly simple situation where I need to run workflow integration tests involving sidekiq jobs and the presence of exceptions that cause the sidekiq_retries_exhausted block to be called (without any exceptions raised to the spec's thread).

With or without your helper method, the spec will inevitably encounter raised exceptions instead of the sidekiq_retries_exhausted block. Or I will have to invent an exception mock, which should be explicitly passed to the helper method. But in this case it is possible to call the mentioned block only for one job class at a time. If several different job classes are involved in the workflow, I'll have to make nested blocks with your helper for each of them. And also the main body of job perform should be executed successfully, without any exceptions, so it becomes a side-effect. Aaaand there is no control on the job instances level. So you cannot run sidekiq_retries_exhausted block for only one job of the workflow if there is a bunch of them under one job class.

In my proposed example above it is enough to add a tag to the "context" or "describe" block and declare such conditions that will cause a real exception when sidekiq processes the job. And there is no added implementation details needed like some mocked exception object or stubbing the original perform method to avoid side-effects of the successful job completion. It simply runs the sidekiq_retries_exhausted block as a rescuer for the raising exception of the worker/job.

[wspurgin]

Honestly without seeing the precise testing code you're trying to describe, this sounds like an esoteric use-case and not widely applicable. A "workflow integration tests involving sidekiq jobs and the presence of exceptions that cause the sidekiq_retries_exhausted block to be called" already sounds beyond a "simple situation" 😄

TBH, I think you'd be better served with adding a specific Sidekiq Testing server middleware that does exactly what you're describing (rescue errors and call the Sidekiq retries exhausted block of the job). The fact that errors get bubbled up is how Sidekiq's Testing harness is designed.

IMHO, you're better served with unit testing the sidekiq_retries_exhausted block with the existing helper for whatever combination of Sidekiq jobs you have rather than having a dedicated integration test for that very specific sad-path.

However, that's not really my business 🫖!

Yet, I don't particularly see the need to add what you're describing to rspec-sidekiq as I understand it now. If you have other details our further examples of this specific use-case and how it's more widely applicable (and not served via the unit testing-oriented helper), please feel free to share. For now though, I'm passing on adding this to rspec-sidekiq.