1 files changed, 117 insertions, 109 deletions

diff --git a/spec/finders/ci/runners_finder_spec.rb b/spec/finders/ci/runners_finder_spec.rb
index e57ad5bc76d..5d249ddb391 100644
--- a/spec/finders/ci/runners_finder_spec.rb
+++ b/spec/finders/ci/runners_finder_spec.rb
@@ -222,12 +222,14 @@ RSpec.describe Ci::RunnersFinder, feature_category: :runner_fleet do
       end
 
       shared_examples 'executes as normal user' do
-        it 'returns no runners' do
+        it 'raises Gitlab::Access::AccessDeniedError' do
           user = create :user
           create :ci_runner, active: true
           create :ci_runner, active: false
 
-          expect(described_class.new(current_user: user, params: {}).execute).to be_empty
+          expect do
+            described_class.new(current_user: user, params: {}).execute
+          end.to raise_error(Gitlab::Access::AccessDeniedError)
         end
       end
 
@@ -250,12 +252,14 @@ RSpec.describe Ci::RunnersFinder, feature_category: :runner_fleet do
       end
 
       context 'when user is nil' do
-        it 'returns no runners' do
+        it 'raises Gitlab::Access::AccessDeniedError' do
           user = nil
           create :ci_runner, active: true
           create :ci_runner, active: false
 
-          expect(described_class.new(current_user: user, params: {}).execute).to be_empty
+          expect do
+            described_class.new(current_user: user, params: {}).execute
+          end.to raise_error(Gitlab::Access::AccessDeniedError)
         end
       end
     end
@@ -306,154 +310,162 @@ RSpec.describe Ci::RunnersFinder, feature_category: :runner_fleet do
 
       shared_examples 'membership equal to :descendants' do
         it 'returns all descendant runners' do
-          expect(subject).to eq([runner_project_7, runner_project_6, runner_project_5,
-                                 runner_project_4, runner_project_3, runner_project_2,
-                                 runner_project_1, runner_sub_group_4, runner_sub_group_3,
-                                 runner_sub_group_2, runner_sub_group_1, runner_group])
+          is_expected.to contain_exactly(
+            runner_project_7, runner_project_6, runner_project_5,
+            runner_project_4, runner_project_3, runner_project_2,
+            runner_project_1, runner_sub_group_4, runner_sub_group_3,
+            runner_sub_group_2, runner_sub_group_1, runner_group)
         end
       end
 
-      context 'with user as group owner' do
-        before do
-          group.add_owner(user)
+      context 'with user is group maintainer or owner' do
+        where(:user_role) do
+          [GroupMember::OWNER, GroupMember::MAINTAINER]
         end
 
-        context 'with :group as target group' do
-          let(:target_group) { group }
-
-          context 'passing no membership params' do
-            it_behaves_like 'membership equal to :descendants'
+        with_them do
+          before do
+            group.add_member(user, user_role)
           end
 
-          context 'with :descendants membership' do
-            let(:membership) { :descendants }
+          context 'with :group as target group' do
+            let(:target_group) { group }
 
-            it_behaves_like 'membership equal to :descendants'
-          end
+            context 'passing no membership params' do
+              it_behaves_like 'membership equal to :descendants'
+            end
 
-          context 'with :direct membership' do
-            let(:membership) { :direct }
+            context 'with :descendants membership' do
+              let(:membership) { :descendants }
 
-            it 'returns runners belonging to group' do
-              expect(subject).to eq([runner_group])
+              it_behaves_like 'membership equal to :descendants'
             end
-          end
 
-          context 'with :all_available membership' do
-            let(:membership) { :all_available }
+            context 'with :direct membership' do
+              let(:membership) { :direct }
 
-            it 'returns runners available to group' do
-              expect(subject).to match_array([runner_project_7, runner_project_6, runner_project_5,
-                                              runner_project_4, runner_project_3, runner_project_2,
-                                              runner_project_1, runner_sub_group_4, runner_sub_group_3,
-                                              runner_sub_group_2, runner_sub_group_1, runner_group, runner_instance])
+              it 'returns runners belonging to group' do
+                is_expected.to contain_exactly(runner_group)
+              end
             end
-          end
 
-          context 'with unknown membership' do
-            let(:membership) { :unsupported }
+            context 'with :all_available membership' do
+              let(:membership) { :all_available }
 
-            it 'raises an error' do
-              expect { subject }.to raise_error(ArgumentError, 'Invalid membership filter')
+              it 'returns runners available to group' do
+                is_expected.to contain_exactly(
+                  runner_project_7, runner_project_6, runner_project_5,
+                  runner_project_4, runner_project_3, runner_project_2,
+                  runner_project_1, runner_sub_group_4, runner_sub_group_3,
+                  runner_sub_group_2, runner_sub_group_1, runner_group, runner_instance)
+              end
             end
-          end
 
-          context 'with nil group' do
-            let(:target_group) { nil }
+            context 'with unknown membership' do
+              let(:membership) { :unsupported }
 
-            it 'returns no runners' do
-              # Query should run against all runners, however since user is not admin, query returns no results
-              expect(subject).to eq([])
+              it 'raises an error' do
+                expect { subject }.to raise_error(ArgumentError, 'Invalid membership filter')
+              end
             end
-          end
 
-          context 'with sort param' do
-            let(:extra_params) { { sort: 'contacted_asc' } }
+            context 'with nil group' do
+              let(:target_group) { nil }
 
-            it 'sorts by specified attribute' do
-              expect(subject).to eq([runner_group, runner_sub_group_1, runner_sub_group_2,
-                                     runner_sub_group_3, runner_sub_group_4, runner_project_1,
-                                     runner_project_2, runner_project_3, runner_project_4,
-                                     runner_project_5, runner_project_6, runner_project_7])
+              it 'raises Gitlab::Access::AccessDeniedError' do
+                # Query should run against all runners, however since user is not admin, we raise an error
+                expect { execute }.to raise_error(Gitlab::Access::AccessDeniedError)
+              end
             end
-          end
 
-          context 'filtering' do
-            context 'by search term' do
-              let(:extra_params) { { search: 'runner_project_search' } }
+            context 'with sort param' do
+              let(:extra_params) { { sort: 'contacted_asc' } }
 
-              it 'returns correct runner' do
-                expect(subject).to match_array([runner_project_3])
+              it 'sorts by specified attribute' do
+                expect(subject).to eq([runner_group, runner_sub_group_1, runner_sub_group_2,
+                                      runner_sub_group_3, runner_sub_group_4, runner_project_1,
+                                      runner_project_2, runner_project_3, runner_project_4,
+                                      runner_project_5, runner_project_6, runner_project_7])
               end
             end
 
-            context 'by active status' do
-              let(:extra_params) { { active: false } }
+            context 'filtering' do
+              context 'by search term' do
+                let(:extra_params) { { search: 'runner_project_search' } }
 
-              it 'returns correct runner' do
-                expect(subject).to match_array([runner_sub_group_1])
+                it 'returns correct runner' do
+                  expect(subject).to match_array([runner_project_3])
+                end
               end
-            end
 
-            context 'by status' do
-              let(:extra_params) { { status_status: 'paused' } }
+              context 'by active status' do
+                let(:extra_params) { { active: false } }
 
-              it 'returns correct runner' do
-                expect(subject).to match_array([runner_sub_group_1])
+                it 'returns correct runner' do
+                  expect(subject).to match_array([runner_sub_group_1])
+                end
               end
-            end
 
-            context 'by tag_name' do
-              let(:extra_params) { { tag_name: %w[runner_tag] } }
+              context 'by status' do
+                let(:extra_params) { { status_status: 'paused' } }
 
-              it 'returns correct runner' do
-                expect(subject).to match_array([runner_project_5])
+                it 'returns correct runner' do
+                  expect(subject).to match_array([runner_sub_group_1])
+                end
               end
-            end
 
-            context 'by runner type' do
-              let(:extra_params) { { type_type: 'project_type' } }
+              context 'by tag_name' do
+                let(:extra_params) { { tag_name: %w[runner_tag] } }
 
-              it 'returns correct runners' do
-                expect(subject).to eq([runner_project_7, runner_project_6,
-                                       runner_project_5, runner_project_4,
-                                       runner_project_3, runner_project_2, runner_project_1])
+                it 'returns correct runner' do
+                  expect(subject).to match_array([runner_project_5])
+                end
+              end
+
+              context 'by runner type' do
+                let(:extra_params) { { type_type: 'project_type' } }
+
+                it 'returns correct runners' do
+                  expect(subject).to eq([runner_project_7, runner_project_6,
+                                        runner_project_5, runner_project_4,
+                                        runner_project_3, runner_project_2, runner_project_1])
+                end
               end
             end
           end
         end
       end
 
-      context 'when user is not group owner' do
-        where(:user_permission) do
-          [:maintainer, :developer, :reporter, :guest]
+      context 'when user is group developer or below' do
+        where(:user_role) do
+          [GroupMember::DEVELOPER, GroupMember::REPORTER, GroupMember::GUEST]
         end
 
         with_them do
           before do
-            create(:group_member, user_permission, group: sub_group_1, user: user)
+            group.add_member(user, user_role)
           end
 
           context 'with :sub_group_1 as target group' do
             let(:target_group) { sub_group_1 }
 
-            it 'returns no runners' do
-              is_expected.to be_empty
+            it 'raises Gitlab::Access::AccessDeniedError' do
+              expect { execute }.to raise_error(Gitlab::Access::AccessDeniedError)
             end
           end
 
           context 'with :group as target group' do
             let(:target_group) { group }
 
-            it 'returns no runners' do
-              is_expected.to be_empty
+            it 'raises Gitlab::Access::AccessDeniedError' do
+              expect { execute }.to raise_error(Gitlab::Access::AccessDeniedError)
             end
 
             context 'with :all_available membership' do
               let(:membership) { :all_available }
 
-              it 'returns no runners' do
-                expect(subject).to be_empty
+              it 'raises Gitlab::Access::AccessDeniedError' do
+                expect { execute }.to raise_error(Gitlab::Access::AccessDeniedError)
               end
             end
           end
@@ -461,35 +473,31 @@ RSpec.describe Ci::RunnersFinder, feature_category: :runner_fleet do
       end
 
       context 'when user has no access' do
-        it 'returns no runners' do
-          expect(subject).to be_empty
+        it 'raises Gitlab::Access::AccessDeniedError' do
+          expect { execute }.to raise_error(Gitlab::Access::AccessDeniedError)
         end
       end
 
       context 'when user is nil' do
-        let_it_be(:user) { nil }
+        let(:user) { nil }
 
-        it 'returns no runners' do
-          expect(subject).to be_empty
+        it 'raises Gitlab::Access::AccessDeniedError' do
+          expect { execute }.to raise_error(Gitlab::Access::AccessDeniedError)
         end
       end
     end
 
     describe '#sort_key' do
-      subject { described_class.new(current_user: user, params: params.merge(group: group)).sort_key }
+      subject(:sort_key) { described_class.new(current_user: user, params: params.merge(group: group)).sort_key }
 
       context 'without params' do
-        it 'returns created_at_desc' do
-          expect(subject).to eq('created_at_desc')
-        end
+        it { is_expected.to eq('created_at_desc') }
       end
 
       context 'with params' do
         let(:extra_params) { { sort: 'contacted_asc' } }
 
-        it 'returns contacted_asc' do
-          expect(subject).to eq('contacted_asc')
-        end
+        it { is_expected.to eq('contacted_asc') }
       end
     end
   end
@@ -504,7 +512,7 @@ RSpec.describe Ci::RunnersFinder, feature_category: :runner_fleet do
     let(:params) { { project: project }.merge(extra_params).reject { |_, v| v.nil? } }
 
     describe '#execute' do
-      subject { described_class.new(current_user: user, params: params).execute }
+      subject(:execute) { described_class.new(current_user: user, params: params).execute }
 
       context 'with user as project admin' do
         before do
@@ -515,7 +523,7 @@ RSpec.describe Ci::RunnersFinder, feature_category: :runner_fleet do
           let_it_be(:runner_project) { create(:ci_runner, :project, contacted_at: 7.minutes.ago, projects: [project]) }
 
           it 'returns runners available to project' do
-            expect(subject).to match_array([runner_project])
+            is_expected.to match_array([runner_project])
           end
         end
 
@@ -524,7 +532,7 @@ RSpec.describe Ci::RunnersFinder, feature_category: :runner_fleet do
           let_it_be(:runner_group) { create(:ci_runner, :group, contacted_at: 12.minutes.ago, groups: [group]) }
 
           it 'returns runners available to project' do
-            expect(subject).to match_array([runner_instance, runner_group])
+            is_expected.to match_array([runner_instance, runner_group])
           end
         end
 
@@ -610,24 +618,24 @@ RSpec.describe Ci::RunnersFinder, feature_category: :runner_fleet do
           project.add_developer(user)
         end
 
-        it 'returns no runners' do
-          expect(subject).to be_empty
+        it 'raises Gitlab::Access::AccessDeniedError' do
+          expect { execute }.to raise_error(Gitlab::Access::AccessDeniedError)
         end
       end
 
       context 'when user is nil' do
         let_it_be(:user) { nil }
 
-        it 'returns no runners' do
-          expect(subject).to be_empty
+        it 'raises Gitlab::Access::AccessDeniedError' do
+          expect { execute }.to raise_error(Gitlab::Access::AccessDeniedError)
         end
       end
 
       context 'with nil project_full_path' do
         let(:project_full_path) { nil }
 
-        it 'returns no runners' do
-          expect(subject).to be_empty
+        it 'raises Gitlab::Access::AccessDeniedError' do
+          expect { execute }.to raise_error(Gitlab::Access::AccessDeniedError)
         end
       end
     end