-
Notifications
You must be signed in to change notification settings - Fork 86
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
rescue exceptions and run job.fail in perform #88
base: master
Are you sure you want to change the base?
Conversation
Oops, well there is definitely at least one problem. It seems that on_failure_hooks are running twice now. |
Hi @mcfiredrill, An alternative approach would be to test fail directly, something like: describe "MyJob.fail" do
it "updates the fail count" do\
expect do
MyJob.fail
end.to change(MyJob, :fail_count)
end
end |
I changed my test slightly and fixed duplicate failure hook bug by upgrading resque. Let me know what you think, although the tests still aren't passing on travis for some reason. |
Ah ok, so I'm actually relying on redis running for that test, I didn't realize that. I'm not sure yet how to test it without relying on redis. |
Hi @mcfiredrill, Have you overridden the fail code in resque? I suspect the default behavior is using redis. Perhaps you should mock out fail and set the expectation that way? |
Nah I'm not overriding it, I'm just calling job.fail directly, like worker.rb does here: Mocking out fail sounds like a good idea, I'll try that out. |
So I did that, let me know what you think. Maybe I'm wary of using |
Hi @mcfiredrill, Hmmm. Looking at that, it is not exactly what we want. We want a test double at runtime for
Make sense? |
Alright, I guess I kind of missed the point there. I'm actually testing a custom failure backend via this, which I guess I shouldn't be doing, I should be doing an integration test with full resque. I can still set inline manually without resque_spec. I'm still having fun hacking on this project though, so I think I'll attempt to write this. |
Any news/plans on this? At the moment I'm using a fork of this gem with this PR merged in. |
bump, I am tracking this too. I am trying to test job retries right now and can not seem to get around this :( |
@kyrylo are you referring to this ::
in that gem? do also use |
It's just a fork where this PR is merged. You don't want to touch |
Hi all, Thanks for following up, this project needs a new maintainer (#122). Please drop a note if interested. Regards. |
I needed to test some of my resque failure code, but I realized that Resque::Job#fail is not actually run if you are using inline. I think using ResqueSpec.inline simplifies testing quite a bit, so I wanted to use it, but I still needed failures to be reported. So I wrote this patch. I'm not sure if this fits everyones use case, or if this is even the right way to go about it.
Let me know what you think.