Reduce factory usage across spec/services area (mastodon#32098)

Author: mjankowski

spec/services/account_statuses_cleanup_service_spec.rb

@@ -27,39 +27,35 @@
       end
 
       context 'when given a normal budget of 10' do
-        it 'reports 3 deleted statuses' do
-          expect(subject.call(account_policy, 10)).to eq 3
-        end
-
-        it 'records the last deleted id' do
-          subject.call(account_policy, 10)
-          expect(account_policy.last_inspected).to eq [old_status.id, another_old_status.id].max
-        end
-
-        it 'actually deletes the statuses' do
-          subject.call(account_policy, 10)
-          expect(Status.find_by(id: [very_old_status.id, old_status.id, another_old_status.id])).to be_nil
-          expect { recent_status.reload }.to_not raise_error
-        end
-
-        it 'preserves recent and unrelated statuses' do
-          subject.call(account_policy, 10)
-          expect { unrelated_status.reload }.to_not raise_error
-          expect { recent_status.reload }.to_not raise_error
+        it 'reports 3 deleted statuses and records last deleted id, deletes statuses, preserves recent unrelated statuses' do
+          expect(subject.call(account_policy, 10))
+            .to eq(3)
+
+          expect(account_policy.last_inspected)
+            .to eq [old_status.id, another_old_status.id].max
+
+          expect(Status.find_by(id: [very_old_status.id, old_status.id, another_old_status.id]))
+            .to be_nil
+          expect { recent_status.reload }
+            .to_not raise_error
+          expect { unrelated_status.reload }
+            .to_not raise_error
+          expect { recent_status.reload }
+            .to_not raise_error
         end
       end
 
       context 'when called repeatedly with a budget of 2' do
-        it 'reports 2 then 1 deleted statuses' do
-          expect(subject.call(account_policy, 2)).to eq 2
-          expect(subject.call(account_policy, 2)).to eq 1
-        end
-
-        it 'actually deletes the statuses in the expected order' do
-          subject.call(account_policy, 2)
-          expect(Status.find_by(id: very_old_status.id)).to be_nil
-          subject.call(account_policy, 2)
-          expect(Status.find_by(id: [very_old_status.id, old_status.id, another_old_status.id])).to be_nil
+        it 'reports 2 then 1 deleted statuses and deletes in expected order' do
+          expect(subject.call(account_policy, 2))
+            .to eq(2)
+          expect(Status.find_by(id: very_old_status.id))
+            .to be_nil
+
+          expect(subject.call(account_policy, 2))
+            .to eq(1)
+          expect(Status.find_by(id: [very_old_status.id, old_status.id, another_old_status.id]))
+            .to be_nil
         end
       end
 
@@ -90,19 +86,24 @@
           end
         end
 
-        it 'reports 0 deleted statuses then 0 then 3 then 0 again' do
-          expect(subject.call(account_policy, 10)).to eq 0
-          expect(subject.call(account_policy, 10)).to eq 0
-          expect(subject.call(account_policy, 10)).to eq 3
-          expect(subject.call(account_policy, 10)).to eq 0
+        it 'reports 0 deleted statuses then 0 then 3 then 0 again, and keeps id under oldest deletable record' do
+          expect(subject.call(account_policy, 10))
+            .to eq(0)
+          expect(subject.call(account_policy, 10))
+            .to eq(0)
+          expect(subject.call(account_policy, 10))
+            .to eq(3)
+          expect(subject.call(account_policy, 10))
+            .to eq(0)
+          expect(account_policy.last_inspected)
+            .to be < oldest_deletable_record_id
         end
 
-        it 'never causes the recorded id to get higher than oldest deletable toot' do
-          subject.call(account_policy, 10)
-          subject.call(account_policy, 10)
-          subject.call(account_policy, 10)
-          subject.call(account_policy, 10)
-          expect(account_policy.last_inspected).to be < Mastodon::Snowflake.id_at(account_policy.min_status_age.seconds.ago, with_random: false)
+        def oldest_deletable_record_id
+          Mastodon::Snowflake.id_at(
+            account_policy.min_status_age.seconds.ago,
+            with_random: false
+          )
         end
       end
     end

spec/services/activitypub/process_status_update_service_spec.rb

@@ -2,10 +2,6 @@
 
 require 'rails_helper'
 
-def poll_option_json(name, votes)
-  { type: 'Note', name: name, replies: { type: 'Collection', totalItems: votes } }
-end
-
 RSpec.describe ActivityPub::ProcessStatusUpdateService do
   subject { described_class.new }
 
@@ -294,7 +290,6 @@ def poll_option_json(name, votes)
     context 'when originally without media attachments' do
       before do
         stub_request(:get, 'https://example.com/foo.png').to_return(body: attachment_fixture('emojo.png'))
-        subject.call(status, json, json)
       end
 
       let(:payload) do
@@ -310,19 +305,18 @@ def poll_option_json(name, votes)
         }
       end
 
-      it 'updates media attachments' do
-        media_attachment = status.reload.ordered_media_attachments.first
+      it 'updates media attachments, fetches attachment, records media change in edit' do
+        subject.call(status, json, json)
 
-        expect(media_attachment).to_not be_nil
-        expect(media_attachment.remote_url).to eq 'https://example.com/foo.png'
-      end
+        expect(status.reload.ordered_media_attachments.first)
+          .to be_present
+          .and(have_attributes(remote_url: 'https://example.com/foo.png'))
 
-      it 'fetches the attachment' do
-        expect(a_request(:get, 'https://example.com/foo.png')).to have_been_made
-      end
+        expect(a_request(:get, 'https://example.com/foo.png'))
+          .to have_been_made
 
-      it 'records media change in edit' do
-        expect(status.edits.reload.last.ordered_media_attachment_ids).to_not be_empty
+        expect(status.edits.reload.last.ordered_media_attachment_ids)
+          .to_not be_empty
       end
     end
 
@@ -344,38 +338,38 @@ def poll_option_json(name, votes)
 
       before do
         allow(RedownloadMediaWorker).to receive(:perform_async)
-        subject.call(status, json, json)
       end
 
-      it 'updates the existing media attachment in-place' do
-        media_attachment = status.media_attachments.ordered.reload.first
+      it 'updates the existing media attachment in-place, does not queue redownload, updates media, records media change' do
+        subject.call(status, json, json)
 
-        expect(media_attachment).to_not be_nil
-        expect(media_attachment.remote_url).to eq 'https://example.com/foo.png'
-        expect(media_attachment.description).to eq 'A picture'
-      end
+        expect(status.media_attachments.ordered.reload.first)
+          .to be_present
+          .and have_attributes(
+            remote_url: 'https://example.com/foo.png',
+            description: 'A picture'
+          )
 
-      it 'does not queue redownload for the existing media attachment' do
-        expect(RedownloadMediaWorker).to_not have_received(:perform_async)
-      end
+        expect(RedownloadMediaWorker)
+          .to_not have_received(:perform_async)
 
-      it 'updates media attachments' do
-        expect(status.ordered_media_attachments.map(&:remote_url)).to eq %w(https://example.com/foo.png)
-      end
+        expect(status.ordered_media_attachments.map(&:remote_url))
+          .to eq %w(https://example.com/foo.png)
 
-      it 'records media change in edit' do
-        expect(status.edits.reload.last.ordered_media_attachment_ids).to_not be_empty
+        expect(status.edits.reload.last.ordered_media_attachment_ids)
+          .to_not be_empty
       end
     end
 
     context 'when originally with a poll' do
       before do
         poll = Fabricate(:poll, status: status)
         status.update(preloadable_poll: poll)
-        subject.call(status, json, json)
       end
 
       it 'removes poll and records media change in edit' do
+        subject.call(status, json, json)
+
         expect(status.reload.poll).to be_nil
         expect(status.edits.reload.last.poll_options).to be_nil
       end
@@ -398,15 +392,13 @@ def poll_option_json(name, votes)
         }
       end
 
-      before do
+      it 'creates a poll and records media change in edit' do
         subject.call(status, json, json)
-      end
 
-      it 'creates a poll and records media change in edit' do
-        poll = status.reload.poll
+        expect(status.reload.poll)
+          .to be_present
+          .and have_attributes(options: %w(Foo Bar Baz))
 
-        expect(poll).to_not be_nil
-        expect(poll.options).to eq %w(Foo Bar Baz)
         expect(status.edits.reload.last.poll_options).to eq %w(Foo Bar Baz)
       end
     end
@@ -419,4 +411,8 @@ def poll_option_json(name, votes)
         .to eq '2021-09-08 22:39:25 UTC'
     end
   end
+
+  def poll_option_json(name, votes)
+    { type: 'Note', name: name, replies: { type: 'Collection', totalItems: votes } }
+  end
 end

spec/services/authorize_follow_service_spec.rb

@@ -12,15 +12,15 @@
 
     before do
       FollowRequest.create(account: bob, target_account: sender)
-      subject.call(bob, sender)
     end
 
-    it 'removes follow request' do
-      expect(bob.requested?(sender)).to be false
-    end
+    it 'removes follow request and creates follow relation' do
+      subject.call(bob, sender)
 
-    it 'creates follow relation' do
-      expect(bob.following?(sender)).to be true
+      expect(bob)
+        .to_not be_requested(sender)
+      expect(bob)
+        .to be_following(sender)
     end
   end
 
@@ -30,19 +30,17 @@
     before do
       FollowRequest.create(account: bob, target_account: sender)
       stub_request(:post, bob.inbox_url).to_return(status: 200)
-      subject.call(bob, sender)
     end
 
-    it 'removes follow request' do
-      expect(bob.requested?(sender)).to be false
-    end
-
-    it 'creates follow relation' do
-      expect(bob.following?(sender)).to be true
-    end
+    it 'removes follow request, creates follow relation, send accept activity', :inline_jobs do
+      subject.call(bob, sender)
 
-    it 'sends an accept activity', :inline_jobs do
-      expect(a_request(:post, bob.inbox_url)).to have_been_made.once
+      expect(bob)
+        .to_not be_requested(sender)
+      expect(bob)
+        .to be_following(sender)
+      expect(a_request(:post, bob.inbox_url))
+        .to have_been_made.once
     end
   end
 end

spec/services/batched_remove_status_service_spec.rb

@@ -24,32 +24,38 @@
 
     status_alice_hello
     status_alice_other
+  end
 
+  it 'removes status records, removes from author and local follower feeds, notifies stream, sends delete' do
     subject.call([status_alice_hello, status_alice_other])
-  end
 
-  it 'removes statuses' do
-    expect { Status.find(status_alice_hello.id) }.to raise_error ActiveRecord::RecordNotFound
-    expect { Status.find(status_alice_other.id) }.to raise_error ActiveRecord::RecordNotFound
-  end
+    expect { Status.find(status_alice_hello.id) }
+      .to raise_error ActiveRecord::RecordNotFound
+    expect { Status.find(status_alice_other.id) }
+      .to raise_error ActiveRecord::RecordNotFound
 
-  it 'removes statuses from author\'s home feed' do
-    expect(HomeFeed.new(alice).get(10).pluck(:id)).to_not include(status_alice_hello.id, status_alice_other.id)
-  end
+    expect(feed_ids_for(alice))
+      .to_not include(status_alice_hello.id, status_alice_other.id)
 
-  it 'removes statuses from local follower\'s home feed' do
-    expect(HomeFeed.new(jeff).get(10).pluck(:id)).to_not include(status_alice_hello.id, status_alice_other.id)
-  end
+    expect(feed_ids_for(jeff))
+      .to_not include(status_alice_hello.id, status_alice_other.id)
 
-  it 'notifies streaming API of followers' do
-    expect(redis).to have_received(:publish).with("timeline:#{jeff.id}", any_args).at_least(:once)
-  end
+    expect(redis)
+      .to have_received(:publish)
+      .with("timeline:#{jeff.id}", any_args).at_least(:once)
+
+    expect(redis)
+      .to have_received(:publish)
+      .with('timeline:public', any_args).at_least(:once)
 
-  it 'notifies streaming API of public timeline' do
-    expect(redis).to have_received(:publish).with('timeline:public', any_args).at_least(:once)
+    expect(a_request(:post, 'http://example.com/inbox'))
+      .to have_been_made.at_least_once
   end
 
-  it 'sends delete activity to followers' do
-    expect(a_request(:post, 'http://example.com/inbox')).to have_been_made.at_least_once
+  def feed_ids_for(account)
+    HomeFeed
+      .new(account)
+      .get(10)
+      .pluck(:id)
   end
 end

spec/services/block_service_spec.rb

@@ -26,15 +26,16 @@
 
     before do
       stub_request(:post, 'http://example.com/inbox').to_return(status: 200)
-      subject.call(sender, bob)
     end
 
-    it 'creates a blocking relation' do
-      expect(sender.blocking?(bob)).to be true
-    end
+    it 'creates a blocking relation and send block activity', :inline_jobs do
+      subject.call(sender, bob)
+
+      expect(sender)
+        .to be_blocking(bob)
 
-    it 'sends a block activity', :inline_jobs do
-      expect(a_request(:post, 'http://example.com/inbox')).to have_been_made.once
+      expect(a_request(:post, 'http://example.com/inbox'))
+        .to have_been_made.once
     end
   end
 end