From 5b595b8a5a0872a5c4eaaca83a60ba7b3652e847 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 3 Sep 2024 11:23:16 -0400 Subject: [PATCH] Remove usage of `assigns` in controller specs (#30195) --- app/views/settings/imports/index.html.haml | 2 +- .../admin/accounts_controller_spec.rb | 11 ++++---- .../admin/domain_allows_controller_spec.rb | 1 - .../admin/domain_blocks_controller_spec.rb | 2 -- .../export_domain_blocks_controller_spec.rb | 22 +++++++++------- .../admin/instances_controller_spec.rb | 9 ++++--- .../admin/invites_controller_spec.rb | 3 ++- .../admin/reports_controller_spec.rb | 26 +++++++++---------- .../authorize_interactions_controller_spec.rb | 10 ++++--- .../account_controller_concern_spec.rb | 7 ++--- .../settings/imports_controller_spec.rb | 8 +++--- .../confirmations_controller_spec.rb | 13 +++++++--- .../recovery_codes_controller_spec.rb | 3 ++- 13 files changed, 66 insertions(+), 51 deletions(-) diff --git a/app/views/settings/imports/index.html.haml b/app/views/settings/imports/index.html.haml index ca815720f..bfddd4546 100644 --- a/app/views/settings/imports/index.html.haml +++ b/app/views/settings/imports/index.html.haml @@ -46,7 +46,7 @@ %th= t('imports.failures') %tbody - @recent_imports.each do |import| - %tr + %tr{ id: dom_id(import) } %td= t("imports.types.#{import.type}") %td - if import.state_unconfirmed? diff --git a/spec/controllers/admin/accounts_controller_spec.rb b/spec/controllers/admin/accounts_controller_spec.rb index f241d261b..89a7239f5 100644 --- a/spec/controllers/admin/accounts_controller_spec.rb +++ b/spec/controllers/admin/accounts_controller_spec.rb @@ -40,15 +40,16 @@ RSpec.describe Admin::AccountsController do expect(response) .to have_http_status(200) - expect(assigns(:accounts)) - .to have_attributes( - count: eq(1), - klass: be(Account) - ) + expect(accounts_table_rows.size) + .to eq(1) expect(AccountFilter) .to have_received(:new) .with(hash_including(params)) end + + def accounts_table_rows + Nokogiri::Slop(response.body).css('table.accounts-table tr') + end end describe 'GET #show' do diff --git a/spec/controllers/admin/domain_allows_controller_spec.rb b/spec/controllers/admin/domain_allows_controller_spec.rb index 6f82f322b..036d22909 100644 --- a/spec/controllers/admin/domain_allows_controller_spec.rb +++ b/spec/controllers/admin/domain_allows_controller_spec.rb @@ -13,7 +13,6 @@ RSpec.describe Admin::DomainAllowsController do it 'assigns a new domain allow' do get :new - expect(assigns(:domain_allow)).to be_instance_of(DomainAllow) expect(response).to have_http_status(200) end end diff --git a/spec/controllers/admin/domain_blocks_controller_spec.rb b/spec/controllers/admin/domain_blocks_controller_spec.rb index eb2c6265d..a99ca6c64 100644 --- a/spec/controllers/admin/domain_blocks_controller_spec.rb +++ b/spec/controllers/admin/domain_blocks_controller_spec.rb @@ -13,7 +13,6 @@ RSpec.describe Admin::DomainBlocksController do it 'assigns a new domain block' do get :new - expect(assigns(:domain_block)).to be_instance_of(DomainBlock) expect(response).to have_http_status(200) end end @@ -171,7 +170,6 @@ RSpec.describe Admin::DomainBlocksController do it 'returns http success' do get :edit, params: { id: domain_block.id } - expect(assigns(:domain_block)).to be_instance_of(DomainBlock) expect(response).to have_http_status(200) end end diff --git a/spec/controllers/admin/export_domain_blocks_controller_spec.rb b/spec/controllers/admin/export_domain_blocks_controller_spec.rb index bfcccfa06..39195716c 100644 --- a/spec/controllers/admin/export_domain_blocks_controller_spec.rb +++ b/spec/controllers/admin/export_domain_blocks_controller_spec.rb @@ -42,11 +42,8 @@ RSpec.describe Admin::ExportDomainBlocksController do post :import, params: { admin_import: { data: fixture_file_upload('domain_blocks.csv') } } end - it 'renders page with expected domain blocks' do - expect(assigns(:domain_blocks).map { |block| [block.domain, block.severity.to_sym] }).to contain_exactly(['bad.domain', :silence], ['worse.domain', :suspend], ['reject.media', :noop]) - end - - it 'returns http success' do + it 'renders page with expected domain blocks and returns http success' do + expect(mapped_batch_table_rows).to contain_exactly(['bad.domain', :silence], ['worse.domain', :suspend], ['reject.media', :noop]) expect(response).to have_http_status(200) end end @@ -56,14 +53,19 @@ RSpec.describe Admin::ExportDomainBlocksController do post :import, params: { admin_import: { data: fixture_file_upload('domain_blocks_list.txt') } } end - it 'renders page with expected domain blocks' do - expect(assigns(:domain_blocks).map { |block| [block.domain, block.severity.to_sym] }).to contain_exactly(['bad.domain', :suspend], ['worse.domain', :suspend], ['reject.media', :suspend]) - end - - it 'returns http success' do + it 'renders page with expected domain blocks and returns http success' do + expect(mapped_batch_table_rows).to contain_exactly(['bad.domain', :suspend], ['worse.domain', :suspend], ['reject.media', :suspend]) expect(response).to have_http_status(200) end end + + def mapped_batch_table_rows + batch_table_rows.map { |row| [row.at_css('[id$=_domain]')['value'], row.at_css('[id$=_severity]')['value'].to_sym] } + end + + def batch_table_rows + Nokogiri::Slop(response.body).css('body div.batch-table__row') + end end it 'displays error on no file selected' do diff --git a/spec/controllers/admin/instances_controller_spec.rb b/spec/controllers/admin/instances_controller_spec.rb index ca64dd90a..a64bbb2c9 100644 --- a/spec/controllers/admin/instances_controller_spec.rb +++ b/spec/controllers/admin/instances_controller_spec.rb @@ -28,12 +28,15 @@ RSpec.describe Admin::InstancesController do it 'renders instances' do get :index, params: { page: 2 } - instances = assigns(:instances).to_a - expect(instances.size).to eq 1 - expect(instances[0].domain).to eq 'less.popular' + expect(instance_directory_links.size).to eq(1) + expect(instance_directory_links.first.text.strip).to match('less.popular') expect(response).to have_http_status(200) end + + def instance_directory_links + Nokogiri::Slop(response.body).css('div.directory__tag a') + end end describe 'GET #show' do diff --git a/spec/controllers/admin/invites_controller_spec.rb b/spec/controllers/admin/invites_controller_spec.rb index 71748cbbe..8638f8e21 100644 --- a/spec/controllers/admin/invites_controller_spec.rb +++ b/spec/controllers/admin/invites_controller_spec.rb @@ -18,7 +18,8 @@ describe Admin::InvitesController do it 'renders index page' do expect(subject).to render_template :index - expect(assigns(:invites)).to include invite + expect(response.body) + .to include(invite.code) end end diff --git a/spec/controllers/admin/reports_controller_spec.rb b/spec/controllers/admin/reports_controller_spec.rb index 5849163b5..67fb28e7a 100644 --- a/spec/controllers/admin/reports_controller_spec.rb +++ b/spec/controllers/admin/reports_controller_spec.rb @@ -13,39 +13,39 @@ describe Admin::ReportsController do describe 'GET #index' do it 'returns http success with no filters' do - specified = Fabricate(:report, action_taken_at: nil) - Fabricate(:report, action_taken_at: Time.now.utc) + specified = Fabricate(:report, action_taken_at: nil, comment: 'First report') + other = Fabricate(:report, action_taken_at: Time.now.utc, comment: 'Second report') get :index - reports = assigns(:reports).to_a - expect(reports.size).to eq 1 - expect(reports[0]).to eq specified expect(response).to have_http_status(200) + expect(response.body) + .to include(specified.comment) + .and not_include(other.comment) end it 'returns http success with resolved filter' do - specified = Fabricate(:report, action_taken_at: Time.now.utc) - Fabricate(:report, action_taken_at: nil) + specified = Fabricate(:report, action_taken_at: Time.now.utc, comment: 'First report') + other = Fabricate(:report, action_taken_at: nil, comment: 'Second report') get :index, params: { resolved: '1' } - reports = assigns(:reports).to_a - expect(reports.size).to eq 1 - expect(reports[0]).to eq specified - expect(response).to have_http_status(200) + expect(response.body) + .to include(specified.comment) + .and not_include(other.comment) end end describe 'GET #show' do it 'renders report' do - report = Fabricate(:report) + report = Fabricate(:report, comment: 'A big problem') get :show, params: { id: report } - expect(assigns(:report)).to eq report expect(response).to have_http_status(200) + expect(response.body) + .to include(report.comment) end end diff --git a/spec/controllers/authorize_interactions_controller_spec.rb b/spec/controllers/authorize_interactions_controller_spec.rb index 5282a196a..ed55df08d 100644 --- a/spec/controllers/authorize_interactions_controller_spec.rb +++ b/spec/controllers/authorize_interactions_controller_spec.rb @@ -46,8 +46,9 @@ describe AuthorizeInteractionsController do get :show, params: { acct: 'http://example.com' } - expect(response).to have_http_status(302) - expect(assigns(:resource)).to eq account + expect(response) + .to have_http_status(302) + .and redirect_to(web_url("@#{account.pretty_acct}")) end it 'sets resource from acct uri' do @@ -58,8 +59,9 @@ describe AuthorizeInteractionsController do get :show, params: { acct: 'acct:found@hostname' } - expect(response).to have_http_status(302) - expect(assigns(:resource)).to eq account + expect(response) + .to have_http_status(302) + .and redirect_to(web_url("@#{account.pretty_acct}")) end end end diff --git a/spec/controllers/concerns/account_controller_concern_spec.rb b/spec/controllers/concerns/account_controller_concern_spec.rb index 6eb970ded..122ef21e9 100644 --- a/spec/controllers/concerns/account_controller_concern_spec.rb +++ b/spec/controllers/concerns/account_controller_concern_spec.rb @@ -7,7 +7,7 @@ describe AccountControllerConcern do include AccountControllerConcern def success - head 200 + render plain: @account.username # rubocop:disable RSpec/InstanceVariable end end @@ -51,12 +51,13 @@ describe AccountControllerConcern do context 'when account is not suspended' do let(:account) { Fabricate(:account, username: 'username') } - it 'assigns @account, returns success, and sets link headers' do + it 'Prepares the account, returns success, and sets link headers' do get 'success', params: { account_username: account.username } - expect(assigns(:account)).to eq account expect(response).to have_http_status(200) expect(response.headers['Link'].to_s).to eq(expected_link_headers) + expect(response.body) + .to include(account.username) end def expected_link_headers diff --git a/spec/controllers/settings/imports_controller_spec.rb b/spec/controllers/settings/imports_controller_spec.rb index 89ec39e54..219b882e6 100644 --- a/spec/controllers/settings/imports_controller_spec.rb +++ b/spec/controllers/settings/imports_controller_spec.rb @@ -21,9 +21,10 @@ RSpec.describe Settings::ImportsController do it 'assigns the expected imports', :aggregate_failures do expect(response).to have_http_status(200) - expect(assigns(:recent_imports)).to eq [import] - expect(assigns(:recent_imports)).to_not include(other_import) expect(response.headers['Cache-Control']).to include('private, no-store') + expect(response.body) + .to include("bulk_import_#{import.id}") + .and not_include("bulk_import_#{other_import.id}") end end @@ -261,7 +262,8 @@ RSpec.describe Settings::ImportsController do it 'does not creates an unconfirmed bulk_import', :aggregate_failures do expect { subject }.to_not(change { user.account.bulk_imports.count }) - expect(assigns(:import).errors).to_not be_empty + expect(response.body) + .to include('field_with_errors') end 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 1b3b0cb0a..1c8b483a0 100644 --- a/spec/controllers/settings/two_factor_authentication/confirmations_controller_spec.rb +++ b/spec/controllers/settings/two_factor_authentication/confirmations_controller_spec.rb @@ -9,11 +9,16 @@ describe Settings::TwoFactorAuthentication::ConfirmationsController do it 'renders the new view' do subject - expect(assigns(:confirmation)).to be_instance_of Form::TwoFactorConfirmation - expect(assigns(:provision_url)).to eq 'otpauth://totp/cb6e6126.ngrok.io:local-part%40domain?secret=thisisasecretforthespecofnewview&issuer=cb6e6126.ngrok.io' - expect(assigns(:qrcode)).to be_instance_of RQRCode::QRCode expect(response).to have_http_status(200) expect(response).to render_template(:new) + expect(response.body) + .to include(qr_code_markup) + end + + def qr_code_markup + RQRCode::QRCode.new( + 'otpauth://totp/cb6e6126.ngrok.io:local-part%40domain?secret=thisisasecretforthespecofnewview&issuer=cb6e6126.ngrok.io' + ).as_svg(padding: 0, module_size: 4) end end @@ -61,10 +66,10 @@ describe Settings::TwoFactorAuthentication::ConfirmationsController do expect { post_create_with_options } .to change { user.reload.otp_secret }.to 'thisisasecretforthespecofnewview' - expect(assigns(:recovery_codes)).to eq otp_backup_codes expect(flash[:notice]).to eq 'Two-factor authentication successfully enabled' expect(response).to have_http_status(200) expect(response).to render_template('settings/two_factor_authentication/recovery_codes/index') + expect(response.body).to include(*otp_backup_codes) end end 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 28a40e138..dbc2e3059 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 @@ -15,10 +15,11 @@ describe Settings::TwoFactorAuthentication::RecoveryCodesController do sign_in user, scope: :user post :create, session: { challenge_passed_at: Time.now.utc } - expect(assigns(:recovery_codes)).to eq otp_backup_codes expect(flash[:notice]).to eq 'Recovery codes successfully regenerated' expect(response).to have_http_status(200) expect(response).to render_template(:index) + expect(response.body) + .to include(*otp_backup_codes) end it 'redirects when not signed in' do