New ActiveJob is not counted when other job executed

[mkllnk]

What Ruby, Rails and RSpec versions are you using?

Ruby version: ruby 3.0.3p157 (2021-11-24 revision 3fb7d2cadc) [x86_64-linux]
Rails version: Rails 7.0.4.3
RSpec version: RSpec 3.12

• rspec-core 3.12.1
• rspec-expectations 3.12.2
• rspec-mocks 3.12.3
• rspec-rails 6.0.1
• rspec-support 3.12.0

Observed behaviour

Consider this example:

    ReportJob.perform_later(*report_args)

    expect {
      perform_enqueued_jobs(only: ReportJob)
    }.to enqueue_job(ActionMailer::MailDeliveryJob)

The ReportJob sends an email and this test should pass but fails:

     Failure/Error:
       expect {
         perform_enqueued_jobs(only: ReportJob)
       }.to enqueue_job(ActionMailer::MailDeliveryJob)
     
       expected to enqueue exactly 1 jobs, but enqueued 0

I found the reason in the matcher simply taking the count of pre-existing jobs and dropping them off the queue when comparing:

rspec-rails/lib/rspec/rails/matchers/active_job.rb

Lines 233 to 235 in 01704c5

	original_enqueued_jobs_count = queue_adapter.enqueued_jobs.count
	proc.call
	in_block_jobs = queue_adapter.enqueued_jobs.drop(original_enqueued_jobs_count)

Expected behaviour

I expected the matcher to only ignore pre-existing jobs and not newly generated jobs. I would also have understood if the matcher would only look for MailDeliveryJobs but the types of jobs executed and enqueued are different here.

I saw in the original pull request that it was counting only the requested job type but I'm wondering if we could actually remember the previous jobs (with id) and drop only those off the queue. I'm not familiar with the adapters but I was wondering if this would work:

            original_enqueued_jobs = queue_adapter.enqueued_jobs
            proc.call
            in_block_jobs = queue_adapter.enqueued_jobs - original_enqueued_jobs

It may have performance implications.

Can you provide an example app?

I hope that the above examples are easy enough but let me know if I need to create an app for this. Maybe more useful, here's an (untested) spec:

diff --git a/spec/rspec/rails/matchers/active_job_spec.rb b/spec/rspec/rails/matchers/active_job_spec.rb
index 38aee9d3..0c24276d 100644
--- a/spec/rspec/rails/matchers/active_job_spec.rb
+++ b/spec/rspec/rails/matchers/active_job_spec.rb
@@ -94,6 +94,14 @@ RSpec.describe "ActiveJob matchers", skip: !RSpec::Rails::FeatureCheck.has_activ
       }.to have_enqueued_job.exactly(1)
     end
 
+    it "counts jobs even when others are removed" do
+      heavy_lifting_job.perform_later
+      expect {
+        perform_enqueued_jobs
+        hello_job.perform_later
+      }.to have_enqueued_job(hello_job)
+    end
+
     it "passes when negated" do
       expect { }.not_to have_enqueued_job
     end

mkllnk@7ecda54

[pirj]

Quite interesting.
It seems that the

queue_adapter.enqueued_jobs.drop(original_enqueued_jobs_count)

is incorrect in this scenario.
I suggest using the "fix" from this PR.

Here's a draft of a spec:

    include ActiveJob::TestHelper

    let(:one) do
      Class.new(ActiveJob::Base) do
        def perform;
          Two.perform_later
        end
        def self.name; "One"; end
      end
    end

    let(:two) do
      Class.new(ActiveJob::Base) do
        def perform; end
        def self.name; "Two"; end
      end
    end

    it do
      stub_const('One', one)
      stub_const('Two', two)

      One.perform_later
      expect {
        perform_enqueued_jobs(only: One)
      }.to enqueue_job(Two)
    end

The code from that PR makes the spec pass.

Would you like to send a PR, @mkllnk ?

[JonRowe]

The original implementation fails to take into account that you are performing jobs, so the array is presumedly mutated and the new job pop'd off by mistake, which is similar to the issue with retry

[mkllnk]

@pirj

Would you like to send a PR, @mkllnk ?

Maybe. But you are saying that the problem is solved with the other PR #2573. If that's the case then the only missing part is a spec. If we are using your spec draft then you should submit that pull request. My issue description contains another little spec addition which is slightly simpler but still demonstrates the same issue. I'm happy to submit that as PR if it's desired. But it would be pending until the other PR is merged. Please advise how to proceed.

[JonRowe]

If you have a spec you like to add to check its covered / fixed please do now that #2573 is merged.

[pirj]

I’ve added it here.

[pirj]

Still, please check the latest code out if it fixes it for you

[mkllnk]

Thank you very much. I just tested with the latest release 6.02 and it fixes the issue. 👍