Fix RSpec/InstanceVariable cop (#27766)

.rubocop_todo.yml

@@ -71,26 +71,6 @@ RSpec/AnyInstance:
 RSpec/ExampleLength:
   Max: 22
 
-# Configuration parameters: AssignmentOnly.
-RSpec/InstanceVariable:
-  Exclude:
-    - 'spec/controllers/api/v1/streaming_controller_spec.rb'
-    - 'spec/controllers/auth/confirmations_controller_spec.rb'
-    - 'spec/controllers/auth/passwords_controller_spec.rb'
-    - 'spec/controllers/auth/sessions_controller_spec.rb'
-    - 'spec/controllers/concerns/export_controller_concern_spec.rb'
-    - 'spec/controllers/home_controller_spec.rb'
-    - 'spec/controllers/settings/two_factor_authentication/webauthn_credentials_controller_spec.rb'
-    - 'spec/controllers/statuses_cleanup_controller_spec.rb'
-    - 'spec/models/concerns/account_finder_concern_spec.rb'
-    - 'spec/models/concerns/account_interactions_spec.rb'
-    - 'spec/models/public_feed_spec.rb'
-    - 'spec/serializers/activitypub/note_serializer_spec.rb'
-    - 'spec/serializers/activitypub/update_poll_serializer_spec.rb'
-    - 'spec/services/remove_status_service_spec.rb'
-    - 'spec/services/search_service_spec.rb'
-    - 'spec/services/unblock_domain_service_spec.rb'
-
 RSpec/LetSetup:
   Exclude:
     - 'spec/controllers/api/v1/accounts/statuses_controller_spec.rb'

spec/controllers/api/v1/streaming_controller_spec.rb

@@ -26,7 +26,6 @@ describe Api::V1::StreamingController do
   context 'with streaming api on different host' do
     before do
       Rails.configuration.x.streaming_api_base_url = "wss://streaming-#{Rails.configuration.x.web_domain}"
-      @streaming_host = URI.parse(Rails.configuration.x.streaming_api_base_url).host
     end
 
     describe 'GET #index' do
@@ -38,7 +37,13 @@ describe Api::V1::StreamingController do
         [:scheme, :path, :query, :fragment].each do |part|
           expect(redirect_to_uri.send(part)).to eq(request_uri.send(part)), "redirect target #{part}"
         end
-        expect(redirect_to_uri.host).to eq(@streaming_host), 'redirect target host'
+        expect(redirect_to_uri.host).to eq(streaming_host), 'redirect target host'
+      end
+
+      private
+
+      def streaming_host
+        URI.parse(Rails.configuration.x.streaming_api_base_url).host
       end
     end
   end

spec/controllers/auth/passwords_controller_spec.rb

@@ -18,12 +18,14 @@ describe Auth::PasswordsController do
 
     before do
       request.env['devise.mapping'] = Devise.mappings[:user]
-      @token = user.send_reset_password_instructions
     end
 
     context 'with valid reset_password_token' do
       it 'returns http success' do
-        get :edit, params: { reset_password_token: @token }
+        token = user.send_reset_password_instructions
+
+        get :edit, params: { reset_password_token: token }
+
         expect(response).to have_http_status(200)
       end
     end
@@ -38,9 +40,9 @@ describe Auth::PasswordsController do
 
   describe 'POST #update' do
     let(:user) { Fabricate(:user) }
+    let(:password) { 'reset0password' }
 
     before do
-      @password = 'reset0password'
       request.env['devise.mapping'] = Devise.mappings[:user]
     end
 
@@ -50,9 +52,9 @@ describe Auth::PasswordsController do
       let!(:web_push_subscription) { Fabricate(:web_push_subscription, access_token: access_token) }
 
       before do
-        @token = user.send_reset_password_instructions
+        token = user.send_reset_password_instructions
 
-        post :update, params: { user: { password: @password, password_confirmation: @password, reset_password_token: @token } }
+        post :update, params: { user: { password: password, password_confirmation: password, reset_password_token: token } }
       end
 
       it 'redirect to sign in' do
@@ -63,7 +65,7 @@ describe Auth::PasswordsController do
         this_user = User.find(user.id)
 
         expect(this_user).to_not be_nil
-        expect(this_user.valid_password?(@password)).to be true
+        expect(this_user.valid_password?(password)).to be true
       end
 
       it 'deactivates all sessions' do
@@ -81,7 +83,7 @@ describe Auth::PasswordsController do
 
     context 'with invalid reset_password_token' do
       before do
-        post :update, params: { user: { password: @password, password_confirmation: @password, reset_password_token: 'some_invalid_value' } }
+        post :update, params: { user: { password: password, password_confirmation: password, reset_password_token: 'some_invalid_value' } }
       end
 
       it 'renders reset password' do

spec/controllers/concerns/export_controller_concern_spec.rb

@@ -11,7 +11,7 @@ describe ExportControllerConcern do
     end
 
     def export_data
-      @export.account.username
+      'body data value'
     end
   end
 
@@ -24,7 +24,7 @@ describe ExportControllerConcern do
       expect(response).to have_http_status(200)
       expect(response.media_type).to eq 'text/csv'
       expect(response.headers['Content-Disposition']).to start_with 'attachment; filename="anonymous.csv"'
-      expect(response.body).to eq user.account.username
+      expect(response.body).to eq 'body data value'
     end
 
     it 'returns unauthorized when not signed in' do

spec/controllers/statuses_cleanup_controller_spec.rb

@@ -5,9 +5,10 @@ require 'rails_helper'
 RSpec.describe StatusesCleanupController do
   render_views
 
+  let!(:user) { Fabricate(:user) }
+
   before do
-    @user = Fabricate(:user)
+    sign_in user, scope: :user
-    sign_in @user, scope: :user
   end
 
   describe 'GET #show' do
@@ -30,9 +31,9 @@ RSpec.describe StatusesCleanupController do
     end
 
     it 'updates the account status cleanup policy' do
-      expect(@user.account.statuses_cleanup_policy.enabled).to be true
+      expect(user.account.statuses_cleanup_policy.enabled).to be true
-      expect(@user.account.statuses_cleanup_policy.keep_direct).to be false
+      expect(user.account.statuses_cleanup_policy.keep_direct).to be false
-      expect(@user.account.statuses_cleanup_policy.keep_polls).to be true
+      expect(user.account.statuses_cleanup_policy.keep_polls).to be true
     end
 
     it 'redirects' do

spec/models/concerns/account_finder_concern_spec.rb

@@ -4,17 +4,15 @@ require 'rails_helper'
 
 describe AccountFinderConcern do
   describe 'local finders' do
-    before do
+    let!(:account) { Fabricate(:account, username: 'Alice') }
-      @account = Fabricate(:account, username: 'Alice')
-    end
 
     describe '.find_local' do
       it 'returns case-insensitive result' do
-        expect(Account.find_local('alice')).to eq(@account)
+        expect(Account.find_local('alice')).to eq(account)
       end
 
       it 'returns correctly cased result' do
-        expect(Account.find_local('Alice')).to eq(@account)
+        expect(Account.find_local('Alice')).to eq(account)
       end
 
       it 'returns nil without a match' do
@@ -36,7 +34,7 @@ describe AccountFinderConcern do
 
     describe '.find_local!' do
       it 'returns matching result' do
-        expect(Account.find_local!('alice')).to eq(@account)
+        expect(Account.find_local!('alice')).to eq(account)
       end
 
       it 'raises on non-matching result' do
@@ -54,17 +52,15 @@ describe AccountFinderConcern do
   end
 
   describe 'remote finders' do
-    before do
+    let!(:account) { Fabricate(:account, username: 'Alice', domain: 'mastodon.social') }
-      @account = Fabricate(:account, username: 'Alice', domain: 'mastodon.social')
-    end
 
     describe '.find_remote' do
       it 'returns exact match result' do
-        expect(Account.find_remote('alice', 'mastodon.social')).to eq(@account)
+        expect(Account.find_remote('alice', 'mastodon.social')).to eq(account)
       end
 
       it 'returns case-insensitive result' do
-        expect(Account.find_remote('ALICE', 'MASTODON.SOCIAL')).to eq(@account)
+        expect(Account.find_remote('ALICE', 'MASTODON.SOCIAL')).to eq(account)
       end
 
       it 'returns nil when username does not match' do
@@ -90,7 +86,7 @@ describe AccountFinderConcern do
 
     describe '.find_remote!' do
       it 'returns matching result' do
-        expect(Account.find_remote!('alice', 'mastodon.social')).to eq(@account)
+        expect(Account.find_remote!('alice', 'mastodon.social')).to eq(account)
       end
 
       it 'raises on non-matching result' do

spec/models/concerns/account_interactions_spec.rb

@@ -648,38 +648,36 @@ describe AccountInteractions do
   end
 
   describe 'ignoring reblogs from an account' do
-    before do
+    let!(:me) { Fabricate(:account, username: 'Me') }
-      @me = Fabricate(:account, username: 'Me')
+    let!(:you) { Fabricate(:account, username: 'You') }
-      @you = Fabricate(:account, username: 'You')
-    end
 
     context 'with the reblogs option unspecified' do
       before do
-        @me.follow!(@you)
+        me.follow!(you)
       end
 
       it 'defaults to showing reblogs' do
-        expect(@me.muting_reblogs?(@you)).to be(false)
+        expect(me.muting_reblogs?(you)).to be(false)
       end
     end
 
     context 'with the reblogs option set to false' do
       before do
-        @me.follow!(@you, reblogs: false)
+        me.follow!(you, reblogs: false)
       end
 
       it 'does mute reblogs' do
-        expect(@me.muting_reblogs?(@you)).to be(true)
+        expect(me.muting_reblogs?(you)).to be(true)
       end
     end
 
     context 'with the reblogs option set to true' do
       before do
-        @me.follow!(@you, reblogs: true)
+        me.follow!(you, reblogs: true)
       end
 
       it 'does not mute reblogs' do
-        expect(@me.muting_reblogs?(@you)).to be(false)
+        expect(me.muting_reblogs?(you)).to be(false)
       end
     end
   end

spec/models/public_feed_spec.rb

@@ -137,15 +137,13 @@ RSpec.describe PublicFeed do
     end
 
     describe 'with an account passed in' do
-      subject { described_class.new(@account).get(20).map(&:id) }
+      subject { described_class.new(account).get(20).map(&:id) }
 
-      before do
+      let!(:account) { Fabricate(:account) }
-        @account = Fabricate(:account)
-      end
 
       it 'excludes statuses from accounts blocked by the account' do
         blocked = Fabricate(:account)
-        @account.block!(blocked)
+        account.block!(blocked)
         blocked_status = Fabricate(:status, account: blocked)
 
         expect(subject).to_not include(blocked_status.id)
@@ -153,7 +151,7 @@ RSpec.describe PublicFeed do
 
       it 'excludes statuses from accounts who have blocked the account' do
         blocker = Fabricate(:account)
-        blocker.block!(@account)
+        blocker.block!(account)
         blocked_status = Fabricate(:status, account: blocker)
 
         expect(subject).to_not include(blocked_status.id)
@@ -161,7 +159,7 @@ RSpec.describe PublicFeed do
 
       it 'excludes statuses from accounts muted by the account' do
         muted = Fabricate(:account)
-        @account.mute!(muted)
+        account.mute!(muted)
         muted_status = Fabricate(:status, account: muted)
 
         expect(subject).to_not include(muted_status.id)
@@ -169,7 +167,7 @@ RSpec.describe PublicFeed do
 
       it 'excludes statuses from accounts from personally blocked domains' do
         blocked = Fabricate(:account, domain: 'example.com')
-        @account.block_domain!(blocked.domain)
+        account.block_domain!(blocked.domain)
         blocked_status = Fabricate(:status, account: blocked)
 
         expect(subject).to_not include(blocked_status.id)
@@ -177,7 +175,7 @@ RSpec.describe PublicFeed do
 
       context 'with language preferences' do
         it 'excludes statuses in languages not allowed by the account user' do
-          @account.user.update(chosen_languages: [:en, :es])
+          account.user.update(chosen_languages: [:en, :es])
           en_status = Fabricate(:status, language: 'en')
           es_status = Fabricate(:status, language: 'es')
           fr_status = Fabricate(:status, language: 'fr')
@@ -188,7 +186,7 @@ RSpec.describe PublicFeed do
         end
 
         it 'includes all languages when user does not have a setting' do
-          @account.user.update(chosen_languages: nil)
+          account.user.update(chosen_languages: nil)
 
           en_status = Fabricate(:status, language: 'en')
           es_status = Fabricate(:status, language: 'es')
@@ -198,7 +196,7 @@ RSpec.describe PublicFeed do
         end
 
         it 'includes all languages when account does not have a user' do
-          @account.update(user: nil)
+          account.update(user: nil)
 
           en_status = Fabricate(:status, language: 'en')
           es_status = Fabricate(:status, language: 'es')

spec/services/remove_status_service_spec.rb

@@ -20,71 +20,70 @@ RSpec.describe RemoveStatusService, type: :service do
   end
 
   context 'when removed status is not a reblog' do
+    let!(:status) { PostStatusService.new.call(alice, text: 'Hello @bob@example.com ThisIsASecret') }
+
     before do
-      @status = PostStatusService.new.call(alice, text: 'Hello @bob@example.com ThisIsASecret')
+      FavouriteService.new.call(jeff, status)
-      FavouriteService.new.call(jeff, @status)
+      Fabricate(:status, account: bill, reblog: status, uri: 'hoge')
-      Fabricate(:status, account: bill, reblog: @status, uri: 'hoge')
     end
 
     it 'removes status from author\'s home feed' do
-      subject.call(@status)
+      subject.call(status)
-      expect(HomeFeed.new(alice).get(10).pluck(:id)).to_not include(@status.id)
+      expect(HomeFeed.new(alice).get(10).pluck(:id)).to_not include(status.id)
     end
 
     it 'removes status from local follower\'s home feed' do
-      subject.call(@status)
+      subject.call(status)
-      expect(HomeFeed.new(jeff).get(10).pluck(:id)).to_not include(@status.id)
+      expect(HomeFeed.new(jeff).get(10).pluck(:id)).to_not include(status.id)
     end
 
     it 'sends Delete activity to followers' do
-      subject.call(@status)
+      subject.call(status)
       expect(a_request(:post, 'http://example.com/inbox').with(
                body: hash_including({
                  'type' => 'Delete',
                  'object' => {
                    'type' => 'Tombstone',
-                   'id' => ActivityPub::TagManager.instance.uri_for(@status),
+                   'id' => ActivityPub::TagManager.instance.uri_for(status),
-                   'atomUri' => OStatus::TagManager.instance.uri_for(@status),
+                   'atomUri' => OStatus::TagManager.instance.uri_for(status),
                  },
                })
              )).to have_been_made.once
     end
 
     it 'sends Delete activity to rebloggers' do
-      subject.call(@status)
+      subject.call(status)
       expect(a_request(:post, 'http://example2.com/inbox').with(
                body: hash_including({
                  'type' => 'Delete',
                  'object' => {
                    'type' => 'Tombstone',
-                   'id' => ActivityPub::TagManager.instance.uri_for(@status),
+                   'id' => ActivityPub::TagManager.instance.uri_for(status),
-                   'atomUri' => OStatus::TagManager.instance.uri_for(@status),
+                   'atomUri' => OStatus::TagManager.instance.uri_for(status),
                  },
                })
              )).to have_been_made.once
     end
 
     it 'remove status from notifications' do
-      expect { subject.call(@status) }.to change {
+      expect { subject.call(status) }.to change {
         Notification.where(activity_type: 'Favourite', from_account: jeff, account: alice).count
       }.from(1).to(0)
     end
   end
 
   context 'when removed status is a private self-reblog' do
-    before do
+    let!(:original_status) { Fabricate(:status, account: alice, text: 'Hello ThisIsASecret', visibility: :private) }
-      @original_status = Fabricate(:status, account: alice, text: 'Hello ThisIsASecret', visibility: :private)
+    let!(:status) { ReblogService.new.call(alice, original_status) }
-      @status = ReblogService.new.call(alice, @original_status)
-    end
 
     it 'sends Undo activity to followers' do
-      subject.call(@status)
+      subject.call(status)
       expect(a_request(:post, 'http://example.com/inbox').with(
                body: hash_including({
                  'type' => 'Undo',
                  'object' => hash_including({
                    'type' => 'Announce',
-                   'object' => ActivityPub::TagManager.instance.uri_for(@original_status),
+                   'object' => ActivityPub::TagManager.instance.uri_for(original_status),
                  }),
                })
              )).to have_been_made.once
@@ -92,19 +91,17 @@ RSpec.describe RemoveStatusService, type: :service do
   end
 
   context 'when removed status is public self-reblog' do
-    before do
+    let!(:original_status) { Fabricate(:status, account: alice, text: 'Hello ThisIsASecret', visibility: :public) }
-      @original_status = Fabricate(:status, account: alice, text: 'Hello ThisIsASecret', visibility: :public)
+    let!(:status) { ReblogService.new.call(alice, original_status) }
-      @status = ReblogService.new.call(alice, @original_status)
-    end
 
     it 'sends Undo activity to followers' do
-      subject.call(@status)
+      subject.call(status)
       expect(a_request(:post, 'http://example.com/inbox').with(
                body: hash_including({
                  'type' => 'Undo',
                  'object' => hash_including({
                    'type' => 'Announce',
-                   'object' => ActivityPub::TagManager.instance.uri_for(@original_status),
+                   'object' => ActivityPub::TagManager.instance.uri_for(original_status),
                  }),
                })
              )).to have_been_made.once

spec/services/search_service_spec.rb

@@ -19,17 +19,15 @@ describe SearchService, type: :service do
     end
 
     describe 'with an url query' do
-      before do
+      let(:query) { 'http://test.host/query' }
-        @query = 'http://test.host/query'
-      end
 
       context 'when it does not find anything' do
         it 'returns the empty results' do
           service = instance_double(ResolveURLService, call: nil)
           allow(ResolveURLService).to receive(:new).and_return(service)
-          results = subject.call(@query, nil, 10, resolve: true)
+          results = subject.call(query, nil, 10, resolve: true)
 
-          expect(service).to have_received(:call).with(@query, on_behalf_of: nil)
+          expect(service).to have_received(:call).with(query, on_behalf_of: nil)
           expect(results).to eq empty_results
         end
       end
@@ -40,8 +38,8 @@ describe SearchService, type: :service do
           service = instance_double(ResolveURLService, call: account)
           allow(ResolveURLService).to receive(:new).and_return(service)
 
-          results = subject.call(@query, nil, 10, resolve: true)
+          results = subject.call(query, nil, 10, resolve: true)
-          expect(service).to have_received(:call).with(@query, on_behalf_of: nil)
+          expect(service).to have_received(:call).with(query, on_behalf_of: nil)
           expect(results).to eq empty_results.merge(accounts: [account])
         end
       end
@@ -52,8 +50,8 @@ describe SearchService, type: :service do
           service = instance_double(ResolveURLService, call: status)
           allow(ResolveURLService).to receive(:new).and_return(service)
 
-          results = subject.call(@query, nil, 10, resolve: true)
+          results = subject.call(query, nil, 10, resolve: true)
-          expect(service).to have_received(:call).with(@query, on_behalf_of: nil)
+          expect(service).to have_received(:call).with(query, on_behalf_of: nil)
           expect(results).to eq empty_results.merge(statuses: [status])
         end
       end

spec/services/unblock_domain_service_spec.rb

@@ -6,38 +6,36 @@ describe UnblockDomainService, type: :service do
   subject { described_class.new }
 
   describe 'call' do
-    before do
+    let!(:independently_suspended) { Fabricate(:account, domain: 'example.com', suspended_at: 1.hour.ago) }
-      @independently_suspended = Fabricate(:account, domain: 'example.com', suspended_at: 1.hour.ago)
+    let!(:independently_silenced) { Fabricate(:account, domain: 'example.com', silenced_at: 1.hour.ago) }
-      @independently_silenced = Fabricate(:account, domain: 'example.com', silenced_at: 1.hour.ago)
+    let!(:domain_block) { Fabricate(:domain_block, domain: 'example.com') }
-      @domain_block = Fabricate(:domain_block, domain: 'example.com')
+    let!(:silenced) { Fabricate(:account, domain: 'example.com', silenced_at: domain_block.created_at) }
-      @silenced = Fabricate(:account, domain: 'example.com', silenced_at: @domain_block.created_at)
+    let!(:suspended) { Fabricate(:account, domain: 'example.com', suspended_at: domain_block.created_at) }
-      @suspended = Fabricate(:account, domain: 'example.com', suspended_at: @domain_block.created_at)
-    end
 
     it 'unsilences accounts and removes block' do
-      @domain_block.update(severity: :silence)
+      domain_block.update(severity: :silence)
 
-      subject.call(@domain_block)
+      subject.call(domain_block)
       expect_deleted_domain_block
-      expect(@silenced.reload.silenced?).to be false
+      expect(silenced.reload.silenced?).to be false
-      expect(@suspended.reload.suspended?).to be true
+      expect(suspended.reload.suspended?).to be true
-      expect(@independently_suspended.reload.suspended?).to be true
+      expect(independently_suspended.reload.suspended?).to be true
-      expect(@independently_silenced.reload.silenced?).to be true
+      expect(independently_silenced.reload.silenced?).to be true
     end
 
     it 'unsuspends accounts and removes block' do
-      @domain_block.update(severity: :suspend)
+      domain_block.update(severity: :suspend)
 
-      subject.call(@domain_block)
+      subject.call(domain_block)
       expect_deleted_domain_block
-      expect(@suspended.reload.suspended?).to be false
+      expect(suspended.reload.suspended?).to be false
-      expect(@silenced.reload.silenced?).to be false
+      expect(silenced.reload.silenced?).to be false
-      expect(@independently_suspended.reload.suspended?).to be true
+      expect(independently_suspended.reload.suspended?).to be true
-      expect(@independently_silenced.reload.silenced?).to be true
+      expect(independently_silenced.reload.silenced?).to be true
     end
   end
 
   def expect_deleted_domain_block
-    expect { @domain_block.reload }.to raise_error(ActiveRecord::RecordNotFound)
+    expect { domain_block.reload }.to raise_error(ActiveRecord::RecordNotFound)
   end
 end