From 8a3d8c6c14f4e9b57967104286fa6eaa305e6412 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Wed, 13 Dec 2023 05:14:19 -0500 Subject: [PATCH] Remove the `stub_stdout` wrapper around CLI specs (#28340) --- lib/mastodon/cli/search.rb | 12 ++- spec/lib/mastodon/cli/accounts_spec.rb | 138 ++++++++++++++++-------- spec/lib/mastodon/cli/ip_blocks_spec.rb | 6 +- spec/lib/mastodon/cli/settings_spec.rb | 3 +- spec/rails_helper.rb | 9 -- 5 files changed, 110 insertions(+), 58 deletions(-) diff --git a/lib/mastodon/cli/search.rb b/lib/mastodon/cli/search.rb index 25a595aadd..77c455f049 100644 --- a/lib/mastodon/cli/search.rb +++ b/lib/mastodon/cli/search.rb @@ -42,7 +42,13 @@ module Mastodon::CLI pool = Concurrent::FixedThreadPool.new(options[:concurrency], max_queue: options[:concurrency] * 10) importers = indices.index_with { |index| "Importer::#{index.name}Importer".constantize.new(batch_size: options[:batch_size], executor: pool) } - progress = ProgressBar.create(total: nil, format: '%t%c/%u |%b%i| %e (%r docs/s)', autofinish: false) + progress = ProgressBar.create( + { + total: nil, + format: '%t%c/%u |%b%i| %e (%r docs/s)', + autofinish: false, + }.merge(progress_output_options) + ) Chewy::Stash::Specification.reset! if options[:reset_chewy] @@ -116,5 +122,9 @@ module Mastodon::CLI say('Cannot run with this batch_size setting, must be at least 1', :red) exit(1) end + + def progress_output_options + Rails.env.test? ? { output: ProgressBar::Outputs::Null } : {} + end end end diff --git a/spec/lib/mastodon/cli/accounts_spec.rb b/spec/lib/mastodon/cli/accounts_spec.rb index 563f6e877d..64df29ebb3 100644 --- a/spec/lib/mastodon/cli/accounts_spec.rb +++ b/spec/lib/mastodon/cli/accounts_spec.rb @@ -77,7 +77,8 @@ describe Mastodon::CLI::Accounts do it_behaves_like 'a new user with given email address and username' it 'creates a new user with confirmed status' do - subject + expect { subject } + .to output_results('New password') user = User.find_by(email: options[:email]) @@ -95,7 +96,8 @@ describe Mastodon::CLI::Accounts do it_behaves_like 'a new user with given email address and username' it 'creates a new user with approved status' do - subject + expect { subject } + .to output_results('New password') user = User.find_by(email: options[:email]) @@ -111,7 +113,8 @@ describe Mastodon::CLI::Accounts do it_behaves_like 'a new user with given email address and username' it 'creates a new user and assigns the specified role' do - subject + expect { subject } + .to output_results('New password') role = User.find_by(email: options[:email])&.role @@ -148,7 +151,8 @@ describe Mastodon::CLI::Accounts do let(:options) { { email: 'tootctl_new@example.com', reattach: true, force: true } } it 'reattaches the account to the new user and deletes the previous user' do - subject + expect { subject } + .to output_results('New password') user = Account.find_local('tootctl_username')&.user @@ -220,7 +224,8 @@ describe Mastodon::CLI::Accounts do let(:options) { { role: default_role.name } } it "updates the user's role to the specified role" do - subject + expect { subject } + .to output_results('OK') role = user.reload.role @@ -235,7 +240,8 @@ describe Mastodon::CLI::Accounts do let(:user) { Fabricate(:user, role: role) } it "removes the user's role successfully" do - subject + expect { subject } + .to output_results('OK') role = user.reload.role @@ -248,13 +254,15 @@ describe Mastodon::CLI::Accounts do let(:options) { { email: 'new_email@email.com' } } it "sets the user's unconfirmed email to the provided email address" do - subject + expect { subject } + .to output_results('OK') expect(user.reload.unconfirmed_email).to eq(options[:email]) end it "does not update the user's original email address" do - subject + expect { subject } + .to output_results('OK') expect(user.reload.email).to eq('old_email@email.com') end @@ -264,13 +272,15 @@ describe Mastodon::CLI::Accounts do let(:options) { { email: 'new_email@email.com', confirm: true } } it "updates the user's email address to the provided email" do - subject + expect { subject } + .to output_results('OK') expect(user.reload.email).to eq(options[:email]) end it "sets the user's email address as confirmed" do - subject + expect { subject } + .to output_results('OK') expect(user.reload.confirmed?).to be(true) end @@ -282,7 +292,8 @@ describe Mastodon::CLI::Accounts do let(:options) { { confirm: true } } it "confirms the user's email address" do - subject + expect { subject } + .to output_results('OK') expect(user.reload.confirmed?).to be(true) end @@ -297,7 +308,9 @@ describe Mastodon::CLI::Accounts do end it 'approves the user' do - expect { subject }.to change { user.reload.approved }.from(false).to(true) + expect { subject } + .to output_results('OK') + .and change { user.reload.approved }.from(false).to(true) end end @@ -306,7 +319,9 @@ describe Mastodon::CLI::Accounts do let(:options) { { disable: true } } it 'disables the user' do - expect { subject }.to change { user.reload.disabled }.from(false).to(true) + expect { subject } + .to output_results('OK') + .and change { user.reload.disabled }.from(false).to(true) end end @@ -315,7 +330,9 @@ describe Mastodon::CLI::Accounts do let(:options) { { enable: true } } it 'enables the user' do - expect { subject }.to change { user.reload.disabled }.from(true).to(false) + expect { subject } + .to output_results('OK') + .and change { user.reload.disabled }.from(true).to(false) end end @@ -335,7 +352,9 @@ describe Mastodon::CLI::Accounts do let(:options) { { disable_2fa: true } } it 'disables the two-factor authentication for the user' do - expect { subject }.to change { user.reload.otp_required_for_login }.from(true).to(false) + expect { subject } + .to output_results('OK') + .and change { user.reload.otp_required_for_login }.from(true).to(false) end end @@ -385,7 +404,8 @@ describe Mastodon::CLI::Accounts do let(:arguments) { [account.username] } it 'deletes the specified user successfully' do - subject + expect { subject } + .to output_results('Deleting') expect(delete_account_service).to have_received(:call).with(account, reserve_email: false).once end @@ -415,7 +435,8 @@ describe Mastodon::CLI::Accounts do let(:options) { { email: account.user.email } } it 'deletes the specified user successfully' do - subject + expect { subject } + .to output_results('Deleting') expect(delete_account_service).to have_received(:call).with(account, reserve_email: false).once end @@ -457,7 +478,8 @@ describe Mastodon::CLI::Accounts do let(:options) { { all: true } } it 'approves all pending registrations' do - subject + expect { subject } + .to output_results('OK') expect(User.pluck(:approved).all?(true)).to be(true) end @@ -468,7 +490,8 @@ describe Mastodon::CLI::Accounts do let(:options) { { number: 2 } } it 'approves the earliest n pending registrations but not the remaining ones' do - subject + expect { subject } + .to output_results('OK') expect(n_earliest_pending_registrations.all?(&:approved?)).to be(true) expect(pending_registrations.all?(&:approved?)).to be(false) @@ -498,7 +521,7 @@ describe Mastodon::CLI::Accounts do it 'approves all users and does not raise any error' do expect { subject } - .to_not raise_error + .to output_results('OK') expect(User.pluck(:approved).all?(true)).to be(true) end end @@ -510,7 +533,8 @@ describe Mastodon::CLI::Accounts do let(:arguments) { [user.account.username] } it 'approves the specified user successfully' do - subject + expect { subject } + .to output_results('OK') expect(user.reload.approved?).to be(true) end @@ -655,7 +679,8 @@ describe Mastodon::CLI::Accounts do allow(remote_account_example_com).to receive(:reset_avatar!) allow(account_example_net).to receive(:reset_avatar!) - cli.refresh + expect { cli.refresh } + .to output_results('Refreshed 2 accounts') expect(cli).to have_received(:parallelize_with_progress).with(scope).once expect(remote_account_example_com).to have_received(:reset_avatar!).once @@ -665,7 +690,8 @@ describe Mastodon::CLI::Accounts do it 'does not refresh avatar for local accounts' do allow(local_account).to receive(:reset_avatar!) - cli.refresh + expect { cli.refresh } + .to output_results('Refreshed 2 accounts') expect(cli).to have_received(:parallelize_with_progress).with(scope).once expect(local_account).to_not have_received(:reset_avatar!) @@ -675,7 +701,8 @@ describe Mastodon::CLI::Accounts do allow(remote_account_example_com).to receive(:reset_header!) allow(account_example_net).to receive(:reset_header!) - cli.refresh + expect { cli.refresh } + .to output_results('Refreshed 2 accounts') expect(cli).to have_received(:parallelize_with_progress).with(scope).once expect(remote_account_example_com).to have_received(:reset_header!).once @@ -685,7 +712,8 @@ describe Mastodon::CLI::Accounts do it 'does not refresh the header for local accounts' do allow(local_account).to receive(:reset_header!) - cli.refresh + expect { cli.refresh } + .to output_results('Refreshed 2 accounts') expect(cli).to have_received(:parallelize_with_progress).with(scope).once expect(local_account).to_not have_received(:reset_header!) @@ -706,7 +734,8 @@ describe Mastodon::CLI::Accounts do allow(remote_account_example_com).to receive(:reset_avatar!) allow(account_example_net).to receive(:reset_avatar!) - cli.refresh + expect { cli.refresh } + .to output_results('Refreshed 2 accounts') expect(cli).to have_received(:parallelize_with_progress).with(scope).once expect(local_account).to_not have_received(:reset_avatar!) @@ -719,7 +748,8 @@ describe Mastodon::CLI::Accounts do allow(remote_account_example_com).to receive(:reset_header!) allow(account_example_net).to receive(:reset_header!) - cli.refresh + expect { cli.refresh } + .to output_results('Refreshed 2 accounts') expect(cli).to have_received(:parallelize_with_progress).with(scope).once expect(local_account).to_not have_received(:reset_header!) @@ -752,7 +782,8 @@ describe Mastodon::CLI::Accounts do allow(account_example_com_a).to receive(:reset_avatar!) allow(account_example_com_b).to receive(:reset_avatar!) - cli.refresh(*arguments) + expect { cli.refresh(*arguments) } + .to output_results('OK') expect(account_example_com_a).to have_received(:reset_avatar!).once expect(account_example_com_b).to have_received(:reset_avatar!).once @@ -761,7 +792,8 @@ describe Mastodon::CLI::Accounts do it 'does not reset the avatar for unspecified accounts' do allow(account_example_net).to receive(:reset_avatar!) - cli.refresh(*arguments) + expect { cli.refresh(*arguments) } + .to output_results('OK') expect(account_example_net).to_not have_received(:reset_avatar!) end @@ -770,7 +802,8 @@ describe Mastodon::CLI::Accounts do allow(account_example_com_a).to receive(:reset_header!) allow(account_example_com_b).to receive(:reset_header!) - cli.refresh(*arguments) + expect { cli.refresh(*arguments) } + .to output_results('OK') expect(account_example_com_a).to have_received(:reset_header!).once expect(account_example_com_b).to have_received(:reset_header!).once @@ -779,7 +812,8 @@ describe Mastodon::CLI::Accounts do it 'does not reset the header for unspecified accounts' do allow(account_example_net).to receive(:reset_header!) - cli.refresh(*arguments) + expect { cli.refresh(*arguments) } + .to output_results('OK') expect(account_example_net).to_not have_received(:reset_header!) end @@ -812,7 +846,8 @@ describe Mastodon::CLI::Accounts do allow(account_example_com_a).to receive(:reset_avatar!) allow(account_example_com_b).to receive(:reset_avatar!) - cli.refresh(*arguments) + expect { cli.refresh(*arguments) } + .to output_results('OK (DRY RUN)') expect(account_example_com_a).to_not have_received(:reset_avatar!) expect(account_example_com_b).to_not have_received(:reset_avatar!) @@ -822,7 +857,8 @@ describe Mastodon::CLI::Accounts do allow(account_example_com_a).to receive(:reset_header!) allow(account_example_com_b).to receive(:reset_header!) - cli.refresh(*arguments) + expect { cli.refresh(*arguments) } + .to output_results('OK (DRY RUN)') expect(account_example_com_a).to_not have_received(:reset_header!) expect(account_example_com_b).to_not have_received(:reset_header!) @@ -848,7 +884,8 @@ describe Mastodon::CLI::Accounts do allow(account_example_com_a).to receive(:reset_avatar!) allow(account_example_com_b).to receive(:reset_avatar!) - cli.refresh + expect { cli.refresh } + .to output_results('Refreshed 2 accounts') expect(cli).to have_received(:parallelize_with_progress).with(scope).once expect(account_example_com_a).to have_received(:reset_avatar!).once @@ -858,7 +895,8 @@ describe Mastodon::CLI::Accounts do it 'does not refresh the avatar for accounts outside specified domain' do allow(account_example_net).to receive(:reset_avatar!) - cli.refresh + expect { cli.refresh } + .to output_results('Refreshed 2 accounts') expect(cli).to have_received(:parallelize_with_progress).with(scope).once expect(account_example_net).to_not have_received(:reset_avatar!) @@ -868,7 +906,8 @@ describe Mastodon::CLI::Accounts do allow(account_example_com_a).to receive(:reset_header!) allow(account_example_com_b).to receive(:reset_header!) - cli.refresh + expect { cli.refresh } + .to output_results('Refreshed 2 accounts') expect(cli).to have_received(:parallelize_with_progress).with(scope) expect(account_example_com_a).to have_received(:reset_header!).once @@ -878,7 +917,8 @@ describe Mastodon::CLI::Accounts do it 'does not refresh the header for accounts outside specified domain' do allow(account_example_net).to receive(:reset_header!) - cli.refresh + expect { cli.refresh } + .to output_results('Refreshed 2 accounts') expect(cli).to have_received(:parallelize_with_progress).with(scope).once expect(account_example_net).to_not have_received(:reset_header!) @@ -913,7 +953,8 @@ describe Mastodon::CLI::Accounts do old_private_key = account.private_key old_public_key = account.public_key - subject + expect { subject } + .to output_results('OK') account.reload expect(account.private_key).to_not eq(old_private_key) @@ -923,7 +964,8 @@ describe Mastodon::CLI::Accounts do it 'broadcasts the new keys for the specified account' do allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_in) - subject + expect { subject } + .to output_results('OK') expect(ActivityPub::UpdateDistributionWorker).to have_received(:perform_in).with(anything, account.id, anything).once end @@ -947,7 +989,8 @@ describe Mastodon::CLI::Accounts do old_private_keys = accounts.map(&:private_key) old_public_keys = accounts.map(&:public_key) - subject + expect { subject } + .to output_results('rotated') accounts.each(&:reload) expect(accounts.map(&:private_key)).to_not eq(old_private_keys) @@ -957,7 +1000,8 @@ describe Mastodon::CLI::Accounts do it 'broadcasts the new keys for each account' do allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_in) - subject + expect { subject } + .to output_results('rotated') accounts.each do |account| expect(ActivityPub::UpdateDistributionWorker).to have_received(:perform_in).with(anything, account.id, anything).once @@ -1036,7 +1080,8 @@ describe Mastodon::CLI::Accounts do end it 'merges `from_account` into `to_account` and deletes `from_account`' do - subject + expect { subject } + .to output_results('OK') expect(to_account).to have_received(:merge_with!).with(from_account).once expect(from_account).to have_received(:destroy).once @@ -1059,7 +1104,8 @@ describe Mastodon::CLI::Accounts do end it 'merges "from_account" into "to_account" and deletes from_account' do - subject + expect { subject } + .to output_results('OK') expect(to_account).to have_received(:merge_with!).with(from_account).once expect(from_account).to have_received(:destroy) @@ -1339,7 +1385,8 @@ describe Mastodon::CLI::Accounts do shared_examples 'a successful migration' do it 'calls the MoveService for the last migration' do - subject + expect { subject } + .to output_results('OK') last_migration = source_account.migrations.last @@ -1449,7 +1496,8 @@ describe Mastodon::CLI::Accounts do end it 'creates a migration for the specified account with the target account' do - subject + expect { subject } + .to output_results('migrated') last_migration = source_account.migrations.last diff --git a/spec/lib/mastodon/cli/ip_blocks_spec.rb b/spec/lib/mastodon/cli/ip_blocks_spec.rb index 1d6c47268e..82be10813e 100644 --- a/spec/lib/mastodon/cli/ip_blocks_spec.rb +++ b/spec/lib/mastodon/cli/ip_blocks_spec.rb @@ -78,7 +78,8 @@ describe Mastodon::CLI::IpBlocks do it 'overwrites the existing IP block record' do expect { subject } - .to change { blocked_ip.reload.severity } + .to output_results('Added 11') + .and change { blocked_ip.reload.severity } .from('no_access') .to('sign_up_requires_approval') end @@ -189,7 +190,8 @@ describe Mastodon::CLI::IpBlocks do let(:options) { { force: true } } it 'removes blocks for IP ranges that cover given IP(s) and keeps other ranges' do - subject + expect { subject } + .to output_results('Removed 2') expect(covered_ranges).to_not exist expect(other_ranges).to exist diff --git a/spec/lib/mastodon/cli/settings_spec.rb b/spec/lib/mastodon/cli/settings_spec.rb index 568ee00393..e1b353eb90 100644 --- a/spec/lib/mastodon/cli/settings_spec.rb +++ b/spec/lib/mastodon/cli/settings_spec.rb @@ -41,7 +41,8 @@ describe Mastodon::CLI::Settings do it 'changes registrations_mode and require_invite_text' do expect { subject } - .to change(Setting, :registrations_mode).from(nil).to('approved') + .to output_results('OK') + .and change(Setting, :registrations_mode).from(nil).to('approved') .and change(Setting, :require_invite_text).from(false).to(true) end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 4394b470e6..0e68fbe121 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -104,7 +104,6 @@ RSpec.configure do |config| end config.before :each, type: :cli do - stub_stdout stub_reset_connection_pools end @@ -163,14 +162,6 @@ def attachment_fixture(name) Rails.root.join('spec', 'fixtures', 'files', name).open end -def stub_stdout - # TODO: Is there a bettery way to: - # - Avoid CLI command output being printed out - # - Allow rspec to assert things against STDOUT - # - Avoid disabling stdout for other desirable output (deprecation warnings, for example) - allow($stdout).to receive(:write) -end - def stub_reset_connection_pools # TODO: Is there a better way to correctly run specs without stubbing this? # (Avoids reset_connection_pools! in test env)