Finish up the CLI spec area pattern adoption for CLI::Accounts#refresh specs (#28764)

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(:options) { { all: true } }
-      let!(:remote_account_example_com) { Fabricate(:account, domain: 'example.com') }
+      let!(:local_account) { Fabricate(:account, domain: nil) }
-      let!(:account_example_net)        { Fabricate(:account, domain: 'example.net') }
+      let(:remote_com_avatar_url) { 'https://example.host/avatar/com' }
-      let(:scope)                       { Account.remote }
+      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
+        stub_parallelize_with_progress!
-        # this makes the assertions harder to write
+
-        allow(cli).to receive(:parallelize_with_progress).and_yield(remote_account_example_com)
+        stub_request(:get, remote_com_avatar_url)
-                                                         .and_yield(account_example_net)
+          .to_return request_fixture('avatar.txt')
-                                                         .and_return([2, nil])
+        stub_request(:get, remote_com_header_url)
-        cli.options = { all: true }
+          .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
+      it 'refreshes the avatar and header for all remote accounts' do
-        allow(remote_account_example_com).to receive(:reset_avatar!)
+        expect { subject }
-        allow(account_example_net).to receive(:reset_avatar!)
-
-        expect { cli.refresh }
           .to output_results('Refreshed 2 accounts')
+          .and not_change(local_account, :updated_at)
 
-        expect(cli).to have_received(:parallelize_with_progress).with(scope).once
+        # One request from factory creation, one more from task
-        expect(remote_account_example_com).to have_received(:reset_avatar!).once
+        expect(a_request(:get, remote_com_avatar_url))
-        expect(account_example_net).to have_received(:reset_avatar!).once
+          .to have_been_made.at_least_times(2)
-      end
+        expect(a_request(:get, remote_com_header_url))
-
+          .to have_been_made.at_least_times(2)
-      it 'does not refresh avatar for local accounts' do
+        expect(a_request(:get, remote_net_avatar_url))
-        allow(local_account).to receive(:reset_avatar!)
+          .to have_been_made.at_least_times(2)
-
+        expect(a_request(:get, remote_net_header_url))
-        expect { cli.refresh }
+          .to have_been_made.at_least_times(2)
-          .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')
       end
 
       context 'with --dry-run option' do
-        before do
+        let(:options) { { all: true, dry_run: true } }
-          cli.options = { all: true, dry_run: true }
-        end
 
-        it 'does not refresh the avatar for any account' do
+        it 'does not refresh the avatar or header for any account' do
-          allow(local_account).to receive(:reset_avatar!)
+          expect { subject }
-          allow(remote_account_example_com).to receive(:reset_avatar!)
-          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
+          # One request from factory creation, none from task due to dry run
-          expect(local_account).to_not have_received(:reset_avatar!)
+          expect(a_request(:get, remote_com_avatar_url))
-          expect(remote_account_example_com).to_not have_received(:reset_avatar!)
+            .to have_been_made.once
-          expect(account_example_net).to_not have_received(:reset_avatar!)
+          expect(a_request(:get, remote_com_header_url))
-        end
+            .to have_been_made.once
-
+          expect(a_request(:get, remote_net_avatar_url))
-        it 'does not refresh the header for any account' do
+            .to have_been_made.once
-          allow(local_account).to receive(:reset_header!)
+          expect(a_request(:get, remote_net_header_url))
-          allow(remote_account_example_com).to receive(:reset_header!)
+            .to have_been_made.once
-          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)')
         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
+        let(:options) { { dry_run: true } }
-          cli.options = { dry_run: true }
-        end
 
         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(:domain) { 'example.com' }
-      let!(:account_example_com_b) { Fabricate(:account, domain: 'example.com') }
+      let(:options) { { domain: domain } }
-      let!(:account_example_net)   { Fabricate(:account, domain: 'example.net') }
+
-      let(:domain)                 { 'example.com' }
+      let(:com_a_avatar_url) { 'https://example.host/avatar/a' }
-      let(:scope)                  { Account.remote.where(domain: domain) }
+      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)
+        stub_parallelize_with_progress!
-                                                         .and_yield(account_example_com_b)
+
-                                                         .and_return([2, nil])
+        stub_request(:get, com_a_avatar_url)
-        cli.options = { domain: domain }
+          .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
+      it 'refreshes the avatar and header for all accounts on specified domain' do
-        allow(account_example_com_a).to receive(:reset_avatar!)
+        expect { subject }
-        allow(account_example_com_b).to receive(:reset_avatar!)
-
-        expect { cli.refresh }
           .to output_results('Refreshed 2 accounts')
 
-        expect(cli).to have_received(:parallelize_with_progress).with(scope).once
+        # One request from factory creation, one more from task
-        expect(account_example_com_a).to have_received(:reset_avatar!).once
+        expect(a_request(:get, com_a_avatar_url))
-        expect(account_example_com_b).to have_received(:reset_avatar!).once
+          .to have_been_made.at_least_times(2)
-      end
+        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
+        # One request from factory creation, none from task
-        allow(account_example_net).to receive(:reset_avatar!)
+        expect(a_request(:get, net_avatar_url))
-
+          .to have_been_made.once
-        expect { cli.refresh }
+        expect(a_request(:get, net_header_url))
-          .to output_results('Refreshed 2 accounts')
+          .to have_been_made.once
-
-        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!)
       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