From b2c5b20ef27edd948eca8d6bd2014b7a5efaec11 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 14 Nov 2023 09:52:59 -0500 Subject: [PATCH] Fix `RSpec/AnyInstance` cop (#27810) --- .rubocop_todo.yml | 17 ---------- .../activitypub/inboxes_controller_spec.rb | 2 +- .../admin/accounts_controller_spec.rb | 3 +- .../admin/resets_controller_spec.rb | 18 ++++++++--- .../auth/sessions_controller_spec.rb | 6 ++-- .../confirmations_controller_spec.rb | 31 +++++++++---------- .../recovery_codes_controller_spec.rb | 6 ++-- spec/lib/request_spec.rb | 5 ++- spec/lib/status_filter_spec.rb | 6 ++-- spec/models/account_spec.rb | 14 +++++++-- spec/models/setting_spec.rb | 6 ++-- .../process_collection_service_spec.rb | 9 ++++-- .../activitypub/delivery_worker_spec.rb | 3 +- .../web/push_notification_worker_spec.rb | 4 +-- 14 files changed, 70 insertions(+), 60 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index f9d14fd55..167204604 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -41,23 +41,6 @@ Metrics/CyclomaticComplexity: Metrics/PerceivedComplexity: Max: 27 -RSpec/AnyInstance: - Exclude: - - 'spec/controllers/activitypub/inboxes_controller_spec.rb' - - 'spec/controllers/admin/accounts_controller_spec.rb' - - 'spec/controllers/admin/resets_controller_spec.rb' - - 'spec/controllers/auth/sessions_controller_spec.rb' - - 'spec/controllers/settings/two_factor_authentication/confirmations_controller_spec.rb' - - 'spec/controllers/settings/two_factor_authentication/recovery_codes_controller_spec.rb' - - 'spec/lib/request_spec.rb' - - 'spec/lib/status_filter_spec.rb' - - 'spec/models/account_spec.rb' - - 'spec/models/setting_spec.rb' - - 'spec/services/activitypub/process_collection_service_spec.rb' - - 'spec/validators/follow_limit_validator_spec.rb' - - 'spec/workers/activitypub/delivery_worker_spec.rb' - - 'spec/workers/web/push_notification_worker_spec.rb' - # Configuration parameters: CountAsOne. RSpec/ExampleLength: Max: 22 diff --git a/spec/controllers/activitypub/inboxes_controller_spec.rb b/spec/controllers/activitypub/inboxes_controller_spec.rb index 030a30326..feca543cb 100644 --- a/spec/controllers/activitypub/inboxes_controller_spec.rb +++ b/spec/controllers/activitypub/inboxes_controller_spec.rb @@ -58,7 +58,7 @@ RSpec.describe ActivityPub::InboxesController do before do allow(ActivityPub::FollowersSynchronizationWorker).to receive(:perform_async).and_return(nil) - allow_any_instance_of(Account).to receive(:local_followers_hash).and_return('somehash') + allow(remote_account).to receive(:local_followers_hash).and_return('somehash') request.headers['Collection-Synchronization'] = synchronization_header post :create, body: '{}' diff --git a/spec/controllers/admin/accounts_controller_spec.rb b/spec/controllers/admin/accounts_controller_spec.rb index 3b8fa2f71..307e81950 100644 --- a/spec/controllers/admin/accounts_controller_spec.rb +++ b/spec/controllers/admin/accounts_controller_spec.rb @@ -227,7 +227,8 @@ RSpec.describe Admin::AccountsController do let(:account) { Fabricate(:account, domain: 'example.com') } before do - allow_any_instance_of(ResolveAccountService).to receive(:call) + service = instance_double(ResolveAccountService, call: nil) + allow(ResolveAccountService).to receive(:new).and_return(service) end context 'when user is admin' do diff --git a/spec/controllers/admin/resets_controller_spec.rb b/spec/controllers/admin/resets_controller_spec.rb index 16adb8a12..14826973c 100644 --- a/spec/controllers/admin/resets_controller_spec.rb +++ b/spec/controllers/admin/resets_controller_spec.rb @@ -13,12 +13,20 @@ describe Admin::ResetsController do describe 'POST #create' do it 'redirects to admin accounts page' do - expect_any_instance_of(User).to receive(:send_reset_password_instructions) do |value| - expect(value.account_id).to eq account.id - end - - post :create, params: { account_id: account.id } + expect do + post :create, params: { account_id: account.id } + end.to change(Devise.mailer.deliveries, :size).by(2) + expect(Devise.mailer.deliveries).to have_attributes( + first: have_attributes( + to: include(account.user.email), + subject: I18n.t('devise.mailer.password_change.subject') + ), + last: have_attributes( + to: include(account.user.email), + subject: I18n.t('devise.mailer.reset_password_instructions.subject') + ) + ) expect(response).to redirect_to(admin_account_path(account.id)) end end diff --git a/spec/controllers/auth/sessions_controller_spec.rb b/spec/controllers/auth/sessions_controller_spec.rb index 049190e2e..f341d75b7 100644 --- a/spec/controllers/auth/sessions_controller_spec.rb +++ b/spec/controllers/auth/sessions_controller_spec.rb @@ -126,7 +126,7 @@ RSpec.describe Auth::SessionsController do let!(:previous_login) { Fabricate(:login_activity, user: user, ip: previous_ip) } before do - allow_any_instance_of(ActionDispatch::Request).to receive(:remote_ip).and_return(current_ip) + allow(controller.request).to receive(:remote_ip).and_return(current_ip) user.update(current_sign_in_at: 1.month.ago) post :create, params: { user: { email: user.email, password: user.password } } end @@ -279,7 +279,9 @@ RSpec.describe Auth::SessionsController do context 'when the server has an decryption error' do before do - allow_any_instance_of(User).to receive(:validate_and_consume_otp!).and_raise(OpenSSL::Cipher::CipherError) + allow(user).to receive(:validate_and_consume_otp!).with(user.current_otp).and_raise(OpenSSL::Cipher::CipherError) + allow(User).to receive(:find_by).with(id: user.id).and_return(user) + 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 diff --git a/spec/controllers/settings/two_factor_authentication/confirmations_controller_spec.rb b/spec/controllers/settings/two_factor_authentication/confirmations_controller_spec.rb index 37381fe1f..a5a35e91d 100644 --- a/spec/controllers/settings/two_factor_authentication/confirmations_controller_spec.rb +++ b/spec/controllers/settings/two_factor_authentication/confirmations_controller_spec.rb @@ -61,6 +61,7 @@ describe Settings::TwoFactorAuthentication::ConfirmationsController do it 'renders page with success' do prepare_user_otp_generation prepare_user_otp_consumption + allow(controller).to receive(:current_user).and_return(user) expect do post :create, @@ -75,30 +76,28 @@ describe Settings::TwoFactorAuthentication::ConfirmationsController do end def prepare_user_otp_generation - expect_any_instance_of(User).to receive(:generate_otp_backup_codes!) do |value| - expect(value).to eq user - otp_backup_codes - end + allow(user) + .to receive(:generate_otp_backup_codes!) + .and_return(otp_backup_codes) end def prepare_user_otp_consumption - expect_any_instance_of(User).to receive(:validate_and_consume_otp!) do |value, code, options| - expect(value).to eq user - expect(code).to eq '123456' - expect(options).to eq({ otp_secret: 'thisisasecretforthespecofnewview' }) - true - end + options = { otp_secret: 'thisisasecretforthespecofnewview' } + allow(user) + .to receive(:validate_and_consume_otp!) + .with('123456', options) + .and_return(true) end end describe 'when creation fails' do subject do - expect_any_instance_of(User).to receive(:validate_and_consume_otp!) do |value, code, options| - expect(value).to eq user - expect(code).to eq '123456' - expect(options).to eq({ otp_secret: 'thisisasecretforthespecofnewview' }) - false - end + options = { otp_secret: 'thisisasecretforthespecofnewview' } + allow(user) + .to receive(:validate_and_consume_otp!) + .with('123456', options) + .and_return(false) + allow(controller).to receive(:current_user).and_return(user) expect do post :create, diff --git a/spec/controllers/settings/two_factor_authentication/recovery_codes_controller_spec.rb b/spec/controllers/settings/two_factor_authentication/recovery_codes_controller_spec.rb index 630cec428..28a40e138 100644 --- a/spec/controllers/settings/two_factor_authentication/recovery_codes_controller_spec.rb +++ b/spec/controllers/settings/two_factor_authentication/recovery_codes_controller_spec.rb @@ -9,10 +9,8 @@ describe Settings::TwoFactorAuthentication::RecoveryCodesController do it 'updates the codes and shows them on a view when signed in' do user = Fabricate(:user) otp_backup_codes = user.generate_otp_backup_codes! - expect_any_instance_of(User).to receive(:generate_otp_backup_codes!) do |value| - expect(value).to eq user - otp_backup_codes - end + allow(user).to receive(:generate_otp_backup_codes!).and_return(otp_backup_codes) + allow(controller).to receive(:current_user).and_return(user) sign_in user, scope: :user post :create, session: { challenge_passed_at: Time.now.utc } diff --git a/spec/lib/request_spec.rb b/spec/lib/request_spec.rb index f0861376b..c7620cf9b 100644 --- a/spec/lib/request_spec.rb +++ b/spec/lib/request_spec.rb @@ -64,8 +64,11 @@ describe Request do end it 'closes underlying connection' do - expect_any_instance_of(HTTP::Client).to receive(:close) + allow(subject.send(:http_client)).to receive(:close) + expect { |block| subject.perform(&block) }.to yield_control + + expect(subject.send(:http_client)).to have_received(:close) end it 'returns response which implements body_with_limit' do diff --git a/spec/lib/status_filter_spec.rb b/spec/lib/status_filter_spec.rb index c994ad419..cf6f3c795 100644 --- a/spec/lib/status_filter_spec.rb +++ b/spec/lib/status_filter_spec.rb @@ -23,7 +23,8 @@ describe StatusFilter do context 'when status policy does not allow show' do it 'filters the status' do - allow_any_instance_of(StatusPolicy).to receive(:show?).and_return(false) + policy = instance_double(StatusPolicy, show?: false) + allow(StatusPolicy).to receive(:new).and_return(policy) expect(filter).to be_filtered end @@ -74,7 +75,8 @@ describe StatusFilter do context 'when status policy does not allow show' do it 'filters the status' do - allow_any_instance_of(StatusPolicy).to receive(:show?).and_return(false) + policy = instance_double(StatusPolicy, show?: false) + allow(StatusPolicy).to receive(:new).and_return(policy) expect(filter).to be_filtered end diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index b5d942412..f77ecb055 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -209,9 +209,13 @@ RSpec.describe Account do expect(account.refresh!).to be_nil end - it 'calls not ResolveAccountService#call' do - expect_any_instance_of(ResolveAccountService).to_not receive(:call).with(acct) + it 'does not call ResolveAccountService#call' do + service = instance_double(ResolveAccountService, call: nil) + allow(ResolveAccountService).to receive(:new).and_return(service) + account.refresh! + + expect(service).to_not have_received(:call).with(acct) end end @@ -219,8 +223,12 @@ RSpec.describe Account do let(:domain) { 'example.com' } it 'calls ResolveAccountService#call' do - expect_any_instance_of(ResolveAccountService).to receive(:call).with(acct).once + service = instance_double(ResolveAccountService, call: nil) + allow(ResolveAccountService).to receive(:new).and_return(service) + account.refresh! + + expect(service).to have_received(:call).with(acct).once end end end diff --git a/spec/models/setting_spec.rb b/spec/models/setting_spec.rb index b08136a1c..e98062810 100644 --- a/spec/models/setting_spec.rb +++ b/spec/models/setting_spec.rb @@ -52,7 +52,8 @@ RSpec.describe Setting do before do allow(RailsSettings::Settings).to receive(:object).with(key).and_return(object) allow(described_class).to receive(:default_settings).and_return(default_settings) - allow_any_instance_of(Settings::ScopedSettings).to receive(:thing_scoped).and_return(records) + settings_double = instance_double(Settings::ScopedSettings, thing_scoped: records) + allow(Settings::ScopedSettings).to receive(:new).and_return(settings_double) Rails.cache.delete(cache_key) end @@ -128,7 +129,8 @@ RSpec.describe Setting do describe '.all_as_records' do before do - allow_any_instance_of(Settings::ScopedSettings).to receive(:thing_scoped).and_return(records) + settings_double = instance_double(Settings::ScopedSettings, thing_scoped: records) + allow(Settings::ScopedSettings).to receive(:new).and_return(settings_double) allow(described_class).to receive(:default_settings).and_return(default_settings) end diff --git a/spec/services/activitypub/process_collection_service_spec.rb b/spec/services/activitypub/process_collection_service_spec.rb index ede9f5c04..df526daf3 100644 --- a/spec/services/activitypub/process_collection_service_spec.rb +++ b/spec/services/activitypub/process_collection_service_spec.rb @@ -76,7 +76,8 @@ RSpec.describe ActivityPub::ProcessCollectionService, type: :service do let(:forwarder) { Fabricate(:account, domain: 'example.com', uri: 'http://example.com/other_account') } it 'does not process payload if no signature exists' do - allow_any_instance_of(ActivityPub::LinkedDataSignature).to receive(:verify_actor!).and_return(nil) + signature_double = instance_double(ActivityPub::LinkedDataSignature, verify_actor!: nil) + allow(ActivityPub::LinkedDataSignature).to receive(:new).and_return(signature_double) allow(ActivityPub::Activity).to receive(:factory) subject.call(json, forwarder) @@ -87,7 +88,8 @@ RSpec.describe ActivityPub::ProcessCollectionService, type: :service do it 'processes payload with actor if valid signature exists' do payload['signature'] = { 'type' => 'RsaSignature2017' } - allow_any_instance_of(ActivityPub::LinkedDataSignature).to receive(:verify_actor!).and_return(actor) + signature_double = instance_double(ActivityPub::LinkedDataSignature, verify_actor!: actor) + allow(ActivityPub::LinkedDataSignature).to receive(:new).and_return(signature_double) allow(ActivityPub::Activity).to receive(:factory).with(instance_of(Hash), actor, instance_of(Hash)) subject.call(json, forwarder) @@ -98,7 +100,8 @@ RSpec.describe ActivityPub::ProcessCollectionService, type: :service do it 'does not process payload if invalid signature exists' do payload['signature'] = { 'type' => 'RsaSignature2017' } - allow_any_instance_of(ActivityPub::LinkedDataSignature).to receive(:verify_actor!).and_return(nil) + signature_double = instance_double(ActivityPub::LinkedDataSignature, verify_actor!: nil) + allow(ActivityPub::LinkedDataSignature).to receive(:new).and_return(signature_double) allow(ActivityPub::Activity).to receive(:factory) subject.call(json, forwarder) diff --git a/spec/workers/activitypub/delivery_worker_spec.rb b/spec/workers/activitypub/delivery_worker_spec.rb index d39393d50..efce610ae 100644 --- a/spec/workers/activitypub/delivery_worker_spec.rb +++ b/spec/workers/activitypub/delivery_worker_spec.rb @@ -11,7 +11,8 @@ describe ActivityPub::DeliveryWorker do let(:payload) { 'test' } before do - allow_any_instance_of(Account).to receive(:remote_followers_hash).with('https://example.com/api').and_return('somehash') + allow(sender).to receive(:remote_followers_hash).with('https://example.com/api').and_return('somehash') + allow(Account).to receive(:find).with(sender.id).and_return(sender) end describe 'perform' do diff --git a/spec/workers/web/push_notification_worker_spec.rb b/spec/workers/web/push_notification_worker_spec.rb index 822ef5257..637206a40 100644 --- a/spec/workers/web/push_notification_worker_spec.rb +++ b/spec/workers/web/push_notification_worker_spec.rb @@ -23,8 +23,8 @@ describe Web::PushNotificationWorker do describe 'perform' do before do - allow_any_instance_of(subscription.class).to receive(:contact_email).and_return(contact_email) - allow_any_instance_of(subscription.class).to receive(:vapid_key).and_return(vapid_key) + allow(subscription).to receive_messages(contact_email: contact_email, vapid_key: vapid_key) + allow(Web::PushSubscription).to receive(:find).with(subscription.id).and_return(subscription) allow(Webpush::Encryption).to receive(:encrypt).and_return(payload) allow(JWT).to receive(:encode).and_return('jwt.encoded.payload')