Merge pull request #2745 from odlp/pr-verify-job-perform-signature

Verify ActiveJob job signature matches arguments

lib/rspec/rails/matchers/active_job.rb

@@ -71,6 +71,8 @@ def thrice
           end
 
           def failure_message
+            return @failure_message if defined?(@failure_message)
+
             "expected to #{self.class::FAILURE_MESSAGE_EXPECTATION_ACTION} #{base_message}".tap do |msg|
               if @unmatching_jobs.any?
                 msg << "\nQueued jobs:"
@@ -109,6 +111,12 @@ def check(jobs)
                 false
               end
             end
+
+            if (signature_mismatch = detect_args_signature_mismatch(@matching_jobs))
+              @failure_message = signature_mismatch
+              return false
+            end
+
             @matching_jobs_count = @matching_jobs.size
 
             case @expectation_type
@@ -152,6 +160,27 @@ def arguments_match?(job)
             end
           end
 
+          def detect_args_signature_mismatch(jobs)
+            jobs.each do |job|
+              args = deserialize_arguments(job)
+
+              if (signature_mismatch = check_args_signature_mismatch(job.fetch(:job), :perform, args))
+                return signature_mismatch
+              end
+            end
+
+            nil
+          end
+
+          def check_args_signature_mismatch(job_class, job_method, args)
+            signature = Support::MethodSignature.new(job_class.public_instance_method(job_method))
+            verifier = Support::StrictSignatureVerifier.new(signature, args)
+
+            unless verifier.valid?
+              "Incorrect arguments passed to #{job_class.name}: #{verifier.error_message}"
+            end
+          end
+
           def queue_match?(job)
             return true unless @queue
 

lib/rspec/rails/matchers/have_enqueued_mail.rb

@@ -41,6 +41,8 @@ def matches?(block)
         end
 
         def failure_message
+          return @failure_message if defined?(@failure_message)
+
           "expected to enqueue #{base_message}".tap do |msg|
             msg << "\n#{unmatching_mail_jobs_message}" if unmatching_mail_jobs.any?
           end
@@ -89,6 +91,22 @@ def arguments_match?(job)
           super(job)
         end
 
+        def detect_args_signature_mismatch(jobs)
+          return if @method_name.nil?
+
+          mailer_class = mailer_class_name.constantize
+
+          jobs.each do |job|
+            mailer_args = extract_args_without_parameterized_params(job)
+
+            if (signature_mismatch = check_args_signature_mismatch(mailer_class, @method_name, mailer_args))
+              return signature_mismatch
+            end
+          end
+
+          nil
+        end
+
         def base_mailer_args
           [mailer_class_name, @method_name.to_s, MAILER_JOB_METHOD]
         end
@@ -157,6 +175,19 @@ def deserialize_arguments(job)
           end
         end
 
+        def extract_args_without_parameterized_params(job)
+          args = deserialize_arguments(job)
+          mailer_args = args - base_mailer_args
+
+          if parameterized_mail?(job)
+            mailer_args = mailer_args[1..-1] # ignore parameterized params
+          elsif mailer_args.last.is_a?(Hash) && mailer_args.last.key?(:args)
+            mailer_args = args.last[:args]
+          end
+
+          mailer_args
+        end
+
         def legacy_mail?(job)
           RSpec::Rails::FeatureCheck.has_action_mailer_legacy_delivery_job? && job[:job] <= ActionMailer::DeliveryJob
         end

spec/rspec/rails/matchers/active_job_spec.rb

@@ -64,6 +64,20 @@ def self.name; "LoggingJob"; end
     end
   end
 
+  let(:two_args_job) do
+    Class.new(ActiveJob::Base) do
+      def perform(one, two); end
+      def self.name; "TwoArgsJob"; end
+    end
+  end
+
+  let(:keyword_args_job) do
+    Class.new(ActiveJob::Base) do
+      def perform(one:, two:); end
+      def self.name; "KeywordArgsJob"; end
+    end
+  end
+
   before do
     ActiveJob::Base.queue_adapter = :test
   end
@@ -138,7 +152,7 @@ def perform; raise StandardError; end
     it "fails when job is not enqueued" do
       expect {
         expect { }.to have_enqueued_job
-      }.to raise_error(/expected to enqueue exactly 1 jobs, but enqueued 0/)
+      }.to fail_with(/expected to enqueue exactly 1 jobs, but enqueued 0/)
     end
 
     it "fails when too many jobs enqueued" do
@@ -147,20 +161,20 @@ def perform; raise StandardError; end
           heavy_lifting_job.perform_later
           heavy_lifting_job.perform_later
         }.to have_enqueued_job.exactly(1)
-      }.to raise_error(/expected to enqueue exactly 1 jobs, but enqueued 2/)
+      }.to fail_with(/expected to enqueue exactly 1 jobs, but enqueued 2/)
     end
 
     it "reports correct number in fail error message" do
       heavy_lifting_job.perform_later
       expect {
         expect { }.to have_enqueued_job.exactly(1)
-      }.to raise_error(/expected to enqueue exactly 1 jobs, but enqueued 0/)
+      }.to fail_with(/expected to enqueue exactly 1 jobs, but enqueued 0/)
     end
 
     it "fails when negated and job is enqueued" do
       expect {
         expect { heavy_lifting_job.perform_later }.not_to have_enqueued_job
-      }.to raise_error(/expected not to enqueue at least 1 jobs, but enqueued 1/)
+      }.to fail_with(/expected not to enqueue at least 1 jobs, but enqueued 1/)
     end
 
     it "fails when negated and several jobs enqueued" do
@@ -169,7 +183,7 @@ def perform; raise StandardError; end
           heavy_lifting_job.perform_later
           heavy_lifting_job.perform_later
         }.not_to have_enqueued_job
-      }.to raise_error(/expected not to enqueue at least 1 jobs, but enqueued 2/)
+      }.to fail_with(/expected not to enqueue at least 1 jobs, but enqueued 2/)
     end
 
     it "passes with job name" do
@@ -224,7 +238,7 @@ def perform; raise StandardError; end
     it "generates failure message with at least hint" do
       expect {
         expect { }.to have_enqueued_job.at_least(:once)
-      }.to raise_error(/expected to enqueue at least 1 jobs, but enqueued 0/)
+      }.to fail_with(/expected to enqueue at least 1 jobs, but enqueued 0/)
     end
 
     it "generates failure message with at most hint" do
@@ -233,7 +247,7 @@ def perform; raise StandardError; end
           hello_job.perform_later
           hello_job.perform_later
         }.to have_enqueued_job.at_most(:once)
-      }.to raise_error(/expected to enqueue at most 1 jobs, but enqueued 2/)
+      }.to fail_with(/expected to enqueue at most 1 jobs, but enqueued 2/)
     end
 
     it "passes with provided queue name as string" do
@@ -277,7 +291,7 @@ def perform; raise StandardError; end
       travel_to time do
         expect {
           expect { hello_job.set(wait: 5).perform_later }.to have_enqueued_job.at(time + 5)
-        }.to raise_error(/expected to enqueue exactly 1 jobs/)
+        }.to fail_with(/expected to enqueue exactly 1 jobs/)
       end
     end
 
@@ -301,7 +315,7 @@ def perform; raise StandardError; end
         expect {
           hello_job.perform_later
         }.to have_enqueued_job.at(date)
-      }.to raise_error(/expected to enqueue exactly 1 jobs, at .+ but enqueued 0/)
+      }.to fail_with(/expected to enqueue exactly 1 jobs, at .+ but enqueued 0/)
     end
 
     it "has an enqueued job when not providing at and there is a wait" do
@@ -324,6 +338,22 @@ def perform; raise StandardError; end
       }.to have_enqueued_job.with(42, "David")
     end
 
+    it "fails if the arguments do not match the job's signature" do
+      expect {
+        expect {
+          two_args_job.perform_later(1)
+        }.to have_enqueued_job.with(1)
+      }.to fail_with(/Incorrect arguments passed to TwoArgsJob: Wrong number of arguments/)
+    end
+
+    it "fails if the job's signature/arguments are mismatched keyword/positional arguments" do
+      expect {
+        expect {
+          keyword_args_job.perform_later(1, 2)
+        }.to have_enqueued_job.with(1, 2)
+      }.to fail_with(/Incorrect arguments passed to KeywordArgsJob: Missing required keyword arguments/)
+    end
+
     it "passes with provided arguments containing global id object" do
       global_id_object = GlobalIdModel.new("42")
 
@@ -348,7 +378,7 @@ def perform; raise StandardError; end
         expect {
           hello_job.perform_later(1)
         }.to have_enqueued_job(hello_job).with(42).on_queue("low").at(date).exactly(2).times
-      }.to raise_error(message)
+      }.to fail_with(message)
     end
 
     it "throws descriptive error when no test adapter set" do
@@ -454,15 +484,31 @@ def perform; raise StandardError; end
     it "fails when job is not enqueued" do
       expect {
         expect(heavy_lifting_job).to have_been_enqueued
-      }.to raise_error(/expected to enqueue exactly 1 jobs, but enqueued 0/)
+      }.to fail_with(/expected to enqueue exactly 1 jobs, but enqueued 0/)
+    end
+
+    it "fails if the arguments do not match the job's signature" do
+      two_args_job.perform_later(1)
+
+      expect {
+        expect(two_args_job).to have_been_enqueued.with(1)
+      }.to fail_with(/Incorrect arguments passed to TwoArgsJob: Wrong number of arguments/)
+    end
+
+    it "fails if the job's signature/arguments are mismatched keyword/positional arguments" do
+      keyword_args_job.perform_later(1, 2)
+
+      expect {
+        expect(keyword_args_job).to have_been_enqueued.with(1, 2)
+      }.to fail_with(/Incorrect arguments passed to KeywordArgsJob: Missing required keyword arguments/)
     end
 
     it "fails when negated and several jobs enqueued" do
       heavy_lifting_job.perform_later
       heavy_lifting_job.perform_later
       expect {
         expect(heavy_lifting_job).not_to have_been_enqueued
-      }.to raise_error(/expected not to enqueue at least 1 jobs, but enqueued 2/)
+      }.to fail_with(/expected not to enqueue at least 1 jobs, but enqueued 2/)
     end
 
     it "accepts composable matchers as an at date" do
@@ -511,7 +557,7 @@ def perform; raise StandardError; end
     it "fails when job is not performed" do
       expect {
         expect { }.to have_performed_job
-      }.to raise_error(/expected to perform exactly 1 jobs, but performed 0/)
+      }.to fail_with(/expected to perform exactly 1 jobs, but performed 0/)
     end
 
     it "fails when too many jobs performed" do
@@ -520,20 +566,20 @@ def perform; raise StandardError; end
           heavy_lifting_job.perform_later
           heavy_lifting_job.perform_later
         }.to have_performed_job.exactly(1)
-      }.to raise_error(/expected to perform exactly 1 jobs, but performed 2/)
+      }.to fail_with(/expected to perform exactly 1 jobs, but performed 2/)
     end
 
     it "reports correct number in fail error message" do
       heavy_lifting_job.perform_later
       expect {
         expect { }.to have_performed_job.exactly(1)
-      }.to raise_error(/expected to perform exactly 1 jobs, but performed 0/)
+      }.to fail_with(/expected to perform exactly 1 jobs, but performed 0/)
     end
 
     it "fails when negated and job is performed" do
       expect {
         expect { heavy_lifting_job.perform_later }.not_to have_performed_job
-      }.to raise_error(/expected not to perform exactly 1 jobs, but performed 1/)
+      }.to fail_with(/expected not to perform exactly 1 jobs, but performed 1/)
     end
 
     it "passes with job name" do
@@ -588,7 +634,7 @@ def perform; raise StandardError; end
     it "generates failure message with at least hint" do
       expect {
         expect { }.to have_performed_job.at_least(:once)
-      }.to raise_error(/expected to perform at least 1 jobs, but performed 0/)
+      }.to fail_with(/expected to perform at least 1 jobs, but performed 0/)
     end
 
     it "generates failure message with at most hint" do
@@ -597,7 +643,7 @@ def perform; raise StandardError; end
           hello_job.perform_later
           hello_job.perform_later
         }.to have_performed_job.at_most(:once)
-      }.to raise_error(/expected to perform at most 1 jobs, but performed 2/)
+      }.to fail_with(/expected to perform at most 1 jobs, but performed 2/)
     end
 
     it "passes with provided queue name as string" do
@@ -649,7 +695,7 @@ def perform; raise StandardError; end
         expect {
           hello_job.perform_later(1)
         }.to have_performed_job(hello_job).with(42).on_queue("low").at(date).exactly(2).times
-      }.to raise_error(message)
+      }.to fail_with(message)
     end
 
     it "throws descriptive error when no test adapter set" do
@@ -734,7 +780,7 @@ def perform; raise StandardError; end
     it "fails when job is not performed" do
       expect {
         expect(heavy_lifting_job).to have_been_performed
-      }.to raise_error(/expected to perform exactly 1 jobs, but performed 0/)
+      }.to fail_with(/expected to perform exactly 1 jobs, but performed 0/)
     end
   end