From 30658924a80434e6a2bceb61267b911ea8d37898 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 28 Mar 2022 12:43:58 +0200 Subject: Fix test-related issues (#17888) * Remove obsolete RSS::Serializer test Since #17828, RSS::Serializer no longer has specific code for deleted statuses, but it is never called on deleted statuses anyway. * Rename erroneously-named test files * Fix failing test * Fix test deprecation warnings * Update CircleCI Ruby orb 1.4.0 has a bug that does not match all the test files due to incorrect globbing --- spec/controllers/admin/accounts_controller_spec.rb | 10 ++--- .../settings/exports/bookmarks_controller_spec.rb | 22 +++++++++++ .../settings/exports/bookmarks_controller_specs.rb | 17 -------- spec/lib/html_aware_formatter.rb | 44 --------------------- spec/lib/html_aware_formatter_spec.rb | 44 +++++++++++++++++++++ spec/lib/rss/serializer_spec.rb | 7 ---- spec/services/after_block_service_spec.rb | 8 ++-- spec/services/delete_account_service_spec.rb | 14 +++---- spec/services/mute_service_spec.rb | 22 ++++------- spec/services/notify_service_spec.rb | 46 +++++++++++----------- spec/services/suspend_account_service_spec.rb | 12 +++--- spec/services/unsuspend_account_service_spec.rb | 26 ++++++------ 12 files changed, 126 insertions(+), 146 deletions(-) create mode 100644 spec/controllers/settings/exports/bookmarks_controller_spec.rb delete mode 100644 spec/controllers/settings/exports/bookmarks_controller_specs.rb delete mode 100644 spec/lib/html_aware_formatter.rb create mode 100644 spec/lib/html_aware_formatter_spec.rb (limited to 'spec') diff --git a/spec/controllers/admin/accounts_controller_spec.rb b/spec/controllers/admin/accounts_controller_spec.rb index 0f71d697c..1779fb7c0 100644 --- a/spec/controllers/admin/accounts_controller_spec.rb +++ b/spec/controllers/admin/accounts_controller_spec.rb @@ -194,9 +194,7 @@ RSpec.describe Admin::AccountsController, type: :controller do end describe 'POST #unblock_email' do - subject do - -> { post :unblock_email, params: { id: account.id } } - end + subject { post :unblock_email, params: { id: account.id } } let(:current_user) { Fabricate(:user, admin: admin) } let(:account) { Fabricate(:account, suspended: true) } @@ -206,11 +204,11 @@ RSpec.describe Admin::AccountsController, type: :controller do let(:admin) { true } it 'succeeds in removing email blocks' do - is_expected.to change { CanonicalEmailBlock.where(reference_account: account).count }.from(1).to(0) + expect { subject }.to change { CanonicalEmailBlock.where(reference_account: account).count }.from(1).to(0) end it 'redirects to admin account path' do - subject.call + subject expect(response).to redirect_to admin_account_path(account.id) end end @@ -219,7 +217,7 @@ RSpec.describe Admin::AccountsController, type: :controller do let(:admin) { false } it 'fails to remove avatar' do - subject.call + subject expect(response).to have_http_status :forbidden end end diff --git a/spec/controllers/settings/exports/bookmarks_controller_spec.rb b/spec/controllers/settings/exports/bookmarks_controller_spec.rb new file mode 100644 index 000000000..a06c02e0c --- /dev/null +++ b/spec/controllers/settings/exports/bookmarks_controller_spec.rb @@ -0,0 +1,22 @@ +require 'rails_helper' + +describe Settings::Exports::BookmarksController do + render_views + + let(:user) { Fabricate(:user) } + let(:account) { Fabricate(:account, domain: 'foo.bar') } + let(:status) { Fabricate(:status, account: account, uri: 'https://foo.bar/statuses/1312') } + + describe 'GET #index' do + before do + user.account.bookmarks.create!(status: status) + end + + it 'returns a csv of the bookmarked toots' do + sign_in user, scope: :user + get :index, format: :csv + + expect(response.body).to eq "https://foo.bar/statuses/1312\n" + end + end +end diff --git a/spec/controllers/settings/exports/bookmarks_controller_specs.rb b/spec/controllers/settings/exports/bookmarks_controller_specs.rb deleted file mode 100644 index 85761577b..000000000 --- a/spec/controllers/settings/exports/bookmarks_controller_specs.rb +++ /dev/null @@ -1,17 +0,0 @@ -require 'rails_helper' - -describe Settings::Exports::BookmarksController do - render_views - - describe 'GET #index' do - it 'returns a csv of the bookmarked toots' do - user = Fabricate(:user) - user.account.bookmarks.create!(status: Fabricate(:status, uri: 'https://foo.bar/statuses/1312')) - - sign_in user, scope: :user - get :index, format: :csv - - expect(response.body).to eq "https://foo.bar/statuses/1312\n" - end - end -end diff --git a/spec/lib/html_aware_formatter.rb b/spec/lib/html_aware_formatter.rb deleted file mode 100644 index 18d23abf5..000000000 --- a/spec/lib/html_aware_formatter.rb +++ /dev/null @@ -1,44 +0,0 @@ -require 'rails_helper' - -RSpec.describe HtmlAwareFormatter do - describe '#to_s' do - subject { described_class.new(text, local).to_s } - - context 'when local' do - let(:local) { true } - let(:text) { 'Foo bar' } - - it 'returns formatted text' do - is_expected.to eq '

Foo bar

' - end - end - - context 'when remote' do - let(:local) { false } - - context 'given plain text' do - let(:text) { 'Beep boop' } - - it 'keeps the plain text' do - is_expected.to include 'Beep boop' - end - end - - context 'given text containing script tags' do - let(:text) { '' } - - it 'strips the scripts' do - is_expected.to_not include '' - end - end - - context 'given text containing malicious classes' do - let(:text) { 'Show more' } - - it 'strips the malicious classes' do - is_expected.to_not include 'status__content__spoiler-link' - end - end - end - end -end diff --git a/spec/lib/html_aware_formatter_spec.rb b/spec/lib/html_aware_formatter_spec.rb new file mode 100644 index 000000000..18d23abf5 --- /dev/null +++ b/spec/lib/html_aware_formatter_spec.rb @@ -0,0 +1,44 @@ +require 'rails_helper' + +RSpec.describe HtmlAwareFormatter do + describe '#to_s' do + subject { described_class.new(text, local).to_s } + + context 'when local' do + let(:local) { true } + let(:text) { 'Foo bar' } + + it 'returns formatted text' do + is_expected.to eq '

Foo bar

' + end + end + + context 'when remote' do + let(:local) { false } + + context 'given plain text' do + let(:text) { 'Beep boop' } + + it 'keeps the plain text' do + is_expected.to include 'Beep boop' + end + end + + context 'given text containing script tags' do + let(:text) { '' } + + it 'strips the scripts' do + is_expected.to_not include '' + end + end + + context 'given text containing malicious classes' do + let(:text) { 'Show more' } + + it 'strips the malicious classes' do + is_expected.to_not include 'status__content__spoiler-link' + end + end + end + end +end diff --git a/spec/lib/rss/serializer_spec.rb b/spec/lib/rss/serializer_spec.rb index 0364d13de..1da45d302 100644 --- a/spec/lib/rss/serializer_spec.rb +++ b/spec/lib/rss/serializer_spec.rb @@ -13,13 +13,6 @@ describe RSS::Serializer do subject { RSS::Serializer.new.send(:status_title, status) } - context 'if destroyed?' do - it 'returns "#{account.acct} deleted status"' do - status.destroy! - expect(subject).to eq "#{account.acct} deleted status" - end - end - context 'on a toot with long text' do let(:text) { "This toot's text is longer than the allowed number of characters" } diff --git a/spec/services/after_block_service_spec.rb b/spec/services/after_block_service_spec.rb index fe5b26b2b..c09425d7c 100644 --- a/spec/services/after_block_service_spec.rb +++ b/spec/services/after_block_service_spec.rb @@ -1,9 +1,7 @@ require 'rails_helper' RSpec.describe AfterBlockService, type: :service do - subject do - -> { described_class.new.call(account, target_account) } - end + subject { described_class.new.call(account, target_account) } let(:account) { Fabricate(:account) } let(:target_account) { Fabricate(:account) } @@ -24,7 +22,7 @@ RSpec.describe AfterBlockService, type: :service do FeedManager.instance.push_to_home(account, other_account_status) FeedManager.instance.push_to_home(account, other_account_reblog) - is_expected.to change { + expect { subject }.to change { Redis.current.zrange(home_timeline_key, 0, -1) }.from([status.id.to_s, other_account_status.id.to_s, other_account_reblog.id.to_s]).to([other_account_status.id.to_s]) end @@ -43,7 +41,7 @@ RSpec.describe AfterBlockService, type: :service do FeedManager.instance.push_to_list(list, other_account_status) FeedManager.instance.push_to_list(list, other_account_reblog) - is_expected.to change { + expect { subject }.to change { Redis.current.zrange(list_timeline_key, 0, -1) }.from([status.id.to_s, other_account_status.id.to_s, other_account_reblog.id.to_s]).to([other_account_status.id.to_s]) end diff --git a/spec/services/delete_account_service_spec.rb b/spec/services/delete_account_service_spec.rb index 9c785fc17..1fbe4d07c 100644 --- a/spec/services/delete_account_service_spec.rb +++ b/spec/services/delete_account_service_spec.rb @@ -23,12 +23,10 @@ RSpec.describe DeleteAccountService, type: :service do let!(:account_note) { Fabricate(:account_note, account: account) } - subject do - -> { described_class.new.call(account) } - end + subject { described_class.new.call(account) } it 'deletes associated owned records' do - is_expected.to change { + expect { subject }.to change { [ account.statuses, account.media_attachments, @@ -43,7 +41,7 @@ RSpec.describe DeleteAccountService, type: :service do end it 'deletes associated target records' do - is_expected.to change { + expect { subject }.to change { [ AccountPin.where(target_account: account), ].map(&:count) @@ -51,7 +49,7 @@ RSpec.describe DeleteAccountService, type: :service do end it 'deletes associated target notifications' do - is_expected.to change { + expect { subject }.to change { [ 'poll', 'favourite', 'status', 'mention', 'follow' ].map { |type| Notification.where(type: type).count } @@ -73,7 +71,7 @@ RSpec.describe DeleteAccountService, type: :service do let!(:local_follower) { Fabricate(:account) } it 'sends a delete actor activity to all known inboxes' do - subject.call + subject expect(a_request(:post, "https://alice.com/inbox")).to have_been_made.once expect(a_request(:post, "https://bob.com/inbox")).to have_been_made.once end @@ -91,7 +89,7 @@ RSpec.describe DeleteAccountService, type: :service do let!(:local_follower) { Fabricate(:account) } it 'sends a reject follow to follower inboxes' do - subject.call + subject expect(a_request(:post, account.inbox_url)).to have_been_made.once end end diff --git a/spec/services/mute_service_spec.rb b/spec/services/mute_service_spec.rb index 4bb839b8d..bdec1c67b 100644 --- a/spec/services/mute_service_spec.rb +++ b/spec/services/mute_service_spec.rb @@ -1,9 +1,7 @@ require 'rails_helper' RSpec.describe MuteService, type: :service do - subject do - -> { described_class.new.call(account, target_account) } - end + subject { described_class.new.call(account, target_account) } let(:account) { Fabricate(:account) } let(:target_account) { Fabricate(:account) } @@ -21,45 +19,41 @@ RSpec.describe MuteService, type: :service do FeedManager.instance.push_to_home(account, status) FeedManager.instance.push_to_home(account, other_account_status) - is_expected.to change { + expect { subject }.to change { Redis.current.zrange(home_timeline_key, 0, -1) }.from([status.id.to_s, other_account_status.id.to_s]).to([other_account_status.id.to_s]) end end it 'mutes account' do - is_expected.to change { + expect { subject }.to change { account.muting?(target_account) }.from(false).to(true) end context 'without specifying a notifications parameter' do it 'mutes notifications from the account' do - is_expected.to change { + expect { subject }.to change { account.muting_notifications?(target_account) }.from(false).to(true) end end context 'with a true notifications parameter' do - subject do - -> { described_class.new.call(account, target_account, notifications: true) } - end + subject { described_class.new.call(account, target_account, notifications: true) } it 'mutes notifications from the account' do - is_expected.to change { + expect { subject }.to change { account.muting_notifications?(target_account) }.from(false).to(true) end end context 'with a false notifications parameter' do - subject do - -> { described_class.new.call(account, target_account, notifications: false) } - end + subject { described_class.new.call(account, target_account, notifications: false) } it 'does not mute notifications from the account' do - is_expected.to_not change { + expect { subject }.to_not change { account.muting_notifications?(target_account) }.from(false) end diff --git a/spec/services/notify_service_spec.rb b/spec/services/notify_service_spec.rb index 7433866b7..294c31b04 100644 --- a/spec/services/notify_service_spec.rb +++ b/spec/services/notify_service_spec.rb @@ -1,9 +1,7 @@ require 'rails_helper' RSpec.describe NotifyService, type: :service do - subject do - -> { described_class.new.call(recipient, type, activity) } - end + subject { described_class.new.call(recipient, type, activity) } let(:user) { Fabricate(:user) } let(:recipient) { user.account } @@ -11,42 +9,42 @@ RSpec.describe NotifyService, type: :service do let(:activity) { Fabricate(:follow, account: sender, target_account: recipient) } let(:type) { :follow } - it { is_expected.to change(Notification, :count).by(1) } + it { expect { subject }.to change(Notification, :count).by(1) } it 'does not notify when sender is blocked' do recipient.block!(sender) - is_expected.to_not change(Notification, :count) + expect { subject }.to_not change(Notification, :count) end it 'does not notify when sender is muted with hide_notifications' do recipient.mute!(sender, notifications: true) - is_expected.to_not change(Notification, :count) + expect { subject }.to_not change(Notification, :count) end it 'does notify when sender is muted without hide_notifications' do recipient.mute!(sender, notifications: false) - is_expected.to change(Notification, :count) + expect { subject }.to change(Notification, :count) end it 'does not notify when sender\'s domain is blocked' do recipient.block_domain!(sender.domain) - is_expected.to_not change(Notification, :count) + expect { subject }.to_not change(Notification, :count) end it 'does still notify when sender\'s domain is blocked but sender is followed' do recipient.block_domain!(sender.domain) recipient.follow!(sender) - is_expected.to change(Notification, :count) + expect { subject }.to change(Notification, :count) end it 'does not notify when sender is silenced and not followed' do sender.silence! - is_expected.to_not change(Notification, :count) + expect { subject }.to_not change(Notification, :count) end it 'does not notify when recipient is suspended' do recipient.suspend! - is_expected.to_not change(Notification, :count) + expect { subject }.to_not change(Notification, :count) end context 'for direct messages' do @@ -61,7 +59,7 @@ RSpec.describe NotifyService, type: :service do let(:enabled) { true } it 'does not notify' do - is_expected.to_not change(Notification, :count) + expect { subject }.to_not change(Notification, :count) end context 'if the message chain is initiated by recipient, but is not direct message' do @@ -70,7 +68,7 @@ RSpec.describe NotifyService, type: :service do let(:activity) { Fabricate(:mention, account: recipient, status: Fabricate(:status, account: sender, visibility: :direct, thread: reply_to)) } it 'does not notify' do - is_expected.to_not change(Notification, :count) + expect { subject }.to_not change(Notification, :count) end end @@ -81,7 +79,7 @@ RSpec.describe NotifyService, type: :service do let(:activity) { Fabricate(:mention, account: recipient, status: Fabricate(:status, account: sender, visibility: :direct, thread: dummy_reply)) } it 'does not notify' do - is_expected.to_not change(Notification, :count) + expect { subject }.to_not change(Notification, :count) end end @@ -91,7 +89,7 @@ RSpec.describe NotifyService, type: :service do let(:activity) { Fabricate(:mention, account: recipient, status: Fabricate(:status, account: sender, visibility: :direct, thread: reply_to)) } it 'does notify' do - is_expected.to change(Notification, :count) + expect { subject }.to change(Notification, :count) end end end @@ -100,7 +98,7 @@ RSpec.describe NotifyService, type: :service do let(:enabled) { false } it 'does notify' do - is_expected.to change(Notification, :count) + expect { subject }.to change(Notification, :count) end end end @@ -112,17 +110,17 @@ RSpec.describe NotifyService, type: :service do it 'shows reblogs by default' do recipient.follow!(sender) - is_expected.to change(Notification, :count) + expect { subject }.to change(Notification, :count) end it 'shows reblogs when explicitly enabled' do recipient.follow!(sender, reblogs: true) - is_expected.to change(Notification, :count) + expect { subject }.to change(Notification, :count) end it 'shows reblogs when disabled' do recipient.follow!(sender, reblogs: false) - is_expected.to change(Notification, :count) + expect { subject }.to change(Notification, :count) end end @@ -134,12 +132,12 @@ RSpec.describe NotifyService, type: :service do it 'does not notify when conversation is muted' do recipient.mute_conversation!(activity.status.conversation) - is_expected.to_not change(Notification, :count) + expect { subject }.to_not change(Notification, :count) end it 'does not notify when it is a reply to a blocked user' do recipient.block!(asshole) - is_expected.to_not change(Notification, :count) + expect { subject }.to_not change(Notification, :count) end end @@ -147,7 +145,7 @@ RSpec.describe NotifyService, type: :service do let(:sender) { recipient } it 'does not notify when recipient is the sender' do - is_expected.to_not change(Notification, :count) + expect { subject }.to_not change(Notification, :count) end end @@ -163,7 +161,7 @@ RSpec.describe NotifyService, type: :service do let(:enabled) { true } it 'sends email' do - is_expected.to change(ActionMailer::Base.deliveries, :count).by(1) + expect { subject }.to change(ActionMailer::Base.deliveries, :count).by(1) end end @@ -171,7 +169,7 @@ RSpec.describe NotifyService, type: :service do let(:enabled) { false } it "doesn't send email" do - is_expected.to_not change(ActionMailer::Base.deliveries, :count).from(0) + expect { subject }.to_not change(ActionMailer::Base.deliveries, :count).from(0) end end end diff --git a/spec/services/suspend_account_service_spec.rb b/spec/services/suspend_account_service_spec.rb index cf7eb257a..5d45e4ffd 100644 --- a/spec/services/suspend_account_service_spec.rb +++ b/spec/services/suspend_account_service_spec.rb @@ -5,9 +5,7 @@ RSpec.describe SuspendAccountService, type: :service do let!(:local_follower) { Fabricate(:user, current_sign_in_at: 1.hour.ago).account } let!(:list) { Fabricate(:list, account: local_follower) } - subject do - -> { described_class.new.call(account) } - end + subject { described_class.new.call(account) } before do allow(FeedManager.instance).to receive(:unmerge_from_home).and_return(nil) @@ -18,13 +16,13 @@ RSpec.describe SuspendAccountService, type: :service do end it "unmerges from local followers' feeds" do - subject.call + subject expect(FeedManager.instance).to have_received(:unmerge_from_home).with(account, local_follower) expect(FeedManager.instance).to have_received(:unmerge_from_list).with(account, list) end it 'marks account as suspended' do - is_expected.to change { account.suspended? }.from(false).to(true) + expect { subject }.to change { account.suspended? }.from(false).to(true) end end @@ -51,7 +49,7 @@ RSpec.describe SuspendAccountService, type: :service do end it 'sends an update actor to followers and reporters' do - subject.call + subject expect(a_request(:post, remote_follower.inbox_url).with { |req| match_update_actor_request(req, account) }).to have_been_made.once expect(a_request(:post, remote_reporter.inbox_url).with { |req| match_update_actor_request(req, account) }).to have_been_made.once end @@ -77,7 +75,7 @@ RSpec.describe SuspendAccountService, type: :service do end it 'sends a reject follow' do - subject.call + subject expect(a_request(:post, account.inbox_url).with { |req| match_reject_follow_request(req, account, local_followee) }).to have_been_made.once end end diff --git a/spec/services/unsuspend_account_service_spec.rb b/spec/services/unsuspend_account_service_spec.rb index 0593beb6f..3ac4cc085 100644 --- a/spec/services/unsuspend_account_service_spec.rb +++ b/spec/services/unsuspend_account_service_spec.rb @@ -5,9 +5,7 @@ RSpec.describe UnsuspendAccountService, type: :service do let!(:local_follower) { Fabricate(:user, current_sign_in_at: 1.hour.ago).account } let!(:list) { Fabricate(:list, account: local_follower) } - subject do - -> { described_class.new.call(account) } - end + subject { described_class.new.call(account) } before do allow(FeedManager.instance).to receive(:merge_into_home).and_return(nil) @@ -33,7 +31,7 @@ RSpec.describe UnsuspendAccountService, type: :service do end it 'marks account as unsuspended' do - is_expected.to change { account.suspended? }.from(true).to(false) + expect { subject }.to change { account.suspended? }.from(true).to(false) end include_examples 'common behavior' do @@ -47,13 +45,13 @@ RSpec.describe UnsuspendAccountService, type: :service do end it "merges back into local followers' feeds" do - subject.call + subject expect(FeedManager.instance).to have_received(:merge_into_home).with(account, local_follower) expect(FeedManager.instance).to have_received(:merge_into_list).with(account, list) end it 'sends an update actor to followers and reporters' do - subject.call + subject expect(a_request(:post, remote_follower.inbox_url).with { |req| match_update_actor_request(req, account) }).to have_been_made.once expect(a_request(:post, remote_reporter.inbox_url).with { |req| match_update_actor_request(req, account) }).to have_been_made.once end @@ -75,18 +73,18 @@ RSpec.describe UnsuspendAccountService, type: :service do end it 're-fetches the account' do - subject.call + subject expect(resolve_account_service).to have_received(:call).with(account) end it "merges back into local followers' feeds" do - subject.call + subject expect(FeedManager.instance).to have_received(:merge_into_home).with(account, local_follower) expect(FeedManager.instance).to have_received(:merge_into_list).with(account, list) end it 'marks account as unsuspended' do - is_expected.to change { account.suspended? }.from(true).to(false) + expect { subject }.to change { account.suspended? }.from(true).to(false) end end @@ -99,18 +97,18 @@ RSpec.describe UnsuspendAccountService, type: :service do end it 're-fetches the account' do - subject.call + subject expect(resolve_account_service).to have_received(:call).with(account) end it "does not merge back into local followers' feeds" do - subject.call + subject expect(FeedManager.instance).to_not have_received(:merge_into_home).with(account, local_follower) expect(FeedManager.instance).to_not have_received(:merge_into_list).with(account, list) end it 'does not mark the account as unsuspended' do - is_expected.not_to change { account.suspended? } + expect { subject }.not_to change { account.suspended? } end end @@ -120,12 +118,12 @@ RSpec.describe UnsuspendAccountService, type: :service do end it 're-fetches the account' do - subject.call + subject expect(resolve_account_service).to have_received(:call).with(account) end it "does not merge back into local followers' feeds" do - subject.call + subject expect(FeedManager.instance).to_not have_received(:merge_into_home).with(account, local_follower) expect(FeedManager.instance).to_not have_received(:merge_into_list).with(account, list) end -- cgit