Skip to content

Commit

Permalink
Fix RSpec/InstanceVariable cop (mastodon#27766)
Browse files Browse the repository at this point in the history
  • Loading branch information
mjankowski authored Nov 8, 2023
1 parent 4329616 commit 69d00e2
Show file tree
Hide file tree
Showing 11 changed files with 97 additions and 124 deletions.
20 changes: 0 additions & 20 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
9 changes: 7 additions & 2 deletions spec/controllers/api/v1/streaming_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
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
Expand All @@ -38,7 +37,13 @@
[: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
Expand Down
16 changes: 9 additions & 7 deletions spec/controllers/auth/passwords_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@

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
Expand All @@ -38,9 +40,9 @@

describe 'POST #update' do
let(:user) { Fabricate(:user) }
let(:password) { 'reset0password' }

before do
@password = 'reset0password'
request.env['devise.mapping'] = Devise.mappings[:user]
end

Expand All @@ -50,9 +52,9 @@
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
Expand All @@ -63,7 +65,7 @@
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
Expand All @@ -81,7 +83,7 @@

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
Expand Down
4 changes: 2 additions & 2 deletions spec/controllers/concerns/export_controller_concern_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def index
end

def export_data
@export.account.username
'body data value'
end
end

Expand All @@ -24,7 +24,7 @@ def export_data
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
Expand Down
11 changes: 6 additions & 5 deletions spec/controllers/statuses_cleanup_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@
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
Expand All @@ -30,9 +31,9 @@
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.keep_direct).to be false
expect(@user.account.statuses_cleanup_policy.keep_polls).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_polls).to be true
end

it 'redirects' do
Expand Down
20 changes: 8 additions & 12 deletions spec/models/concerns/account_finder_concern_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,15 @@

describe AccountFinderConcern do
describe 'local finders' do
before do
@account = Fabricate(:account, username: 'Alice')
end
let!(:account) { Fabricate(:account, username: 'Alice') }

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
Expand All @@ -36,7 +34,7 @@

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
Expand All @@ -54,17 +52,15 @@
end

describe 'remote finders' do
before do
@account = Fabricate(:account, username: 'Alice', domain: 'mastodon.social')
end
let!(:account) { Fabricate(:account, username: 'Alice', domain: 'mastodon.social') }

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
Expand All @@ -90,7 +86,7 @@

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
Expand Down
18 changes: 8 additions & 10 deletions spec/models/concerns/account_interactions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -648,38 +648,36 @@
end

describe 'ignoring reblogs from an account' do
before do
@me = Fabricate(:account, username: 'Me')
@you = Fabricate(:account, username: 'You')
end
let!(:me) { Fabricate(:account, username: 'Me') }
let!(:you) { Fabricate(:account, username: 'You') }

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
Expand Down
20 changes: 9 additions & 11 deletions spec/models/public_feed_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -137,47 +137,45 @@
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
@account = Fabricate(:account)
end
let!(:account) { Fabricate(:account) }

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

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

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

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

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')
Expand All @@ -188,7 +186,7 @@
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')
Expand All @@ -198,7 +196,7 @@
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')
Expand Down
Loading

0 comments on commit 69d00e2

Please sign in to comment.