From 63c9102f8afb7853d6b42f335d44601139cf113a Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 9 Nov 2023 07:57:23 -0500 Subject: [PATCH] Fix `RSpec/MessageChain` cop (#27776) --- .rubocop_todo.yml | 6 ---- .../session_activation_fabricator.rb | 2 +- spec/models/concerns/remotable_spec.rb | 4 ++- spec/models/session_activation_spec.rb | 36 ++++++++++++------- spec/models/setting_spec.rb | 7 ++-- 5 files changed, 32 insertions(+), 23 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index b3228bac6e9..f9d14fd551e 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -104,12 +104,6 @@ RSpec/LetSetup: - 'spec/services/unsuspend_account_service_spec.rb' - 'spec/workers/scheduler/user_cleanup_scheduler_spec.rb' -RSpec/MessageChain: - Exclude: - - 'spec/models/concerns/remotable_spec.rb' - - 'spec/models/session_activation_spec.rb' - - 'spec/models/setting_spec.rb' - RSpec/MultipleExpectations: Max: 8 diff --git a/spec/fabricators/session_activation_fabricator.rb b/spec/fabricators/session_activation_fabricator.rb index 4b5244cec6e..655ec37aa8a 100644 --- a/spec/fabricators/session_activation_fabricator.rb +++ b/spec/fabricators/session_activation_fabricator.rb @@ -2,5 +2,5 @@ Fabricator(:session_activation) do user { Fabricate.build(:user) } - session_id 'MyString' + session_id { sequence(:session_id) { |i| "session_id_#{i}" } } end diff --git a/spec/models/concerns/remotable_spec.rb b/spec/models/concerns/remotable_spec.rb index 6413b6f4679..db690da3c21 100644 --- a/spec/models/concerns/remotable_spec.rb +++ b/spec/models/concerns/remotable_spec.rb @@ -69,7 +69,9 @@ RSpec.describe Remotable do context 'with an invalid URL' do before do - allow(Addressable::URI).to receive_message_chain(:parse, :normalize).with(url).with(no_args).and_raise(Addressable::URI::InvalidURIError) + parsed = instance_double(Addressable::URI) + allow(parsed).to receive(:normalize).with(no_args).and_raise(Addressable::URI::InvalidURIError) + allow(Addressable::URI).to receive(:parse).with(url).and_return(parsed) end it 'makes no request' do diff --git a/spec/models/session_activation_spec.rb b/spec/models/session_activation_spec.rb index 4012d46fd70..bed411c3698 100644 --- a/spec/models/session_activation_spec.rb +++ b/spec/models/session_activation_spec.rb @@ -98,34 +98,44 @@ RSpec.describe SessionActivation do end context 'when id exists' do - let(:id) { '1' } + let!(:session_activation) { Fabricate(:session_activation) } - it 'calls where.destroy_all' do - expect(described_class).to receive_message_chain(:where, :destroy_all) - .with(session_id: id).with(no_args) + it 'destroys the record' do + described_class.deactivate(session_activation.session_id) - described_class.deactivate(id) + expect { session_activation.reload }.to raise_error(ActiveRecord::RecordNotFound) end end end describe '.purge_old' do - it 'calls order.offset.destroy_all' do - expect(described_class).to receive_message_chain(:order, :offset, :destroy_all) - .with('created_at desc').with(Rails.configuration.x.max_session_activations).with(no_args) + around do |example| + before = Rails.configuration.x.max_session_activations + Rails.configuration.x.max_session_activations = 1 + example.run + Rails.configuration.x.max_session_activations = before + end + let!(:oldest_session_activation) { Fabricate(:session_activation, created_at: 10.days.ago) } + let!(:newest_session_activation) { Fabricate(:session_activation, created_at: 5.days.ago) } + + it 'preserves the newest X records based on config' do described_class.purge_old + + expect { oldest_session_activation.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect { newest_session_activation.reload }.to_not raise_error end end describe '.exclusive' do - let(:id) { '1' } + let!(:unwanted_session_activation) { Fabricate(:session_activation) } + let!(:wanted_session_activation) { Fabricate(:session_activation) } - it 'calls where.destroy_all' do - expect(described_class).to receive_message_chain(:where, :not, :destroy_all) - .with(session_id: id).with(no_args) + it 'preserves supplied record and destroys all others' do + described_class.exclusive(wanted_session_activation.session_id) - described_class.exclusive(id) + expect { unwanted_session_activation.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect { wanted_session_activation.reload }.to_not raise_error end end end diff --git a/spec/models/setting_spec.rb b/spec/models/setting_spec.rb index 5f53ee56341..b08136a1c17 100644 --- a/spec/models/setting_spec.rb +++ b/spec/models/setting_spec.rb @@ -77,10 +77,13 @@ RSpec.describe Setting do let(:default_value) { { default_value: 'default_value' } } it 'calls default_value.with_indifferent_access.merge!' do - expect(default_value).to receive_message_chain(:with_indifferent_access, :merge!) - .with(db_val.value) + indifferent_hash = instance_double(Hash, merge!: nil) + allow(default_value).to receive(:with_indifferent_access).and_return(indifferent_hash) described_class[key] + + expect(default_value).to have_received(:with_indifferent_access) + expect(indifferent_hash).to have_received(:merge!).with(db_val.value) end end