From 50e2c7260ebe78e7ff765e5213f24068a2c40c28 Mon Sep 17 00:00:00 2001 From: Fabio Napoleoni Date: Fri, 10 Sep 2021 15:17:16 +0200 Subject: [PATCH 01/10] Change HaveEnqueuedEmail to work as usual in Rails 6 --- lib/rspec/rails/matchers/have_enqueued_mail.rb | 15 +++++++++++++-- .../rails/matchers/have_enqueued_mail_spec.rb | 18 +++++++++++------- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/lib/rspec/rails/matchers/have_enqueued_mail.rb b/lib/rspec/rails/matchers/have_enqueued_mail.rb index b7756346e9..e10e474848 100644 --- a/lib/rspec/rails/matchers/have_enqueued_mail.rb +++ b/lib/rspec/rails/matchers/have_enqueued_mail.rb @@ -76,7 +76,7 @@ def job_match?(job) def arguments_match?(job) @args = if @mail_args.any? - base_mailer_args + @mail_args + base_mailer_args + process_arguments(job, @mail_args) elsif @mailer_class && @method_name base_mailer_args + [any_args] elsif @mailer_class @@ -88,6 +88,18 @@ def arguments_match?(job) super(job) end + def process_arguments(job, given_mail_args) + if job[:job] == ActionMailer::MailDeliveryJob + if given_mail_args.first.is_a?(Hash) && job[:args][3]['params'].present? + [hash_including(params: given_mail_args[0], args: given_mail_args.drop(1))] + else + [hash_including(args: given_mail_args)] + end + else + given_mail_args + end + end + def base_mailer_args [mailer_class_name, @method_name.to_s, MAILER_JOB_METHOD] end @@ -120,7 +132,6 @@ def unmatching_mail_jobs_message def mail_job_message(job) mailer_method = job[:args][0..1].join('.') - mailer_args = job[:args][3..-1] msg_parts = [] msg_parts << "with #{mailer_args}" if mailer_args.any? diff --git a/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb b/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb index 152e02fffc..119779352e 100644 --- a/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb +++ b/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb @@ -393,18 +393,22 @@ def self.name; "NonMailerJob"; end }.to have_enqueued_mail(UnifiedMailer, :test_email).and have_enqueued_mail(UnifiedMailer, :email_with_args) end - it "passes with provided argument matchers" do + it "matches arguments when mailer has only args" do + expect { + UnifiedMailer.email_with_args(1, 2).deliver_later + }.to have_enqueued_mail(UnifiedMailer, :email_with_args).with(1, 2) + end + + it "matches arguments when mailer is parameterized" do expect { UnifiedMailer.with('foo' => 'bar').test_email.deliver_later - }.to have_enqueued_mail(UnifiedMailer, :test_email).with( - a_hash_including(params: {'foo' => 'bar'}) - ) + }.to have_enqueued_mail(UnifiedMailer, :test_email).with('foo' => 'bar') + end + it "matches arguments when mixing parameterized and non-parameterized emails" do expect { UnifiedMailer.with('foo' => 'bar').email_with_args(1, 2).deliver_later - }.to have_enqueued_mail(UnifiedMailer, :email_with_args).with( - a_hash_including(params: {'foo' => 'bar'}, args: [1, 2]) - ) + }.to have_enqueued_mail(UnifiedMailer, :email_with_args).with({'foo' => 'bar'}, 1, 2) end it "passes when using a mailer with `delivery_job` set to a sub class of `ActionMailer::DeliveryJob`" do From 7adcef22b5634e66919bead2f9fa9e5c494e07ba Mon Sep 17 00:00:00 2001 From: Fabio Napoleoni Date: Fri, 10 Sep 2021 15:22:09 +0200 Subject: [PATCH 02/10] Add missing relish documentation for parameter matching --- .../have_enqueued_mail_matcher.feature | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/features/matchers/have_enqueued_mail_matcher.feature b/features/matchers/have_enqueued_mail_matcher.feature index ab1ae50210..b05fe78e45 100644 --- a/features/matchers/have_enqueued_mail_matcher.feature +++ b/features/matchers/have_enqueued_mail_matcher.feature @@ -38,3 +38,40 @@ Feature: have_enqueued_mail matcher """ When I run `rspec spec/mailers/user_mailer_spec.rb` Then the examples should all pass + + Scenario: Checking mailer arguments + Given a file named "app/mailers/my_mailer.rb" with: + """ruby + class MyMailer < ApplicationMailer + + def signup(user = nil) + @user = user + + mail to: "to@example.org" + end + end + """ + Given a file named "spec/mailers/my_mailer_spec.rb" with: + """ruby + require "rails_helper" + + RSpec.describe MyMailer do + it "matches with enqueued mailer" do + ActiveJob::Base.queue_adapter = :test + # Works with plain args + expect { + MyMailer.signup('user').deliver_later + }.to have_enqueued_mail(MyMailer, :signup).with('user') + # Works with named parameters + expect { + MyMailer.with('foo' => 'bar').signup.deliver_later + }.to have_enqueued_mail(MyMailer, :signup).with('foo' => 'bar') + # Works also with both, named parameters match first argument + expect { + MyMailer.with('foo' => 'bar').signup('user').deliver_later + }.to have_enqueued_mail(MyMailer, :signup).with({'foo' => 'bar'}, 'user') + end + end + """ + When I run `rspec spec/mailers/my_mailer_spec.rb` + Then the examples should all pass From 1092437da18ba8502e0c5e52937249910e081e0f Mon Sep 17 00:00:00 2001 From: Fabio Napoleoni Date: Sat, 11 Sep 2021 16:29:07 +0200 Subject: [PATCH 03/10] Ensure ActionMailer::MailDeliveryJob is defined --- lib/rspec/rails/matchers/have_enqueued_mail.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/rspec/rails/matchers/have_enqueued_mail.rb b/lib/rspec/rails/matchers/have_enqueued_mail.rb index e10e474848..af71a6671e 100644 --- a/lib/rspec/rails/matchers/have_enqueued_mail.rb +++ b/lib/rspec/rails/matchers/have_enqueued_mail.rb @@ -89,14 +89,14 @@ def arguments_match?(job) end def process_arguments(job, given_mail_args) - if job[:job] == ActionMailer::MailDeliveryJob - if given_mail_args.first.is_a?(Hash) && job[:args][3]['params'].present? - [hash_including(params: given_mail_args[0], args: given_mail_args.drop(1))] - else - [hash_including(args: given_mail_args)] - end + # Old matcher behavior working with all builtin classes but ActionMailer::MailDeliveryJob + return given_mail_args unless defined?(ActionMailer::MailDeliveryJob) && job[:job] <= ActionMailer::MailDeliveryJob + + # If matching args starts with a hash and job instance has params match with them + if given_mail_args.first.is_a?(Hash) && job[:args][3]['params'].present? + [hash_including(params: given_mail_args[0], args: given_mail_args.drop(1))] else - given_mail_args + [hash_including(args: given_mail_args)] end end From 54d969226d1fad21201624859000872afa61de17 Mon Sep 17 00:00:00 2001 From: Fabio Napoleoni Date: Sat, 11 Sep 2021 16:30:23 +0200 Subject: [PATCH 04/10] Silence rubocop for HaveEnqueuedMail class length --- lib/rspec/rails/matchers/have_enqueued_mail.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/rspec/rails/matchers/have_enqueued_mail.rb b/lib/rspec/rails/matchers/have_enqueued_mail.rb index af71a6671e..8f12876bc9 100644 --- a/lib/rspec/rails/matchers/have_enqueued_mail.rb +++ b/lib/rspec/rails/matchers/have_enqueued_mail.rb @@ -7,6 +7,7 @@ module RSpec module Rails module Matchers + # rubocop: disable Metrics/ClassLength # Matcher class for `have_enqueued_mail`. Should not be instantiated directly. # # @private @@ -153,6 +154,8 @@ def unified_mail?(job) RSpec::Rails::FeatureCheck.has_action_mailer_unified_delivery? && job[:job] <= ActionMailer::MailDeliveryJob end end + # rubocop: enable Metrics/ClassLength + # @api public # Passes if an email has been enqueued inside block. # May chain with to specify expected arguments. From 84ac4f8ace86232a549fd46572ded7ca8d80ba65 Mon Sep 17 00:00:00 2001 From: Fabio Napoleoni Date: Mon, 22 Nov 2021 15:13:22 +0100 Subject: [PATCH 05/10] Split all syntax in different examples --- .../have_enqueued_mail_matcher.feature | 56 ++++++++++++++++++- 1 file changed, 53 insertions(+), 3 deletions(-) diff --git a/features/matchers/have_enqueued_mail_matcher.feature b/features/matchers/have_enqueued_mail_matcher.feature index b05fe78e45..2091920a4b 100644 --- a/features/matchers/have_enqueued_mail_matcher.feature +++ b/features/matchers/have_enqueued_mail_matcher.feature @@ -62,14 +62,64 @@ Feature: have_enqueued_mail matcher expect { MyMailer.signup('user').deliver_later }.to have_enqueued_mail(MyMailer, :signup).with('user') + end + end + """ + When I run `rspec spec/mailers/my_mailer_spec.rb` + Then the examples should all pass + + Scenario: Parameterize the mailer + Given a file named "app/mailers/my_mailer.rb" with: + """ruby + class MyMailer < ApplicationMailer + + def signup(user = nil) + @user = user + + mail to: "to@example.org" + end + end + """ + Given a file named "spec/mailers/my_mailer_spec.rb" with: + """ruby + require "rails_helper" + + RSpec.describe MyMailer do + it "matches with enqueued mailer" do + ActiveJob::Base.queue_adapter = :test # Works with named parameters expect { - MyMailer.with('foo' => 'bar').signup.deliver_later - }.to have_enqueued_mail(MyMailer, :signup).with('foo' => 'bar') + MyMailer.with(foo: 'bar').signup.deliver_later + }.to have_enqueued_mail(MyMailer, :signup).with(foo: 'bar') + end + end + """ + When I run `rspec spec/mailers/my_mailer_spec.rb` + Then the examples should all pass + + Scenario: Parameterize and pass an argument to the mailer + Given a file named "app/mailers/my_mailer.rb" with: + """ruby + class MyMailer < ApplicationMailer + + def signup(user = nil) + @user = user + + mail to: "to@example.org" + end + end + """ + Given a file named "spec/mailers/my_mailer_spec.rb" with: + """ruby + require "rails_helper" + + RSpec.describe MyMailer do + it "matches with enqueued mailer" do + ActiveJob::Base.queue_adapter = :test # Works also with both, named parameters match first argument expect { MyMailer.with('foo' => 'bar').signup('user').deliver_later - }.to have_enqueued_mail(MyMailer, :signup).with({'foo' => 'bar'}, 'user') + }.to have_enqueued_mail(MyMailer, :signup).with({foo: 'bar'}, 'user') end end """ From 08d92ceb91c61bfd8cc7f932df3b31411eca9dd0 Mon Sep 17 00:00:00 2001 From: Fabio Napoleoni Date: Tue, 23 Nov 2021 10:19:23 +0100 Subject: [PATCH 06/10] Clarify examples with better params usage --- features/matchers/have_enqueued_mail_matcher.feature | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/features/matchers/have_enqueued_mail_matcher.feature b/features/matchers/have_enqueued_mail_matcher.feature index 2091920a4b..8def19cf3c 100644 --- a/features/matchers/have_enqueued_mail_matcher.feature +++ b/features/matchers/have_enqueued_mail_matcher.feature @@ -73,8 +73,8 @@ Feature: have_enqueued_mail matcher """ruby class MyMailer < ApplicationMailer - def signup(user = nil) - @user = user + def signup + @foo = params[:foo] mail to: "to@example.org" end @@ -102,8 +102,9 @@ Feature: have_enqueued_mail matcher """ruby class MyMailer < ApplicationMailer - def signup(user = nil) + def signup(user) @user = user + @foo = params[:foo] mail to: "to@example.org" end @@ -118,7 +119,7 @@ Feature: have_enqueued_mail matcher ActiveJob::Base.queue_adapter = :test # Works also with both, named parameters match first argument expect { - MyMailer.with('foo' => 'bar').signup('user').deliver_later + MyMailer.with(foo: 'bar').signup('user').deliver_later }.to have_enqueued_mail(MyMailer, :signup).with({foo: 'bar'}, 'user') end end From ed71aedc9fda010341b3cc296e075eff94496f63 Mon Sep 17 00:00:00 2001 From: mhenrixon Date: Tue, 23 Nov 2021 20:59:48 +0100 Subject: [PATCH 07/10] Add compatibility for have_enqueued_mail (rails 7) 1. ActionMailer::DeliveryJob does not exist anymore 2. ActionMailer::Parameterized::DeliveryJob According to my research they have both been superseeded by ActionMailer::MailDeliveryJob Closes #2531 --- lib/rspec/rails/feature_check.rb | 6 +++++- lib/rspec/rails/matchers/have_enqueued_mail.rb | 2 +- spec/rspec/rails/matchers/have_enqueued_mail_spec.rb | 7 ++++++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/lib/rspec/rails/feature_check.rb b/lib/rspec/rails/feature_check.rb index 2570d6659b..4a4d855d75 100644 --- a/lib/rspec/rails/feature_check.rb +++ b/lib/rspec/rails/feature_check.rb @@ -28,13 +28,17 @@ def has_action_cable_testing? end def has_action_mailer_parameterized? - has_action_mailer? && defined?(::ActionMailer::Parameterized) + has_action_mailer? && defined?(::ActionMailer::Parameterized::DeliveryJob) end def has_action_mailer_unified_delivery? has_action_mailer? && defined?(::ActionMailer::MailDeliveryJob) end + def has_action_mailer_legacy_delivery_job? + defined?(ActionMailer::DeliveryJob) + end + def has_action_mailbox? defined?(::ActionMailbox) end diff --git a/lib/rspec/rails/matchers/have_enqueued_mail.rb b/lib/rspec/rails/matchers/have_enqueued_mail.rb index 8f12876bc9..d887388a5a 100644 --- a/lib/rspec/rails/matchers/have_enqueued_mail.rb +++ b/lib/rspec/rails/matchers/have_enqueued_mail.rb @@ -143,7 +143,7 @@ def mail_job_message(job) end def legacy_mail?(job) - job[:job] <= ActionMailer::DeliveryJob + RSpec::Rails::FeatureCheck.has_action_mailer_legacy_delivery_job? && job[:job] <= ActionMailer::DeliveryJob end def parameterized_mail?(job) diff --git a/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb b/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb index 119779352e..9413be1aa0 100644 --- a/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb +++ b/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb @@ -22,7 +22,12 @@ def test_email; end def email_with_args(arg1, arg2); end end - class DeliveryJobSubClass < ActionMailer::DeliveryJob + if RSpec::Rails::FeatureCheck.has_action_mailer_legacy_delivery_job? + class DeliveryJobSubClass < ActionMailer::DeliveryJob + end + else + class DeliveryJobSubClass < ActionMailer::MailDeliveryJob + end end class UnifiedMailerWithDeliveryJobSubClass < ActionMailer::Base From 1dbacd7e80c6a1e047db7c69b33b500007791f43 Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Tue, 21 Dec 2021 01:12:16 +0300 Subject: [PATCH 08/10] Fix unified mailer args --- lib/rspec/rails/matchers/have_enqueued_mail.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/rspec/rails/matchers/have_enqueued_mail.rb b/lib/rspec/rails/matchers/have_enqueued_mail.rb index d887388a5a..92d7f1e9fb 100644 --- a/lib/rspec/rails/matchers/have_enqueued_mail.rb +++ b/lib/rspec/rails/matchers/have_enqueued_mail.rb @@ -106,7 +106,14 @@ def base_mailer_args end def yield_mail_args(block) - proc { |*job_args| block.call(*(job_args - base_mailer_args)) } + proc do |*job_args| + mailer_args = job_args - base_mailer_args + if mailer_args.first.is_a?(Hash) + block.call(*mailer_args.first[:args]) + else + block.call(*mailer_args) + end + end end def check_active_job_adapter @@ -133,7 +140,8 @@ def unmatching_mail_jobs_message def mail_job_message(job) mailer_method = job[:args][0..1].join('.') - mailer_args = job[:args][3..-1] + mailer_args = deserialize_arguments(job)[3..-1] + mailer_args = mailer_args.first[:args] if unified_mail?(job) msg_parts = [] msg_parts << "with #{mailer_args}" if mailer_args.any? msg_parts << "on queue #{job[:queue]}" if job[:queue] && job[:queue] != 'mailers' From a9cbac6c4949c40834ac5a13cca8c25d2f08e3cf Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Tue, 21 Dec 2021 01:58:37 +0300 Subject: [PATCH 09/10] Fix system spec spec on Rails 7 ActionPack is checking `instance_created?` now, and it was `false`, so `take_screenshot` was not called. ``` def take_failed_screenshot take_screenshot if failed? && supports_screenshot? && Capybara::Session.instance_created? end ``` https://github.com/rails/rails/commit/1dfcffb583a89e1ef064db8dc6a45deaf157b147 --- spec/rspec/rails/example/system_example_group_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/rspec/rails/example/system_example_group_spec.rb b/spec/rspec/rails/example/system_example_group_spec.rb index cd5328e2bf..cdc135946a 100644 --- a/spec/rspec/rails/example/system_example_group_spec.rb +++ b/spec/rspec/rails/example/system_example_group_spec.rb @@ -72,6 +72,7 @@ module RSpec::Rails describe '#after' do it 'sets the :extra_failure_lines metadata to an array of STDOUT lines' do + allow(Capybara::Session).to receive(:instance_created?).and_return(true) group = RSpec::Core::ExampleGroup.describe do include SystemExampleGroup From dd48c6b970fba00055783563b959fe4fb0f3ab8f Mon Sep 17 00:00:00 2001 From: Phil Pirozhkov Date: Mon, 10 Jan 2022 22:43:07 +0300 Subject: [PATCH 10/10] Add Changelog entry --- Changelog.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Changelog.md b/Changelog.md index 8f4e7bd6c9..ec662e3709 100644 --- a/Changelog.md +++ b/Changelog.md @@ -12,6 +12,9 @@ Bug Fixes: * Properly name params in controller and request spec templates when using the `--model-name` parameter. (@kenzo-tanaka, #2534) +* Fix parameter matching with mail delivery job and + ActionMailer::MailDeliveryJob. (Fabio Napoleoni, #2516, #2546) +* Fix Rails 7 `have_enqueued_mail` compatibility (Mikael Henriksson, #2537, #2546) ### 5.0.2 / 2021-08-14 [Full Changelog](https://github.com/rspec/rspec-rails/compare/v5.0.1...v5.0.2)