cryo28 / sidekiq_status

Extension to Sidekiq to pass job execution metadata such as status and result back to the client
MIT License
152 stars 53 forks source link

Ideas for new features #39

Open cryo28 opened 8 years ago

cryo28 commented 8 years ago

Please suggest any ideas of what has to be added to the project.

salmanasiddiqui commented 8 years ago

support for testing? I am not able to test anything where a worker is being invoked which includes SidekiqStatus::Worker or where I use SidekiqStatus::Container. It fails to connect to redis. SidekiqStatus should support the same testing interface that sidekiq provides.

cryo28 commented 8 years ago

@salmanasiddiqui Could you please specify what exactly does not work for you? This is an exerpt from one of my one tests:


    let(:container) do
      jid = described_class.perform_async(client_id, user_id, path) # this is required for SidekiqStatus to propertly initialize container
      container = Workers::AsyncJobWorkerMixin::Container.load(jid)
    end
    let(:payload) { container.payload.fetch(InternalApi::Users::AsyncJobsController::PAYLOAD_FOR_CLIENT).symbolize_keys }

    around do |example|
      Sidekiq::Testing.inline! do
        example.call
      end
    end

    it "works" do
      stub_entity_assignments_call_for(user_id).and_return(build_entity_assignments_response([item1, item2].map(&:catalog_node).map(&:id)))
      expect(payload).to include(status: 'success')
    end
end

I am sorry it has a lot of unrelated (client project-specific test code) but it shows the idea:

  1. Set Sidekiq::Testing.inline!
  2. Schedule a job. The job will immediately execute (because of Inline) and return jid.
  3. Load container using jid and assert on its attributes/payload.

Or do you mean some other use case for tests?

salmanasiddiqui commented 8 years ago

There is this action in controller which is supposed to return status of job

def get_job_status
  container = SidekiqStatus::Container.load(params[:job_id])
  @result = { status: container.status, completed: container.at, total: container.total }
end

RESPONSE:

{
  status: 'complete'|'waiting'|'working'|'failed',
  total: integer
  completed: integer
}

So I wanted to write two tests for this action

  1. action should return status as 'completed' and total should be equal to completed.
  2. action should return status as 'waiting' if job is queued and hasn't started yet.

For first test:

Sidekiq::Testing.inline! do
  job_id = MyWorker.perform_async(some_id)
  get :get_job_status, job_id: job_id
  assert_response :success
  result = assigns(:result)

  assert_equal 'complete', result[:status]
  assert_equal result[:total], result[:completed]
end

MyWorker includes SidekiqStatus::Worker, and on line job_id = MyWorker.perform_async(some_id) it gives error that it cant connect to redis.

For second test:

job_id = MyWorker.perform_async(some_id)
get :get_job_status, job_id: job_id
assert_response :success
result = assigns(:result)

assert_equal 'waiting', result[:status]

no Sidekiq::Testing.inline! as I dont want the job to be processed. But this too fails on job_id = MyWorker.perform_async(some_id) as it cant connect to redis.

I already have included require 'sidekiq/testing' in test_helper.rb, and there are some other tests for other sidekiq workers and they are being tested fine.

Sidekiq Version: 3.4.0

cryo28 commented 8 years ago

@salmanasiddiqui Looks like you just need to simply mock the SidekiqStatus::Container and the load call. Like this (rspec syntax):

expect(SidekiqStatus::Container).to receive(:load).with(jid).and_return(SidekiqStatus::Container.new(jid, {
    status: __here goes the desired status__,
    total: integer,
    completed: integer,
}))

as you are essentially not testing your Sidekiq::Status job here, but you test the behaviour of your controller depending on the status of the job. So just stub the call.

salmanasiddiqui commented 8 years ago

and what if I wanted to test my job? :smile: Sidekiq does this mocking of workers by itself in test environment. I suggest that this gem (and any other sidekiq plugin aswell) should override those classes in sidekiq as needed. https://github.com/mperham/sidekiq/blob/master/lib/sidekiq/testing.rb

If I could then I would have definitely loved to contribute in terms of PR but right now I lack both the time and knowledge for this :disappointed: