From 64f9939e397c6b0b0dbcbc0a29380b544de91329 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Mon, 19 Feb 2024 10:57:47 -0500 Subject: [PATCH] Use `capture_emails` helper to improve email assertions in specs (#29245) --- .../admin/disputes/appeals_controller_spec.rb | 25 +++++++++----- .../admin/resets_controller_spec.rb | 10 +++--- .../auth/sessions_controller_spec.rb | 34 ++++++++++++------- .../disputes/appeals_controller_spec.rb | 14 +++++--- spec/models/user_spec.rb | 18 +++++----- spec/rails_helper.rb | 1 + spec/requests/api/v1/reports_spec.rb | 10 ++++-- spec/services/notify_service_spec.rb | 16 ++++++--- spec/services/report_service_spec.rb | 5 +-- spec/workers/backup_worker_spec.rb | 11 ++++-- 10 files changed, 97 insertions(+), 47 deletions(-) diff --git a/spec/controllers/admin/disputes/appeals_controller_spec.rb b/spec/controllers/admin/disputes/appeals_controller_spec.rb index d365233167..bf7f9bd704 100644 --- a/spec/controllers/admin/disputes/appeals_controller_spec.rb +++ b/spec/controllers/admin/disputes/appeals_controller_spec.rb @@ -35,7 +35,7 @@ RSpec.describe Admin::Disputes::AppealsController do let(:current_user) { Fabricate(:user, role: UserRole.find_by(name: 'Admin')) } it 'redirects back to the strike page and notifies target account about approved appeal', :sidekiq_inline do - subject + emails = capture_emails { subject } expect(response) .to redirect_to(disputes_strike_path(appeal.strike)) @@ -43,9 +43,13 @@ RSpec.describe Admin::Disputes::AppealsController do expect(target_account.reload) .to_not be_suspended - expect(UserMailer.deliveries.size).to eq(1) - expect(UserMailer.deliveries.first.to.first).to eq(target_account.user.email) - expect(UserMailer.deliveries.first.subject).to eq(I18n.t('user_mailer.appeal_approved.subject', date: I18n.l(appeal.created_at))) + expect(emails.size) + .to eq(1) + expect(emails.first) + .to have_attributes( + to: contain_exactly(target_account.user.email), + subject: eq(I18n.t('user_mailer.appeal_approved.subject', date: I18n.l(appeal.created_at))) + ) end end @@ -55,14 +59,19 @@ RSpec.describe Admin::Disputes::AppealsController do let(:current_user) { Fabricate(:user, role: UserRole.find_by(name: 'Admin')) } it 'redirects back to the strike page and notifies target account about rejected appeal', :sidekiq_inline do - subject + emails = capture_emails { subject } expect(response) .to redirect_to(disputes_strike_path(appeal.strike)) - expect(UserMailer.deliveries.size).to eq(1) - expect(UserMailer.deliveries.first.to.first).to eq(target_account.user.email) - expect(UserMailer.deliveries.first.subject).to eq(I18n.t('user_mailer.appeal_rejected.subject', date: I18n.l(appeal.created_at))) + expect(emails.size) + .to eq(1) + + expect(emails.first) + .to have_attributes( + to: contain_exactly(target_account.user.email), + subject: eq(I18n.t('user_mailer.appeal_rejected.subject', date: I18n.l(appeal.created_at))) + ) end end end diff --git a/spec/controllers/admin/resets_controller_spec.rb b/spec/controllers/admin/resets_controller_spec.rb index e82a3a6afa..10ed2cf969 100644 --- a/spec/controllers/admin/resets_controller_spec.rb +++ b/spec/controllers/admin/resets_controller_spec.rb @@ -5,6 +5,8 @@ require 'rails_helper' describe Admin::ResetsController do render_views + subject { post :create, params: { account_id: account.id } } + let(:account) { Fabricate(:account) } before do @@ -13,11 +15,11 @@ describe Admin::ResetsController do describe 'POST #create', :sidekiq_inline do it 'redirects to admin accounts page' do - expect do - post :create, params: { account_id: account.id } - end.to change(Devise.mailer.deliveries, :size).by(2) + emails = capture_emails { subject } - expect(Devise.mailer.deliveries).to have_attributes( + expect(emails.size) + .to eq(2) + expect(emails).to have_attributes( first: have_attributes( to: include(account.user.email), subject: I18n.t('devise.mailer.password_change.subject') diff --git a/spec/controllers/auth/sessions_controller_spec.rb b/spec/controllers/auth/sessions_controller_spec.rb index dcbaf1fcbb..7adafc6f14 100644 --- a/spec/controllers/auth/sessions_controller_spec.rb +++ b/spec/controllers/auth/sessions_controller_spec.rb @@ -124,7 +124,7 @@ RSpec.describe Auth::SessionsController do end it 'logs the user in and sends suspicious email and redirects home', :sidekiq_inline do - subject + emails = capture_emails { subject } expect(response) .to redirect_to(root_path) @@ -132,9 +132,13 @@ RSpec.describe Auth::SessionsController do expect(controller.current_user) .to eq user - expect(UserMailer.deliveries.size).to eq(1) - expect(UserMailer.deliveries.first.to.first).to eq(user.email) - expect(UserMailer.deliveries.first.subject).to eq(I18n.t('user_mailer.suspicious_sign_in.subject')) + expect(emails.size) + .to eq(1) + expect(emails.first) + .to have_attributes( + to: contain_exactly(user.email), + subject: eq(I18n.t('user_mailer.suspicious_sign_in.subject')) + ) end end @@ -260,21 +264,27 @@ RSpec.describe Auth::SessionsController do end it 'does not log the user in, sets a flash message, and sends a suspicious sign in email', :sidekiq_inline do - Auth::SessionsController::MAX_2FA_ATTEMPTS_PER_HOUR.times do - post :create, params: { user: { otp_attempt: '1234' } }, session: { attempt_user_id: user.id, attempt_user_updated_at: user.updated_at.to_s } - expect(controller.current_user).to be_nil + emails = capture_emails do + Auth::SessionsController::MAX_2FA_ATTEMPTS_PER_HOUR.times do + post :create, params: { user: { otp_attempt: '1234' } }, session: { attempt_user_id: user.id, attempt_user_updated_at: user.updated_at.to_s } + expect(controller.current_user).to be_nil + end + post :create, params: { user: { otp_attempt: user.current_otp } }, session: { attempt_user_id: user.id, attempt_user_updated_at: user.updated_at.to_s } end - post :create, params: { user: { otp_attempt: user.current_otp } }, session: { attempt_user_id: user.id, attempt_user_updated_at: user.updated_at.to_s } - expect(controller.current_user) .to be_nil + expect(flash[:alert]) .to match I18n.t('users.rate_limited') - expect(UserMailer.deliveries.size).to eq(1) - expect(UserMailer.deliveries.first.to.first).to eq(user.email) - expect(UserMailer.deliveries.first.subject).to eq(I18n.t('user_mailer.failed_2fa.subject')) + expect(emails.size) + .to eq(1) + expect(emails.first) + .to have_attributes( + to: contain_exactly(user.email), + subject: eq(I18n.t('user_mailer.failed_2fa.subject')) + ) end end diff --git a/spec/controllers/disputes/appeals_controller_spec.rb b/spec/controllers/disputes/appeals_controller_spec.rb index d763068ebe..99d5a8b17f 100644 --- a/spec/controllers/disputes/appeals_controller_spec.rb +++ b/spec/controllers/disputes/appeals_controller_spec.rb @@ -18,9 +18,15 @@ RSpec.describe Disputes::AppealsController do let(:params) { { strike_id: strike.id, appeal: { text: 'Foo' } } } it 'notifies staff about new appeal and redirects back to strike page', :sidekiq_inline do - subject + emails = capture_emails { subject } - expect(ActionMailer::Base.deliveries.first.to).to eq([admin.email]) + expect(emails.size) + .to eq(1) + expect(emails.first) + .to have_attributes( + to: contain_exactly(admin.email), + subject: eq(I18n.t('admin_mailer.new_appeal.subject', username: current_user.account.acct, instance: Rails.configuration.x.local_domain)) + ) expect(response).to redirect_to(disputes_strike_path(strike.id)) end end @@ -31,9 +37,9 @@ RSpec.describe Disputes::AppealsController do let(:params) { { strike_id: strike.id, appeal: { text: '' } } } it 'does not send email and renders strike show page', :sidekiq_inline do - subject + emails = capture_emails { subject } - expect(ActionMailer::Base.deliveries.size).to eq(0) + expect(emails).to be_empty expect(response).to render_template('disputes/strikes/show') end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 1baa3ccbf9..39986f476c 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -455,18 +455,20 @@ RSpec.describe User do let!(:user) { Fabricate(:user, confirmed_at: confirmed_at) } - before { ActionMailer::Base.deliveries.clear } - - after { ActionMailer::Base.deliveries.clear } - context 'when user is new' do let(:confirmed_at) { nil } it 'confirms user and delivers welcome email', :sidekiq_inline do - subject + emails = capture_emails { subject } expect(user.confirmed_at).to be_present - expect(ActionMailer::Base.deliveries.count).to eq 1 + expect(emails.size) + .to eq(1) + expect(emails.first) + .to have_attributes( + to: contain_exactly(user.email), + subject: eq(I18n.t('user_mailer.welcome.subject')) + ) end end @@ -474,10 +476,10 @@ RSpec.describe User do let(:confirmed_at) { Time.zone.now } it 'confirms user but does not deliver welcome email' do - subject + emails = capture_emails { subject } expect(user.confirmed_at).to be_present - expect(ActionMailer::Base.deliveries.count).to eq 0 + expect(emails).to be_empty end end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index cde5a439db..3e84d68738 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -82,6 +82,7 @@ RSpec.configure do |config| config.include Devise::Test::ControllerHelpers, type: :view config.include Devise::Test::IntegrationHelpers, type: :feature config.include Devise::Test::IntegrationHelpers, type: :request + config.include ActionMailer::TestHelper config.include Paperclip::Shoulda::Matchers config.include ActiveSupport::Testing::TimeHelpers config.include Chewy::Rspec::Helpers diff --git a/spec/requests/api/v1/reports_spec.rb b/spec/requests/api/v1/reports_spec.rb index ba3d2b3060..94baf8cb98 100644 --- a/spec/requests/api/v1/reports_spec.rb +++ b/spec/requests/api/v1/reports_spec.rb @@ -35,7 +35,7 @@ RSpec.describe 'Reports' do it 'creates a report', :aggregate_failures do perform_enqueued_jobs do - subject + emails = capture_emails { subject } expect(response).to have_http_status(200) expect(body_as_json).to match( @@ -49,7 +49,13 @@ RSpec.describe 'Reports' do expect(target_account.targeted_reports).to_not be_empty expect(target_account.targeted_reports.first.comment).to eq 'reasons' - expect(ActionMailer::Base.deliveries.first.to).to eq([admin.email]) + expect(emails.size) + .to eq(1) + expect(emails.first) + .to have_attributes( + to: contain_exactly(admin.email), + subject: eq(I18n.t('admin_mailer.new_report.subject', instance: Rails.configuration.x.local_domain, id: target_account.targeted_reports.first.id)) + ) end end diff --git a/spec/services/notify_service_spec.rb b/spec/services/notify_service_spec.rb index 38a33c522d..e818fadcbe 100644 --- a/spec/services/notify_service_spec.rb +++ b/spec/services/notify_service_spec.rb @@ -157,8 +157,6 @@ RSpec.describe NotifyService, type: :service do describe 'email' do before do - ActionMailer::Base.deliveries.clear - user.settings.update('notification_emails.follow': enabled) user.save end @@ -167,7 +165,15 @@ RSpec.describe NotifyService, type: :service do let(:enabled) { true } it 'sends email', :sidekiq_inline do - expect { subject }.to change(ActionMailer::Base.deliveries, :count).by(1) + emails = capture_emails { subject } + + expect(emails.size) + .to eq(1) + expect(emails.first) + .to have_attributes( + to: contain_exactly(user.email), + subject: eq(I18n.t('notification_mailer.follow.subject', name: sender.acct)) + ) end end @@ -175,7 +181,9 @@ RSpec.describe NotifyService, type: :service do let(:enabled) { false } it "doesn't send email" do - expect { subject }.to_not change(ActionMailer::Base.deliveries, :count).from(0) + emails = capture_emails { subject } + + expect(emails).to be_empty end end end diff --git a/spec/services/report_service_spec.rb b/spec/services/report_service_spec.rb index 39e14a6a1c..2caeb189d9 100644 --- a/spec/services/report_service_spec.rb +++ b/spec/services/report_service_spec.rb @@ -158,13 +158,14 @@ RSpec.describe ReportService, type: :service do before do Fabricate(:report, target_account: target_account) - ActionMailer::Base.deliveries.clear source_account.user.settings['notification_emails.report'] = true source_account.user.save end it 'does not send an e-mail' do - expect { subject.call }.to_not change(ActionMailer::Base.deliveries, :count).from(0) + emails = capture_emails { subject.call } + + expect(emails).to be_empty end end end diff --git a/spec/workers/backup_worker_spec.rb b/spec/workers/backup_worker_spec.rb index 987cbc7d60..74928c7ca6 100644 --- a/spec/workers/backup_worker_spec.rb +++ b/spec/workers/backup_worker_spec.rb @@ -15,12 +15,17 @@ describe BackupWorker do let!(:other_backup) { Fabricate(:backup, user: backup.user) } it 'sends the backup to the service and removes other backups', :sidekiq_inline do - expect do - worker.perform(backup.id) - end.to change(UserMailer.deliveries, :size).by(1) + emails = capture_emails { worker.perform(backup.id) } expect(service).to have_received(:call).with(backup) expect { other_backup.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect(emails.size) + .to eq(1) + expect(emails.first) + .to have_attributes( + to: contain_exactly(backup.user.email), + subject: I18n.t('user_mailer.backup_ready.subject') + ) end context 'when sidekiq retries are exhausted' do