Bladeren bron

Improve tests involving push_bulk (#17508)

sidekiq-bulk's push_bulk can either accept arguments directly or run them
through a block.

Setting expectations on the result of evaluating the blocks allows testing
more code (the block itself) and the test is moved closer to the *interface*
of the tested code than its precise implementation.
Claire 2 jaren geleden
bovenliggende
commit
2af03164cb

+ 6 - 12
spec/services/activitypub/fetch_replies_service_spec.rb

@@ -34,9 +34,8 @@ RSpec.describe ActivityPub::FetchRepliesService, type: :service do
     context 'when the payload is a Collection with inlined replies' do
       context 'when passing the collection itself' do
         it 'spawns workers for up to 5 replies on the same server' do
-          allow(FetchReplyWorker).to receive(:push_bulk)
+          expect(FetchReplyWorker).to receive(:push_bulk).with(['http://example.com/self-reply-1', 'http://example.com/self-reply-2', 'http://example.com/self-reply-3', 'http://example.com/self-reply-4', 'http://example.com/self-reply-5'])
           subject.call(status, payload)
-          expect(FetchReplyWorker).to have_received(:push_bulk).with(['http://example.com/self-reply-1', 'http://example.com/self-reply-2', 'http://example.com/self-reply-3', 'http://example.com/self-reply-4', 'http://example.com/self-reply-5'])
         end
       end
 
@@ -46,9 +45,8 @@ RSpec.describe ActivityPub::FetchRepliesService, type: :service do
         end
 
         it 'spawns workers for up to 5 replies on the same server' do
-          allow(FetchReplyWorker).to receive(:push_bulk)
+          expect(FetchReplyWorker).to receive(:push_bulk).with(['http://example.com/self-reply-1', 'http://example.com/self-reply-2', 'http://example.com/self-reply-3', 'http://example.com/self-reply-4', 'http://example.com/self-reply-5'])
           subject.call(status, collection_uri)
-          expect(FetchReplyWorker).to have_received(:push_bulk).with(['http://example.com/self-reply-1', 'http://example.com/self-reply-2', 'http://example.com/self-reply-3', 'http://example.com/self-reply-4', 'http://example.com/self-reply-5'])
         end
       end
     end
@@ -65,9 +63,8 @@ RSpec.describe ActivityPub::FetchRepliesService, type: :service do
 
       context 'when passing the collection itself' do
         it 'spawns workers for up to 5 replies on the same server' do
-          allow(FetchReplyWorker).to receive(:push_bulk)
+          expect(FetchReplyWorker).to receive(:push_bulk).with(['http://example.com/self-reply-1', 'http://example.com/self-reply-2', 'http://example.com/self-reply-3', 'http://example.com/self-reply-4', 'http://example.com/self-reply-5'])
           subject.call(status, payload)
-          expect(FetchReplyWorker).to have_received(:push_bulk).with(['http://example.com/self-reply-1', 'http://example.com/self-reply-2', 'http://example.com/self-reply-3', 'http://example.com/self-reply-4', 'http://example.com/self-reply-5'])
         end
       end
 
@@ -77,9 +74,8 @@ RSpec.describe ActivityPub::FetchRepliesService, type: :service do
         end
 
         it 'spawns workers for up to 5 replies on the same server' do
-          allow(FetchReplyWorker).to receive(:push_bulk)
+          expect(FetchReplyWorker).to receive(:push_bulk).with(['http://example.com/self-reply-1', 'http://example.com/self-reply-2', 'http://example.com/self-reply-3', 'http://example.com/self-reply-4', 'http://example.com/self-reply-5'])
           subject.call(status, collection_uri)
-          expect(FetchReplyWorker).to have_received(:push_bulk).with(['http://example.com/self-reply-1', 'http://example.com/self-reply-2', 'http://example.com/self-reply-3', 'http://example.com/self-reply-4', 'http://example.com/self-reply-5'])
         end
       end
     end
@@ -100,9 +96,8 @@ RSpec.describe ActivityPub::FetchRepliesService, type: :service do
 
       context 'when passing the collection itself' do
         it 'spawns workers for up to 5 replies on the same server' do
-          allow(FetchReplyWorker).to receive(:push_bulk)
+          expect(FetchReplyWorker).to receive(:push_bulk).with(['http://example.com/self-reply-1', 'http://example.com/self-reply-2', 'http://example.com/self-reply-3', 'http://example.com/self-reply-4', 'http://example.com/self-reply-5'])
           subject.call(status, payload)
-          expect(FetchReplyWorker).to have_received(:push_bulk).with(['http://example.com/self-reply-1', 'http://example.com/self-reply-2', 'http://example.com/self-reply-3', 'http://example.com/self-reply-4', 'http://example.com/self-reply-5'])
         end
       end
 
@@ -112,9 +107,8 @@ RSpec.describe ActivityPub::FetchRepliesService, type: :service do
         end
 
         it 'spawns workers for up to 5 replies on the same server' do
-          allow(FetchReplyWorker).to receive(:push_bulk)
+          expect(FetchReplyWorker).to receive(:push_bulk).with(['http://example.com/self-reply-1', 'http://example.com/self-reply-2', 'http://example.com/self-reply-3', 'http://example.com/self-reply-4', 'http://example.com/self-reply-5'])
           subject.call(status, collection_uri)
-          expect(FetchReplyWorker).to have_received(:push_bulk).with(['http://example.com/self-reply-1', 'http://example.com/self-reply-2', 'http://example.com/self-reply-3', 'http://example.com/self-reply-4', 'http://example.com/self-reply-5'])
         end
       end
     end

+ 7 - 0
spec/spec_helper.rb

@@ -57,3 +57,10 @@ end
 def json_str_to_hash(str)
   JSON.parse(str, symbolize_names: true)
 end
+
+def expect_push_bulk_to_match(klass, matcher)
+  expect(Sidekiq::Client).to receive(:push_bulk).with(hash_including({
+    "class" => klass,
+    "args" => matcher
+  }))
+end

+ 1 - 2
spec/workers/activitypub/distribute_poll_update_worker_spec.rb

@@ -10,13 +10,12 @@ describe ActivityPub::DistributePollUpdateWorker do
 
   describe '#perform' do
     before do
-      allow(ActivityPub::DeliveryWorker).to receive(:push_bulk)
       follower.follow!(account)
     end
 
     it 'delivers to followers' do
+      expect_push_bulk_to_match(ActivityPub::DeliveryWorker, [[kind_of(String), account.id, 'http://example.com']])
       subject.perform(status.id)
-      expect(ActivityPub::DeliveryWorker).to have_received(:push_bulk).with(['http://example.com'])
     end
   end
 end

+ 3 - 4
spec/workers/activitypub/distribution_worker_spec.rb

@@ -8,7 +8,6 @@ describe ActivityPub::DistributionWorker do
 
   describe '#perform' do
     before do
-      allow(ActivityPub::DeliveryWorker).to receive(:push_bulk)
       follower.follow!(status.account)
     end
 
@@ -18,8 +17,8 @@ describe ActivityPub::DistributionWorker do
       end
 
       it 'delivers to followers' do
+        expect_push_bulk_to_match(ActivityPub::DeliveryWorker, [[kind_of(String), status.account.id, 'http://example.com', anything]])
         subject.perform(status.id)
-        expect(ActivityPub::DeliveryWorker).to have_received(:push_bulk).with(['http://example.com'])
       end
     end
 
@@ -29,8 +28,8 @@ describe ActivityPub::DistributionWorker do
       end
 
       it 'delivers to followers' do
+        expect_push_bulk_to_match(ActivityPub::DeliveryWorker, [[kind_of(String), status.account.id, 'http://example.com', anything]])
         subject.perform(status.id)
-        expect(ActivityPub::DeliveryWorker).to have_received(:push_bulk).with(['http://example.com'])
       end
     end
 
@@ -43,8 +42,8 @@ describe ActivityPub::DistributionWorker do
       end
 
       it 'delivers to mentioned accounts' do
+        expect_push_bulk_to_match(ActivityPub::DeliveryWorker, [[kind_of(String), status.account.id, 'https://foo.bar/inbox', anything]])
         subject.perform(status.id)
-        expect(ActivityPub::DeliveryWorker).to have_received(:push_bulk).with(['https://foo.bar/inbox'])
       end
     end
   end

+ 4 - 2
spec/workers/activitypub/move_distribution_worker_spec.rb

@@ -9,14 +9,16 @@ describe ActivityPub::MoveDistributionWorker do
 
   describe '#perform' do
     before do
-      allow(ActivityPub::DeliveryWorker).to receive(:push_bulk)
       follower.follow!(migration.account)
       blocker.block!(migration.account)
     end
 
     it 'delivers to followers and known blockers' do
+      expect_push_bulk_to_match(ActivityPub::DeliveryWorker, [
+        [kind_of(String), migration.account.id, 'http://example.com'],
+        [kind_of(String), migration.account.id, 'http://example2.com']
+      ])
       subject.perform(migration.id)
-        expect(ActivityPub::DeliveryWorker).to have_received(:push_bulk).with(['http://example.com', 'http://example2.com'])
     end
   end
 end

+ 2 - 6
spec/workers/activitypub/status_update_distribution_worker_spec.rb

@@ -23,9 +23,7 @@ describe ActivityPub::StatusUpdateDistributionWorker do
       end
 
       it 'delivers to followers' do
-        expect(ActivityPub::DeliveryWorker).to receive(:push_bulk) do |items, &block|
-          expect(items.map(&block)).to match([[kind_of(String), status.account.id, 'http://example.com', anything]])
-        end
+        expect_push_bulk_to_match(ActivityPub::DeliveryWorker, [[kind_of(String), status.account.id, 'http://example.com', anything]])
 
         subject.perform(status.id)
       end
@@ -37,9 +35,7 @@ describe ActivityPub::StatusUpdateDistributionWorker do
       end
 
       it 'delivers to followers' do
-        expect(ActivityPub::DeliveryWorker).to receive(:push_bulk) do |items, &block|
-          expect(items.map(&block)).to match([[kind_of(String), status.account.id, 'http://example.com', anything]])
-        end
+        expect_push_bulk_to_match(ActivityPub::DeliveryWorker, [[kind_of(String), status.account.id, 'http://example.com', anything]])
 
         subject.perform(status.id)
       end

+ 1 - 2
spec/workers/activitypub/update_distribution_worker_spec.rb

@@ -8,13 +8,12 @@ describe ActivityPub::UpdateDistributionWorker do
 
   describe '#perform' do
     before do
-      allow(ActivityPub::DeliveryWorker).to receive(:push_bulk)
       follower.follow!(account)
     end
 
     it 'delivers to followers' do
+      expect_push_bulk_to_match(ActivityPub::DeliveryWorker, [[kind_of(String), account.id, 'http://example.com', anything]])
       subject.perform(account.id)
-      expect(ActivityPub::DeliveryWorker).to have_received(:push_bulk).with(['http://example.com'])
     end
   end
 end

+ 2 - 3
spec/workers/move_worker_spec.rb

@@ -21,7 +21,6 @@ describe MoveWorker do
     blocking_account.block!(source_account)
     muting_account.mute!(source_account)
 
-    allow(UnfollowFollowWorker).to receive(:push_bulk)
     allow(BlockService).to receive(:new).and_return(block_service)
     allow(block_service).to receive(:call)
   end
@@ -78,8 +77,8 @@ describe MoveWorker do
   context 'both accounts are distant' do
     describe 'perform' do
       it 'calls UnfollowFollowWorker' do
+        expect_push_bulk_to_match(UnfollowFollowWorker, [[local_follower.id, source_account.id, target_account.id, false]])
         subject.perform(source_account.id, target_account.id)
-        expect(UnfollowFollowWorker).to have_received(:push_bulk).with([local_follower.id])
       end
 
       include_examples 'user note handling'
@@ -92,8 +91,8 @@ describe MoveWorker do
 
     describe 'perform' do
       it 'calls UnfollowFollowWorker' do
+        expect_push_bulk_to_match(UnfollowFollowWorker, [[local_follower.id, source_account.id, target_account.id, true]])
         subject.perform(source_account.id, target_account.id)
-        expect(UnfollowFollowWorker).to have_received(:push_bulk).with([local_follower.id])
       end
 
       include_examples 'user note handling'