From a43702d3c8b89f4485e3a39f29b284c31678db17 Mon Sep 17 00:00:00 2001 From: Oli Peate Date: Fri, 25 Oct 2024 15:55:48 +0100 Subject: [PATCH 1/4] Group job/mailer signature check specs --- spec/rspec/rails/matchers/active_job_spec.rb | 48 ++++++++++--------- .../rails/matchers/have_enqueued_mail_spec.rb | 12 +++-- 2 files changed, 33 insertions(+), 27 deletions(-) diff --git a/spec/rspec/rails/matchers/active_job_spec.rb b/spec/rspec/rails/matchers/active_job_spec.rb index 86e191ae5..5c94efd0d 100644 --- a/spec/rspec/rails/matchers/active_job_spec.rb +++ b/spec/rspec/rails/matchers/active_job_spec.rb @@ -372,20 +372,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 { + describe "verifying the arguments passed match the job's signature" do + it "fails if there is an arity mismatch" do 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 + 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 { + it "fails if there is a keyword/positional arguments mismatch" do 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/) + 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 end it "passes with provided arguments containing global id object" do @@ -521,20 +523,22 @@ def perform; raise StandardError; end }.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) + describe "verifying the arguments passed match the job's signature" do + it "fails if there is an arity mismatch" 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 + 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) + it "fails if there is a keyword/positional arguments mismatch" 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/) + 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 end it "fails when negated and several jobs enqueued" do diff --git a/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb b/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb index 14b886189..71719bce2 100644 --- a/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb +++ b/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb @@ -251,12 +251,14 @@ def test_email; end }.not_to have_enqueued_mail(TestMailer, :email_with_args).with(3, 4) end - it "fails if the arguments do not match the mailer method's signature" do - expect { + describe "verifying the arguments passed match the mailer's signature" do + it "fails if there is a mismatch" do expect { - TestMailer.email_with_args(1).deliver_later - }.to have_enqueued_mail(TestMailer, :email_with_args).with(1) - }.to fail_with(/Incorrect arguments passed to TestMailer: Wrong number of arguments/) + expect { + TestMailer.email_with_args(1).deliver_later + }.to have_enqueued_mail(TestMailer, :email_with_args).with(1) + }.to fail_with(/Incorrect arguments passed to TestMailer: Wrong number of arguments/) + end end it "generates a failure message" do From 2f519d426243f0d2d52bf8df5c3cd18bdb65329e Mon Sep 17 00:00:00 2001 From: Oli Peate Date: Fri, 25 Oct 2024 16:17:34 +0100 Subject: [PATCH 2/4] Respect #verify_partial_doubles? config for job/mailer signature verification https://github.com/rspec/rspec-rails/issues/2801#issuecomment-2387109797 --- lib/rspec/rails/matchers/active_job.rb | 2 ++ .../rails/matchers/have_enqueued_mail.rb | 1 + spec/rspec/rails/matchers/active_job_spec.rb | 20 +++++++++++++++++++ .../rails/matchers/have_enqueued_mail_spec.rb | 13 ++++++++++++ spec/support/temporary_assignment.rb | 15 ++++++++++++++ 5 files changed, 51 insertions(+) create mode 100644 spec/support/temporary_assignment.rb diff --git a/lib/rspec/rails/matchers/active_job.rb b/lib/rspec/rails/matchers/active_job.rb index 40c676c0c..e61b28374 100644 --- a/lib/rspec/rails/matchers/active_job.rb +++ b/lib/rspec/rails/matchers/active_job.rb @@ -178,6 +178,8 @@ def arguments_match?(job) end def detect_args_signature_mismatch(jobs) + return unless RSpec::Mocks.configuration.verify_partial_doubles? + jobs.each do |job| args = deserialize_arguments(job) diff --git a/lib/rspec/rails/matchers/have_enqueued_mail.rb b/lib/rspec/rails/matchers/have_enqueued_mail.rb index 85aaea274..b5cab310c 100644 --- a/lib/rspec/rails/matchers/have_enqueued_mail.rb +++ b/lib/rspec/rails/matchers/have_enqueued_mail.rb @@ -93,6 +93,7 @@ def arguments_match?(job) def detect_args_signature_mismatch(jobs) return if @method_name.nil? + return unless RSpec::Mocks.configuration.verify_partial_doubles? mailer_class = mailer_class_name.constantize diff --git a/spec/rspec/rails/matchers/active_job_spec.rb b/spec/rspec/rails/matchers/active_job_spec.rb index 5c94efd0d..6ed7007b7 100644 --- a/spec/rspec/rails/matchers/active_job_spec.rb +++ b/spec/rspec/rails/matchers/active_job_spec.rb @@ -1,4 +1,5 @@ require "rspec/rails/feature_check" +require "support/temporary_assignment" if RSpec::Rails::FeatureCheck.has_active_job? require "rspec/rails/matchers/active_job" @@ -34,6 +35,7 @@ def self.find(_id) RSpec.describe "ActiveJob matchers", skip: !RSpec::Rails::FeatureCheck.has_active_job? do include ActiveSupport::Testing::TimeHelpers + include TemporaryAssignment around do |example| original_logger = ActiveJob::Base.logger @@ -388,6 +390,14 @@ def perform; raise StandardError; end }.to have_enqueued_job.with(1, 2) }.to fail_with(/Incorrect arguments passed to KeywordArgsJob: Missing required keyword arguments/) end + + context "with partial double verification disabled" do + it "skips signature checks" do + with_temporary_assignment(RSpec::Mocks.configuration, :verify_partial_doubles, false) { + expect { two_args_job.perform_later(1) }.to have_enqueued_job.with(1) + } + end + end end it "passes with provided arguments containing global id object" do @@ -539,6 +549,16 @@ def perform; raise StandardError; end expect(keyword_args_job).to have_been_enqueued.with(1, 2) }.to fail_with(/Incorrect arguments passed to KeywordArgsJob: Missing required keyword arguments/) end + + context "with partial double verification disabled" do + it "skips signature checks" do + keyword_args_job.perform_later(1, 2) + + with_temporary_assignment(RSpec::Mocks.configuration, :verify_partial_doubles, false) { + expect(keyword_args_job).to have_been_enqueued.with(1, 2) + } + end + end end it "fails when negated and several jobs enqueued" do diff --git a/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb b/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb index 71719bce2..0401c95ce 100644 --- a/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb +++ b/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb @@ -1,4 +1,5 @@ require "rspec/rails/feature_check" +require "support/temporary_assignment" if RSpec::Rails::FeatureCheck.has_active_job? require "action_mailer" @@ -49,6 +50,8 @@ def test_email; end end RSpec.describe "HaveEnqueuedMail matchers", skip: !RSpec::Rails::FeatureCheck.has_active_job? do + include TemporaryAssignment + before do ActiveJob::Base.queue_adapter = :test end @@ -259,6 +262,16 @@ def test_email; end }.to have_enqueued_mail(TestMailer, :email_with_args).with(1) }.to fail_with(/Incorrect arguments passed to TestMailer: Wrong number of arguments/) end + + context "with partial double verification disabled" do + it "skips signature checks" do + with_temporary_assignment(RSpec::Mocks.configuration, :verify_partial_doubles, false) { + expect { + TestMailer.email_with_args(1).deliver_later + }.to have_enqueued_mail(TestMailer, :email_with_args).with(1) + } + end + end end it "generates a failure message" do diff --git a/spec/support/temporary_assignment.rb b/spec/support/temporary_assignment.rb new file mode 100644 index 000000000..d07aadcad --- /dev/null +++ b/spec/support/temporary_assignment.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module TemporaryAssignment + def with_temporary_assignment(assignee, attribute, temporary_value) + predicate = "#{attribute}?" + attribute_reader = assignee.respond_to?(predicate) ? predicate : attribute + attribute_writer = "#{attribute}=" + + original_value = assignee.public_send(attribute_reader) + assignee.public_send(attribute_writer, temporary_value) + yield + ensure + assignee.public_send(attribute_writer, original_value) + end +end From 67a538fd307c4243642226b6792cf769f96a4fd0 Mon Sep 17 00:00:00 2001 From: Oli Peate Date: Fri, 25 Oct 2024 16:22:57 +0100 Subject: [PATCH 3/4] Respect #without_partial_double_verification block for job/mailer signature verification --- lib/rspec/rails/matchers/active_job.rb | 7 ++++++- .../rails/matchers/have_enqueued_mail.rb | 2 +- spec/rspec/rails/matchers/active_job_spec.rb | 20 +++++++++++++++++++ .../rails/matchers/have_enqueued_mail_spec.rb | 10 ++++++++++ 4 files changed, 37 insertions(+), 2 deletions(-) diff --git a/lib/rspec/rails/matchers/active_job.rb b/lib/rspec/rails/matchers/active_job.rb index e61b28374..1bdd323e5 100644 --- a/lib/rspec/rails/matchers/active_job.rb +++ b/lib/rspec/rails/matchers/active_job.rb @@ -178,7 +178,7 @@ def arguments_match?(job) end def detect_args_signature_mismatch(jobs) - return unless RSpec::Mocks.configuration.verify_partial_doubles? + return if skip_signature_verification? jobs.each do |job| args = deserialize_arguments(job) @@ -191,6 +191,11 @@ def detect_args_signature_mismatch(jobs) nil end + def skip_signature_verification? + !RSpec::Mocks.configuration.verify_partial_doubles? || + RSpec::Mocks.configuration.temporarily_suppress_partial_double_verification + 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) diff --git a/lib/rspec/rails/matchers/have_enqueued_mail.rb b/lib/rspec/rails/matchers/have_enqueued_mail.rb index b5cab310c..cf4576e57 100644 --- a/lib/rspec/rails/matchers/have_enqueued_mail.rb +++ b/lib/rspec/rails/matchers/have_enqueued_mail.rb @@ -93,7 +93,7 @@ def arguments_match?(job) def detect_args_signature_mismatch(jobs) return if @method_name.nil? - return unless RSpec::Mocks.configuration.verify_partial_doubles? + return if skip_signature_verification? mailer_class = mailer_class_name.constantize diff --git a/spec/rspec/rails/matchers/active_job_spec.rb b/spec/rspec/rails/matchers/active_job_spec.rb index 6ed7007b7..54f73de28 100644 --- a/spec/rspec/rails/matchers/active_job_spec.rb +++ b/spec/rspec/rails/matchers/active_job_spec.rb @@ -398,6 +398,16 @@ def perform; raise StandardError; end } end end + + context "when partial double verification is temporarily suspended" do + it "skips signature checks" do + without_partial_double_verification { + expect { + two_args_job.perform_later(1) + }.to have_enqueued_job.with(1) + } + end + end end it "passes with provided arguments containing global id object" do @@ -559,6 +569,16 @@ def perform; raise StandardError; end } end end + + context "when partial double verification is temporarily suspended" do + it "skips signature checks" do + keyword_args_job.perform_later(1, 2) + + without_partial_double_verification { + expect(keyword_args_job).to have_been_enqueued.with(1, 2) + } + end + end end it "fails when negated and several jobs enqueued" do diff --git a/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb b/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb index 0401c95ce..bc5da9b46 100644 --- a/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb +++ b/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb @@ -272,6 +272,16 @@ def test_email; end } end end + + context "when partial double verification is temporarily suspended" do + it "skips signature checks" do + without_partial_double_verification { + expect { + TestMailer.email_with_args(1).deliver_later + }.to have_enqueued_mail(TestMailer, :email_with_args).with(1) + } + end + end end it "generates a failure message" do From 19baec8921527cb3f240946dfd8d3d84997e9cbb Mon Sep 17 00:00:00 2001 From: Oli Peate Date: Mon, 4 Nov 2024 10:00:54 +0000 Subject: [PATCH 4/4] Use around/before hooks to edit verify partial doubles config https://github.com/rspec/rspec-rails/pull/2808#pullrequestreview-2411993274 --- spec/rspec/rails/matchers/active_job_spec.rb | 25 +++++++++++++------ .../rails/matchers/have_enqueued_mail_spec.rb | 21 ++++++++++------ spec/support/temporary_assignment.rb | 15 ----------- 3 files changed, 30 insertions(+), 31 deletions(-) delete mode 100644 spec/support/temporary_assignment.rb diff --git a/spec/rspec/rails/matchers/active_job_spec.rb b/spec/rspec/rails/matchers/active_job_spec.rb index 54f73de28..5b299a28b 100644 --- a/spec/rspec/rails/matchers/active_job_spec.rb +++ b/spec/rspec/rails/matchers/active_job_spec.rb @@ -1,5 +1,4 @@ require "rspec/rails/feature_check" -require "support/temporary_assignment" if RSpec::Rails::FeatureCheck.has_active_job? require "rspec/rails/matchers/active_job" @@ -35,7 +34,6 @@ def self.find(_id) RSpec.describe "ActiveJob matchers", skip: !RSpec::Rails::FeatureCheck.has_active_job? do include ActiveSupport::Testing::TimeHelpers - include TemporaryAssignment around do |example| original_logger = ActiveJob::Base.logger @@ -44,6 +42,13 @@ def self.find(_id) ActiveJob::Base.logger = original_logger end + around do |example| + original_value = RSpec::Mocks.configuration.verify_partial_doubles? + example.run + ensure + RSpec::Mocks.configuration.verify_partial_doubles = original_value + end + let(:heavy_lifting_job) do Class.new(ActiveJob::Base) do def perform; end @@ -392,10 +397,12 @@ def perform; raise StandardError; end end context "with partial double verification disabled" do + before do + RSpec::Mocks.configuration.verify_partial_doubles = false + end + it "skips signature checks" do - with_temporary_assignment(RSpec::Mocks.configuration, :verify_partial_doubles, false) { - expect { two_args_job.perform_later(1) }.to have_enqueued_job.with(1) - } + expect { two_args_job.perform_later(1) }.to have_enqueued_job.with(1) end end @@ -561,12 +568,14 @@ def perform; raise StandardError; end end context "with partial double verification disabled" do + before do + RSpec::Mocks.configuration.verify_partial_doubles = false + end + it "skips signature checks" do keyword_args_job.perform_later(1, 2) - with_temporary_assignment(RSpec::Mocks.configuration, :verify_partial_doubles, false) { - expect(keyword_args_job).to have_been_enqueued.with(1, 2) - } + expect(keyword_args_job).to have_been_enqueued.with(1, 2) end end diff --git a/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb b/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb index bc5da9b46..d2cc57ba4 100644 --- a/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb +++ b/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb @@ -1,5 +1,4 @@ require "rspec/rails/feature_check" -require "support/temporary_assignment" if RSpec::Rails::FeatureCheck.has_active_job? require "action_mailer" @@ -50,8 +49,6 @@ def test_email; end end RSpec.describe "HaveEnqueuedMail matchers", skip: !RSpec::Rails::FeatureCheck.has_active_job? do - include TemporaryAssignment - before do ActiveJob::Base.queue_adapter = :test end @@ -63,6 +60,13 @@ def test_email; end ActiveJob::Base.logger = original_logger end + around do |example| + original_value = RSpec::Mocks.configuration.verify_partial_doubles? + example.run + ensure + RSpec::Mocks.configuration.verify_partial_doubles = original_value + end + describe "have_enqueued_mail" do it "passes when a mailer method is called with deliver_later" do expect { @@ -264,12 +268,13 @@ def test_email; end end context "with partial double verification disabled" do + before do + RSpec::Mocks.configuration.verify_partial_doubles = false + end + it "skips signature checks" do - with_temporary_assignment(RSpec::Mocks.configuration, :verify_partial_doubles, false) { - expect { - TestMailer.email_with_args(1).deliver_later - }.to have_enqueued_mail(TestMailer, :email_with_args).with(1) - } + expect { TestMailer.email_with_args(1).deliver_later } + .to have_enqueued_mail(TestMailer, :email_with_args).with(1) end end diff --git a/spec/support/temporary_assignment.rb b/spec/support/temporary_assignment.rb deleted file mode 100644 index d07aadcad..000000000 --- a/spec/support/temporary_assignment.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -module TemporaryAssignment - def with_temporary_assignment(assignee, attribute, temporary_value) - predicate = "#{attribute}?" - attribute_reader = assignee.respond_to?(predicate) ? predicate : attribute - attribute_writer = "#{attribute}=" - - original_value = assignee.public_send(attribute_reader) - assignee.public_send(attribute_writer, temporary_value) - yield - ensure - assignee.public_send(attribute_writer, original_value) - end -end