wspurgin / rspec-sidekiq

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

have_enqueued_job failing on 3.0.2 in cases where have_enqueued_sidekiq_job passes #135

Closed aprescott closed 7 years ago

aprescott commented 7 years ago

Some of my tests started failing with have_enqueued_job on 3.0.2, whereas before they were passing on 3.0.0. The failure is:

     Failure/Error: expect(FooJob).to have_enqueued_job(1)

       expected to have an enqueued FooJob job
         arguments: [[1]]
       found
         arguments: [[1], [2], [3], [4], [5], [6], [7]]

I noticed the deprecation warning about switching to using have_enqueued_sidekiq_job, and that seems to work fine. Despite the deprecation, I would expect the two matchers to work the same and continue to be aliases.

By looking at the comparison for v3.0.0...v3.0.2 I noticed in https://github.com/philostler/rspec-sidekiq/commit/32d2f99756b23abb391830e69dffa8b89b7e524b that the aliasing method was replaced with a def + regular method call:

-      if Gem::Dependency.new('rspec-rails', '>= 3.4.0').matching_specs.max_by(&:version)
+      def have_enqueued_job(*expected_arguments)
         warn "[DEPRECATION] `have_enqueued_job` is deprecated.  Please use `have_enqueued_sidekiq_job` instead."
-        alias have_enqueued_sidekiq_job have_enqueued_job
+        have_enqueued_sidekiq_job(expected_arguments)
       end

I think this introduced the failures. have_enqueued_job takes *expected_args, but it then calls have_enqueued_sidekiq_job(expected_args), not have_enqueued_sidekiq_job(*expected_args).

aprescott commented 7 years ago

I managed to put the fix for this together in #136, along with a test.

packrat386 commented 7 years ago

Closed by #136

For future reference, you can put "Closes #135" in the body of the PR and this will get closed automagically when I merge.

aprescott commented 7 years ago

Yep, I know that trick, but forgot to include it and thought it best to not edit the PR.

Thanks for the merge!