Skip to content
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

any thoughts on how to support delay class methods? #18

Closed
dfguo opened this issue Sep 6, 2013 · 5 comments
Closed

any thoughts on how to support delay class methods? #18

dfguo opened this issue Sep 6, 2013 · 5 comments

Comments

@dfguo
Copy link

dfguo commented Sep 6, 2013

No description provided.

@philostler
Copy link
Collaborator

I've not had need to use the delayed extensions kit in Sidekiq so haven't given this any thought up to now.

FYI from looking at some of the Sidekiq code there are comments that using the delayed methods are discouraged anyway due to serialization of the entire object to JSON.

However, if I read Sidekiq's source correctly, this should be testable with the current matchers (don't hold me to that).

Scanning over the extensions directory in Sidekiq's source, everything hangs round the Proxy class and it's method_missing method. The end game appears to be the job enqueued via the usual #client_push method but with some additional arguments added (most importantly the name of the method being called). I've not looked any further but there must be logic in Sidekiq that looks at arguments and calls the relevant method when the job is invoked.

So you should be able check a job in enqueued correctly and the expected arguments (You'll have to take a peek to see what those arguments are). As for the method being called, I'd test that the job is enqueued as expected (as above) and then test the method itself separately (else your testing that Sidekiq is actually performing it's job correctly which is already tested in Sidekiq itself).

@dfguo
Copy link
Author

dfguo commented Sep 6, 2013

It's discouraged to use active_record extension. Using class methods actually will not have any serialization problem as long as the arguments are plain.

I'm actually interested in testing if arguments are correctly passed in, using have_enqueued_job. Currently I can only test if the size of the DelayClass has changed using Sidekiq::Extensions::DelayedClass.jobs.

Using have_enqueud_job on Sidekiq::Extensions::DelayedClass gives me:

[["---\n- !ruby/class 'EmailListManager'\n- :add_user_to_list\n- - 1\n - free\n"]]

when I called EmailListManager.delay.add_user_to_lis(1, 'free')

I'll look into how this argument get parsed.

@philostler
Copy link
Collaborator

Did you get any further with this?

@dfguo
Copy link
Author

dfguo commented Sep 11, 2013

Hi Phil,

I realized that It's not very readable to test Sidekiq::Extensions::DelayedClass. So I wrote a simple dsl to generate sidekiq job class instead.

@dfguo dfguo closed this as completed Sep 11, 2013
@sosaucily
Copy link
Contributor

I've got some helper methods for testing the delay class methods. Will create a pull request.

sosaucily added a commit to sosaucily/rspec-sidekiq that referenced this issue Nov 4, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants