-
Notifications
You must be signed in to change notification settings - Fork 133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Testing callbacks within batches? #86
Comments
I'm also curious about this. Any suggestions? |
First thanks @kmayer for the job already done.
But I would like to know if rspec-sidekiq plan to add more support for batch callbacks. |
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 ***@***.***> wrote:
First thanks @kmayer <https://github.com/kmayer> for the job already done.
I have the same problem. From this discussion
<sidekiq/sidekiq#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
<#86 (comment)>,
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
|
Sorry for the bump @kmayer. But I'm still interested ;) |
If mike perham is against it we're probably not going to add it. |
He said "If anyone comes up with a solution they think is useful to all Sidekiq Pro users, I'm happy to review" |
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. |
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 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 |
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?
The text was updated successfully, but these errors were encountered: