From 9a475ea8b32eed5ce163600d4747fe553bb7045e Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Wed, 17 Jan 2024 08:38:37 -0500 Subject: [PATCH] Finish up the CLI spec area pattern adoption for `CLI::Accounts#refresh` specs (#28764) --- spec/lib/mastodon/cli/accounts_spec.rb | 258 +++++++++++-------------- 1 file changed, 111 insertions(+), 147 deletions(-) diff --git a/spec/lib/mastodon/cli/accounts_spec.rb b/spec/lib/mastodon/cli/accounts_spec.rb index 64df29ebb..26ad983bb 100644 --- a/spec/lib/mastodon/cli/accounts_spec.rb +++ b/spec/lib/mastodon/cli/accounts_spec.rb @@ -660,106 +660,69 @@ describe Mastodon::CLI::Accounts do end describe '#refresh' do + let(:action) { :refresh } + context 'with --all option' do - let!(:local_account) { Fabricate(:account, domain: nil) } - let!(:remote_account_example_com) { Fabricate(:account, domain: 'example.com') } - let!(:account_example_net) { Fabricate(:account, domain: 'example.net') } - let(:scope) { Account.remote } + let(:options) { { all: true } } + let!(:local_account) { Fabricate(:account, domain: nil) } + let(:remote_com_avatar_url) { 'https://example.host/avatar/com' } + let(:remote_com_header_url) { 'https://example.host/header/com' } + let(:remote_account_example_com) { Fabricate(:account, domain: 'example.com', avatar_remote_url: remote_com_avatar_url, header_remote_url: remote_com_header_url) } + let(:remote_net_avatar_url) { 'https://example.host/avatar/net' } + let(:remote_net_header_url) { 'https://example.host/header/net' } + let(:account_example_net) { Fabricate(:account, domain: 'example.net', avatar_remote_url: remote_net_avatar_url, header_remote_url: remote_net_header_url) } + let(:scope) { Account.remote } before do - # TODO: we should be using `stub_parallelize_with_progress!` but - # this makes the assertions harder to write - allow(cli).to receive(:parallelize_with_progress).and_yield(remote_account_example_com) - .and_yield(account_example_net) - .and_return([2, nil]) - cli.options = { all: true } + stub_parallelize_with_progress! + + stub_request(:get, remote_com_avatar_url) + .to_return request_fixture('avatar.txt') + stub_request(:get, remote_com_header_url) + .to_return request_fixture('avatar.txt') + stub_request(:get, remote_net_avatar_url) + .to_return request_fixture('avatar.txt') + stub_request(:get, remote_net_header_url) + .to_return request_fixture('avatar.txt') + + remote_account_example_com + .update_column(:avatar_file_name, nil) + account_example_net + .update_column(:avatar_file_name, nil) end - it 'refreshes the avatar for all remote accounts' do - allow(remote_account_example_com).to receive(:reset_avatar!) - allow(account_example_net).to receive(:reset_avatar!) - - expect { cli.refresh } + it 'refreshes the avatar and header for all remote accounts' do + expect { subject } .to output_results('Refreshed 2 accounts') + .and not_change(local_account, :updated_at) - expect(cli).to have_received(:parallelize_with_progress).with(scope).once - expect(remote_account_example_com).to have_received(:reset_avatar!).once - expect(account_example_net).to have_received(:reset_avatar!).once - end - - it 'does not refresh avatar for local accounts' do - allow(local_account).to receive(:reset_avatar!) - - 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!) - end - - it 'refreshes the header for all remote accounts' do - allow(remote_account_example_com).to receive(:reset_header!) - allow(account_example_net).to receive(:reset_header!) - - 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 - expect(account_example_net).to have_received(:reset_header!).once - end - - it 'does not refresh the header for local accounts' do - allow(local_account).to receive(:reset_header!) - - 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!) - end - - it 'displays a successful message' do - expect { cli.refresh } - .to output_results('Refreshed 2 accounts') + # One request from factory creation, one more from task + expect(a_request(:get, remote_com_avatar_url)) + .to have_been_made.at_least_times(2) + expect(a_request(:get, remote_com_header_url)) + .to have_been_made.at_least_times(2) + expect(a_request(:get, remote_net_avatar_url)) + .to have_been_made.at_least_times(2) + expect(a_request(:get, remote_net_header_url)) + .to have_been_made.at_least_times(2) end context 'with --dry-run option' do - before do - cli.options = { all: true, dry_run: true } - end + let(:options) { { all: true, dry_run: true } } - it 'does not refresh the avatar for any account' do - allow(local_account).to receive(:reset_avatar!) - allow(remote_account_example_com).to receive(:reset_avatar!) - allow(account_example_net).to receive(:reset_avatar!) - - expect { cli.refresh } + it 'does not refresh the avatar or header for any account' do + expect { subject } .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!) - expect(remote_account_example_com).to_not have_received(:reset_avatar!) - expect(account_example_net).to_not have_received(:reset_avatar!) - end - - it 'does not refresh the header for any account' do - allow(local_account).to receive(:reset_header!) - allow(remote_account_example_com).to receive(:reset_header!) - allow(account_example_net).to receive(:reset_header!) - - 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!) - expect(remote_account_example_com).to_not have_received(:reset_header!) - expect(account_example_net).to_not have_received(:reset_header!) - end - - it 'displays a successful message with (DRY RUN)' do - expect { cli.refresh } - .to output_results('Refreshed 2 accounts (DRY RUN)') + # One request from factory creation, none from task due to dry run + expect(a_request(:get, remote_com_avatar_url)) + .to have_been_made.once + expect(a_request(:get, remote_com_header_url)) + .to have_been_made.once + expect(a_request(:get, remote_net_avatar_url)) + .to have_been_made.once + expect(a_request(:get, remote_net_header_url)) + .to have_been_made.once end end end @@ -782,7 +745,7 @@ describe Mastodon::CLI::Accounts do allow(account_example_com_a).to receive(:reset_avatar!) allow(account_example_com_b).to receive(:reset_avatar!) - expect { cli.refresh(*arguments) } + expect { subject } .to output_results('OK') expect(account_example_com_a).to have_received(:reset_avatar!).once @@ -792,7 +755,7 @@ describe Mastodon::CLI::Accounts do it 'does not reset the avatar for unspecified accounts' do allow(account_example_net).to receive(:reset_avatar!) - expect { cli.refresh(*arguments) } + expect { subject } .to output_results('OK') expect(account_example_net).to_not have_received(:reset_avatar!) @@ -802,7 +765,7 @@ describe Mastodon::CLI::Accounts do allow(account_example_com_a).to receive(:reset_header!) allow(account_example_com_b).to receive(:reset_header!) - expect { cli.refresh(*arguments) } + expect { subject } .to output_results('OK') expect(account_example_com_a).to have_received(:reset_header!).once @@ -812,7 +775,7 @@ describe Mastodon::CLI::Accounts do it 'does not reset the header for unspecified accounts' do allow(account_example_net).to receive(:reset_header!) - expect { cli.refresh(*arguments) } + expect { subject } .to output_results('OK') expect(account_example_net).to_not have_received(:reset_header!) @@ -822,7 +785,7 @@ describe Mastodon::CLI::Accounts do it 'displays a failure message' do allow(account_example_com_a).to receive(:reset_avatar!).and_raise(Mastodon::UnexpectedResponseError) - expect { cli.refresh(*arguments) } + expect { subject } .to output_results("Account failed: #{account_example_com_a.username}@#{account_example_com_a.domain}") end end @@ -831,22 +794,20 @@ describe Mastodon::CLI::Accounts do it 'exits with an error message' do allow(Account).to receive(:find_remote).with(account_example_com_b.username, account_example_com_b.domain).and_return(nil) - expect { cli.refresh(*arguments) } + expect { subject } .to output_results('No such account') .and raise_error(SystemExit) end end context 'with --dry-run option' do - before do - cli.options = { dry_run: true } - end + let(:options) { { dry_run: true } } it 'does not refresh the avatar for any account' do allow(account_example_com_a).to receive(:reset_avatar!) allow(account_example_com_b).to receive(:reset_avatar!) - expect { cli.refresh(*arguments) } + expect { subject } .to output_results('OK (DRY RUN)') expect(account_example_com_a).to_not have_received(:reset_avatar!) @@ -857,7 +818,7 @@ describe Mastodon::CLI::Accounts do allow(account_example_com_a).to receive(:reset_header!) allow(account_example_com_b).to receive(:reset_header!) - expect { cli.refresh(*arguments) } + expect { subject } .to output_results('OK (DRY RUN)') expect(account_example_com_a).to_not have_received(:reset_header!) @@ -867,67 +828,70 @@ describe Mastodon::CLI::Accounts do end context 'with --domain option' do - let!(:account_example_com_a) { Fabricate(:account, domain: 'example.com') } - let!(:account_example_com_b) { Fabricate(:account, domain: 'example.com') } - let!(:account_example_net) { Fabricate(:account, domain: 'example.net') } - let(:domain) { 'example.com' } - let(:scope) { Account.remote.where(domain: domain) } + let(:domain) { 'example.com' } + let(:options) { { domain: domain } } + + let(:com_a_avatar_url) { 'https://example.host/avatar/a' } + let(:com_a_header_url) { 'https://example.host/header/a' } + let(:account_example_com_a) { Fabricate(:account, domain: domain, avatar_remote_url: com_a_avatar_url, header_remote_url: com_a_header_url) } + + let(:com_b_avatar_url) { 'https://example.host/avatar/b' } + let(:com_b_header_url) { 'https://example.host/header/b' } + let(:account_example_com_b) { Fabricate(:account, domain: domain, avatar_remote_url: com_b_avatar_url, header_remote_url: com_b_header_url) } + + let(:net_avatar_url) { 'https://example.host/avatar/net' } + let(:net_header_url) { 'https://example.host/header/net' } + let(:account_example_net) { Fabricate(:account, domain: 'example.net', avatar_remote_url: net_avatar_url, header_remote_url: net_header_url) } before do - allow(cli).to receive(:parallelize_with_progress).and_yield(account_example_com_a) - .and_yield(account_example_com_b) - .and_return([2, nil]) - cli.options = { domain: domain } + stub_parallelize_with_progress! + + stub_request(:get, com_a_avatar_url) + .to_return request_fixture('avatar.txt') + stub_request(:get, com_a_header_url) + .to_return request_fixture('avatar.txt') + stub_request(:get, com_b_avatar_url) + .to_return request_fixture('avatar.txt') + stub_request(:get, com_b_header_url) + .to_return request_fixture('avatar.txt') + stub_request(:get, net_avatar_url) + .to_return request_fixture('avatar.txt') + stub_request(:get, net_header_url) + .to_return request_fixture('avatar.txt') + + account_example_com_a + .update_column(:avatar_file_name, nil) + account_example_com_b + .update_column(:avatar_file_name, nil) + account_example_net + .update_column(:avatar_file_name, nil) end - it 'refreshes the avatar for all accounts on specified domain' do - allow(account_example_com_a).to receive(:reset_avatar!) - allow(account_example_com_b).to receive(:reset_avatar!) - - expect { cli.refresh } + it 'refreshes the avatar and header for all accounts on specified domain' do + expect { subject } .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 - expect(account_example_com_b).to have_received(:reset_avatar!).once - end + # One request from factory creation, one more from task + expect(a_request(:get, com_a_avatar_url)) + .to have_been_made.at_least_times(2) + expect(a_request(:get, com_a_header_url)) + .to have_been_made.at_least_times(2) + expect(a_request(:get, com_b_avatar_url)) + .to have_been_made.at_least_times(2) + expect(a_request(:get, com_b_header_url)) + .to have_been_made.at_least_times(2) - it 'does not refresh the avatar for accounts outside specified domain' do - allow(account_example_net).to receive(:reset_avatar!) - - 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!) - end - - it 'refreshes the header for all accounts on specified domain' do - allow(account_example_com_a).to receive(:reset_header!) - allow(account_example_com_b).to receive(:reset_header!) - - 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 - expect(account_example_com_b).to have_received(:reset_header!).once - end - - it 'does not refresh the header for accounts outside specified domain' do - allow(account_example_net).to receive(:reset_header!) - - 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!) + # One request from factory creation, none from task + expect(a_request(:get, net_avatar_url)) + .to have_been_made.once + expect(a_request(:get, net_header_url)) + .to have_been_made.once end end context 'when neither a list of accts nor options are provided' do it 'exits with an error message' do - expect { cli.refresh } + expect { subject } .to output_results('No account(s) given') .and raise_error(SystemExit) end