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.

diff --git a/spec/services/activitypub/fetch_replies_service_spec.rb b/spec/services/activitypub/fetch_replies_service_spec.rb
index 65c453341143ee5d9c36c6503ea895a0a8810c40..fe49b18c195db841916d91ff0304536efeec29de 100644 (file)
--- a/spec/services/activitypub/fetch_replies_service_spec.rb
+++ b/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

diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index b6d127a0840217158e1a067e4854252beb98b323..0414ba9ed62b3ab257641f87e8322453899e2b4d 100644 (file)
--- a/spec/spec_helper.rb
+++ b/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

diff --git a/spec/workers/activitypub/distribute_poll_update_worker_spec.rb b/spec/workers/activitypub/distribute_poll_update_worker_spec.rb
index 7eb6119fd8091f791729952d0450e816cf4dfa67..d68a695b780851289c9a7757719d98b66e78f0b3 100644 (file)
--- a/spec/workers/activitypub/distribute_poll_update_worker_spec.rb
+++ b/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

diff --git a/spec/workers/activitypub/distribution_worker_spec.rb b/spec/workers/activitypub/distribution_worker_spec.rb
index c017b4da1afc98c1c606ba144e4dffbd915d41a5..3a5900d9b8d71c611d93c7c0b87d1a5d00cdadfd 100644 (file)
--- a/spec/workers/activitypub/distribution_worker_spec.rb
+++ b/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

diff --git a/spec/workers/activitypub/move_distribution_worker_spec.rb b/spec/workers/activitypub/move_distribution_worker_spec.rb
index b52788e54e78d3e6619be02ffc474dc98fbbf0f2..af8c44cc013f9a6e552bf316ca59d610828db879 100644 (file)
--- a/spec/workers/activitypub/move_distribution_worker_spec.rb
+++ b/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

diff --git a/spec/workers/activitypub/status_update_distribution_worker_spec.rb b/spec/workers/activitypub/status_update_distribution_worker_spec.rb
index 6633b601fc6224f6a6c4ef0b5b72c96060499683..c014c6790ebad8d4502e2856e56ffbaa40f9385f 100644 (file)
--- a/spec/workers/activitypub/status_update_distribution_worker_spec.rb
+++ b/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

diff --git a/spec/workers/activitypub/update_distribution_worker_spec.rb b/spec/workers/activitypub/update_distribution_worker_spec.rb
index 688a424d55ef183069b2bbe4b9b28b29fb8b0f09..0e057fd0bcea0cca3634e91f6de6029253439675 100644 (file)
--- a/spec/workers/activitypub/update_distribution_worker_spec.rb
+++ b/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

diff --git a/spec/workers/move_worker_spec.rb b/spec/workers/move_worker_spec.rb
index 4db5810f196db599c6664c1fe14853a15d315979..be02d31924dcb5b9179d495e07ea22de2bc5e268 100644 (file)
--- a/spec/workers/move_worker_spec.rb
+++ b/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'