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

Drain method doesn't invoke server middleware #1846

Closed
vhldanluria opened this issue Jul 15, 2014 · 5 comments
Closed

Drain method doesn't invoke server middleware #1846

vhldanluria opened this issue Jul 15, 2014 · 5 comments

Comments

@vhldanluria
Copy link

When testing with Sidekiq, the drain method doesn't invoke the server middleware chain. However, queueing a job invokes the client middleware chain. This makes it impossible to do a full middleware integration test without writing a custom drain method.

Is there a proper way to test the server middleware, or should I implement my own method?

@mperham
Copy link
Collaborator

mperham commented Jul 15, 2014

Correct. Sidekiq does not try to provide a full integration test server environment as the law of diminishing returns applies here: very few people need it and it's a lot of work. NewRelic has a middleware integration test system they built to run the Sidekiq server in-process so they can verify their server middleware.

https://github.com/newrelic/rpm/tree/master/test/multiverse/suites/sidekiq

@mperham mperham closed this as completed Jul 15, 2014
@jpcody
Copy link

jpcody commented Sep 8, 2014

@mperham Without knowing how extensive implementation work would be, I'd argue this violates the principle of least surprise.

When running Sidekiq tests inline or draining them, especially as client middleware is run, I'd expect server middleware to run. It's essentially already an integration test at the point of providing inline/drain, just with an important caveat that one of two types of middleware won't run.

And this middleware can be very important to test as it often has impacts on guarantees you can make about your system (environment-dependent information, multi-tenancy, etc…).

I'd love to hear if I'm misunderstanding something.

@ryansch
Copy link
Contributor

ryansch commented Sep 8, 2014

I've bumped my head on this again as we're now using server side sidekiq middleware to enforce multi-tenancy in our application. I might have to drop by happy hour this week for a chat.

@mperham
Copy link
Collaborator

mperham commented Sep 8, 2014

You can test workers which depend on middleware:

worker = MyWorker.new
MyMiddleware.new.call(msg, worker, 'default') do
  worker.perform(*msg['args'])
end

There's no surprise here: a worker is a basic Ruby class. If you need some context set up, you have to set it up.

@ryansch
Copy link
Contributor

ryansch commented Sep 8, 2014

Thanks Mike. I'll give that a try.

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

4 participants