DEV: Ensure DiscourseEvent handlers cleaned up during specs (#11205)

lib/discourse_event.rb

@@ -23,6 +23,7 @@ class DiscourseEvent
   end
 
   def self.off(event_name, &block)
+    raise ArgumentError.new "DiscourseEvent.off must reference a block" if block.nil?
     events[event_name].delete(block)
   end
 

spec/components/discourse_event_spec.rb

@@ -31,10 +31,16 @@ describe DiscourseEvent do
       )
     }
 
+    let(:event_handler) do
+      Proc.new { |user| user.name = 'Two Face' }
+    end
+
     before do
-      DiscourseEvent.on(:acid_face) do |user|
+      DiscourseEvent.on(:acid_face, &event_handler)
-        user.name = 'Two Face'
+    end
-      end
+
+    after do
+      DiscourseEvent.off(:acid_face, &event_handler)
     end
 
     context 'when event does not exist' do
@@ -60,14 +66,19 @@ describe DiscourseEvent do
 
     context 'when multiple events exist' do
 
-      before do
+      let(:event_handler_2) do
-        DiscourseEvent.on(:acid_face) do |user|
+        Proc.new { |user| user.job = 'Supervillian' }
-          user.job = 'Supervillian'
+      end
-        end
 
+      before do
+        DiscourseEvent.on(:acid_face, &event_handler_2)
         DiscourseEvent.trigger(:acid_face, harvey)
       end
 
+      after do
+        DiscourseEvent.off(:acid_face, &event_handler_2)
+      end
+
       it 'triggers both events' do
         expect(harvey.job).to eq('Supervillian')
         expect(harvey.name).to eq('Two Face')

spec/jobs/automatic_group_membership_spec.rb

@@ -22,21 +22,22 @@ describe Jobs::AutomaticGroupMembership do
 
     group = Fabricate(:group, automatic_membership_email_domains: "wat.com")
 
+    automatic = nil
+    called = false
+
+    blk = Proc.new do |_u, _g, options|
+      automatic = options[:automatic]
+      called = true
+    end
+
     begin
-      automatic = nil
+      DiscourseEvent.on(:user_added_to_group, &blk)
-      called = false
-
-      DiscourseEvent.on(:user_added_to_group) do |_u, _g, options|
-        automatic = options[:automatic]
-        called = true
-      end
-
       Jobs::AutomaticGroupMembership.new.execute(group_id: group.id)
 
       expect(automatic).to eql(true)
       expect(called).to eq(true)
     ensure
-      DiscourseEvent.off(:user_added_to_group)
+      DiscourseEvent.off(:user_added_to_group, &blk)
     end
 
     group.reload

spec/models/group_spec.rb

@@ -853,21 +853,22 @@ describe Group do
     end
 
     it 'triggers a user_added_to_group event' do
-      begin
+      automatic = nil
-        automatic = nil
+      called = false
-        called = false
 
-        DiscourseEvent.on(:user_added_to_group) do |_u, _g, options|
+      block = Proc.new do |_u, _g, options|
-          automatic = options[:automatic]
+        automatic = options[:automatic]
-          called = true
+        called = true
-        end
+      end
+      begin
+        DiscourseEvent.on(:user_added_to_group, &block)
 
         group.add(user)
 
         expect(automatic).to eql(false)
         expect(called).to eq(true)
       ensure
-        DiscourseEvent.off(:user_added_to_group)
+        DiscourseEvent.off(:user_added_to_group, &block)
       end
     end
 

spec/models/topic_spec.rb

@@ -2344,14 +2344,18 @@ describe Topic do
     user.admin = true
     @topic_status_event_triggered = false
 
-    DiscourseEvent.on(:topic_status_updated) do
+    blk = Proc.new do
       @topic_status_event_triggered = true
     end
 
+    DiscourseEvent.on(:topic_status_updated, &blk)
+
     topic.update_status('closed', true, user)
     topic.reload
 
     expect(@topic_status_event_triggered).to eq(true)
+  ensure
+    DiscourseEvent.off(:topic_status_updated, &blk)
   end
 
   it 'allows users to normalize counts' do

spec/models/topic_user_spec.rb

@@ -109,15 +109,16 @@ describe TopicUser do
 
   describe 'notifications' do
     it 'should trigger the right DiscourseEvent' do
+      called = false
+      blk = Proc.new { called = true }
       begin
-        called = false
+        DiscourseEvent.on(:topic_notification_level_changed, &blk)
-        DiscourseEvent.on(:topic_notification_level_changed) { called = true }
 
         TopicUser.change(user.id, topic.id, notification_level: TopicUser.notification_levels[:tracking])
 
         expect(called).to eq(true)
       ensure
-        DiscourseEvent.off(:topic_notification_level_changed) { called = true }
+        DiscourseEvent.off(:topic_notification_level_changed, &blk)
       end
     end
 

spec/rails_helper.rb

@@ -266,6 +266,13 @@ RSpec.configure do |config|
 
   config.before :each, &TestSetup.method(:test_setup)
 
+  config.around :each do |example|
+    before_event_count = DiscourseEvent.events.values.sum(&:count)
+    example.run
+    after_event_count = DiscourseEvent.events.values.sum(&:count)
+    expect(before_event_count).to eq(after_event_count), "DiscourseEvent registrations were not cleaned up"
+  end
+
   config.before(:each, type: :multisite) do
     Rails.configuration.multisite = true # rubocop:disable Discourse/NoDirectMultisiteManipulation