1 files changed, 75 insertions, 38 deletions

diff --git a/spec/models/hooks/active_hook_filter_spec.rb b/spec/models/hooks/active_hook_filter_spec.rb
index 1f693ce9fde..47c0fbdb106 100644
--- a/spec/models/hooks/active_hook_filter_spec.rb
+++ b/spec/models/hooks/active_hook_filter_spec.rb
@@ -6,65 +6,102 @@ RSpec.describe ActiveHookFilter do
   subject(:filter) { described_class.new(hook) }
 
   describe '#matches?' do
-    context 'for push event hooks' do
+    using RSpec::Parameterized::TableSyntax
+
+    context 'for various types of branch_filter' do
       let(:hook) do
-        create(:project_hook, push_events: true, push_events_branch_filter: branch_filter)
+        build(:project_hook, push_events: true, issues_events: true)
       end
 
-      context 'branch filter is specified' do
-        let(:branch_filter) { 'master' }
+      where(:branch_filter_strategy, :branch_filter, :ref, :expected_matches?) do
+        'all_branches'  | 'master'        | 'refs/heads/master'                       | true
+        'all_branches'  | ''              | 'refs/heads/master'                       | true
+        'all_branches'  | nil             | 'refs/heads/master'                       | true
+        'all_branches'  | '.*'            | 'refs/heads/master'                       | true
+        'wildcard'      | 'master'        | 'refs/heads/master'                       | true
+        'wildcard'      | 'master'        | 'refs/heads/my_branch'                    | false
+        'wildcard'      | 'features/*'    | 'refs/heads/features/my-branch'           | true
+        'wildcard'      | 'features/*'    | 'refs/heads/features/my-branch/something' | true
+        'wildcard'      | 'features/*'    | 'refs/heads/master'                       | false
+        'wildcard'      | nil             | 'refs/heads/master'                       | true
+        'wildcard'      | ''              | 'refs/heads/master'                       | true
+        'regex'         | 'master'        | 'refs/heads/master'                       | true
+        'regex'         | 'master'        | 'refs/heads/my_branch'                    | false
+        'regex'         | 'features/*'    | 'refs/heads/xxxx/features/my-branch'      | true
+        'regex'         | 'features/*'    | 'refs/heads/features/'                    | true
+        'regex'         | 'features/*'    | 'refs/heads/features'                     | true
+        'regex'         | 'features/.*'   | 'refs/heads/features/my-branch'           | true
+        'regex'         | 'features/.*'   | 'refs/heads/features/my-branch/something' | true
+        'regex'         | 'features/.*'   | 'refs/heads/master'                       | false
+        'regex'         | '(feature|dev)' | 'refs/heads/feature'                      | true
+        'regex'         | '(feature|dev)' | 'refs/heads/dev'                          | true
+        'regex'         | '(feature|dev)' | 'refs/heads/master'                       | false
+        'regex'         | nil             | 'refs/heads/master'                       | true
+        'regex'         | ''              | 'refs/heads/master'                       | true
+      end
 
-        it 'returns true if branch matches' do
-          expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be true
+      with_them do
+        before do
+          hook.assign_attributes(
+            push_events_branch_filter: branch_filter,
+            branch_filter_strategy: branch_filter_strategy
+          )
         end
 
-        it 'returns false if branch does not match' do
-          expect(filter.matches?(:push_hooks, { ref: 'refs/heads/my_branch' })).to be false
-        end
+        it { expect(filter.matches?(:push_hooks, { ref: ref })).to be expected_matches? }
+        it { expect(filter.matches?(:issues_events, { ref: ref })).to be true }
+      end
 
-        it 'returns false if ref is nil' do
-          expect(filter.matches?(:push_hooks, {})).to be false
+      context 'when the branch filter is a invalid regex' do
+        let(:hook) do
+          build(
+            :project_hook,
+            push_events: true,
+            push_events_branch_filter: 'master',
+            branch_filter_strategy: 'regex'
+          )
         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 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 be false
-          end
+        before do
+          allow(hook).to receive(:push_events_branch_filter).and_return("invalid-regex[")
         end
-      end
 
-      context 'branch filter is not specified' do
-        let(:branch_filter) { nil }
-
-        it 'returns true' do
-          expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be true
-        end
+        it { expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be false }
       end
 
-      context 'branch filter is empty string' do
-        let(:branch_filter) { '' }
+      context 'when the branch filter is not properly set to nil' do
+        let(:hook) do
+          build(
+            :project_hook,
+            push_events: true,
+            branch_filter_strategy: 'all_branches'
+          )
+        end
 
-        it 'acts like branch is not specified' do
-          expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be true
+        before do
+          allow(hook).to receive(:push_events_branch_filter).and_return("master")
         end
+
+        it { expect(filter.matches?(:push_hooks, { ref: 'refs/heads/feature1' })).to be true }
       end
     end
 
-    context 'for non-push-events hooks' do
-      let(:hook) do
-        create(:project_hook, issues_events: true, push_events: false, push_events_branch_filter: '')
+    context 'when feature flag is disabled' do
+      before do
+        stub_feature_flags(enhanced_webhook_support_regex: false)
       end
 
-      it 'returns true as branch filters are not yet supported for these' do
-        expect(filter.matches?(:issues_events, { ref: 'refs/heads/master' })).to be true
+      let(:hook) do
+        build(
+          :project_hook,
+          push_events: true,
+          push_events_branch_filter: '(master)',
+          branch_filter_strategy: 'regex'
+        )
       end
+
+      it { expect(filter.matches?(:push_hooks, { ref: 'refs/heads/master' })).to be false }
+      it { expect(filter.matches?(:push_hooks, { ref: 'refs/heads/(master)' })).to be true }
     end
   end
 end