3 files changed, 46 insertions, 21 deletions
diff --git a/spec/models/concerns/triggerable_hooks_spec.rb b/spec/models/concerns/triggerable_hooks_spec.rb
index 621d2d38eae..265abd6bd72 100644
--- a/spec/models/concerns/triggerable_hooks_spec.rb
+++ b/spec/models/concerns/triggerable_hooks_spec.rb
@@ -40,4 +40,28 @@ RSpec.describe TriggerableHooks do
       end
     end
   end
+
+  describe '.select_active' do
+    it 'returns hooks that match the active filter' do
+      TestableHook.create!(url: 'http://example1.com', push_events: true)
+      TestableHook.create!(url: 'http://example2.com', push_events: true)
+      filter1 = double(:filter1)
+      filter2 = double(:filter2)
+      allow(ActiveHookFilter).to receive(:new).exactly(2).times.and_return(filter1, filter2)
+      expect(filter1).to receive(:matches?).and_return(true)
+      expect(filter2).to receive(:matches?).and_return(false)
+
+      hooks = TestableHook.push_hooks.order_id_asc
+      expect(hooks.select_active(:push_hooks, {})).to eq [hooks.first]
+    end
+
+    it 'returns empty list if no hooks match the active filter' do
+      TestableHook.create!(url: 'http://example1.com', push_events: true)
+      filter = double(:filter)
+      allow(ActiveHookFilter).to receive(:new).and_return(filter)
+      expect(filter).to receive(:matches?).and_return(false)
+
+      expect(TestableHook.push_hooks.select_active(:push_hooks, {})).to eq []
+    end
+  end
 end
diff --git a/spec/models/hooks/active_hook_filter_spec.rb b/spec/models/hooks/active_hook_filter_spec.rb
index c97003eb542..df7edda2213 100644
--- a/spec/models/hooks/active_hook_filter_spec.rb
+++ b/spec/models/hooks/active_hook_filter_spec.rb
@@ -11,28 +11,29 @@ describe ActiveHookFilter do
 
       context 'branch filter is specified' do
         let(:branch_filter) { 'master' }
+
         it 'returns true if branch matches' do
-          expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to eq(true)
+          expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be true
         end
 
         it 'returns false if branch does not match' do
-          expect(filter.matches?(:push_hooks, { ref: 'refs/heads/my_branch' })).to eq(false)
+          expect(filter.matches?(:push_hooks, { ref: 'refs/heads/my_branch' })).to be false
         end
 
         it 'returns false if ref is nil' do
-          expect(filter.matches?(:push_hooks, {})).to eq(false)
+          expect(filter.matches?(:push_hooks, {})).to be false
         end
 
         context 'branch filter contains wildcard' do
           let(:branch_filter) { 'features/*' }
 
           it 'returns true if branch matches' do
-            expect(filter.matches?(:push_hooks, { ref: 'refs/heads/features/my-branch' })).to eq(true)
-            expect(filter.matches?(:push_hooks, { ref: 'refs/heads/features/my-branch/something' })).to eq(true)
+            expect(filter.matches?(:push_hooks, { ref: 'refs/heads/features/my-branch' })).to be true
+            expect(filter.matches?(:push_hooks, { ref: 'refs/heads/features/my-branch/something' })).to be true
           end
 
           it 'returns false if branch does not match' do
-            expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to eq(false)
+            expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be false
           end
         end
       end
@@ -41,7 +42,7 @@ describe ActiveHookFilter do
         let(:branch_filter) { nil }
 
         it 'returns true' do
-          expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to eq(true)
+          expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be true
         end
       end
 
@@ -49,7 +50,7 @@ describe ActiveHookFilter do
         let(:branch_filter) { '' }
 
         it 'acts like branch is not specified' do
-          expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to eq(true)
+          expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be true
         end
       end
     end
@@ -60,7 +61,7 @@ describe ActiveHookFilter do
       end
 
       it 'returns true as branch filters are not yet supported for these' do
-        expect(filter.matches?(:issues_events, { ref: 'refs/heads/master' })).to eq(true)
+        expect(filter.matches?(:issues_events, { ref: 'refs/heads/master' })).to be true
       end
     end
   end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index cea7fa31b16..d3dc95548aa 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -3672,36 +3672,36 @@ describe Project do
   describe '#execute_hooks' do
     let(:data) { { ref: 'refs/heads/master', data: 'data' } }
     it 'executes active projects hooks with the specified scope' do
-      expect_any_instance_of(ActiveHookFilter).to receive(:matches?)
-        .with(:tag_push_hooks, data)
-        .and_return(true)
-      hook = create(:project_hook, merge_requests_events: false, tag_push_events: true)
+      hook = create(:project_hook, merge_requests_events: false, push_events: true)
+      expect(ProjectHook).to receive(:select_active)
+        .with(:push_hooks, data)
+        .and_return([hook])
       project = create(:project, hooks: [hook])
 
       expect_any_instance_of(ProjectHook).to receive(:async_execute).once
 
-      project.execute_hooks(data, :tag_push_hooks)
+      project.execute_hooks(data, :push_hooks)
     end
 
     it 'does not execute project hooks that dont match the specified scope' do
-      hook = create(:project_hook, merge_requests_events: true, tag_push_events: false)
+      hook = create(:project_hook, merge_requests_events: true, push_events: false)
       project = create(:project, hooks: [hook])
 
       expect_any_instance_of(ProjectHook).not_to receive(:async_execute).once
 
-      project.execute_hooks(data, :tag_push_hooks)
+      project.execute_hooks(data, :push_hooks)
     end
 
     it 'does not execute project hooks which are not active' do
-      expect_any_instance_of(ActiveHookFilter).to receive(:matches?)
-        .with(:tag_push_hooks, data)
-        .and_return(false)
-      hook = create(:project_hook, tag_push_events: true)
+      hook = create(:project_hook, push_events: true)
+      expect(ProjectHook).to receive(:select_active)
+        .with(:push_hooks, data)
+        .and_return([])
       project = create(:project, hooks: [hook])
 
       expect_any_instance_of(ProjectHook).not_to receive(:async_execute).once
 
-      project.execute_hooks(data, :tag_push_hooks)
+      project.execute_hooks(data, :push_hooks)
     end
 
     it 'executes the system hooks with the specified scope' do