Sidekiq: 'testing' module doesn't clear #jobs between runs

[subwindow]

This may just be a doc fix, but I find this behavior surprising.

Take this incredibly trite example:

describe MyJob do
  before { MyJob.perform_async('test') }
  subject { MyJob.jobs }
  its(:size) { should eq 1 }
  its(:size) { should eq 1 }
end

The last assertion fails because at that point, #jobs returns two items. To fix it you have to add:

before { MyJob.instance_variable_set(:@pushed, []) }

That seems a bit unintuitive to me. I know that the 'testing' module, as of today, is deliberately ignorant of RSpec, but perhaps that's not the best approach long-term. At the very least it'd be nice if there were some documentation to say that resetting the variable is up to you.

[mperham]

before { MyJob.jobs.clear }

What does ActionMailer do?

[subwindow]

https://github.com/rails/rails/blob/a152fd34d3eb0e4980c4a44dd1f71510890c5417/actionmailer/lib/action_mailer/test_case.rb#L49

Called from a 'setup' hook for Test::Unit, which I think translates in a straightforward manner to RSpec.

[cvincent]

+1 it would be nice to be able to clear all workers in a single call in the global after hook in RSpec.

[mperham]

There's at least three different test libraries I'd need to inject magic into and they all work differently. I'd rather have you guys just use a one-line before/setup block like above. ActionMailer has the benefit of running within its own special TestCase subclass.

I'm happy to take PRs for this but I just don't have the time or expertise to do it.

[cvincent]

There wouldn't need to be library-specific magic injection necessarily. All we really need is a line like this:

Sidekiq::Worker.clear_jobs

Or something similar. (Obviously the above wouldn't work as Sidekiq::Worker is a module not a class, but it reflects the concept.) Right now we can only clear jobs for a specific worker; any time we add new workers we would have to add another line to our global teardown or remember to clear specific workers which are involved in our specific tests. A better solution would be to have a one-liner which clears jobs across all workers.

I managed to hack together a solution with a global variable which keeps an array of workers, then calls #jobs.clear on each of them, but the global variable solution is dirty of course. A cleaner alternative would be to append worker jobs to a single, globally-accessible array rather than per-worker arrays. Then we would just clear that array in our test/spec helper global teardown. This is closer to the way ActiveMailer, Resque, and a number of other libraries work with testing environments. Each library has a single array to clear, rather than one per subclass or worker.

For reference, here's my dirty global variable based solution:

# Keep track of workers with jobs added to allow their
# jobs to be cleared after each test.
$sidekiq_workers = []

module Sidekiq::Worker
  module ClassMethods
    alias_method :perform_async_orig, :perform_async
    def perform_async(*args)
      $sidekiq_workers << self unless $sidekiq_workers.include?(self)
      perform_async_orig(*args)
    end

    def clear_jobs
      @pushed.clear
    end
  end
end

Then I have this helper method:

  def clear_sidekiq_jobs
    $sidekiq_workers.each do |worker|
      worker.clear_jobs
    end
  end

I call that method in my spec_helper.rb global after hook. Just like that, all my jobs are cleared after each test, regardless of how many workers may have enqueued jobs. It works for me in my limited time, but obviously we would need a cleaner solution for Sidekiq proper.

[seivan]

+1

[mperham]

Sidekiq::Worker.clear_all and Sidekiq::Worker.drain_all are now available.

[stulentsev]

:heart:

[seivan]

I still got code running from @cvincent snippet. Thanks everyone! :)