diff options
Diffstat (limited to 'spec/models')
-rw-r--r-- | spec/models/hooks/active_hook_filter_spec.rb | 115 | ||||
-rw-r--r-- | spec/models/hooks/web_hook_spec.rb | 81 |
2 files changed, 145 insertions, 51 deletions
diff --git a/spec/models/hooks/active_hook_filter_spec.rb b/spec/models/hooks/active_hook_filter_spec.rb index 1f693ce9fde..38e4d9d3a72 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 - let(:hook) do - create(:project_hook, push_events: true, push_events_branch_filter: branch_filter) + using RSpec::Parameterized::TableSyntax + + context 'for various types of branch_filter' do + let_it_be_with_reload(:hook) do + create(: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_it_be(:hook) do + create( + :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_it_be(:hook) do + create( + :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_it_be(:hook) do + create( + :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 diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb index da8c10b67a6..dd5bf8b99dd 100644 --- a/spec/models/hooks/web_hook_spec.rb +++ b/spec/models/hooks/web_hook_spec.rb @@ -113,24 +113,81 @@ RSpec.describe WebHook do end describe 'push_events_branch_filter' do - it { is_expected.to allow_values("good_branch_name", "another/good-branch_name").for(:push_events_branch_filter) } - it { is_expected.to allow_values("").for(:push_events_branch_filter) } - it { is_expected.not_to allow_values("bad branch name", "bad~branchname").for(:push_events_branch_filter) } - - it 'gets rid of whitespace' do - hook.push_events_branch_filter = ' branch ' - hook.save! + before do + subject.branch_filter_strategy = strategy + end + + context 'with "all branches" strategy' do + let(:strategy) { 'all_branches' } + + it { + is_expected.to allow_values( + "good_branch_name", + "another/good-branch_name", + "good branch name", + "good~branchname", + "good_branchname(", + "good_branchname[", + "" + ).for(:push_events_branch_filter) + } + end + + context 'with "wildcard" strategy' do + let(:strategy) { 'wildcard' } + + it { + is_expected.to allow_values( + "good_branch_name", + "another/good-branch_name", + "good_branch_name(", + "" + ).for(:push_events_branch_filter) + } + + it { + is_expected.not_to allow_values( + "bad branch name", + "bad~branchname", + "bad_branch_name[" + ).for(:push_events_branch_filter) + } + + it 'gets rid of whitespace' do + hook.push_events_branch_filter = ' branch ' + hook.save! + + expect(hook.push_events_branch_filter).to eq('branch') + end - expect(hook.push_events_branch_filter).to eq('branch') + it 'stores whitespace only as empty' do + hook.push_events_branch_filter = ' ' + hook.save! + expect(hook.push_events_branch_filter).to eq('') + end end - it 'stores whitespace only as empty' do - hook.push_events_branch_filter = ' ' - hook.save! + context 'with "regex" strategy' do + let(:strategy) { 'regex' } + + it { + is_expected.to allow_values( + "good_branch_name", + "another/good-branch_name", + "good branch name", + "good~branch~name", + "" + ).for(:push_events_branch_filter) + } - expect(hook.push_events_branch_filter).to eq('') + it { is_expected.not_to allow_values("bad_branch_name(", "bad_branch_name[").for(:push_events_branch_filter) } end end + + it "only consider these branch filter strategies are valid" do + expected_valid_types = %w[all_branches regex wildcard] + expect(described_class.branch_filter_strategies.keys).to contain_exactly(*expected_valid_types) + end end describe 'encrypted attributes' do |