wspurgin / rspec-sidekiq

RSpec for Sidekiq
https://github.com/wspurgin/rspec-sidekiq
Other
651 stars 133 forks source link

Can't test several delayed jobs together #137

Closed arpadlukacs closed 2 months ago

arpadlukacs commented 6 years ago

I'm delaying sending two emails, one right now and another one later on.

This is the related code in my Appointment#book!

       ScheduleMailer.delay.confirmation(id)
       ScheduleMailer.delay_until(deadline).confirmation(id)

I'm trying to test like this:


    context 'email notifications' do
        before do
          Sidekiq::Testing.fake!
          expect { appointment.book! }.to change(Sidekiq::Extensions::DelayedMailer.jobs, :size).from(0).to(5)
        end
        it 'sends emails' do
          expect(ScheduleMailer.instance_method :confirmation).to be_delayed(appointment.id)
          expect(ScheduleMailer.instance_method :confirmation).to be_delayed(appointment.id).until appointment.deadline
        end
      end
    end

The last expect line always fails. If I comment out the " ScheduleMailer.delay.confirmation(id)" line in the appointment model, the line passes.

I looked into rspec-sidekiq's code and it seems the be_delayed#find_job does not find the job that was delayed.until (the one which has the "at" key/value), only the one which was delayed without "until". It seems the problem happens because the find consider both job the same: expected_method_receiver, expected_method and expected_arguments are all the same for both jobs. The condition does not include the "until" argument here. Those are supposed to be handled in the "matches?" method but since that one uses the find_job which does not returns the delayed.until job the matches? method always returns false.

I saw another issue about a TZ issue, but I think that has been fixed already and my problem is not because of that.

That looks like a bug to me, but please let me know if I'm wrong.

wspurgin commented 2 months ago

7 years later (😅) - Thanks for the issue, but be_delayed is deprecated for Sidekiq >=7 and I'm not planning to invest more into it.