Skip to content

Commit

Permalink
Fix RSpec/AnyInstance cop (mastodon#27810)
Browse files Browse the repository at this point in the history
  • Loading branch information
mjankowski authored Nov 14, 2023
1 parent d562fb8 commit b2c5b20
Show file tree
Hide file tree
Showing 14 changed files with 70 additions and 60 deletions.
17 changes: 0 additions & 17 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion spec/controllers/activitypub/inboxes_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@

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: '{}'
Expand Down
3 changes: 2 additions & 1 deletion spec/controllers/admin/accounts_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,8 @@
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
Expand Down
18 changes: 13 additions & 5 deletions spec/controllers/admin/resets_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,20 @@

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
Expand Down
6 changes: 4 additions & 2 deletions spec/controllers/auth/sessions_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@
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
Expand Down Expand Up @@ -279,7 +279,9 @@

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
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,
Expand All @@ -75,30 +76,28 @@
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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,8 @@
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 }
Expand Down
5 changes: 4 additions & 1 deletion spec/lib/request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,11 @@
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
Expand Down
6 changes: 4 additions & 2 deletions spec/lib/status_filter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@

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
Expand Down Expand Up @@ -74,7 +75,8 @@

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
Expand Down
14 changes: 11 additions & 3 deletions spec/models/account_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -209,18 +209,26 @@
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

context 'when domain is present' 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
Expand Down
6 changes: 4 additions & 2 deletions spec/models/setting_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@
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

Expand Down Expand Up @@ -128,7 +129,8 @@

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

Expand Down
9 changes: 6 additions & 3 deletions spec/services/activitypub/process_collection_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@
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)
Expand All @@ -87,7 +88,8 @@
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)
Expand All @@ -98,7 +100,8 @@
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)
Expand Down
3 changes: 2 additions & 1 deletion spec/workers/activitypub/delivery_worker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
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
Expand Down
4 changes: 2 additions & 2 deletions spec/workers/web/push_notification_worker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@

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')

Expand Down

0 comments on commit b2c5b20

Please sign in to comment.