rspec-sidekiq icon indicating copy to clipboard operation
rspec-sidekiq copied to clipboard

Testing callbacks within batches?

Open IanWhitney opened this issue 9 years ago • 8 comments

In the Batches feature of Sidekiq Pro we can declare a callback to be called when the batch completes/succeeds.

rspec-sidekiq has made it much easier for us to test the batch functionality (which I appreciate!), but I don't see a way to test this callback functionality. I'm guessing it's because of the NullObject implementation of Batch, but I'm not sure.

Is there a way to test the callback functionality within rspec-sidekiq? Or, if not, any suggestions on how I might start tackling this PR?

IanWhitney avatar Nov 24 '15 22:11 IanWhitney

I'm also curious about this. Any suggestions?

scifisamurai avatar Jan 18 '17 00:01 scifisamurai

First thanks @kmayer for the job already done. I have the same problem. From this discussion, Mike Perham say:

I don't support automated testing batches in-process. You can test your worker code, you can test your callback code. You cannot test the entire workflow. People have hacked together solutions but nothing that I want to support long-term.

But I would like to know if rspec-sidekiq plan to add more support for batch callbacks.

benoittgt avatar Jun 20 '17 14:06 benoittgt

This is an old discussion, I'm pretty sure that we came up with a satisfactory solution, since I'm not worried about it any more :-) I'll check my code.

Ken

On Tue, Jun 20, 2017 at 7:05 AM, Benoit Tigeot [email protected] wrote:

First thanks @kmayer https://github.com/kmayer for the job already done. I have the same problem. From this discussion https://github.com/mperham/sidekiq/issues/2700, Mike Perham say:

I don't support automated testing batches in-process. You can test your worker code, you can test your callback code. You cannot test the entire workflow. People have hacked together solutions but nothing that I want to support long-term.

But I would like to know if rspec-sidekiq plan to add more support for batch callbacks.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/philostler/rspec-sidekiq/issues/86#issuecomment-309765344, or mute the thread https://github.com/notifications/unsubscribe-auth/AABM5IhyU-DPH0aM_Gnrjxtm9uStb7_2ks5sF9GhgaJpZM4GoxLh .

-- CTO, PacerPro, Inc. https://www.pacerpro.com Want to talk to me? Schedule an appointment here: https://calendly.com/ken-pacerpro/15min

kmayer avatar Jun 20 '17 16:06 kmayer

Sorry for the bump @kmayer. But I'm still interested ;)

benoittgt avatar Jun 29 '17 08:06 benoittgt

If mike perham is against it we're probably not going to add it.

packrat386 avatar Jun 29 '17 15:06 packrat386

He said "If anyone comes up with a solution they think is useful to all Sidekiq Pro users, I'm happy to review"

benoittgt avatar Jun 29 '17 15:06 benoittgt

I mean, PRs are welcome, but I feel reasonably confident in speaking for the team that we're not going to to spend time working on it if it's a feature that perham thinks is unlikely to work well.

packrat386 avatar Jun 29 '17 15:06 packrat386

Here's how we do it at PacerPro:

We modified the worker so it returns a Sidekiq::Batch object:

class BatchSearch
  include Sidekiq::Worker

  def perform(search_id)
    @search = Search.find(search_id)

    Sidekiq::Batch.new.tap do |batch|
      batch.on(:complete, Callback, {'search_id' => search_id})
      batch.jobs do
        @search.end_points.each do |court|
          SearchWorker.perform_async(court.id, @search.id, jid)
        end
      end
    end
  end
end

and in RSpec, we call #perform to run synchronously, then batch.status.join to run the callbacks and check behavior.

      it 'changes the state of the search to "finished"' do
        expect {
          batch = worker.perform(search.id)
          batch.status.join
        }.to change { search.reload.status }.from("searching").to("finished")
      end

kmayer avatar Jun 29 '17 15:06 kmayer